[PATCH] lib/acpi: Enable QEMU special cases for ARM

[PATCH] lib/acpi: Enable QEMU special cases for ARM

[Alexander Graf]

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)
 static int acpi_write_all(struct acpi_ctx *ctx)
 {
 	const struct acpi_writer *writer =
-- 
2.32.0

Re: [PATCH] lib/acpi: Enable QEMU special cases for ARM

[Simon Glass]

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>

Re: [PATCH] lib/acpi: Enable QEMU special cases for ARM

[Heinrich Schuchardt]

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 =

Re: [PATCH] lib/acpi: Enable QEMU special cases for ARM

[Simon Glass]

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