| Message ID | 20220227141035.37088-1-agraf@csgraf.de |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | lib/acpi: Enable QEMU special cases for ARM | expand |
On Sun, 27 Feb 2022 at 07:10, Alexander Graf <agraf@csgraf.de> wrote: > > With QEMU, we receive ACPI tables from the hypervisor rather than build > them ourselves in U-Boot. That logic however hard codes the target to the > x86 QEMU target, leaving ARM out on the fun. > > Let's add the ARM QEMU config as well to the ifdefs so that we can consume > QEMU provided firmware tables there as well. > > Signed-off-by: Alexander Graf <agraf@csgraf.de> > --- > lib/acpi/Makefile | 2 ++ > lib/acpi/acpi_writer.c | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) Reviewed-by: Simon Glass <sjg@chromium.org>
On 2/27/22 15:10, Alexander Graf wrote: > With QEMU, we receive ACPI tables from the hypervisor rather than build > them ourselves in U-Boot. That logic however hard codes the target to the > x86 QEMU target, leaving ARM out on the fun. > > Let's add the ARM QEMU config as well to the ifdefs so that we can consume > QEMU provided firmware tables there as well. > > Signed-off-by: Alexander Graf <agraf@csgraf.de> > --- > lib/acpi/Makefile | 2 ++ > lib/acpi/acpi_writer.c | 2 +- > 2 files changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/acpi/Makefile b/lib/acpi/Makefile > index 956b5a0d72..66ba0e94ac 100644 > --- a/lib/acpi/Makefile > +++ b/lib/acpi/Makefile > @@ -9,6 +9,7 @@ obj-y += acpi_writer.o > > # With QEMU the ACPI tables come from there, not from U-Boot > ifndef CONFIG_QEMU > +ifndef CONFIG_ARCH_QEMU > obj-y += base.o > obj-y += csrt.o > obj-y += mcfg.o > @@ -21,3 +22,4 @@ endif > obj-y += facs.o > obj-y += ssdt.o > endif > +endif > diff --git a/lib/acpi/acpi_writer.c b/lib/acpi/acpi_writer.c > index 946f90e8e7..9c252aa58d 100644 > --- a/lib/acpi/acpi_writer.c > +++ b/lib/acpi/acpi_writer.c > @@ -48,7 +48,7 @@ int acpi_write_one(struct acpi_ctx *ctx, const struct acpi_writer *entry) > return 0; > } > > -#ifndef CONFIG_QEMU > +#if !defined(CONFIG_QEMU) && !defined(CONFIG_ARCH_QEMU) Why would you treat RISC-V QEMU differently? Why should a symbol called CONFIG_QEMU relate to x86 and not to any QEMU? Why should a symbol called CONFIG_ARCH_QEMU relate to ARM? It is preferable to clean up the chaotic naming of QEMU related symbols and to ensure that CONFIG_QEMU means any QEMU: arch/arm/Kconfig:971:config ARCH_QEMU arch/arm/mach-qemu/Kconfig:16:config TARGET_QEMU_ARM_32BIT arch/arm/mach-qemu/Kconfig:23:config TARGET_QEMU_ARM_64BIT arch/powerpc/cpu/mpc85xx/Kconfig:108:config TARGET_QEMU_PPCE500 arch/powerpc/cpu/mpc85xx/Kconfig:638:config ARCH_QEMU_E500 arch/riscv/Kconfig:17:config TARGET_QEMU_VIRT arch/x86/cpu/qemu/Kconfig:5:config QEMU board/emulation/Kconfig:10:config TARGET_QEMU_X86 board/emulation/Kconfig:19:config TARGET_QEMU_X86_64 board/emulation/common/qemu_dfu.c and drivers/timer/tsc_timer.c are other instances where one QEMU architecture is treated differently to others without reason. Best regards Heinrich > static int acpi_write_all(struct acpi_ctx *ctx) > { > const struct acpi_writer *writer =
Hi, On Mon, 14 Mar 2022 at 01:04, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote: > > On 2/27/22 15:10, Alexander Graf wrote: > > With QEMU, we receive ACPI tables from the hypervisor rather than build > > them ourselves in U-Boot. That logic however hard codes the target to the > > x86 QEMU target, leaving ARM out on the fun. > > > > Let's add the ARM QEMU config as well to the ifdefs so that we can consume > > QEMU provided firmware tables there as well. > > > > Signed-off-by: Alexander Graf <agraf@csgraf.de> > > --- > > lib/acpi/Makefile | 2 ++ > > lib/acpi/acpi_writer.c | 2 +- > > 2 files changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/lib/acpi/Makefile b/lib/acpi/Makefile > > index 956b5a0d72..66ba0e94ac 100644 > > --- a/lib/acpi/Makefile > > +++ b/lib/acpi/Makefile > > @@ -9,6 +9,7 @@ obj-y += acpi_writer.o > > > > # With QEMU the ACPI tables come from there, not from U-Boot > > ifndef CONFIG_QEMU > > +ifndef CONFIG_ARCH_QEMU > > obj-y += base.o > > obj-y += csrt.o > > obj-y += mcfg.o > > @@ -21,3 +22,4 @@ endif > > obj-y += facs.o > > obj-y += ssdt.o > > endif > > +endif > > diff --git a/lib/acpi/acpi_writer.c b/lib/acpi/acpi_writer.c > > index 946f90e8e7..9c252aa58d 100644 > > --- a/lib/acpi/acpi_writer.c > > +++ b/lib/acpi/acpi_writer.c > > @@ -48,7 +48,7 @@ int acpi_write_one(struct acpi_ctx *ctx, const struct acpi_writer *entry) > > return 0; > > } > > > > -#ifndef CONFIG_QEMU > > +#if !defined(CONFIG_QEMU) && !defined(CONFIG_ARCH_QEMU) > > Why would you treat RISC-V QEMU differently? > > Why should a symbol called CONFIG_QEMU relate to x86 and not to any > QEMU? Why should a symbol called CONFIG_ARCH_QEMU relate to ARM? > > It is preferable to clean up the chaotic naming of QEMU related symbols > and to ensure that CONFIG_QEMU means any QEMU: > > arch/arm/Kconfig:971:config ARCH_QEMU > arch/arm/mach-qemu/Kconfig:16:config TARGET_QEMU_ARM_32BIT > arch/arm/mach-qemu/Kconfig:23:config TARGET_QEMU_ARM_64BIT > arch/powerpc/cpu/mpc85xx/Kconfig:108:config TARGET_QEMU_PPCE500 > arch/powerpc/cpu/mpc85xx/Kconfig:638:config ARCH_QEMU_E500 > arch/riscv/Kconfig:17:config TARGET_QEMU_VIRT > arch/x86/cpu/qemu/Kconfig:5:config QEMU > board/emulation/Kconfig:10:config TARGET_QEMU_X86 > board/emulation/Kconfig:19:config TARGET_QEMU_X86_64 Yes that would be good! > > board/emulation/common/qemu_dfu.c and drivers/timer/tsc_timer.c are > other instances where one QEMU architecture is treated differently to > others without reason. > > Best regards > > Heinrich > > > static int acpi_write_all(struct acpi_ctx *ctx) > > { > > const struct acpi_writer *writer = > Regards, SImon
diff --git a/lib/acpi/Makefile b/lib/acpi/Makefile index 956b5a0d72..66ba0e94ac 100644 --- a/lib/acpi/Makefile +++ b/lib/acpi/Makefile @@ -9,6 +9,7 @@ obj-y += acpi_writer.o # With QEMU the ACPI tables come from there, not from U-Boot ifndef CONFIG_QEMU +ifndef CONFIG_ARCH_QEMU obj-y += base.o obj-y += csrt.o obj-y += mcfg.o @@ -21,3 +22,4 @@ endif obj-y += facs.o obj-y += ssdt.o endif +endif diff --git a/lib/acpi/acpi_writer.c b/lib/acpi/acpi_writer.c index 946f90e8e7..9c252aa58d 100644 --- a/lib/acpi/acpi_writer.c +++ b/lib/acpi/acpi_writer.c @@ -48,7 +48,7 @@ int acpi_write_one(struct acpi_ctx *ctx, const struct acpi_writer *entry) return 0; } -#ifndef CONFIG_QEMU +#if !defined(CONFIG_QEMU) && !defined(CONFIG_ARCH_QEMU) static int acpi_write_all(struct acpi_ctx *ctx) { const struct acpi_writer *writer =
With QEMU, we receive ACPI tables from the hypervisor rather than build them ourselves in U-Boot. That logic however hard codes the target to the x86 QEMU target, leaving ARM out on the fun. Let's add the ARM QEMU config as well to the ifdefs so that we can consume QEMU provided firmware tables there as well. Signed-off-by: Alexander Graf <agraf@csgraf.de> --- lib/acpi/Makefile | 2 ++ lib/acpi/acpi_writer.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-)