| Message ID | 20230712163943.98994-2-sunilvl@ventanamicro.com |
|---|---|
| State | New |
| Headers | show |
| Series | RISC-V: ACPI: Enable AIA and update RHC | expand |
On 7/12/23 13:39, Sunil V L wrote: > The functions which add fw_cfg and virtio to DSDT are same for ARM > and RISC-V. So, instead of duplicating in RISC-V, move them from > hw/arm/virt-acpi-build.c to common aml-build.c. Nice. > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > --- Reviewed-by: Daniel Henrique Barboza <dbarboza@ventanamicro.com> > hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++ > hw/arm/virt-acpi-build.c | 42 ------------------------------------- > hw/riscv/virt-acpi-build.c | 16 -------------- > include/hw/acpi/aml-build.h | 6 ++++++ > 4 files changed, 47 insertions(+), 58 deletions(-) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index ea331a20d1..eeb1263c8c 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) > > return var; > } > + > +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > +{ > + Aml *dev = aml_device("FWCF"); > + aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > + /* device present, functioning, decoding, not shown in UI */ > + aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > + > + Aml *crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > + fw_cfg_memmap->size, AML_READ_WRITE)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > +} > + > +void acpi_dsdt_add_virtio(Aml *scope, > + const MemMapEntry *virtio_mmio_memmap, > + uint32_t mmio_irq, int num) > +{ > + hwaddr base = virtio_mmio_memmap->base; > + hwaddr size = virtio_mmio_memmap->size; > + int i; > + > + for (i = 0; i < num; i++) { > + uint32_t irq = mmio_irq + i; > + Aml *dev = aml_device("VR%02u", i); > + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(i))); > + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > + > + Aml *crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); > + aml_append(crs, > + aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, &irq, 1)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > + base += size; > + } > +} > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 6b674231c2..fdedb68e2b 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -35,7 +35,6 @@ > #include "target/arm/cpu.h" > #include "hw/acpi/acpi-defs.h" > #include "hw/acpi/acpi.h" > -#include "hw/nvram/fw_cfg.h" > #include "hw/acpi/bios-linker-loader.h" > #include "hw/acpi/aml-build.h" > #include "hw/acpi/utils.h" > @@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, > aml_append(scope, dev); > } > > -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > -{ > - Aml *dev = aml_device("FWCF"); > - aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > - /* device present, functioning, decoding, not shown in UI */ > - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > - fw_cfg_memmap->size, AML_READ_WRITE)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > -} > - > static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) > { > Aml *dev, *crs; > @@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) > aml_append(scope, dev); > } > > -static void acpi_dsdt_add_virtio(Aml *scope, > - const MemMapEntry *virtio_mmio_memmap, > - uint32_t mmio_irq, int num) > -{ > - hwaddr base = virtio_mmio_memmap->base; > - hwaddr size = virtio_mmio_memmap->size; > - int i; > - > - for (i = 0; i < num; i++) { > - uint32_t irq = mmio_irq + i; > - Aml *dev = aml_device("VR%02u", i); > - aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > - aml_append(dev, aml_name_decl("_UID", aml_int(i))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); > - aml_append(crs, > - aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > - AML_EXCLUSIVE, &irq, 1)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > - base += size; > - } > -} > - > static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, > uint32_t irq, VirtMachineState *vms) > { > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c > index 7331248f59..01843e4509 100644 > --- a/hw/riscv/virt-acpi-build.c > +++ b/hw/riscv/virt-acpi-build.c > @@ -97,22 +97,6 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s) > } > } > > -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > -{ > - Aml *dev = aml_device("FWCF"); > - aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > - > - /* device present, functioning, decoding, not shown in UI */ > - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > - fw_cfg_memmap->size, AML_READ_WRITE)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > -} > - > /* RHCT Node[N] starts at offset 56 */ > #define RHCT_NODE_ARRAY_OFFSET 56 > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index d1fb08514b..c4a8967310 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -3,6 +3,7 @@ > > #include "hw/acpi/acpi-defs.h" > #include "hw/acpi/bios-linker-loader.h" > +#include "hw/nvram/fw_cfg.h" > > #define ACPI_BUILD_APPNAME6 "BOCHS " > #define ACPI_BUILD_APPNAME8 "BXPC " > @@ -497,4 +498,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f, > > void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog, > const char *oem_id, const char *oem_table_id); > + > +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap); > +void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap, > + uint32_t mmio_irq, int num); > + > #endif
On Thu, Jul 13, 2023 at 2:41 AM Sunil V L <sunilvl@ventanamicro.com> wrote: > > The functions which add fw_cfg and virtio to DSDT are same for ARM > and RISC-V. So, instead of duplicating in RISC-V, move them from > hw/arm/virt-acpi-build.c to common aml-build.c. > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> Reviewed-by: Alistair Francis <alistair.francis@wdc.com> Alistair > --- > hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++ > hw/arm/virt-acpi-build.c | 42 ------------------------------------- > hw/riscv/virt-acpi-build.c | 16 -------------- > include/hw/acpi/aml-build.h | 6 ++++++ > 4 files changed, 47 insertions(+), 58 deletions(-) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > index ea331a20d1..eeb1263c8c 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) > > return var; > } > + > +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > +{ > + Aml *dev = aml_device("FWCF"); > + aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > + /* device present, functioning, decoding, not shown in UI */ > + aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > + > + Aml *crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > + fw_cfg_memmap->size, AML_READ_WRITE)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > +} > + > +void acpi_dsdt_add_virtio(Aml *scope, > + const MemMapEntry *virtio_mmio_memmap, > + uint32_t mmio_irq, int num) > +{ > + hwaddr base = virtio_mmio_memmap->base; > + hwaddr size = virtio_mmio_memmap->size; > + int i; > + > + for (i = 0; i < num; i++) { > + uint32_t irq = mmio_irq + i; > + Aml *dev = aml_device("VR%02u", i); > + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(i))); > + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > + > + Aml *crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); > + aml_append(crs, > + aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, &irq, 1)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > + base += size; > + } > +} > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 6b674231c2..fdedb68e2b 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -35,7 +35,6 @@ > #include "target/arm/cpu.h" > #include "hw/acpi/acpi-defs.h" > #include "hw/acpi/acpi.h" > -#include "hw/nvram/fw_cfg.h" > #include "hw/acpi/bios-linker-loader.h" > #include "hw/acpi/aml-build.h" > #include "hw/acpi/utils.h" > @@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, > aml_append(scope, dev); > } > > -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > -{ > - Aml *dev = aml_device("FWCF"); > - aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > - /* device present, functioning, decoding, not shown in UI */ > - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > - fw_cfg_memmap->size, AML_READ_WRITE)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > -} > - > static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) > { > Aml *dev, *crs; > @@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) > aml_append(scope, dev); > } > > -static void acpi_dsdt_add_virtio(Aml *scope, > - const MemMapEntry *virtio_mmio_memmap, > - uint32_t mmio_irq, int num) > -{ > - hwaddr base = virtio_mmio_memmap->base; > - hwaddr size = virtio_mmio_memmap->size; > - int i; > - > - for (i = 0; i < num; i++) { > - uint32_t irq = mmio_irq + i; > - Aml *dev = aml_device("VR%02u", i); > - aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > - aml_append(dev, aml_name_decl("_UID", aml_int(i))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); > - aml_append(crs, > - aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > - AML_EXCLUSIVE, &irq, 1)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > - base += size; > - } > -} > - > static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, > uint32_t irq, VirtMachineState *vms) > { > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c > index 7331248f59..01843e4509 100644 > --- a/hw/riscv/virt-acpi-build.c > +++ b/hw/riscv/virt-acpi-build.c > @@ -97,22 +97,6 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s) > } > } > > -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > -{ > - Aml *dev = aml_device("FWCF"); > - aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > - > - /* device present, functioning, decoding, not shown in UI */ > - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > - fw_cfg_memmap->size, AML_READ_WRITE)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > -} > - > /* RHCT Node[N] starts at offset 56 */ > #define RHCT_NODE_ARRAY_OFFSET 56 > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index d1fb08514b..c4a8967310 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -3,6 +3,7 @@ > > #include "hw/acpi/acpi-defs.h" > #include "hw/acpi/bios-linker-loader.h" > +#include "hw/nvram/fw_cfg.h" > > #define ACPI_BUILD_APPNAME6 "BOCHS " > #define ACPI_BUILD_APPNAME8 "BXPC " > @@ -497,4 +498,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f, > > void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog, > const char *oem_id, const char *oem_table_id); > + > +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap); > +void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap, > + uint32_t mmio_irq, int num); > + > #endif > -- > 2.39.2 > >
On Wed, 12 Jul 2023 22:09:34 +0530 Sunil V L <sunilvl@ventanamicro.com> wrote: > The functions which add fw_cfg and virtio to DSDT are same for ARM > and RISC-V. So, instead of duplicating in RISC-V, move them from > hw/arm/virt-acpi-build.c to common aml-build.c. > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > --- > hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++ > hw/arm/virt-acpi-build.c | 42 ------------------------------------- > hw/riscv/virt-acpi-build.c | 16 -------------- > include/hw/acpi/aml-build.h | 6 ++++++ > 4 files changed, 47 insertions(+), 58 deletions(-) > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c patch looks fine modulo, I'd put these into respective device files instead of generic aml-build.c which was intended for basic AML primitives (it 's got polluted over time with device specific functions but that's not the reason to continue doing that). Also having those functions along with devices models goes along with self enumerating ACPI devices (currently it works for x86 PCI/ISA device but there is no reason that it can't work with other types as well when I get there) > index ea331a20d1..eeb1263c8c 100644 > --- a/hw/acpi/aml-build.c > +++ b/hw/acpi/aml-build.c > @@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) > > return var; > } > + > +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > +{ > + Aml *dev = aml_device("FWCF"); > + aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > + /* device present, functioning, decoding, not shown in UI */ > + aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > + > + Aml *crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > + fw_cfg_memmap->size, AML_READ_WRITE)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > +} > + > +void acpi_dsdt_add_virtio(Aml *scope, > + const MemMapEntry *virtio_mmio_memmap, > + uint32_t mmio_irq, int num) > +{ > + hwaddr base = virtio_mmio_memmap->base; > + hwaddr size = virtio_mmio_memmap->size; > + int i; > + > + for (i = 0; i < num; i++) { > + uint32_t irq = mmio_irq + i; > + Aml *dev = aml_device("VR%02u", i); > + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > + aml_append(dev, aml_name_decl("_UID", aml_int(i))); > + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > + > + Aml *crs = aml_resource_template(); > + aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); > + aml_append(crs, > + aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > + AML_EXCLUSIVE, &irq, 1)); > + aml_append(dev, aml_name_decl("_CRS", crs)); > + aml_append(scope, dev); > + base += size; > + } > +} > diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c > index 6b674231c2..fdedb68e2b 100644 > --- a/hw/arm/virt-acpi-build.c > +++ b/hw/arm/virt-acpi-build.c > @@ -35,7 +35,6 @@ > #include "target/arm/cpu.h" > #include "hw/acpi/acpi-defs.h" > #include "hw/acpi/acpi.h" > -#include "hw/nvram/fw_cfg.h" > #include "hw/acpi/bios-linker-loader.h" > #include "hw/acpi/aml-build.h" > #include "hw/acpi/utils.h" > @@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, > aml_append(scope, dev); > } > > -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > -{ > - Aml *dev = aml_device("FWCF"); > - aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > - /* device present, functioning, decoding, not shown in UI */ > - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > - fw_cfg_memmap->size, AML_READ_WRITE)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > -} > - > static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) > { > Aml *dev, *crs; > @@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) > aml_append(scope, dev); > } > > -static void acpi_dsdt_add_virtio(Aml *scope, > - const MemMapEntry *virtio_mmio_memmap, > - uint32_t mmio_irq, int num) > -{ > - hwaddr base = virtio_mmio_memmap->base; > - hwaddr size = virtio_mmio_memmap->size; > - int i; > - > - for (i = 0; i < num; i++) { > - uint32_t irq = mmio_irq + i; > - Aml *dev = aml_device("VR%02u", i); > - aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); > - aml_append(dev, aml_name_decl("_UID", aml_int(i))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); > - aml_append(crs, > - aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, > - AML_EXCLUSIVE, &irq, 1)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > - base += size; > - } > -} > - > static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, > uint32_t irq, VirtMachineState *vms) > { > diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c > index 7331248f59..01843e4509 100644 > --- a/hw/riscv/virt-acpi-build.c > +++ b/hw/riscv/virt-acpi-build.c > @@ -97,22 +97,6 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s) > } > } > > -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) > -{ > - Aml *dev = aml_device("FWCF"); > - aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); > - > - /* device present, functioning, decoding, not shown in UI */ > - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); > - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); > - > - Aml *crs = aml_resource_template(); > - aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, > - fw_cfg_memmap->size, AML_READ_WRITE)); > - aml_append(dev, aml_name_decl("_CRS", crs)); > - aml_append(scope, dev); > -} > - > /* RHCT Node[N] starts at offset 56 */ > #define RHCT_NODE_ARRAY_OFFSET 56 > > diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h > index d1fb08514b..c4a8967310 100644 > --- a/include/hw/acpi/aml-build.h > +++ b/include/hw/acpi/aml-build.h > @@ -3,6 +3,7 @@ > > #include "hw/acpi/acpi-defs.h" > #include "hw/acpi/bios-linker-loader.h" > +#include "hw/nvram/fw_cfg.h" > > #define ACPI_BUILD_APPNAME6 "BOCHS " > #define ACPI_BUILD_APPNAME8 "BXPC " > @@ -497,4 +498,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f, > > void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog, > const char *oem_id, const char *oem_table_id); > + > +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap); > +void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap, > + uint32_t mmio_irq, int num); > + > #endif
On Mon, Jul 24, 2023 at 05:18:59PM +0200, Igor Mammedov wrote: > On Wed, 12 Jul 2023 22:09:34 +0530 > Sunil V L <sunilvl@ventanamicro.com> wrote: > > > The functions which add fw_cfg and virtio to DSDT are same for ARM > > and RISC-V. So, instead of duplicating in RISC-V, move them from > > hw/arm/virt-acpi-build.c to common aml-build.c. > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > --- > > hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++ > > hw/arm/virt-acpi-build.c | 42 ------------------------------------- > > hw/riscv/virt-acpi-build.c | 16 -------------- > > include/hw/acpi/aml-build.h | 6 ++++++ > > 4 files changed, 47 insertions(+), 58 deletions(-) > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > patch looks fine modulo, > I'd put these into respective device files instead of generic > aml-build.c which was intended for basic AML primitives > (it 's got polluted over time with device specific functions > but that's not the reason to continue doing that). > > Also having those functions along with devices models > goes along with self enumerating ACPI devices (currently > it works for x86 PCI/ISA device but there is no reason > that it can't work with other types as well when > I get there) > Thanks!, Igor. Let me add them to device specific files as per your recommendation. Thanks! Sunil
On Tue, 25 Jul 2023 22:20:36 +0530 Sunil V L <sunilvl@ventanamicro.com> wrote: > On Mon, Jul 24, 2023 at 05:18:59PM +0200, Igor Mammedov wrote: > > On Wed, 12 Jul 2023 22:09:34 +0530 > > Sunil V L <sunilvl@ventanamicro.com> wrote: > > > > > The functions which add fw_cfg and virtio to DSDT are same for ARM > > > and RISC-V. So, instead of duplicating in RISC-V, move them from > > > hw/arm/virt-acpi-build.c to common aml-build.c. > > > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > > --- > > > hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++ > > > hw/arm/virt-acpi-build.c | 42 ------------------------------------- > > > hw/riscv/virt-acpi-build.c | 16 -------------- > > > include/hw/acpi/aml-build.h | 6 ++++++ > > > 4 files changed, 47 insertions(+), 58 deletions(-) > > > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > > > patch looks fine modulo, > > I'd put these into respective device files instead of generic > > aml-build.c which was intended for basic AML primitives > > (it 's got polluted over time with device specific functions > > but that's not the reason to continue doing that). > > > > Also having those functions along with devices models > > goes along with self enumerating ACPI devices (currently > > it works for x86 PCI/ISA device but there is no reason > > that it can't work with other types as well when > > I get there) > > > Thanks!, Igor. Let me add them to device specific files as per your > recommendation. just be careful and build test other targets (while disabling the rest) at least no to regress them due to build deps. (I'd pick 2 with ACPI support that use and not uses affected code) and 1 that uses device model but doesn't use ACPI at all (if such exists) > > Thanks! > Sunil >
On 7/26/23 05:25, Igor Mammedov wrote: > On Tue, 25 Jul 2023 22:20:36 +0530 > Sunil V L <sunilvl@ventanamicro.com> wrote: > >> On Mon, Jul 24, 2023 at 05:18:59PM +0200, Igor Mammedov wrote: >>> On Wed, 12 Jul 2023 22:09:34 +0530 >>> Sunil V L <sunilvl@ventanamicro.com> wrote: >>> >>>> The functions which add fw_cfg and virtio to DSDT are same for ARM >>>> and RISC-V. So, instead of duplicating in RISC-V, move them from >>>> hw/arm/virt-acpi-build.c to common aml-build.c. >>>> >>>> Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> >>>> --- >>>> hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++ >>>> hw/arm/virt-acpi-build.c | 42 ------------------------------------- >>>> hw/riscv/virt-acpi-build.c | 16 -------------- >>>> include/hw/acpi/aml-build.h | 6 ++++++ >>>> 4 files changed, 47 insertions(+), 58 deletions(-) >>>> >>>> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c >>> >>> patch looks fine modulo, >>> I'd put these into respective device files instead of generic >>> aml-build.c which was intended for basic AML primitives >>> (it 's got polluted over time with device specific functions >>> but that's not the reason to continue doing that). >>> >>> Also having those functions along with devices models >>> goes along with self enumerating ACPI devices (currently >>> it works for x86 PCI/ISA device but there is no reason >>> that it can't work with other types as well when >>> I get there) >>> >> Thanks!, Igor. Let me add them to device specific files as per your >> recommendation. > just be careful and build test other targets (while disabling the rest) > at least no to regress them due to build deps. (I'd pick 2 with ACPI > support that use and not uses affected code) and 1 that uses device > model but doesn't use ACPI at all (if such exists) Sunil is already aware of it but I'll also mention here since it seems relevant to Igor's point. This patch breaks i386-softmmu build: FAILED: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o cc -m64 -mcx16 -Ilibqemu-i386-softmmu.fa.p -I. -I.. -Itarget/i386 -I../target/i386 -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -isystem /home/danielhb/work/qemu/linux-headers -isystem linux-headers -iquote . -iquote /home/danielhb/work/qemu -iquote /home/danielhb/work/qemu/include -iquote /home/danielhb/work/qemu/host/include/x86_64 -iquote /home/danielhb/work/qemu/host/include/generic -iquote /home/danielhb/work/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="i386-softmmu-config-target.h"' '-DCONFIG_DEVICES="i386-softmmu-config-devices.h"' -MD -MQ libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o -MF libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o.d -o libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o -c ../hw/i386/acpi-microvm.c ../hw/i386/acpi-microvm.c:48:13: error: conflicting types for ‘acpi_dsdt_add_virtio’; have ‘void(Aml *, MicrovmMachineState *)’ 48 | static void acpi_dsdt_add_virtio(Aml *scope, | ^~~~~~~~~~~~~~~~~~~~ In file included from /home/danielhb/work/qemu/include/hw/acpi/acpi_aml_interface.h:5, from ../hw/i386/acpi-microvm.c:29: /home/danielhb/work/qemu/include/hw/acpi/aml-build.h:503:6: note: previous declaration of ‘acpi_dsdt_add_virtio’ with type ‘void(Aml *, const MemMapEntry *, uint32_t, int)’ {aka ‘void(Aml *, const MemMapEntry *, unsigned int, int)’} 503 | void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap, | ^~~~~~~~~~~~~~~~~~~~ [5/714] Compiling C object libqemu-i386-softmmu.fa.p/hw_i386_kvm_clock.c.o This happens because the common 'acpi_dsdt_add_virtio' function matches a local function with the same name in hw/i386/acpi-microvm.c. We would need to either rename the shared helper or rename the local acpi-microvm function or do something like Igor mentioned to avoid this name collision. Thanks, Daniel > >> >> Thanks! >> Sunil >> >
On Wed, Aug 16, 2023 at 03:51:58PM -0300, Daniel Henrique Barboza wrote: > > > On 7/26/23 05:25, Igor Mammedov wrote: > > On Tue, 25 Jul 2023 22:20:36 +0530 > > Sunil V L <sunilvl@ventanamicro.com> wrote: > > > > > On Mon, Jul 24, 2023 at 05:18:59PM +0200, Igor Mammedov wrote: > > > > On Wed, 12 Jul 2023 22:09:34 +0530 > > > > Sunil V L <sunilvl@ventanamicro.com> wrote: > > > > > The functions which add fw_cfg and virtio to DSDT are same for ARM > > > > > and RISC-V. So, instead of duplicating in RISC-V, move them from > > > > > hw/arm/virt-acpi-build.c to common aml-build.c. > > > > > > > > > > Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> > > > > > --- > > > > > hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++ > > > > > hw/arm/virt-acpi-build.c | 42 ------------------------------------- > > > > > hw/riscv/virt-acpi-build.c | 16 -------------- > > > > > include/hw/acpi/aml-build.h | 6 ++++++ > > > > > 4 files changed, 47 insertions(+), 58 deletions(-) > > > > > > > > > > diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c > > > > > > > > patch looks fine modulo, > > > > I'd put these into respective device files instead of generic > > > > aml-build.c which was intended for basic AML primitives > > > > (it 's got polluted over time with device specific functions > > > > but that's not the reason to continue doing that). > > > > > > > > Also having those functions along with devices models > > > > goes along with self enumerating ACPI devices (currently > > > > it works for x86 PCI/ISA device but there is no reason > > > > that it can't work with other types as well when > > > > I get there) > > > Thanks!, Igor. Let me add them to device specific files as per your > > > recommendation. > > just be careful and build test other targets (while disabling the rest) > > at least no to regress them due to build deps. (I'd pick 2 with ACPI > > support that use and not uses affected code) and 1 that uses device > > model but doesn't use ACPI at all (if such exists) > > Sunil is already aware of it but I'll also mention here since it seems relevant > to Igor's point. > Thanks! Daniel. Yes, I am aware of the issue and will fix it along with Igor's suggestion. I need to fix this irrespective of the approach. Thanks, Sunil > > This patch breaks i386-softmmu build: > > > FAILED: libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o > cc -m64 -mcx16 -Ilibqemu-i386-softmmu.fa.p -I. -I.. -Itarget/i386 -I../target/i386 -Iqapi -Itrace -Iui -Iui/shader -I/usr/include/pixman-1 -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include -I/usr/include/sysprof-4 -fdiagnostics-color=auto -Wall -Winvalid-pch -Werror -std=gnu11 -O2 -g -fstack-protector-strong -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -Wundef -Wwrite-strings -Wmissing-prototypes -Wstrict-prototypes -Wredundant-decls -Wold-style-declaration -Wold-style-definition -Wtype-limits -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wempty-body -Wnested-externs -Wendif-labels -Wexpansion-to-defined -Wimplicit-fallthrough=2 -Wmissing-format-attribute -Wno-missing-include-dirs -Wno-shift-negative-value -Wno-psabi -isystem /home/danielhb/work/qemu/linux-headers -isystem linux-headers -iquote . -iquote /home/danielhb/work/qemu -iquote /home/danielhb/work/qemu/include -iquote /home/danielhb/work/qemu/host/include/x86_64 -iquote /home/danielhb/work/qemu/host/include/generic -iquote /home/danielhb/work/qemu/tcg/i386 -pthread -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -fno-strict-aliasing -fno-common -fwrapv -fPIE -isystem../linux-headers -isystemlinux-headers -DNEED_CPU_H '-DCONFIG_TARGET="i386-softmmu-config-target.h"' '-DCONFIG_DEVICES="i386-softmmu-config-devices.h"' -MD -MQ libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o -MF libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o.d -o libqemu-i386-softmmu.fa.p/hw_i386_acpi-microvm.c.o -c ../hw/i386/acpi-microvm.c > ../hw/i386/acpi-microvm.c:48:13: error: conflicting types for ‘acpi_dsdt_add_virtio’; have ‘void(Aml *, MicrovmMachineState *)’ > 48 | static void acpi_dsdt_add_virtio(Aml *scope, > | ^~~~~~~~~~~~~~~~~~~~ > In file included from /home/danielhb/work/qemu/include/hw/acpi/acpi_aml_interface.h:5, > from ../hw/i386/acpi-microvm.c:29: > /home/danielhb/work/qemu/include/hw/acpi/aml-build.h:503:6: note: previous declaration of ‘acpi_dsdt_add_virtio’ with type ‘void(Aml *, const MemMapEntry *, uint32_t, int)’ {aka ‘void(Aml *, const MemMapEntry *, unsigned int, int)’} > 503 | void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap, > | ^~~~~~~~~~~~~~~~~~~~ > [5/714] Compiling C object libqemu-i386-softmmu.fa.p/hw_i386_kvm_clock.c.o > > This happens because the common 'acpi_dsdt_add_virtio' function matches a local > function with the same name in hw/i386/acpi-microvm.c. We would need to either > rename the shared helper or rename the local acpi-microvm function or do something > like Igor mentioned to avoid this name collision. > > > Thanks, > > Daniel > > > > > > > > > > > > > > > > Thanks! > > > Sunil > > > > >
diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c index ea331a20d1..eeb1263c8c 100644 --- a/hw/acpi/aml-build.c +++ b/hw/acpi/aml-build.c @@ -2467,3 +2467,44 @@ Aml *aml_i2c_serial_bus_device(uint16_t address, const char *resource_source) return var; } + +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) +{ + Aml *dev = aml_device("FWCF"); + aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); + /* device present, functioning, decoding, not shown in UI */ + aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); + + Aml *crs = aml_resource_template(); + aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, + fw_cfg_memmap->size, AML_READ_WRITE)); + aml_append(dev, aml_name_decl("_CRS", crs)); + aml_append(scope, dev); +} + +void acpi_dsdt_add_virtio(Aml *scope, + const MemMapEntry *virtio_mmio_memmap, + uint32_t mmio_irq, int num) +{ + hwaddr base = virtio_mmio_memmap->base; + hwaddr size = virtio_mmio_memmap->size; + int i; + + for (i = 0; i < num; i++) { + uint32_t irq = mmio_irq + i; + Aml *dev = aml_device("VR%02u", i); + aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); + aml_append(dev, aml_name_decl("_UID", aml_int(i))); + aml_append(dev, aml_name_decl("_CCA", aml_int(1))); + + Aml *crs = aml_resource_template(); + aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); + aml_append(crs, + aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, + AML_EXCLUSIVE, &irq, 1)); + aml_append(dev, aml_name_decl("_CRS", crs)); + aml_append(scope, dev); + base += size; + } +} diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c index 6b674231c2..fdedb68e2b 100644 --- a/hw/arm/virt-acpi-build.c +++ b/hw/arm/virt-acpi-build.c @@ -35,7 +35,6 @@ #include "target/arm/cpu.h" #include "hw/acpi/acpi-defs.h" #include "hw/acpi/acpi.h" -#include "hw/nvram/fw_cfg.h" #include "hw/acpi/bios-linker-loader.h" #include "hw/acpi/aml-build.h" #include "hw/acpi/utils.h" @@ -94,21 +93,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap, aml_append(scope, dev); } -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) -{ - Aml *dev = aml_device("FWCF"); - aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); - /* device present, functioning, decoding, not shown in UI */ - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); - - Aml *crs = aml_resource_template(); - aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, - fw_cfg_memmap->size, AML_READ_WRITE)); - aml_append(dev, aml_name_decl("_CRS", crs)); - aml_append(scope, dev); -} - static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) { Aml *dev, *crs; @@ -133,32 +117,6 @@ static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap) aml_append(scope, dev); } -static void acpi_dsdt_add_virtio(Aml *scope, - const MemMapEntry *virtio_mmio_memmap, - uint32_t mmio_irq, int num) -{ - hwaddr base = virtio_mmio_memmap->base; - hwaddr size = virtio_mmio_memmap->size; - int i; - - for (i = 0; i < num; i++) { - uint32_t irq = mmio_irq + i; - Aml *dev = aml_device("VR%02u", i); - aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0005"))); - aml_append(dev, aml_name_decl("_UID", aml_int(i))); - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); - - Aml *crs = aml_resource_template(); - aml_append(crs, aml_memory32_fixed(base, size, AML_READ_WRITE)); - aml_append(crs, - aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH, - AML_EXCLUSIVE, &irq, 1)); - aml_append(dev, aml_name_decl("_CRS", crs)); - aml_append(scope, dev); - base += size; - } -} - static void acpi_dsdt_add_pci(Aml *scope, const MemMapEntry *memmap, uint32_t irq, VirtMachineState *vms) { diff --git a/hw/riscv/virt-acpi-build.c b/hw/riscv/virt-acpi-build.c index 7331248f59..01843e4509 100644 --- a/hw/riscv/virt-acpi-build.c +++ b/hw/riscv/virt-acpi-build.c @@ -97,22 +97,6 @@ static void acpi_dsdt_add_cpus(Aml *scope, RISCVVirtState *s) } } -static void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap) -{ - Aml *dev = aml_device("FWCF"); - aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0002"))); - - /* device present, functioning, decoding, not shown in UI */ - aml_append(dev, aml_name_decl("_STA", aml_int(0xB))); - aml_append(dev, aml_name_decl("_CCA", aml_int(1))); - - Aml *crs = aml_resource_template(); - aml_append(crs, aml_memory32_fixed(fw_cfg_memmap->base, - fw_cfg_memmap->size, AML_READ_WRITE)); - aml_append(dev, aml_name_decl("_CRS", crs)); - aml_append(scope, dev); -} - /* RHCT Node[N] starts at offset 56 */ #define RHCT_NODE_ARRAY_OFFSET 56 diff --git a/include/hw/acpi/aml-build.h b/include/hw/acpi/aml-build.h index d1fb08514b..c4a8967310 100644 --- a/include/hw/acpi/aml-build.h +++ b/include/hw/acpi/aml-build.h @@ -3,6 +3,7 @@ #include "hw/acpi/acpi-defs.h" #include "hw/acpi/bios-linker-loader.h" +#include "hw/nvram/fw_cfg.h" #define ACPI_BUILD_APPNAME6 "BOCHS " #define ACPI_BUILD_APPNAME8 "BXPC " @@ -497,4 +498,9 @@ void build_fadt(GArray *tbl, BIOSLinker *linker, const AcpiFadtData *f, void build_tpm2(GArray *table_data, BIOSLinker *linker, GArray *tcpalog, const char *oem_id, const char *oem_table_id); + +void acpi_dsdt_add_fw_cfg(Aml *scope, const MemMapEntry *fw_cfg_memmap); +void acpi_dsdt_add_virtio(Aml *scope, const MemMapEntry *virtio_mmio_memmap, + uint32_t mmio_irq, int num); + #endif
The functions which add fw_cfg and virtio to DSDT are same for ARM and RISC-V. So, instead of duplicating in RISC-V, move them from hw/arm/virt-acpi-build.c to common aml-build.c. Signed-off-by: Sunil V L <sunilvl@ventanamicro.com> --- hw/acpi/aml-build.c | 41 ++++++++++++++++++++++++++++++++++++ hw/arm/virt-acpi-build.c | 42 ------------------------------------- hw/riscv/virt-acpi-build.c | 16 -------------- include/hw/acpi/aml-build.h | 6 ++++++ 4 files changed, 47 insertions(+), 58 deletions(-)