Re: [PATCH] xen/arm: ACPI: Remove EXPERT dependancy, default on for ARM64

From: Stefano Stabellini <sstabellini@kernel.org>
To: Elliott Mitchell <ehem+xen@m5p.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
	Julien Grall <julien@xen.org>,
	 xen-devel@lists.xenproject.org,
	 Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [PATCH] xen/arm: ACPI: Remove EXPERT dependancy, default on for ARM64
Date: Fri, 23 Oct 2020 10:07:32 -0700 (PDT)	[thread overview]
Message-ID: <alpine.DEB.2.21.2010231002300.12247@sstabellini-ThinkPad-T480s> (raw)
In-Reply-To: <20201023033506.GC83870@mattapan.m5p.com>

On Thu, 22 Oct 2020, Elliott Mitchell wrote:
> Linux requires UEFI support to be enabled on ARM64 devices.  While many
> ARM64 devices lack ACPI, the writing seems to be on the wall of UEFI/ACPI
> potentially taking over.  Some common devices may require ACPI table
> support to boot.

Let's not make guesses on the direction of the industry in a commit
message :-)

The following would suffice:

Some common ARM64 devices require ACPI to boot (no device tree is
available).

> For devices which can boot in either mode, continue defaulting to
> device-tree.  Add warnings about using ACPI advising users of present
> situation.
> 
> Signed-off-by: Elliott Mitchell <ehem+xen@m5p.com>
> ---
> Okay, hopefully this is okay.  Warning in Kconfig, warning on boot.
> Perhaps "default y if ARM_64" is redundant, yet if someone tries to make
> it possible to boot aarch32 on a ACPI machine...
> 
> I also want a date in the message.  Theory is this won't be there
> forever, so a date is essential.
> ---
>  xen/arch/arm/Kconfig     | 7 ++++++-
>  xen/arch/arm/acpi/boot.c | 9 +++++++++
>  2 files changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 2777388265..29624d03fa 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -32,13 +32,18 @@ menu "Architecture Features"
>  source "arch/Kconfig"
>  
>  config ACPI
> -	bool "ACPI (Advanced Configuration and Power Interface) Support" if EXPERT
> +	bool "ACPI (Advanced Configuration and Power Interface) Support"
>  	depends on ARM_64
> +	default y if ARM_64

I am not so sure about the "default y" for the reason that the option is
not technically "supported", so it is probably best to take the default
line out. Otherwise we end up with a default "unsupported" kconfig which
is not great.

>  	---help---
>  
>  	  Advanced Configuration and Power Interface (ACPI) support for Xen is
>  	  an alternative to device tree on ARM64.
>  
> +	  Note this is presently EXPERIMENTAL.  If a given device has both
> +	  device-tree and ACPI support, it is presently (October 2020)
> +	  recommended to boot using the device-tree.

Please remove the date from the message. We'll update as needed in the
future. The following works:

 Note this is presently EXPERIMENTAL.  If a given device has both
 device-tree and ACPI support, it is recommended to boot using the
 device-tree.

>  config GICV3
>  	bool "GICv3 driver"
>  	depends on ARM_64 && !NEW_VGIC
> diff --git a/xen/arch/arm/acpi/boot.c b/xen/arch/arm/acpi/boot.c
> index 30e4bd1bc5..c0e8f85325 100644
> --- a/xen/arch/arm/acpi/boot.c
> +++ b/xen/arch/arm/acpi/boot.c
> @@ -254,6 +254,15 @@ int __init acpi_boot_table_init(void)
>                                     dt_scan_depth1_nodes, NULL) )
>          goto disable;
>  
> +    printk("\n"
> +"*************************************************************************\n"
> +"*    WARNING WARNING WARNING WARNING WARNING WARNING WARNING WARNING    *\n"
> +"*                                                                       *\n"
> +"* Xen-ARM ACPI support is EXPERIMENTAL.  It is presently (October 2020) *\n"
> +"* recommended you boot your system in device-tree mode if you can.      *\n"
> +"*************************************************************************\n"
> +            "\n");

Please use warning_add and remove the date from the message.

>      /*
>       * ACPI is disabled at this point. Enable it in order to parse
>       * the ACPI tables.
> -- 
> 2.20.1
> 
> 
> -- 
> (\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
>  \BS (    |         ehem+sigmsg@m5p.com  PGP 87145445         |    )   /
>   \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
> 8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445
> 
> 