[Qemu-devel] [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space configurable
This is in preparation for adding support for ARM64 platforms
where it doesn't use port mapped IO for ACPI IO space. We are
making changes so that MMIO region can be accommodated
and board can pass the base address into the aml build function.
Also move few MEMORY_* definitions to header so that other memory
hotplug event signalling mechanisms (eg. Generic Event Device on
HW-reduced acpi platforms) can use the same from their respective
event handler code.
Signed-off-by: Shameer Kolothum
---
v8 --> v9
-base address is an input into build_memory_hotplug_aml()
-Removed R-by tags from Igor and Eric for now.
---
hw/acpi/memory_hotplug.c | 29 ++---
hw/i386/acpi-build.c | 4 +++-
hw/i386/pc.c | 3 +++
include/hw/acpi/memory_hotplug.h | 9 +++--
include/hw/i386/pc.h | 3 +++
5 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
index 297812d5f7..1734d4b44f 100644
--- a/hw/acpi/memory_hotplug.c
+++ b/hw/acpi/memory_hotplug.c
@@ -29,12 +29,7 @@
#define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
#define MEMORY_SLOT_EJECT_METHOD "MEJ0"
#define MEMORY_SLOT_NOTIFY_METHOD"MTFY"
-#define MEMORY_SLOT_SCAN_METHOD "MSCN"
#define MEMORY_HOTPLUG_DEVICE"MHPD"
-#define MEMORY_HOTPLUG_IO_LEN 24
-#define MEMORY_DEVICES_CONTAINER "\\_SB.MHPC"
-
-static uint16_t memhp_io_base;
static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
{
@@ -209,7 +204,7 @@ static const MemoryRegionOps acpi_memory_hotplug_ops = {
};
void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
- MemHotplugState *state, uint16_t io_base)
+ MemHotplugState *state, hwaddr io_base)
{
MachineState *machine = MACHINE(qdev_get_machine());
@@ -218,12 +213,10 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object
*owner,
return;
}
-assert(!memhp_io_base);
-memhp_io_base = io_base;
state->devs = g_malloc0(sizeof(*state->devs) * state->dev_count);
memory_region_init_io(&state->io, owner, &acpi_memory_hotplug_ops, state,
"acpi-mem-hotplug", MEMORY_HOTPLUG_IO_LEN);
-memory_region_add_subregion(as, memhp_io_base, &state->io);
+memory_region_add_subregion(as, io_base, &state->io);
}
/**
@@ -342,7 +335,8 @@ const VMStateDescription vmstate_memory_hotplug = {
void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
const char *res_root,
- const char *event_handler_method)
+ const char *event_handler_method,
+ AmlRegionSpace rs, hwaddr memhp_io_base)
{
int i;
Aml *ifctx;
@@ -365,14 +359,19 @@ void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
aml_name_decl("_UID", aml_string("Memory hotplug resources")));
crs = aml_resource_template();
-aml_append(crs,
-aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
- MEMORY_HOTPLUG_IO_LEN)
-);
+if (rs == AML_SYSTEM_IO) {
+aml_append(crs,
+aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
+ MEMORY_HOTPLUG_IO_LEN)
+);
+} else {
+aml_append(crs, aml_memory32_fixed(memhp_io_base,
+MEMORY_HOTPLUG_IO_LEN, AML_READ_WRITE));
+}
aml_append(mem_ctrl_dev, aml_name_decl("_CRS", crs));
aml_append(mem_ctrl_dev, aml_operation_region(
-MEMORY_HOTPLUG_IO_REGION, AML_SYSTEM_IO,
+MEMORY_HOTPLUG_IO_REGION, rs,
aml_int(memhp_io_base), MEMORY_HOTPLUG_IO_LEN)
);
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index f3fdfefcd5..e76d6631ea 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1871,7 +1871,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
"\\_SB.PCI0", "\\_GPE._E02");
}
-build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0", "\\_GPE._E03");
+build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
+ "\\_GPE._E03", AML_SYSTEM_IO,
+ pcms->memhp_io_base);
scope = aml_scope("_GPE");
{
diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index 549c437050..be973cea99 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -1936,6 +1936,9 @@ void pc_memory_init(PCMachineState *pcms,
/* Init default IOAPIC address space */
pcms->ioapic_as = &address_space_memory;
+
+/* Init ACPI memory hotplug IO base address */
+pcms->memhp_io_base = ACPI_MEMORY_HOTPLUG_BASE;
}
/*
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index 77c65765d6..dfe9cf3fde 100644
--- a/include/hw/acpi/me
Re: [Qemu-devel] [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space configurable
> -Original Message-
> From: Linuxarm [mailto:linuxarm-boun...@huawei.com] On Behalf Of Shameer
> Kolothum
> Sent: 13 August 2019 22:05
> To: qemu-devel@nongnu.org; qemu-...@nongnu.org;
> eric.au...@redhat.com; imamm...@redhat.com
> Cc: peter.mayd...@linaro.org; sa...@linux.intel.com;
> ard.biesheu...@linaro.org; Linuxarm ;
> shannon.zha...@gmail.com; sebastien.bo...@intel.com; ler...@redhat.com
> Subject: [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space
> configurable
>
> This is in preparation for adding support for ARM64 platforms
> where it doesn't use port mapped IO for ACPI IO space. We are
> making changes so that MMIO region can be accommodated
> and board can pass the base address into the aml build function.
Looks like, this now breaks the "make check" on x86_64 and needs
updating bios-tables-test-allowed-diff.h with DSDT entries. But I am
not sure what changed now compared to v8(and older ones) that makes
it to complain now!.
Patchew URL:
https://patchew.org/QEMU/20190813210539.31164-1-shameerali.kolothum.th...@huawei.com/
ERROR:/tmp/qemu-test/src/tests/bios-tables-test.c:447:test_acpi_asl: assertion
failed: (all_tables_match)
Thanks,
Shameer
> Also move few MEMORY_* definitions to header so that other memory
> hotplug event signalling mechanisms (eg. Generic Event Device on
> HW-reduced acpi platforms) can use the same from their respective
> event handler code.
>
> Signed-off-by: Shameer Kolothum
> ---
> v8 --> v9
> -base address is an input into build_memory_hotplug_aml()
> -Removed R-by tags from Igor and Eric for now.
> ---
> hw/acpi/memory_hotplug.c | 29 ++---
> hw/i386/acpi-build.c | 4 +++-
> hw/i386/pc.c | 3 +++
> include/hw/acpi/memory_hotplug.h | 9 +++--
> include/hw/i386/pc.h | 3 +++
> 5 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 297812d5f7..1734d4b44f 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -29,12 +29,7 @@
> #define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
> #define MEMORY_SLOT_EJECT_METHOD "MEJ0"
> #define MEMORY_SLOT_NOTIFY_METHOD"MTFY"
> -#define MEMORY_SLOT_SCAN_METHOD "MSCN"
> #define MEMORY_HOTPLUG_DEVICE"MHPD"
> -#define MEMORY_HOTPLUG_IO_LEN 24
> -#define MEMORY_DEVICES_CONTAINER "\\_SB.MHPC"
> -
> -static uint16_t memhp_io_base;
>
> static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
> {
> @@ -209,7 +204,7 @@ static const MemoryRegionOps
> acpi_memory_hotplug_ops = {
> };
>
> void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
> - MemHotplugState *state, uint16_t
> io_base)
> + MemHotplugState *state, hwaddr
> io_base)
> {
> MachineState *machine = MACHINE(qdev_get_machine());
>
> @@ -218,12 +213,10 @@ void acpi_memory_hotplug_init(MemoryRegion *as,
> Object *owner,
> return;
> }
>
> -assert(!memhp_io_base);
> -memhp_io_base = io_base;
> state->devs = g_malloc0(sizeof(*state->devs) * state->dev_count);
> memory_region_init_io(&state->io, owner, &acpi_memory_hotplug_ops,
> state,
>"acpi-mem-hotplug",
> MEMORY_HOTPLUG_IO_LEN);
> -memory_region_add_subregion(as, memhp_io_base, &state->io);
> +memory_region_add_subregion(as, io_base, &state->io);
> }
>
> /**
> @@ -342,7 +335,8 @@ const VMStateDescription vmstate_memory_hotplug
> = {
>
> void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
>const char *res_root,
> - const char *event_handler_method)
> + const char *event_handler_method,
> + AmlRegionSpace rs, hwaddr
> memhp_io_base)
> {
> int i;
> Aml *ifctx;
> @@ -365,14 +359,19 @@ void build_memory_hotplug_aml(Aml *table,
> uint32_t nr_mem,
> aml_name_decl("_UID", aml_string("Memory hotplug
> resources")));
>
> crs = aml_resource_template();
> -aml_append(crs,
> -aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
> - MEMORY_HOTPLUG_IO_LEN)
> -);
> +if (rs == AML_SYSTEM_IO) {
> +aml_append(crs,
> +aml_io(AML_DECODE16, memhp_io_base,
> memhp_io_base, 0,
> + MEMORY_HOTPLUG_IO_LEN)
> +);
> +} else {
> +aml_append(crs, aml_memory32_fixed(memhp_io_base,
> +MEMORY_HOTPLUG_IO_LEN,
> AML_READ_WRITE));
> +}
> aml_append(mem_ctrl_dev, aml_name_decl("_CRS", crs));
>
> aml_append(mem_ctrl_dev, aml_operation_region(
> -MEMORY_HOTPLUG_IO_REGION, AML_SYSTEM_IO,
> +MEMORY_HOTPLUG_IO_REGION, rs,
> aml_int(memhp_io_base), MEMORY_HOTPLUG_IO_LEN)
> );
>
>
Re: [Qemu-devel] [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space configurable
Hi Shameer,
On 8/13/19 11:05 PM, Shameer Kolothum wrote:
> This is in preparation for adding support for ARM64 platforms
> where it doesn't use port mapped IO for ACPI IO space. We are
> making changes so that MMIO region can be accommodated
> and board can pass the base address into the aml build function.
>
> Also move few MEMORY_* definitions to header so that other memory
> hotplug event signalling mechanisms (eg. Generic Event Device on
> HW-reduced acpi platforms) can use the same from their respective
> event handler code.
>
> Signed-off-by: Shameer Kolothum
> ---
> v8 --> v9
> -base address is an input into build_memory_hotplug_aml()
> -Removed R-by tags from Igor and Eric for now.
> ---
> hw/acpi/memory_hotplug.c | 29 ++---
> hw/i386/acpi-build.c | 4 +++-
> hw/i386/pc.c | 3 +++
> include/hw/acpi/memory_hotplug.h | 9 +++--
> include/hw/i386/pc.h | 3 +++
> 5 files changed, 30 insertions(+), 18 deletions(-)
>
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 297812d5f7..1734d4b44f 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -29,12 +29,7 @@
> #define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
> #define MEMORY_SLOT_EJECT_METHOD "MEJ0"
> #define MEMORY_SLOT_NOTIFY_METHOD"MTFY"
> -#define MEMORY_SLOT_SCAN_METHOD "MSCN"
> #define MEMORY_HOTPLUG_DEVICE"MHPD"
> -#define MEMORY_HOTPLUG_IO_LEN 24
> -#define MEMORY_DEVICES_CONTAINER "\\_SB.MHPC"
> -
> -static uint16_t memhp_io_base;
>
> static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
> {
> @@ -209,7 +204,7 @@ static const MemoryRegionOps acpi_memory_hotplug_ops = {
> };
>
> void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
> - MemHotplugState *state, uint16_t io_base)
> + MemHotplugState *state, hwaddr io_base)
> {
> MachineState *machine = MACHINE(qdev_get_machine());
>
> @@ -218,12 +213,10 @@ void acpi_memory_hotplug_init(MemoryRegion *as, Object
> *owner,
> return;
> }
>
> -assert(!memhp_io_base);
> -memhp_io_base = io_base;
> state->devs = g_malloc0(sizeof(*state->devs) * state->dev_count);
> memory_region_init_io(&state->io, owner, &acpi_memory_hotplug_ops, state,
>"acpi-mem-hotplug", MEMORY_HOTPLUG_IO_LEN);
> -memory_region_add_subregion(as, memhp_io_base, &state->io);
> +memory_region_add_subregion(as, io_base, &state->io);
> }
>
> /**
> @@ -342,7 +335,8 @@ const VMStateDescription vmstate_memory_hotplug = {
>
> void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
>const char *res_root,
> - const char *event_handler_method)
> + const char *event_handler_method,
> + AmlRegionSpace rs, hwaddr memhp_io_base)
> {
> int i;
> Aml *ifctx;
> @@ -365,14 +359,19 @@ void build_memory_hotplug_aml(Aml *table, uint32_t
> nr_mem,
> aml_name_decl("_UID", aml_string("Memory hotplug resources")));
do you want to keep
if (!memhp_io_base) {
return;
}
at the beginning of the function? I think we should rather call the
function only if all args are resolved?
>
> crs = aml_resource_template();
> -aml_append(crs,
> -aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
> - MEMORY_HOTPLUG_IO_LEN)
> -);
> +if (rs == AML_SYSTEM_IO) {
> +aml_append(crs,
> +aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
> + MEMORY_HOTPLUG_IO_LEN)
> +);
> +} else {
> +aml_append(crs, aml_memory32_fixed(memhp_io_base,
> +MEMORY_HOTPLUG_IO_LEN, AML_READ_WRITE));
> +}
> aml_append(mem_ctrl_dev, aml_name_decl("_CRS", crs));
>
> aml_append(mem_ctrl_dev, aml_operation_region(
> -MEMORY_HOTPLUG_IO_REGION, AML_SYSTEM_IO,
> +MEMORY_HOTPLUG_IO_REGION, rs,
> aml_int(memhp_io_base), MEMORY_HOTPLUG_IO_LEN)
> );
>
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index f3fdfefcd5..e76d6631ea 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1871,7 +1871,9 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
> "\\_SB.PCI0", "\\_GPE._E02");
> }
> -build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0", "\\_GPE._E03");
> +build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
> + "\\_GPE._E03", AML_SYSTEM_IO,
> + pcms->memhp_io_base);
>
> scope = aml_scope("_GPE");
> {
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 549c437050..
Re: [Qemu-devel] [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space configurable
On Thu, 15 Aug 2019 08:42:48 +
Shameerali Kolothum Thodi wrote:
> > -Original Message-
> > From: Linuxarm [mailto:linuxarm-boun...@huawei.com] On Behalf Of Shameer
> > Kolothum
> > Sent: 13 August 2019 22:05
> > To: qemu-devel@nongnu.org; qemu-...@nongnu.org;
> > eric.au...@redhat.com; imamm...@redhat.com
> > Cc: peter.mayd...@linaro.org; sa...@linux.intel.com;
> > ard.biesheu...@linaro.org; Linuxarm ;
> > shannon.zha...@gmail.com; sebastien.bo...@intel.com; ler...@redhat.com
> > Subject: [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space
> > configurable
> >
> > This is in preparation for adding support for ARM64 platforms
> > where it doesn't use port mapped IO for ACPI IO space. We are
> > making changes so that MMIO region can be accommodated
> > and board can pass the base address into the aml build function.
>
> Looks like, this now breaks the "make check" on x86_64 and needs
> updating bios-tables-test-allowed-diff.h with DSDT entries. But I am
> not sure what changed now compared to v8(and older ones) that makes
> it to complain now!.
you could see diff of what's changed but running test manually with
V=1 env var if you have 'iasl' installed
V=1 QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64 tests/bios-tables-test
> Patchew URL:
> https://patchew.org/QEMU/20190813210539.31164-1-shameerali.kolothum.th...@huawei.com/
>
> ERROR:/tmp/qemu-test/src/tests/bios-tables-test.c:447:test_acpi_asl:
> assertion failed: (all_tables_match)
>
> Thanks,
> Shameer
>
> > Also move few MEMORY_* definitions to header so that other memory
> > hotplug event signalling mechanisms (eg. Generic Event Device on
> > HW-reduced acpi platforms) can use the same from their respective
> > event handler code.
> >
> > Signed-off-by: Shameer Kolothum
> > ---
> > v8 --> v9
> > -base address is an input into build_memory_hotplug_aml()
> > -Removed R-by tags from Igor and Eric for now.
> > ---
> > hw/acpi/memory_hotplug.c | 29 ++---
> > hw/i386/acpi-build.c | 4 +++-
> > hw/i386/pc.c | 3 +++
> > include/hw/acpi/memory_hotplug.h | 9 +++--
> > include/hw/i386/pc.h | 3 +++
> > 5 files changed, 30 insertions(+), 18 deletions(-)
> >
> > diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> > index 297812d5f7..1734d4b44f 100644
> > --- a/hw/acpi/memory_hotplug.c
> > +++ b/hw/acpi/memory_hotplug.c
> > @@ -29,12 +29,7 @@
> > #define MEMORY_SLOT_PROXIMITY_METHOD "MPXM"
> > #define MEMORY_SLOT_EJECT_METHOD "MEJ0"
> > #define MEMORY_SLOT_NOTIFY_METHOD"MTFY"
> > -#define MEMORY_SLOT_SCAN_METHOD "MSCN"
> > #define MEMORY_HOTPLUG_DEVICE"MHPD"
> > -#define MEMORY_HOTPLUG_IO_LEN 24
> > -#define MEMORY_DEVICES_CONTAINER "\\_SB.MHPC"
> > -
> > -static uint16_t memhp_io_base;
> >
> > static ACPIOSTInfo *acpi_memory_device_status(int slot, MemStatus *mdev)
> > {
> > @@ -209,7 +204,7 @@ static const MemoryRegionOps
> > acpi_memory_hotplug_ops = {
> > };
> >
> > void acpi_memory_hotplug_init(MemoryRegion *as, Object *owner,
> > - MemHotplugState *state, uint16_t
> > io_base)
> > + MemHotplugState *state, hwaddr
> > io_base)
> > {
> > MachineState *machine = MACHINE(qdev_get_machine());
> >
> > @@ -218,12 +213,10 @@ void acpi_memory_hotplug_init(MemoryRegion *as,
> > Object *owner,
> > return;
> > }
> >
> > -assert(!memhp_io_base);
> > -memhp_io_base = io_base;
> > state->devs = g_malloc0(sizeof(*state->devs) * state->dev_count);
> > memory_region_init_io(&state->io, owner, &acpi_memory_hotplug_ops,
> > state,
> >"acpi-mem-hotplug",
> > MEMORY_HOTPLUG_IO_LEN);
> > -memory_region_add_subregion(as, memhp_io_base, &state->io);
> > +memory_region_add_subregion(as, io_base, &state->io);
> > }
> >
> > /**
> > @@ -342,7 +335,8 @@ const VMStateDescription vmstate_memory_hotplug
> > = {
> >
> > void build_memory_hotplug_aml(Aml *table, uint32_t nr_mem,
> >const char *res_root,
> > - const char *event_handler_method)
> > + const char *event_handler_method,
> > + AmlRegionSpace rs, hwaddr
> > memhp_io_base)
> > {
> > int i;
> > Aml *ifctx;
> > @@ -365,14 +359,19 @@ void build_memory_hotplug_aml(Aml *table,
> > uint32_t nr_mem,
> > aml_name_decl("_UID", aml_string("Memory hotplug
> > resources")));
> >
> > crs = aml_resource_template();
> > -aml_append(crs,
> > -aml_io(AML_DECODE16, memhp_io_base, memhp_io_base, 0,
> > - MEMORY_HOTPLUG_IO_LEN)
> > -);
> > +if (rs == AML_SYSTEM_IO) {
> > +aml_append(crs,
> > +aml_io(AML_DECODE16, memhp_io_base,
> > memhp_io_base, 0,
> > + MEMORY_HOTPLUG_IO_
Re: [Qemu-devel] [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space configurable
Hi Igor,
> -Original Message-
> From: Igor Mammedov [mailto:imamm...@redhat.com]
> Sent: 29 August 2019 09:45
> To: Shameerali Kolothum Thodi
> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org;
> eric.au...@redhat.com; peter.mayd...@linaro.org; sa...@linux.intel.com;
> ard.biesheu...@linaro.org; Linuxarm ;
> shannon.zha...@gmail.com; sebastien.bo...@intel.com; ler...@redhat.com
> Subject: Re: [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space
> configurable
>
> On Thu, 15 Aug 2019 08:42:48 +
> Shameerali Kolothum Thodi wrote:
>
> > > -Original Message-
> > > From: Linuxarm [mailto:linuxarm-boun...@huawei.com] On Behalf Of
> Shameer
> > > Kolothum
> > > Sent: 13 August 2019 22:05
> > > To: qemu-devel@nongnu.org; qemu-...@nongnu.org;
> > > eric.au...@redhat.com; imamm...@redhat.com
> > > Cc: peter.mayd...@linaro.org; sa...@linux.intel.com;
> > > ard.biesheu...@linaro.org; Linuxarm ;
> > > shannon.zha...@gmail.com; sebastien.bo...@intel.com;
> ler...@redhat.com
> > > Subject: [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space
> > > configurable
> > >
> > > This is in preparation for adding support for ARM64 platforms
> > > where it doesn't use port mapped IO for ACPI IO space. We are
> > > making changes so that MMIO region can be accommodated
> > > and board can pass the base address into the aml build function.
> >
> > Looks like, this now breaks the "make check" on x86_64 and needs
> > updating bios-tables-test-allowed-diff.h with DSDT entries. But I am
> > not sure what changed now compared to v8(and older ones) that makes
> > it to complain now!.
>
> you could see diff of what's changed but running test manually with
> V=1 env var if you have 'iasl' installed
>
> V=1 QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> tests/bios-tables-test
Thanks for that tip and please find below output.
/x86_64/acpi/piix4: Could not access KVM kernel module: No such file or
directory
qemu-system-x86_64: failed to initialize KVM: No such file or directory
qemu-system-x86_64: Back to tcg accelerator
acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-RGE76Z],
Expected [aml:tests/data/acpi/pc/DSDT].
acpi-test: Warning! DSDT mismatch. Actual [asl:/tmp/asl-TAE76Z.dsl,
aml:/tmp/aml-RGE76Z], Expected [asl:/tmp/asl-O6B76Z.dsl,
aml:tests/data/acpi/pc/DSDT].
diff --git a/tmp/asl-O6B76Z.dsl b/tmp/asl-TAE76Z.dsl
index 823ff002ec..4de5bd3221 100644
--- a/tmp/asl-O6B76Z.dsl
+++ b/tmp/asl-TAE76Z.dsl
@@ -5,13 +5,13 @@
*
* Disassembling to symbolic ASL+ operators
*
- * Disassembly of tests/data/acpi/pc/DSDT, Thu Aug 29 10:40:40 2019
+ * Disassembly of /tmp/aml-RGE76Z, Thu Aug 29 10:40:40 2019
*
* Original Table Header:
* Signature"DSDT"
- * Length 0x140B (5131)
+ * Length 0x17E4 (6116)
* Revision 0x01 32-bit table (V1), no 64-bit math support
- * Checksum 0xB1
+ * Checksum 0x8B
* OEM ID "BOCHS "
* OEM Table ID "BXPCDSDT"
* OEM Revision 0x0001 (1)
@@ -787,6 +787,206 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT",
0x0001)
\_SB.CPUS.CSCN ()
}
+Device (\_SB.PCI0.MHPD)
+{
+Name (_HID, "PNP0A06" /* Generic Container Device */) // _HID:
Hardware ID
+Name (_UID, "Memory hotplug resources") // _UID: Unique ID
+Name (_CRS, Reso
I think what happens is since we are now passing the memhp_io_base directly
into the
build_memory_hotplug_aml() and removed the "static uint16_t memhp_io_base", on
x86, memory hotplug aml code is always built by default irrespective of whether
acpi_memory_hotplug_init() is invoked or not.
I could either reintroduce a check in build_memory_hotplug_aml() to make sure
acpi_memory_hotplug_init() is called, or could do something like below,
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 3995f9a40f..17756c2191 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -1873,9 +1873,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
"\\_SB.PCI0", "\\_GPE._E02");
}
-build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
- "\\_GPE._E03", AML_SYSTEM_IO,
- pcms->memhp_io_base);
+
+if (acpi_enabled && pcms->acpi_dev && nr_mem) {
+build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
+ "\\_GPE._E03", AML_SYSTEM_IO,
+ pcms->memhp_io_base);
+}
I prefer the latter if there are no other issues with that. Please let me know.
Thanks,
Shameer
>
> > Patchew URL:
> https://patchew.org/QEMU/20190813210539.31164-1-shameerali.kolothum.t
> h...@huawei.com/
> >
> > ERROR:/tmp/qemu-test/src/tests/bios-tables-test.c:447:test_acpi_asl:
> assertion failed: (all_tables_match)
> >
> > Thanks,
> > Shameer
> >
> > >
Re: [Qemu-devel] [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space configurable
On Thu, 29 Aug 2019 11:04:27 +
Shameerali Kolothum Thodi wrote:
> Hi Igor,
>
> > -Original Message-
> > From: Igor Mammedov [mailto:imamm...@redhat.com]
> > Sent: 29 August 2019 09:45
> > To: Shameerali Kolothum Thodi
> > Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org;
> > eric.au...@redhat.com; peter.mayd...@linaro.org; sa...@linux.intel.com;
> > ard.biesheu...@linaro.org; Linuxarm ;
> > shannon.zha...@gmail.com; sebastien.bo...@intel.com; ler...@redhat.com
> > Subject: Re: [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space
> > configurable
> >
> > On Thu, 15 Aug 2019 08:42:48 +
> > Shameerali Kolothum Thodi wrote:
> >
> > > > -Original Message-
> > > > From: Linuxarm [mailto:linuxarm-boun...@huawei.com] On Behalf Of
> > Shameer
> > > > Kolothum
> > > > Sent: 13 August 2019 22:05
> > > > To: qemu-devel@nongnu.org; qemu-...@nongnu.org;
> > > > eric.au...@redhat.com; imamm...@redhat.com
> > > > Cc: peter.mayd...@linaro.org; sa...@linux.intel.com;
> > > > ard.biesheu...@linaro.org; Linuxarm ;
> > > > shannon.zha...@gmail.com; sebastien.bo...@intel.com;
> > ler...@redhat.com
> > > > Subject: [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space
> > > > configurable
> > > >
> > > > This is in preparation for adding support for ARM64 platforms
> > > > where it doesn't use port mapped IO for ACPI IO space. We are
> > > > making changes so that MMIO region can be accommodated
> > > > and board can pass the base address into the aml build function.
> > >
> > > Looks like, this now breaks the "make check" on x86_64 and needs
> > > updating bios-tables-test-allowed-diff.h with DSDT entries. But I am
> > > not sure what changed now compared to v8(and older ones) that makes
> > > it to complain now!.
> >
> > you could see diff of what's changed but running test manually with
> > V=1 env var if you have 'iasl' installed
> >
> > V=1 QTEST_QEMU_BINARY=x86_64-softmmu/qemu-system-x86_64
> > tests/bios-tables-test
>
> Thanks for that tip and please find below output.
>
> /x86_64/acpi/piix4: Could not access KVM kernel module: No such file or
> directory
> qemu-system-x86_64: failed to initialize KVM: No such file or directory
> qemu-system-x86_64: Back to tcg accelerator
> acpi-test: Warning! DSDT binary file mismatch. Actual [aml:/tmp/aml-RGE76Z],
> Expected [aml:tests/data/acpi/pc/DSDT].
> acpi-test: Warning! DSDT mismatch. Actual [asl:/tmp/asl-TAE76Z.dsl,
> aml:/tmp/aml-RGE76Z], Expected [asl:/tmp/asl-O6B76Z.dsl,
> aml:tests/data/acpi/pc/DSDT].
>
> diff --git a/tmp/asl-O6B76Z.dsl b/tmp/asl-TAE76Z.dsl
> index 823ff002ec..4de5bd3221 100644
> --- a/tmp/asl-O6B76Z.dsl
> +++ b/tmp/asl-TAE76Z.dsl
> @@ -5,13 +5,13 @@
> *
> * Disassembling to symbolic ASL+ operators
> *
> - * Disassembly of tests/data/acpi/pc/DSDT, Thu Aug 29 10:40:40 2019
> + * Disassembly of /tmp/aml-RGE76Z, Thu Aug 29 10:40:40 2019
> *
> * Original Table Header:
> * Signature"DSDT"
> - * Length 0x140B (5131)
> + * Length 0x17E4 (6116)
> * Revision 0x01 32-bit table (V1), no 64-bit math support
> - * Checksum 0xB1
> + * Checksum 0x8B
> * OEM ID "BOCHS "
> * OEM Table ID "BXPCDSDT"
> * OEM Revision 0x0001 (1)
> @@ -787,6 +787,206 @@ DefinitionBlock ("", "DSDT", 1, "BOCHS ", "BXPCDSDT",
> 0x0001)
> \_SB.CPUS.CSCN ()
> }
>
> +Device (\_SB.PCI0.MHPD)
> +{
> +Name (_HID, "PNP0A06" /* Generic Container Device */) // _HID:
> Hardware ID
> +Name (_UID, "Memory hotplug resources") // _UID: Unique ID
> +Name (_CRS, Reso
>
> I think what happens is since we are now passing the memhp_io_base directly
> into the
> build_memory_hotplug_aml() and removed the "static uint16_t memhp_io_base",
> on
> x86, memory hotplug aml code is always built by default irrespective of
> whether
> acpi_memory_hotplug_init() is invoked or not.
>
> I could either reintroduce a check in build_memory_hotplug_aml() to make sure
> acpi_memory_hotplug_init() is called, or could do something like below,
fix looks fine to me, see minor comment below
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index 3995f9a40f..17756c2191 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -1873,9 +1873,12 @@ build_dsdt(GArray *table_data, BIOSLinker *linker,
> build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
> "\\_SB.PCI0", "\\_GPE._E02");
> }
> -build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
> - "\\_GPE._E03", AML_SYSTEM_IO,
> - pcms->memhp_io_base);
> +
> +if (acpi_enabled && pcms->acpi_dev && nr_mem) {
double-check call path and see if
acpi_enabled && pcms->acpi_dev
is really necessary
> +build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
> + "\
Re: [Qemu-devel] [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space configurable
> -Original Message-
> From: Igor Mammedov [mailto:imamm...@redhat.com]
> Sent: 29 August 2019 13:38
> To: Shameerali Kolothum Thodi
> Cc: qemu-devel@nongnu.org; qemu-...@nongnu.org;
> eric.au...@redhat.com; peter.mayd...@linaro.org; sa...@linux.intel.com;
> ard.biesheu...@linaro.org; Linuxarm ;
> shannon.zha...@gmail.com; sebastien.bo...@intel.com; ler...@redhat.com
> Subject: Re: [PATCH-for-4.2 v9 01/12] hw/acpi: Make ACPI IO address space
> configurable
>
> On Thu, 29 Aug 2019 11:04:27 +
> Shameerali Kolothum Thodi wrote:
[...]
> >
> > I think what happens is since we are now passing the memhp_io_base
> directly into the
> > build_memory_hotplug_aml() and removed the "static uint16_t
> memhp_io_base", on
> > x86, memory hotplug aml code is always built by default irrespective of
> whether
> > acpi_memory_hotplug_init() is invoked or not.
> >
> > I could either reintroduce a check in build_memory_hotplug_aml() to make
> sure
> > acpi_memory_hotplug_init() is called, or could do something like below,
> fix looks fine to me, see minor comment below
Ok
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index 3995f9a40f..17756c2191 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -1873,9 +1873,12 @@ build_dsdt(GArray *table_data, BIOSLinker
> *linker,
> > build_cpus_aml(dsdt, machine, opts, pm->cpu_hp_io_base,
> > "\\_SB.PCI0", "\\_GPE._E02");
> > }
> > -build_memory_hotplug_aml(dsdt, nr_mem, "\\_SB.PCI0",
> > - "\\_GPE._E03", AML_SYSTEM_IO,
> > - pcms->memhp_io_base);
> > +
> > +if (acpi_enabled && pcms->acpi_dev && nr_mem) {
> double-check call path and see if
> acpi_enabled && pcms->acpi_dev
> is really necessary
Right, looks like those are always true. I will remove those.
Also appreciate if you could take a look at rest of the series and then I can
re-spin along with this.
Thanks,
Shameer
