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

[Heinrich Schuchardt <xypron.glpk@gmx.de>]

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 =