On Tue, 28 Jun 2016 20:08:42 +0300 Marcel Apfelbaum <[email protected]> wrote:
> On 06/28/2016 05:39 PM, Igor Mammedov wrote: > > On Tue, 28 Jun 2016 12:59:27 +0300 > > Marcel Apfelbaum <[email protected]> wrote: > > > >> In build_crs(), the calculation and merging of the ranges already happens > >> in 64-bit, but the entry boundaries are silently truncated to 32-bit in the > >> call to aml_dword_memory(). Fix it by handling the 64-bit MMIO ranges > >> separately. > >> This fixes 64-bit BARs behind PXBs. > >> > >> Signed-off-by: Marcel Apfelbaum <[email protected]> > > patch indeed fixes issue with truncating in aml_dword_memory() > > as per hunk > > > > @@ -3306,12 +3306,12 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", > > "BXPCDSDT", 0x00000001) > > 0x00000000, // Translation Offset > > 0x00200000, // Length > > ,, , AddressRangeMemory, TypeStatic) > > - DWordMemory (ResourceProducer, PosDecode, MinFixed, > > MaxFixed, NonCacheable, ReadWrite, > > - 0x00000000, // Granularity > > - 0x00000000, // Range Minimum > > - 0xFFFFFFFF, // Range Maximum > > - 0x00000000, // Translation Offset > > - 0x00000000, // Length > > + QWordMemory (ResourceProducer, PosDecode, MinFixed, > > MaxFixed, NonCacheable, ReadWrite, > > + 0x0000000000000000, // Granularity > > + 0x0000000100000000, // Range Minimum > > + 0x00000001FFFFFFFF, // Range Maximum > > + 0x0000000000000000, // Translation Offset > > + 0x0000000100000000, // Length > > ,, , AddressRangeMemory, TypeStatic) > > WordBusNumber (ResourceProducer, MinFixed, MaxFixed, > > PosDecode, > > 0x0000, // Granularity > > > > how a second hunk is present which touches 32bit part of _CRS: > > > > @@ -3372,9 +3372,9 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", > > 0x00000001) > > DWordMemory (ResourceProducer, PosDecode, MinFixed, MaxFixed, > > NonCacheable, ReadWrite, > > 0x00000000, // Granularity > > 0xFEA00000, // Range Minimum > > - 0xFFFFFFFF, // Range Maximum > > + 0xFEBFFFFF, // Range Maximum > > 0x00000000, // Translation Offset > > - 0x01600000, // Length > > + 0x00200000, // Length > > ,, , AddressRangeMemory, TypeStatic) > > }) > > > > was it expected? Why? > > > > Yes, it is expected. It is the same bug. If you try a pc machine > without pxb you will have 0xFEBFFFFF as the 32-bit upper IOMMU limit. > > However, when having 32-bit ranges being merged with 64-bit ranges will result > in a wrong upper limit. > > So this is a second fix to the same problem. Reviewed-by: Igor Mammedov <[email protected]> > > > Thanks, > Marcel > > >> --- > >> hw/i386/acpi-build.c | 53 > >> +++++++++++++++++++++++++++++++++++++++++++--------- > >> 1 file changed, 44 insertions(+), 9 deletions(-) > >> > >> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c > >> index f306ae3..3808347 100644 > >> --- a/hw/i386/acpi-build.c > >> +++ b/hw/i386/acpi-build.c > >> @@ -746,18 +746,22 @@ static void crs_range_free(gpointer data) > >> typedef struct CrsRangeSet { > >> GPtrArray *io_ranges; > >> GPtrArray *mem_ranges; > >> + GPtrArray *mem_64bit_ranges; > >> } CrsRangeSet; > >> > >> static void crs_range_set_init(CrsRangeSet *range_set) > >> { > >> range_set->io_ranges = > >> g_ptr_array_new_with_free_func(crs_range_free); > >> range_set->mem_ranges = > >> g_ptr_array_new_with_free_func(crs_range_free); > >> + range_set->mem_64bit_ranges = > >> + g_ptr_array_new_with_free_func(crs_range_free); > >> } > >> > >> static void crs_range_set_free(CrsRangeSet *range_set) > >> { > >> g_ptr_array_free(range_set->io_ranges, true); > >> g_ptr_array_free(range_set->mem_ranges, true); > >> + g_ptr_array_free(range_set->mem_64bit_ranges, true); > >> } > >> > >> static gint crs_range_compare(gconstpointer a, gconstpointer b) > >> @@ -915,8 +919,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet > >> *range_set) > >> * that do not support multiple root buses > >> */ > >> if (range_base && range_base <= range_limit) { > >> - crs_range_insert(temp_range_set.mem_ranges, > >> - range_base, range_limit); > >> + uint64_t length = range_limit - range_base + 1; > >> + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) { > >> + crs_range_insert(temp_range_set.mem_ranges, > >> + range_base, range_limit); > >> + } else { > >> + crs_range_insert(temp_range_set.mem_64bit_ranges, > >> + range_base, range_limit); > >> + } > >> } > >> > >> range_base = > >> @@ -929,8 +939,14 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet > >> *range_set) > >> * that do not support multiple root buses > >> */ > >> if (range_base && range_base <= range_limit) { > >> - crs_range_insert(temp_range_set.mem_ranges, > >> - range_base, range_limit); > >> + uint64_t length = range_limit - range_base + 1; > >> + if (range_limit <= UINT32_MAX && length <= UINT32_MAX) { > >> + crs_range_insert(temp_range_set.mem_ranges, > >> + range_base, range_limit); > >> + } else { > >> + crs_range_insert(temp_range_set.mem_64bit_ranges, > >> + range_base, range_limit); > >> + } > >> } > >> } > >> } > >> @@ -958,6 +974,19 @@ static Aml *build_crs(PCIHostState *host, CrsRangeSet > >> *range_set) > >> crs_range_insert(range_set->mem_ranges, entry->base, > >> entry->limit); > >> } > >> > >> + crs_range_merge(temp_range_set.mem_64bit_ranges); > >> + for (i = 0; i < temp_range_set.mem_64bit_ranges->len; i++) { > >> + entry = g_ptr_array_index(temp_range_set.mem_64bit_ranges, i); > >> + aml_append(crs, > >> + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, > >> + AML_MAX_FIXED, AML_NON_CACHEABLE, > >> + AML_READ_WRITE, > >> + 0, entry->base, entry->limit, 0, > >> + entry->limit - entry->base + 1)); > >> + crs_range_insert(range_set->mem_64bit_ranges, > >> + entry->base, entry->limit); > >> + } > >> + > >> crs_range_set_free(&temp_range_set); > >> > >> aml_append(crs, > >> @@ -2079,11 +2108,17 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, > >> } > >> > >> if (pci->w64.begin) { > >> - aml_append(crs, > >> - aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED, > >> - AML_CACHEABLE, AML_READ_WRITE, > >> - 0, pci->w64.begin, pci->w64.end - 1, 0, > >> - pci->w64.end - pci->w64.begin)); > >> + crs_replace_with_free_ranges(crs_range_set.mem_64bit_ranges, > >> + pci->w64.begin, pci->w64.end - 1); > >> + for (i = 0; i < crs_range_set.mem_64bit_ranges->len; i++) { > >> + entry = g_ptr_array_index(crs_range_set.mem_64bit_ranges, i); > >> + aml_append(crs, > >> + aml_qword_memory(AML_POS_DECODE, AML_MIN_FIXED, > >> + AML_MAX_FIXED, > >> + AML_CACHEABLE, AML_READ_WRITE, > >> + 0, entry->base, entry->limit, > >> + 0, entry->limit - entry->base + > >> 1)); > >> + } > >> } > >> > >> if (misc->tpm_version != TPM_VERSION_UNSPEC) { > > >
