Re: [Xen-devel] [PATCH v5 2/8] ACPI: add config for BIOS table scan

[Jonathan Creekmore <jonathan.creekmore@xxxxxxxxx>]

Shannon Zhao writes:

> From: Graeme Gregory <graeme.gregory@xxxxxxxxxx>
>
> With the addition of ARM64 that does not have a traditional BIOS to
> scan, add a config option which is selected on x86 (ia64 doesn't need
> it either, it is EFI/UEFI based system) to do the traditional BIOS
> scanning for tables.
>
> Signed-off-by: Graeme Gregory <graeme.gregory@xxxxxxxxxx>
> Signed-off-by: Hanjun Guo <hanjun.guo@xxxxxxxxxx>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
> [Linux commit 8a1664be0b922dd6afd60eca96a992ef5ec22c40]
> Signed-off-by: Shannon Zhao <shannon.zhao@xxxxxxxxxx>
> ---
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> ---
>  xen/arch/x86/Kconfig     | 1 +
>  xen/drivers/acpi/Kconfig | 3 +++
>  xen/drivers/acpi/osl.c   | 4 +++-
>  3 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 7d2ed96..3a25288 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -5,6 +5,7 @@ config X86
>       def_bool y
>       select COMPAT
>       select HAS_ACPI
> +     select ACPI_LEGACY_TABLES_LOOKUP if HAS_ACPI

Since HAS_ACPI is selected right above this, it seems pointless to do
the if HAS_ACPI here. Just select ACPI_LEGACY_TABLES_LOOKUP. Or, see below.

>       select HAS_CPUFREQ
>       select HAS_EHCI
>       select HAS_GDBSX
> diff --git a/xen/drivers/acpi/Kconfig b/xen/drivers/acpi/Kconfig
> index 11ab5e4..82d73ca 100644
> --- a/xen/drivers/acpi/Kconfig
> +++ b/xen/drivers/acpi/Kconfig
> @@ -2,3 +2,6 @@
>  # Select HAS_ACPI if ACPI is supported
>  config HAS_ACPI
>       bool
> +
> +config ACPI_LEGACY_TABLES_LOOKUP
> +     bool

Or, better, default the value of ACPI_LEGACY_TABLES_LOOKUP based on
HAS_ACPI. That way, you only select HAS_ACPI to default this to on and,
if another platform besides X86 ever enabled HAS_ACPI, it would turn on
this option without you having to select it as well.

> diff --git a/xen/drivers/acpi/osl.c b/xen/drivers/acpi/osl.c
> index ce15470..a2fc8c4 100644
> --- a/xen/drivers/acpi/osl.c
> +++ b/xen/drivers/acpi/osl.c
> @@ -75,12 +75,14 @@ acpi_physical_address __init 
> acpi_os_get_root_pointer(void)
>                              "System description tables not found\n");
>                       return 0;
>               }
> -     } else {
> +     } else if (IS_ENABLED(CONFIG_ACPI_LEGACY_TABLES_LOOKUP)) {

I would use an #ifdef CONFIG_ACPI_LEGACY_TABLES_LOOKUP instead of using
the kconfig.h IS_ENABLED macro to keep from pulling that file in.

>               acpi_physical_address pa = 0;
>
>               acpi_find_root_pointer(&pa);
>               return pa;
>       }
> +
> +     return 0;
>  }
>
>  void __iomem *

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel