* [Qemu-devel] [PATCH V3 1/3] acpi: refactor pxb crs computation
2016-06-28 9:59 [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation Marcel Apfelbaum
@ 2016-06-28 9:59 ` Marcel Apfelbaum
2016-06-28 14:29 ` Igor Mammedov
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly Marcel Apfelbaum
` (2 subsequent siblings)
3 siblings, 1 reply; 12+ messages in thread
From: Marcel Apfelbaum @ 2016-06-28 9:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, mst, pbonzini, imammedo, lersek, ehabkost
Instead of always passing both IO and MEM ranges when
computing CRS ranges, define a new CrsRangeSet structure
that include them both.
This is done before introducing a third type of range,
64-bit MEM, so it will be easier to pass them all around.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
hw/i386/acpi-build.c | 82 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 51 insertions(+), 31 deletions(-)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 5a594be..f306ae3 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -743,6 +743,23 @@ static void crs_range_free(gpointer data)
g_free(entry);
}
+typedef struct CrsRangeSet {
+ GPtrArray *io_ranges;
+ GPtrArray *mem_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);
+}
+
+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);
+}
+
static gint crs_range_compare(gconstpointer a, gconstpointer b)
{
CrsRangeEntry *entry_a = *(CrsRangeEntry **)a;
@@ -827,18 +844,17 @@ static void crs_range_merge(GPtrArray *range)
g_ptr_array_free(tmp, true);
}
-static Aml *build_crs(PCIHostState *host,
- GPtrArray *io_ranges, GPtrArray *mem_ranges)
+static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
{
Aml *crs = aml_resource_template();
- GPtrArray *host_io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
- GPtrArray *host_mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+ CrsRangeSet temp_range_set;
CrsRangeEntry *entry;
uint8_t max_bus = pci_bus_num(host->bus);
uint8_t type;
int devfn;
int i;
+ crs_range_set_init(&temp_range_set);
for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) {
uint64_t range_base, range_limit;
PCIDevice *dev = host->bus->devices[devfn];
@@ -862,9 +878,11 @@ static Aml *build_crs(PCIHostState *host,
}
if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
- crs_range_insert(host_io_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.io_ranges,
+ range_base, range_limit);
} else { /* "memory" */
- crs_range_insert(host_mem_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.mem_ranges,
+ range_base, range_limit);
}
}
@@ -883,7 +901,8 @@ static Aml *build_crs(PCIHostState *host,
* that do not support multiple root buses
*/
if (range_base && range_base <= range_limit) {
- crs_range_insert(host_io_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.io_ranges,
+ range_base, range_limit);
}
range_base =
@@ -896,7 +915,8 @@ static Aml *build_crs(PCIHostState *host,
* that do not support multiple root buses
*/
if (range_base && range_base <= range_limit) {
- crs_range_insert(host_mem_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.mem_ranges,
+ range_base, range_limit);
}
range_base =
@@ -909,35 +929,36 @@ static Aml *build_crs(PCIHostState *host,
* that do not support multiple root buses
*/
if (range_base && range_base <= range_limit) {
- crs_range_insert(host_mem_ranges, range_base, range_limit);
+ crs_range_insert(temp_range_set.mem_ranges,
+ range_base, range_limit);
}
}
}
- crs_range_merge(host_io_ranges);
- for (i = 0; i < host_io_ranges->len; i++) {
- entry = g_ptr_array_index(host_io_ranges, i);
+ crs_range_merge(temp_range_set.io_ranges);
+ for (i = 0; i < temp_range_set.io_ranges->len; i++) {
+ entry = g_ptr_array_index(temp_range_set.io_ranges, i);
aml_append(crs,
aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
AML_POS_DECODE, AML_ENTIRE_RANGE,
0, entry->base, entry->limit, 0,
entry->limit - entry->base + 1));
- crs_range_insert(io_ranges, entry->base, entry->limit);
+ crs_range_insert(range_set->io_ranges, entry->base, entry->limit);
}
- g_ptr_array_free(host_io_ranges, true);
- crs_range_merge(host_mem_ranges);
- for (i = 0; i < host_mem_ranges->len; i++) {
- entry = g_ptr_array_index(host_mem_ranges, i);
+ crs_range_merge(temp_range_set.mem_ranges);
+ for (i = 0; i < temp_range_set.mem_ranges->len; i++) {
+ entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
aml_append(crs,
aml_dword_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(mem_ranges, entry->base, entry->limit);
+ crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
}
- g_ptr_array_free(host_mem_ranges, true);
+
+ crs_range_set_free(&temp_range_set);
aml_append(crs,
aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
@@ -1894,8 +1915,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
{
CrsRangeEntry *entry;
Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
- GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
- GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
+ CrsRangeSet crs_range_set;
PCMachineState *pcms = PC_MACHINE(machine);
PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
uint32_t nr_mem = machine->ram_slots;
@@ -1984,6 +2004,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
}
aml_append(dsdt, scope);
+ crs_range_set_init(&crs_range_set);
bus = PC_MACHINE(machine)->bus;
if (bus) {
QLIST_FOREACH(bus, &bus->child, sibling) {
@@ -2010,8 +2031,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
}
aml_append(dev, build_prt(false));
- crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent),
- io_ranges, mem_ranges);
+ crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), &crs_range_set);
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(scope, dev);
aml_append(dsdt, scope);
@@ -2032,9 +2052,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
AML_POS_DECODE, AML_ENTIRE_RANGE,
0x0000, 0x0000, 0x0CF7, 0x0000, 0x0CF8));
- crs_replace_with_free_ranges(io_ranges, 0x0D00, 0xFFFF);
- for (i = 0; i < io_ranges->len; i++) {
- entry = g_ptr_array_index(io_ranges, i);
+ crs_replace_with_free_ranges(crs_range_set.io_ranges, 0x0D00, 0xFFFF);
+ for (i = 0; i < crs_range_set.io_ranges->len; i++) {
+ entry = g_ptr_array_index(crs_range_set.io_ranges, i);
aml_append(crs,
aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
AML_POS_DECODE, AML_ENTIRE_RANGE,
@@ -2047,9 +2067,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
AML_CACHEABLE, AML_READ_WRITE,
0, 0x000A0000, 0x000BFFFF, 0, 0x00020000));
- crs_replace_with_free_ranges(mem_ranges, pci->w32.begin, pci->w32.end - 1);
- for (i = 0; i < mem_ranges->len; i++) {
- entry = g_ptr_array_index(mem_ranges, i);
+ crs_replace_with_free_ranges(crs_range_set.mem_ranges,
+ pci->w32.begin, pci->w32.end - 1);
+ for (i = 0; i < crs_range_set. mem_ranges->len; i++) {
+ entry = g_ptr_array_index(crs_range_set.mem_ranges, i);
aml_append(crs,
aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
AML_NON_CACHEABLE, AML_READ_WRITE,
@@ -2084,8 +2105,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
aml_append(dev, aml_name_decl("_CRS", crs));
aml_append(scope, dev);
- g_ptr_array_free(io_ranges, true);
- g_ptr_array_free(mem_ranges, true);
+ crs_range_set_free(&crs_range_set);
/* reserve PCIHP resources */
if (pm->pcihp_io_len) {
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH V3 1/3] acpi: refactor pxb crs computation
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 1/3] acpi: refactor pxb crs computation Marcel Apfelbaum
@ 2016-06-28 14:29 ` Igor Mammedov
0 siblings, 0 replies; 12+ messages in thread
From: Igor Mammedov @ 2016-06-28 14:29 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, ehabkost, mst, pbonzini, lersek
On Tue, 28 Jun 2016 12:59:26 +0300
Marcel Apfelbaum <marcel@redhat.com> wrote:
> Instead of always passing both IO and MEM ranges when
> computing CRS ranges, define a new CrsRangeSet structure
> that include them both.
>
> This is done before introducing a third type of range,
> 64-bit MEM, so it will be easier to pass them all around.
>
> Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
Reviewed-by: Igor Mammedov <imammedo@redhat.com>
> ---
> hw/i386/acpi-build.c | 82 ++++++++++++++++++++++++++++++++--------------------
> 1 file changed, 51 insertions(+), 31 deletions(-)
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 5a594be..f306ae3 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -743,6 +743,23 @@ static void crs_range_free(gpointer data)
> g_free(entry);
> }
>
> +typedef struct CrsRangeSet {
> + GPtrArray *io_ranges;
> + GPtrArray *mem_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);
> +}
> +
> +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);
> +}
> +
> static gint crs_range_compare(gconstpointer a, gconstpointer b)
> {
> CrsRangeEntry *entry_a = *(CrsRangeEntry **)a;
> @@ -827,18 +844,17 @@ static void crs_range_merge(GPtrArray *range)
> g_ptr_array_free(tmp, true);
> }
>
> -static Aml *build_crs(PCIHostState *host,
> - GPtrArray *io_ranges, GPtrArray *mem_ranges)
> +static Aml *build_crs(PCIHostState *host, CrsRangeSet *range_set)
> {
> Aml *crs = aml_resource_template();
> - GPtrArray *host_io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
> - GPtrArray *host_mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
> + CrsRangeSet temp_range_set;
> CrsRangeEntry *entry;
> uint8_t max_bus = pci_bus_num(host->bus);
> uint8_t type;
> int devfn;
> int i;
>
> + crs_range_set_init(&temp_range_set);
> for (devfn = 0; devfn < ARRAY_SIZE(host->bus->devices); devfn++) {
> uint64_t range_base, range_limit;
> PCIDevice *dev = host->bus->devices[devfn];
> @@ -862,9 +878,11 @@ static Aml *build_crs(PCIHostState *host,
> }
>
> if (r->type & PCI_BASE_ADDRESS_SPACE_IO) {
> - crs_range_insert(host_io_ranges, range_base, range_limit);
> + crs_range_insert(temp_range_set.io_ranges,
> + range_base, range_limit);
> } else { /* "memory" */
> - crs_range_insert(host_mem_ranges, range_base, range_limit);
> + crs_range_insert(temp_range_set.mem_ranges,
> + range_base, range_limit);
> }
> }
>
> @@ -883,7 +901,8 @@ static Aml *build_crs(PCIHostState *host,
> * that do not support multiple root buses
> */
> if (range_base && range_base <= range_limit) {
> - crs_range_insert(host_io_ranges, range_base, range_limit);
> + crs_range_insert(temp_range_set.io_ranges,
> + range_base, range_limit);
> }
>
> range_base =
> @@ -896,7 +915,8 @@ static Aml *build_crs(PCIHostState *host,
> * that do not support multiple root buses
> */
> if (range_base && range_base <= range_limit) {
> - crs_range_insert(host_mem_ranges, range_base, range_limit);
> + crs_range_insert(temp_range_set.mem_ranges,
> + range_base, range_limit);
> }
>
> range_base =
> @@ -909,35 +929,36 @@ static Aml *build_crs(PCIHostState *host,
> * that do not support multiple root buses
> */
> if (range_base && range_base <= range_limit) {
> - crs_range_insert(host_mem_ranges, range_base, range_limit);
> + crs_range_insert(temp_range_set.mem_ranges,
> + range_base, range_limit);
> }
> }
> }
>
> - crs_range_merge(host_io_ranges);
> - for (i = 0; i < host_io_ranges->len; i++) {
> - entry = g_ptr_array_index(host_io_ranges, i);
> + crs_range_merge(temp_range_set.io_ranges);
> + for (i = 0; i < temp_range_set.io_ranges->len; i++) {
> + entry = g_ptr_array_index(temp_range_set.io_ranges, i);
> aml_append(crs,
> aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
> AML_POS_DECODE, AML_ENTIRE_RANGE,
> 0, entry->base, entry->limit, 0,
> entry->limit - entry->base + 1));
> - crs_range_insert(io_ranges, entry->base, entry->limit);
> + crs_range_insert(range_set->io_ranges, entry->base, entry->limit);
> }
> - g_ptr_array_free(host_io_ranges, true);
>
> - crs_range_merge(host_mem_ranges);
> - for (i = 0; i < host_mem_ranges->len; i++) {
> - entry = g_ptr_array_index(host_mem_ranges, i);
> + crs_range_merge(temp_range_set.mem_ranges);
> + for (i = 0; i < temp_range_set.mem_ranges->len; i++) {
> + entry = g_ptr_array_index(temp_range_set.mem_ranges, i);
> aml_append(crs,
> aml_dword_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(mem_ranges, entry->base, entry->limit);
> + crs_range_insert(range_set->mem_ranges, entry->base, entry->limit);
> }
> - g_ptr_array_free(host_mem_ranges, true);
> +
> + crs_range_set_free(&temp_range_set);
>
> aml_append(crs,
> aml_word_bus_number(AML_MIN_FIXED, AML_MAX_FIXED, AML_POS_DECODE,
> @@ -1894,8 +1915,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> {
> CrsRangeEntry *entry;
> Aml *dsdt, *sb_scope, *scope, *dev, *method, *field, *pkg, *crs;
> - GPtrArray *mem_ranges = g_ptr_array_new_with_free_func(crs_range_free);
> - GPtrArray *io_ranges = g_ptr_array_new_with_free_func(crs_range_free);
> + CrsRangeSet crs_range_set;
> PCMachineState *pcms = PC_MACHINE(machine);
> PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(machine);
> uint32_t nr_mem = machine->ram_slots;
> @@ -1984,6 +2004,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> }
> aml_append(dsdt, scope);
>
> + crs_range_set_init(&crs_range_set);
> bus = PC_MACHINE(machine)->bus;
> if (bus) {
> QLIST_FOREACH(bus, &bus->child, sibling) {
> @@ -2010,8 +2031,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> }
>
> aml_append(dev, build_prt(false));
> - crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent),
> - io_ranges, mem_ranges);
> + crs = build_crs(PCI_HOST_BRIDGE(BUS(bus)->parent), &crs_range_set);
> aml_append(dev, aml_name_decl("_CRS", crs));
> aml_append(scope, dev);
> aml_append(dsdt, scope);
> @@ -2032,9 +2052,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> AML_POS_DECODE, AML_ENTIRE_RANGE,
> 0x0000, 0x0000, 0x0CF7, 0x0000, 0x0CF8));
>
> - crs_replace_with_free_ranges(io_ranges, 0x0D00, 0xFFFF);
> - for (i = 0; i < io_ranges->len; i++) {
> - entry = g_ptr_array_index(io_ranges, i);
> + crs_replace_with_free_ranges(crs_range_set.io_ranges, 0x0D00, 0xFFFF);
> + for (i = 0; i < crs_range_set.io_ranges->len; i++) {
> + entry = g_ptr_array_index(crs_range_set.io_ranges, i);
> aml_append(crs,
> aml_word_io(AML_MIN_FIXED, AML_MAX_FIXED,
> AML_POS_DECODE, AML_ENTIRE_RANGE,
> @@ -2047,9 +2067,10 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> AML_CACHEABLE, AML_READ_WRITE,
> 0, 0x000A0000, 0x000BFFFF, 0, 0x00020000));
>
> - crs_replace_with_free_ranges(mem_ranges, pci->w32.begin, pci->w32.end - 1);
> - for (i = 0; i < mem_ranges->len; i++) {
> - entry = g_ptr_array_index(mem_ranges, i);
> + crs_replace_with_free_ranges(crs_range_set.mem_ranges,
> + pci->w32.begin, pci->w32.end - 1);
> + for (i = 0; i < crs_range_set. mem_ranges->len; i++) {
> + entry = g_ptr_array_index(crs_range_set.mem_ranges, i);
> aml_append(crs,
> aml_dword_memory(AML_POS_DECODE, AML_MIN_FIXED, AML_MAX_FIXED,
> AML_NON_CACHEABLE, AML_READ_WRITE,
> @@ -2084,8 +2105,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> aml_append(dev, aml_name_decl("_CRS", crs));
> aml_append(scope, dev);
>
> - g_ptr_array_free(io_ranges, true);
> - g_ptr_array_free(mem_ranges, true);
> + crs_range_set_free(&crs_range_set);
>
> /* reserve PCIHP resources */
> if (pm->pcihp_io_len) {
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly
2016-06-28 9:59 [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation Marcel Apfelbaum
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 1/3] acpi: refactor pxb crs computation Marcel Apfelbaum
@ 2016-06-28 9:59 ` Marcel Apfelbaum
2016-06-28 11:17 ` Igor Mammedov
2016-06-28 14:39 ` Igor Mammedov
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 3/3] tests/acpi: add pxb/pxb-pcie tests Marcel Apfelbaum
2016-06-28 14:27 ` [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation Igor Mammedov
3 siblings, 2 replies; 12+ messages in thread
From: Marcel Apfelbaum @ 2016-06-28 9:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, mst, pbonzini, imammedo, lersek, ehabkost
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 <marcel@redhat.com>
---
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) {
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly Marcel Apfelbaum
@ 2016-06-28 11:17 ` Igor Mammedov
2016-06-28 11:28 ` Marcel Apfelbaum
2016-06-28 14:39 ` Igor Mammedov
1 sibling, 1 reply; 12+ messages in thread
From: Igor Mammedov @ 2016-06-28 11:17 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, mst, pbonzini, lersek, ehabkost
On Tue, 28 Jun 2016 12:59:27 +0300
Marcel Apfelbaum <marcel@redhat.com> 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 <marcel@redhat.com>
> ---
> 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) {
Isn't range_limit <= UINT32_MAX a sufficient, why length check is required?
> + 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) {
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly
2016-06-28 11:17 ` Igor Mammedov
@ 2016-06-28 11:28 ` Marcel Apfelbaum
0 siblings, 0 replies; 12+ messages in thread
From: Marcel Apfelbaum @ 2016-06-28 11:28 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, mst, pbonzini, lersek, ehabkost
On 06/28/2016 02:17 PM, Igor Mammedov wrote:
> On Tue, 28 Jun 2016 12:59:27 +0300
> Marcel Apfelbaum <marcel@redhat.com> 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 <marcel@redhat.com>
>> ---
>> 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) {
> Isn't range_limit <= UINT32_MAX a sufficient, why length check is required?
>
Hi Igor,
Thanks for the review!
Please see Laszlo's mathematical completeness theory :) :
https://www.mail-archive.com/qemu-devel@nongnu.org/msg359583.html
I preferred to keep it.
Thanks,
Marcel
>> + 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) {
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly Marcel Apfelbaum
2016-06-28 11:17 ` Igor Mammedov
@ 2016-06-28 14:39 ` Igor Mammedov
2016-06-28 17:08 ` Marcel Apfelbaum
1 sibling, 1 reply; 12+ messages in thread
From: Igor Mammedov @ 2016-06-28 14:39 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, ehabkost, mst, pbonzini, lersek
On Tue, 28 Jun 2016 12:59:27 +0300
Marcel Apfelbaum <marcel@redhat.com> 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 <marcel@redhat.com>
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?
> ---
> 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) {
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly
2016-06-28 14:39 ` Igor Mammedov
@ 2016-06-28 17:08 ` Marcel Apfelbaum
2016-06-29 9:24 ` Igor Mammedov
0 siblings, 1 reply; 12+ messages in thread
From: Marcel Apfelbaum @ 2016-06-28 17:08 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, ehabkost, mst, pbonzini, lersek
On 06/28/2016 05:39 PM, Igor Mammedov wrote:
> On Tue, 28 Jun 2016 12:59:27 +0300
> Marcel Apfelbaum <marcel@redhat.com> 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 <marcel@redhat.com>
> 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.
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) {
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly
2016-06-28 17:08 ` Marcel Apfelbaum
@ 2016-06-29 9:24 ` Igor Mammedov
0 siblings, 0 replies; 12+ messages in thread
From: Igor Mammedov @ 2016-06-29 9:24 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, ehabkost, mst, pbonzini, lersek
On Tue, 28 Jun 2016 20:08:42 +0300
Marcel Apfelbaum <marcel@redhat.com> wrote:
> On 06/28/2016 05:39 PM, Igor Mammedov wrote:
> > On Tue, 28 Jun 2016 12:59:27 +0300
> > Marcel Apfelbaum <marcel@redhat.com> 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 <marcel@redhat.com>
> > 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 <imammedo@redhat.com>
>
>
> 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) {
> >
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Qemu-devel] [PATCH V3 3/3] tests/acpi: add pxb/pxb-pcie tests
2016-06-28 9:59 [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation Marcel Apfelbaum
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 1/3] acpi: refactor pxb crs computation Marcel Apfelbaum
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 2/3] hw/apci: handle 64-bit MMIO regions correctly Marcel Apfelbaum
@ 2016-06-28 9:59 ` Marcel Apfelbaum
2016-06-28 14:27 ` [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation Igor Mammedov
3 siblings, 0 replies; 12+ messages in thread
From: Marcel Apfelbaum @ 2016-06-28 9:59 UTC (permalink / raw)
To: qemu-devel; +Cc: marcel, mst, pbonzini, imammedo, lersek, ehabkost
Add an ivshmem device with 4G shared memory to
pxb in order to check the ACPI code of 64bit MMIO allocation.
Signed-off-by: Marcel Apfelbaum <marcel@redhat.com>
---
tests/acpi-test-data/pc/DSDT.pxb | Bin 0 -> 6329 bytes
tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
tests/bios-tables-test.c | 37 +++++++++++++++++++++++++++++++++
3 files changed, 37 insertions(+)
create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie
diff --git a/tests/acpi-test-data/pc/DSDT.pxb b/tests/acpi-test-data/pc/DSDT.pxb
new file mode 100644
index 0000000000000000000000000000000000000000..8c0dbf714e7fb7b7a7cdd32945f07f721d45d7d8
GIT binary patch
literal 6329
zcmb_g-ER}w6+hP>$@tny#>o%@fyD~26uN*vAh2px#h!6u6OS`^97;QAXB>j+psg5X
zLE3;;>v*Ls%iGGOYM*LQp5eJq{TKF6*!~M!b>WHhoEcxoI2nnRYAMb==lt&Zxc8iM
z@3m%X^?%L)uyA2TEt#ppZ9~<N#{huF^j|JV?ts<O3QIPNha$|{j(cPLDoDyFT48mG
z{jKTz)o~tv>#&9kYcJ*JYYq3Cr_LG>=+T<XISu4F+bq~td)83PX~`~?wM7009+vF9
zRs+diEo)5HB_C6iN=C6RKny`BMmI`EU55m~5AWg-v9@KRr*0LiTFR(tc1oGI&3YbO
z;y5m`8jl?=JKe<h9S1s2d~a_Ty5rw>8t|7!0{;Eq=q~K?3CUJ#W-`ggcVZYZDu)Qb
z7RYBWkvHV7)jnq2&g48_6JcQJijm2wA>e!RLUaJ8M#^U1_Hnqdgu9YfL)_u$?#G{9
zA}ur%pmZ5VD@L)(;5vlWJPpEX0sqPBsmozun6z2#i?zCnO8q5^<6@D&GyKRBbHN2U
zGzV;nZNesmMz+9eUt2C{eRw#m#JpAM2kK39zyXG{)?pq0&xJ5vt9?#iS?y1pm>Qla
z@#q}L1k6i|n%dvQ(Vj=Md}fLL6#gj;ktOyf{00|lZUHVtJl&baijfzay{MHZQ!?-<
zU*T!SU-J+@i+_`RmhbV;`OkcUOOm~)6_ZD~LTNbo2_a#%e^)W;eP1J>%=lYROX_q-
zaJzTmMWi3I%*U44m*FqNk~G)y{3%A%6+I&sHOl-96l!Qgg4DbXzYT*lCwpoZNlkW0
z4G;D8;Eo3B3B3)W9trixC22p2T83)YSsY$*2~(U16?1Skg<o3AYMI3wSc=js%BVOI
zY9U>`fr3I+ud%r73y4q))m1%Zu|h>%2IZ`w7H@>C(?sp<xlIzfTWkYpx^<4&yC7Gw
zgbKF2%qi^^joU`84(?>A<+#vzS%^Jv@Rs|y!9%d-KG|%n0T097uH3ncpcs}-rMj%a
zdiPA{F6$;c4Ib&UO;~?1)~E6aN_Ot@P@hxial(C$3hVAyTWeefEL?cH4&Xj%puMN-
z4BYjnP6Hk_b{%W`%;hk7OSMAP#*Es{w1i(7`pVK*PEw}IH*r<Fu_wIM#G;Sw^0+cz
zzNyw?2&ko#<2?S|2*kW(+k$7r<5B!BcjM2S7;}@xyTrEIl~D{6li4rG1U3+APZF+`
znVjZR-;F~o9K|f<_!q~&El?)>8YCykJtb*tWfQ)lF>iUC394$?C6_$<sBP9v_N?i9
zf!(ZDCVtO)VaPh>?+07Y3|P;2)-&kep!JM$xb=+H)+u&%$T|=G0`u7c^I6Y)cF24-
zz<jo6UK=)#2gC<QpBpfr^UUXl%;y5k=dAWh1fw^H%}0FmB8BJ_!7UeUH?E9$o2|tC
zH6~Y)MY}7PulWa_%J1wqeWO<Fo8Ti}a*e6RYO3_$q5JuxKczPBeevMY=G}W=fW=g!
zvfOLGW1KHBcnz;v0(&$jmDfdH^Mtj1nut84R620fa@AyNO~1vkA*2~ZR8`lo)l3p7
zteVA23XxjAo>CHaxmuQ7ASNc>CwIIqg}(UyjpbFNoK9!b<bi*lN0p>nU;IO;%R5cj
zQPb)4hEf{A9+r<;r|(>jh=Yw{B;TnM4YSdh3r_cVXIMbqyL`g2&K;0cmUkURk5WXx
z%n%z^g&Gdkuaw4BQ>{utVKC=qKHhQgNXAw<tQ1$cyA(MPkhc@KT*Gi&eC8hT4ACxg
zUMxqtrGu)x{XAS14<FQpaMX2ppn6PQG3$w-Ko8xo1I}W^?#3Z;xjZz?CFuQOCN9CD
z-dW$^$&G1+HHII|P)sdpml6z<P|w$saV~*GcQ2I2ZOU*;3Q)#v3@T2ML3882GK;yq
zWEAaUI*YsIUAQQ(X^Z9R#r@N`oxiA;48P-y8C4^lPtT(zZ1`jPy+1$L+)6bMl%+D|
zb^C6tqifX~-bC=Qv6${1!}$>%O~Aek^Tj$%vc0{RzkTk|4A;@I2D(!nof1UHUB$Fw
zM7)^pDjl8n+T+sB61C~jUXqesrd-si@(0wqe{X`PFy7INrrj$c5A7)V867)6){O6h
zz)E^;{z@{()6aelfLGq3&$~VUBiKE14^K{-$j8yyia(shVaXqUTqz3f1swLc(|Z1P
z4|^PkC0DN%N;HWE6;gzQN{J@Wph8MEiA@8VON1w;*<Iy1@kqW&H1n8AkI5ePIW($}
ztkTs+JH!SIIhReI;wQd6Iv6sAN?}rnUU<1I=cwMsm~c3bb9=*SbjWteGKpPMPC}Rb
zBvEgNXj*?be?>W?8@F{fya6t9ZLz72@+%z^H*x^9p|2Egx%W}1`YM$<9kU&;dGBv0
zrU~8@eTZBhq0ILVv0ewqlvX~)Qpf-megF00Ulj)!hFbNi@;!ud3}@yBTOJh*Ww~5Q
z-szohNkhGk4+bn7-V(<rJ^l)XDsL$ZF+3Cp?wQrm54>mM#gp~#i~qknrT@aJM)@DT
zT<E<(T9^v|c;}~l3cSL2?`=W^WyyO?<PrUJ3_|#Dg%)h%H$AuHl}Z0NrMs(dup;rN
z0toQ62VY`u6K@ktLO%2B@D1vDo9NSHUV8p#FljluLHADjgkUfPkoNYz?uF3$c~H{D
z;QrMQjvjex1hd3{<P9}G;t$+H@X=n+^SB>_cMJLP^^dPvgp}Nf7$u|T=i2+-6T?2_
zMsrXXyn<Iu!Sy<S2m7v>vnNL)43`wgSL{hCg6~Fr5h8V!BowmzoBf5g+Lso8uP*uy
z+$`#=cywe5J>;<*m7{cTlC{TP%HmHE19|VA0A4R22okUN9A@aMRWj)11)R@V+oy;K
zQ|C%iAH%N-jVd_8f=*f6Ckc>;1@84?>=fAHy@pQjI&1rgKc8-4RNMGvswriJ+I>h~
za=m0JpcL0OZl=@t1g76lto8*n(<3lFLZD!X84Zsf8f3*FM+2*U-ppM0Cn6T#vbMQe
z<zZ#gJ1yy%NCt>W&@8DX0N!%Z6wbicb^w&L5;Q8H(I99}XvO=juXQ94DzuIY=x7l1
ziqML`9{5_NK&a3f6VO-?^s3Ox1(XLug;uNv-+_1#bY5sp2xuY@DzuIX=vWZ+n$S8f
zpyPp1p%ve+zONHO(Cb30ETD2ARA@~KXfg=;sn9wppp$`6p>;|?r-Gn)p%pXV4@(Jz
z3av*3^hgl2Ahb>k=yV`dXgw;RM}wfM&^jZaGl5W{^_YMj3xaAw>u~`+9tag$X9aXN
z2wD_cPYCFVK&a4qQb11zK^KMAj|B9iK&a4qN<dE?231P4+O29m)J;5T;!|LUdfkRc
z$Vgx}^NAD@Lb8Aab|hcYB7!7iPnvN&ymw4&D?KViR9_ZB3Y$YuMz*f@Wn>IXyeA`T
z^L-ha!$R%J$lf)QO+0C0cg3S^$HamYG#+k9@5}Ls+cB|pgdE>GNbk#KqOk~sJmt~*
z@&%$XorS#R(fe{QVoXdS$HQdDR0re~HKtf!PH}4ka*7<2r7uqrJs@8|8n?YKZxM}h
e<vd+4@ds`CWs<fK&$?CC8)!WI*(KUa!u$_%*A-g;
literal 0
HcmV?d00001
diff --git a/tests/acpi-test-data/q35/DSDT.pxb_pcie b/tests/acpi-test-data/q35/DSDT.pxb_pcie
new file mode 100644
index 0000000000000000000000000000000000000000..838ca876dece056934ed511197c2a8fa0d27fd11
GIT binary patch
literal 9098
zcmb7KU2hx56`dt1YB{8&rL>aeuZXaVrb*+3l3XX~M__W7B3W@Iilp;V8XzSrt(>&T
zLa~83Mi5B>62}h(62?LMRNByU{zCH;^40)->SOy_6!BBkb7yww8BzjbJ}l?Xo^$V<
z-Pt+IUHVPGb^jt`*1wDEUZt5UzESsl^jVBCYSXt<O<ZT~J-=A)Sm{{8YVW5-8=Io-
zzTg+Hm94+*bie9$Z++UeHbb`c$a%1}8Gd}PyTu4}dn@Fe8qRZ_X0g-Wt9#9sm1bY_
zjA$3FbmnVHi~i!(x>qgQ&U!{b56T(DdAn1o`kBJF+_pQOY{l(P!EZ3TbFJ!IcJo@j
z&TOyMs4>@1b=I3+;{H3#^Iu==uZ;J0vNPOi2mf6De&O8JuU@}Z_~tkN`2GENcmjYW
zeB1b54k@5K)L5oecWJ}V)3)z?(YtAVh&_t}z1W}oqNi}O+o7EyfvNvtM)p#P#lv@;
zz?OPScBNiwDLoccVEj0`m3k{+8OFZ-A<Yu)eTU+i*Gg-CuHNuFIrmDZ*($J*db%NX
zZGO<@PPd==yw_#DZu;oxko6}&?`^U#H#6*C{~J4GhkOc0tI^D6`Q#@loJb^R3C8X)
z=j!;2G#om#?E^1aZ5BG>aAE)p7)-s|*og5iot_%zLyYh7SbP68wVAgb6ynUnR}o*e
zddrUqR4K*}2sH~-iFt+6n&lO-snxM@uAX`+t}0Ts_Q9n_%R^ClUeh>_G}3uKTed<L
zGADL{S!HXN?Xp<%4r?F0yjt`J@Oa!!iQ_SlD2QFwWfoZ5Y#ZMfVo=&Xcs5wSHXu&T
z3=?fWc7Ztrd=**-zyx#W<q@axcl${k<wN!`F(~NpiL&*6{QbCXU)U3cN(mg3MWtxW
zk~34Kp#ySKn#b|Sab{m|M4A;!vow^3#|C+DOi#=cdmKX^8+q({)mve1rh{)Q+@Z+%
z^P3i(0>}H0xTNZhR(*Q)y}jvl0<(wJ)>cbQwk@<**mQ>ac?>~W1#tuwHa(HZ935e4
zkhmx!7J!p-pWp$iLSiDnz{Ug<*eD{Vg2tF&0xFmYiHXq&Y+Nwa2x$I^tO6RBOh8pg
zOjQN*USdU_gkUOYLNe9h5$Za}44q?wsi0$$sRoZw*O@eQCIwSLlcr9Dy3TP!=eVJB
z+|-Fs*J&F%Z9}JR>O`pP#0sGu>6D=}W$Hwz>*R(`Zs_EuPK3J7w4pO?=uDeB5$Zaz
zQfcQiW9ZD7IuYtRCk&kvhRz97CqiB4q@i=t&^c-9M5ybWGIUNEI;TvX2z8x~q0=#R
zI;Kv9y3VYjGi&I~nmQ5cI;Rbt(}vDzQzt@Q=Zv9q#?U!q>O`pPbPb)Zq0=>WBGh%x
z8aihUowKG+gu2c-L+6~KbI#O>P}h0N(0R(xdCJs@P}ezc=$ton&YL<B>N-ygW+7S$
zPYY%-TI^0sruQTyrXOL?7|b&U^Nh(vs58$R%(DjbtjR>EGtU{!a|ZLA$wa6#&l}A1
z2J^hhM5r^L5lrPW&j_Yk<DZdCwQwUOrgD-6qvnE9bHS{MP}a0zpaso9%gyS*K+#6w
z$UqUGtPJ7^R8bhH#0g8XaK9sf$}wXSszPEaX)sWURR*d-$v`DkFv&m>Vl>J?B{qtv
zrt(@cPzfCvC_)TfU}2yV8%4xa2bT;~LIsly6rs`y1C?08gn?>MGEfN>Ofpb}N+%3d
zVg(ZhszJ#>B~&oUKoKgPFi?pVOc<yJB?FaE!6XAksC2?WB~~zDpc<46R6+%l3>2Z#
z2?Lc_!GwWoP%=;n6-+Wvgi0q2RAL1a2C6~HKqXW#$v_b*oiI>|6-*eY1|<WPP{AYv
zMW}SbKqXc%VW1k63{*k|lMEE0(g_2VSiyvWYEUvz2^CB-P=rb+3{+wT69%e5$v`Dk
zFv&m>DxEM;i4{y3s0Jkil~BPX14XEG!ayZfFkzq?lnhit1(OUEq0$Khl~}=qfof1P
zPze=GGEjs{Ck#|#1rr9ULCHWRR4~av5h|T9P>B^x7^ns%1C>z0Bm+gLbizO-Rxn|p
z8k7uFLIsly6rs`y1C?08gn?>MGEfN>Ofpb}N+%3dVg(ZhszJ#>B~&oUKoKgPFi?pV
zOc<yJB?FaE!6XAksC2?WB~~zDpc<46R6+%l3>2Z#2?Ir>87Ly%KoRN&icm99jR^zQ
zm}H<DlMGa2!ay}93{+#1foe=LP>l%#)tE3)jY$ToG08wRCJa<#!ay}98K}l21J#%?
zP(<qN!axz>jERFu28s};L50Lr#})>PNF7@kC?a)i$v_dRV@n2#&}Z(M4lI=QGd=NR
z^^ks$K1|YAp}qghm*Vs*m7W#Q(+I2H*eJkpH;T0JXye-q&qg*%)Lx-YnKl*MovCi%
z@lro^m+$T1AxP?wr`^ld4X=>`ppnl`^7Lm(mZC>n_@#WG`uG^O)$r~PpJLR)AR2fw
zQX9uTLP7l&&){(iJ*=@Y>}Gb3&wMt?QgOH&!}`sz5+j>CC7(;s5HOm8o9+13Ci{@u
zd{6XD(UT5#NCOh_sZO)ewC?XjW1#ZXA5||7RmWq6A6>nuRWFL_MT}3cUUZMIUThx(
zq}>~;&SOte-qXr^qP#a$-aDbZ7nS#i%j0p`k5=z%<$Y1!A1d#kP~MNqmxjwHo}zq7
zD_;`jOGD*LCzLNm<yVHwk3B{C6|MY=D8DjPe&vMnD^dCKaQWm@lrL-L%c6XFsC@Z^
z^5v*}Ww`wKQ<Sf0<tw6mWvG1Rgz^=XPtZ~}T;7r8)#8EM3VO^Yo?4oHZc<$G_@e7N
zN;iBN^Q@T;_ZlPVWZ$vrMy|_gGaYU~M$*Z?W7CaXXESCx+?9-^lYPgg8@X;=GaYVK
zM$*Z?W7CaX2eW26+`o*ZlYPgg8=2R0W;)!|jHHu&$EF*Zr>D$xxYHR)C;QBFPW5H^
zhQQ<3pFAtCH%d)46FDwG_|Jq_zm}`Kdn^3o?LXvp-+J%e+q*a3dXKd&uU=b?X5<9t
zW$U}lcNr^#N89indf)P01`qW2FH%roIgg&pd)2k3<u!uWEU)z2yu}ip7x;F2f0{sX
zuUV?)fb<Kk93CZC*Q$2Ns1x4w!gcWm0b`Nln<#koYCgZ1rxD0EG-GboYpuK)>+{|Y
z`^3xV^Ecf}60d#=srJR|FD2Cb0MbZfKROC`hPzQLWZnLm-tZ0@eaNS}?Po{!)K(9>
zZZJlQp01EJoC+C^&F@wwy{5NjpCrTTJ}RVpUA)QYhHN+~j&Q%67!f4S1P<3Q90%VD
zM@9y8Xmua1Ci<1pRN{OdpNhvva{-QA#|sTo-g>i@IZ5cT=WArJP#oSoMpwvV!@UH3
zd$<#a;Mmc%KgK&3Oe<Vr_+W-&YT1W0!7vGfLMxl*HpB0Ey3=#h9h%`ZDVRHn7lf@e
zDPub~-*(SpE|=@2PAR{HvnAeB(Y&Tp?5thV-jo$C1(mv7EGOz~^?V_J1(oQU4Bq<F
zySsOCJ0q8+J0org`>9^wuQl)v33sDyE2w#^%q=7XlweQxVZO94(!QgkN56j1B}Ui{
za1HcldI3$40B04`%A(-mT;J^lbgo4&omq-@yL2uo$)V+53h2t$(uRNCVIpC=7nGWv
z=n@j>u3MN7aC3-jMh-z??Vxexa(0>L@Bf^!3|_&QpC69oNA*VG5pJ!C$R{z_nru#^
zS&_}3)=H}Pc{HQmxuEbyggt|1B@7zH3X!N$qeN)bDny`0jgstAH#LY$f!Iy#zWads
zX#Xz7mgvliPV3g7hgvn-SL^%ZJ?dtVyu6fM;O9QYQvF0SNhO&iQOH|f;vDHa(1hkB
zxKVQs1L}lSnRbVioOXsZPTCcnA)*bMS1!9h3F>bI*6;}k(c4icwd?*Yz{E|AfNlos
zrPso@vD1?&-3tL`du#+~p*tFYBPPBum}mjVjCN1sN?6v{Et>T-Rh?S)dSp`By7vlR
z%VX~Iy@5gD_PkhJ<9qJal=x{EPRI7J0&YH98-K9L_rec0d5mp^cXv0p@FS4j=sUd|
zESyfsmvcVbrVeY+Q9wtwcZ0_UJ;kUeKIrk-cKG3)E$%RSiFt3EvGDFDa^KswSh#(!
zyUA{E9(Lt@+S(ZZoj9T5rypZ`IG=y}LC)J86SF(oBl{?zQ*CsjkK%h1EQUvDY>(|@
zORt(8Z&G@q{KEph&aT<~o&qxVk${)2$C<|&%SOK?XV&+~^EfkzC&V24uRilQhC#nC
z=@VnR1oq9*(MOSly;t=;eTe(}Kpef`_7Yg+<X<U<Rxj$;ZYA(ng;s&5<v|m-61ik9
zY2huo^W1eTc+KU`bTVP#Fra<8Gi@h$3@4VNU(g;Jrt_;O2cv!P;>zouyp5aKC<WKB
zW-k$XsERY@jL{8?<4=ichCfYcw?bm+6^p1gF#Po9Y@Y6MD)pd&|CDe(-`-!KAWln0
zjusGXYSgNsg)1r7-ak)(0=vq^B8W>AORHE(ab@6-D6^7Jw?A}IVbk<-Zj#p7m{T5;
zf15}*^LhMVfc}KiK6t*Fe?<m3nHIwhZ*$SdYEO+N)mWxR`(UcM7!iSuF*jNF)>`cU
D-{y{a
literal 0
HcmV?d00001
diff --git a/tests/bios-tables-test.c b/tests/bios-tables-test.c
index 92c90dd..3d87f9e 100644
--- a/tests/bios-tables-test.c
+++ b/tests/bios-tables-test.c
@@ -838,6 +838,41 @@ static void test_acpi_piix4_tcg_ipmi(void)
free_test_data(&data);
}
+static void test_acpi_piix4_tcg_pxb(void)
+{
+ test_data data;
+
+ memset(&data, 0, sizeof(data));
+ data.machine = MACHINE_PC;
+ data.variant = ".pxb";
+ data.required_struct_types = base_required_struct_types;
+ data.required_struct_types_len = ARRAY_SIZE(base_required_struct_types);
+ test_acpi_one("-machine accel=tcg"
+ " -device pxb,id=pxb,bus_nr=0x80,bus=pci.0"
+ " -object memory-backend-file,size=4G,mem-path=/tmp/shmem,share,id=mb"
+ " -device ivshmem-plain,memdev=mb,bus=pxb",
+ &data);
+ free_test_data(&data);
+}
+
+static void test_acpi_q35_tcg_pxb_pcie(void)
+{
+ test_data data;
+
+ memset(&data, 0, sizeof(data));
+ data.machine = MACHINE_Q35;
+ data.variant = ".pxb_pcie";
+ data.required_struct_types = base_required_struct_types;
+ data.required_struct_types_len = ARRAY_SIZE(ipmi_required_struct_types);
+ test_acpi_one("-machine q35,accel=tcg"
+ " -device pxb-pcie,id=pxb,bus_nr=0x80,bus=pcie.0"
+ " -device ioh3420,id=rp,bus=pxb,slot=1"
+ " -object memory-backend-file,size=4G,mem-path=/tmp/shmem,share,id=mb"
+ " -device ivshmem-plain,memdev=mb,bus=rp",
+ &data);
+ free_test_data(&data);
+}
+
int main(int argc, char *argv[])
{
const char *arch = qtest_get_arch();
@@ -856,6 +891,8 @@ int main(int argc, char *argv[])
qtest_add_func("acpi/q35/tcg/bridge", test_acpi_q35_tcg_bridge);
qtest_add_func("acpi/piix4/tcg/ipmi", test_acpi_piix4_tcg_ipmi);
qtest_add_func("acpi/q35/tcg/ipmi", test_acpi_q35_tcg_ipmi);
+ qtest_add_func("acpi/piix4/tcg/pxb", test_acpi_piix4_tcg_pxb);
+ qtest_add_func("acpi/q35/tcg/pxb-pcie", test_acpi_q35_tcg_pxb_pcie);
}
ret = g_test_run();
boot_sector_cleanup(disk);
--
2.4.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation
2016-06-28 9:59 [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation Marcel Apfelbaum
` (2 preceding siblings ...)
2016-06-28 9:59 ` [Qemu-devel] [PATCH V3 3/3] tests/acpi: add pxb/pxb-pcie tests Marcel Apfelbaum
@ 2016-06-28 14:27 ` Igor Mammedov
2016-06-28 16:54 ` Marcel Apfelbaum
3 siblings, 1 reply; 12+ messages in thread
From: Igor Mammedov @ 2016-06-28 14:27 UTC (permalink / raw)
To: Marcel Apfelbaum; +Cc: qemu-devel, mst, pbonzini, lersek, ehabkost
On Tue, 28 Jun 2016 12:59:25 +0300
Marcel Apfelbaum <marcel@redhat.com> wrote:
> This series fixes 64-bit BARs allocations for devices behind PXBs/PXB-PCIEs.
>
> 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.
I'd rearrange pathc order by putting test without DSDT.pxb blob,
as the first patch so changes to AML would become observable
for each of following patches and finish series with blob update
(provided below issue also fixed within series, otherwise it will
become hidden and we probably will forget about it)
on piix4 machine PXB is marked as hotpluggable and as result
it generates following bogus change:
@@ -1156,8 +1280,8 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
Device (S18)
{
- Name (_SUN, 0x03) // _SUN: Slot User Number
Name (_ADR, 0x00030000) // _ADR: Address
+ Name (_SUN, 0x03) // _SUN: Slot User Number
Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device
{
PCEJ (BSEL, _SUN)
@@ -1597,6 +1721,7 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
BNUM = Zero
DVNT (PCIU, One)
DVNT (PCID, 0x03)
+ ^S18.PCNT ()
}
}
}
with ^S18.PCNT() leading nowhere,
so question is:
Is PXB present at boot ACPI hot(un)pluggable itself?
perhaps you need to play with bridge_in_acpi or dc->hotpluggable
to make it not hotpluggable so won't create non existent call.
> v2 -> v3:
> - split original series "pci: better support for 64-bit MMIO allocation" into 2 series:
> - this is the first part dealing with correct 64-bit MMIO ACPI computation
> - the second one will include 64-bit MMIO reservation for PCI hotplug
> - Add pxb/pxb-pcie tests (Igor) - See diffs below (*)
> - Re-based on latest master.
>
> v1 -> v2:
> - resolved some styling issues (Laszlo)
> - rebase on latest master (Laszlo)
>
> Thank you,
> Marcel
>
>
>
> (*)
>
> PC/pxb
> =======================================================================================================
> 8c8
> < * Disassembly of /tmp/aml-5UR3JY, Tue Jun 28 12:51:27 2016
> ---
> > * Disassembly of tests/acpi-test-data/pc/DSDT.pxb, Tue Jun 28 12:51:27 2016
> 12c12
> < * Length 0x000018A5 (6309)
> ---
> > * Length 0x000018B9 (6329)
> 14c14
> < * Checksum 0xC3
> ---
> > * Checksum 0x03
> 21c21
> < DefinitionBlock ("/tmp/aml-5UR3JY.aml", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
> ---
> > DefinitionBlock ("tests/acpi-test-data/pc/DSDT.aml", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
> 1063,1068c1063,1068
> < 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
> 1129c1129
> < 0xFFFFFFFF, // Range Maximum
> ---
> > 0xFEBFFFFF, // Range Maximum
> 1131c1131
> < 0x01600000, // Length
> ---
> > 0x00200000, // Length
>
> Q35/pxb-pcie
> ============================================================================================================
> 8c8
> < * Disassembly of /tmp/aml-U1VPJY, Tue Jun 28 12:51:31 2016
> ---
> > * Disassembly of tests/acpi-test-data/q35/DSDT.pxb_pcie, Tue Jun 28 12:51:31 2016
> 12c12
> < * Length 0x00002376 (9078)
> ---
> > * Length 0x0000238A (9098)
> 14c14
> < * Checksum 0xA9
> ---
> > * Checksum 0xE9
> 21c21
> < DefinitionBlock ("/tmp/aml-U1VPJY.aml", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
> ---
> > DefinitionBlock ("tests/acpi-test-data/q35/DSDT.aml", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
> 3309,3314c3309,3314
> < 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
> 3375c3375
> < 0xFFFFFFFF, // Range Maximum
> ---
> > 0xFEBFFFFF, // Range Maximum
> 3377c3377
> < 0x01600000, // Length
> ---
> > 0x00200000, // Length
>
>
>
>
> Marcel Apfelbaum (3):
> acpi: refactor pxb crs computation
> hw/apci: handle 64-bit MMIO regions correctly
> tests/acpi: add pxb/pxb-pcie tests
>
> hw/i386/acpi-build.c | 127 +++++++++++++++++++++++----------
> tests/acpi-test-data/pc/DSDT.pxb | Bin 0 -> 6329 bytes
> tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
> tests/bios-tables-test.c | 37 ++++++++++
> 4 files changed, 128 insertions(+), 36 deletions(-)
> create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
> create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie
>
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation
2016-06-28 14:27 ` [Qemu-devel] [PATCH V3 0/3] pxb: fix 64-bit MMIO allocation Igor Mammedov
@ 2016-06-28 16:54 ` Marcel Apfelbaum
0 siblings, 0 replies; 12+ messages in thread
From: Marcel Apfelbaum @ 2016-06-28 16:54 UTC (permalink / raw)
To: Igor Mammedov; +Cc: qemu-devel, mst, pbonzini, lersek, ehabkost
On 06/28/2016 05:27 PM, Igor Mammedov wrote:
> On Tue, 28 Jun 2016 12:59:25 +0300
> Marcel Apfelbaum <marcel@redhat.com> wrote:
>
>> This series fixes 64-bit BARs allocations for devices behind PXBs/PXB-PCIEs.
>>
>> 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.
>
> I'd rearrange pathc order by putting test without DSDT.pxb blob,
> as the first patch so changes to AML would become observable
> for each of following patches and finish series with blob update
> (provided below issue also fixed within series, otherwise it will
> become hidden and we probably will forget about it)
>
Sure, I'll do that for next version.
> on piix4 machine PXB is marked as hotpluggable and as result
> it generates following bogus change:
>
> @@ -1156,8 +1280,8 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
>
> Device (S18)
> {
> - Name (_SUN, 0x03) // _SUN: Slot User Number
> Name (_ADR, 0x00030000) // _ADR: Address
> + Name (_SUN, 0x03) // _SUN: Slot User Number
> Method (_EJ0, 1, NotSerialized) // _EJx: Eject Device
> {
> PCEJ (BSEL, _SUN)
> @@ -1597,6 +1721,7 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
> BNUM = Zero
> DVNT (PCIU, One)
> DVNT (PCID, 0x03)
> + ^S18.PCNT ()
> }
> }
> }
>
> with ^S18.PCNT() leading nowhere,
> so question is:
> Is PXB present at boot ACPI hot(un)pluggable itself?
>
No, is not, and it should definitively be marked as not hot-pluggable.
I remember you already commented about that, I was testing it
with q35 and didn't see the issue, sorry for missing it earlier.
> perhaps you need to play with bridge_in_acpi or dc->hotpluggable
> to make it not hotpluggable so won't create non existent call.
>
I'll be sure to add a patch for that, thanks!
Marcel
>> v2 -> v3:
>> - split original series "pci: better support for 64-bit MMIO allocation" into 2 series:
>> - this is the first part dealing with correct 64-bit MMIO ACPI computation
>> - the second one will include 64-bit MMIO reservation for PCI hotplug
>> - Add pxb/pxb-pcie tests (Igor) - See diffs below (*)
>> - Re-based on latest master.
>>
>> v1 -> v2:
>> - resolved some styling issues (Laszlo)
>> - rebase on latest master (Laszlo)
>>
>> Thank you,
>> Marcel
>>
>>
>>
>> (*)
>>
>> PC/pxb
>> =======================================================================================================
>> 8c8
>> < * Disassembly of /tmp/aml-5UR3JY, Tue Jun 28 12:51:27 2016
>> ---
>> > * Disassembly of tests/acpi-test-data/pc/DSDT.pxb, Tue Jun 28 12:51:27 2016
>> 12c12
>> < * Length 0x000018A5 (6309)
>> ---
>> > * Length 0x000018B9 (6329)
>> 14c14
>> < * Checksum 0xC3
>> ---
>> > * Checksum 0x03
>> 21c21
>> < DefinitionBlock ("/tmp/aml-5UR3JY.aml", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
>> ---
>> > DefinitionBlock ("tests/acpi-test-data/pc/DSDT.aml", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
>> 1063,1068c1063,1068
>> < 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
>> 1129c1129
>> < 0xFFFFFFFF, // Range Maximum
>> ---
>> > 0xFEBFFFFF, // Range Maximum
>> 1131c1131
>> < 0x01600000, // Length
>> ---
>> > 0x00200000, // Length
>>
>> Q35/pxb-pcie
>> ============================================================================================================
>> 8c8
>> < * Disassembly of /tmp/aml-U1VPJY, Tue Jun 28 12:51:31 2016
>> ---
>> > * Disassembly of tests/acpi-test-data/q35/DSDT.pxb_pcie, Tue Jun 28 12:51:31 2016
>> 12c12
>> < * Length 0x00002376 (9078)
>> ---
>> > * Length 0x0000238A (9098)
>> 14c14
>> < * Checksum 0xA9
>> ---
>> > * Checksum 0xE9
>> 21c21
>> < DefinitionBlock ("/tmp/aml-U1VPJY.aml", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
>> ---
>> > DefinitionBlock ("tests/acpi-test-data/q35/DSDT.aml", "DSDT", 1, "BOCHS ", "BXPCDSDT", 0x00000001)
>> 3309,3314c3309,3314
>> < 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
>> 3375c3375
>> < 0xFFFFFFFF, // Range Maximum
>> ---
>> > 0xFEBFFFFF, // Range Maximum
>> 3377c3377
>> < 0x01600000, // Length
>> ---
>> > 0x00200000, // Length
>>
>>
>>
>>
>> Marcel Apfelbaum (3):
>> acpi: refactor pxb crs computation
>> hw/apci: handle 64-bit MMIO regions correctly
>> tests/acpi: add pxb/pxb-pcie tests
>>
>> hw/i386/acpi-build.c | 127 +++++++++++++++++++++++----------
>> tests/acpi-test-data/pc/DSDT.pxb | Bin 0 -> 6329 bytes
>> tests/acpi-test-data/q35/DSDT.pxb_pcie | Bin 0 -> 9098 bytes
>> tests/bios-tables-test.c | 37 ++++++++++
>> 4 files changed, 128 insertions(+), 36 deletions(-)
>> create mode 100644 tests/acpi-test-data/pc/DSDT.pxb
>> create mode 100644 tests/acpi-test-data/q35/DSDT.pxb_pcie
>>
>
^ permalink raw reply [flat|nested] 12+ messages in thread