[PATCH] xen/arm: Kconfig: ACPI should depend on UEFI

Julien Grall posted 1 patch 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20230705115534.26004-1-julien@xen.org
xen/arch/arm/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[PATCH] xen/arm: Kconfig: ACPI should depend on UEFI
Posted by Julien Grall 10 months ago
From: Julien Grall <jgrall@amazon.com>

On Arm, it is not possible to use ACPI without UEFI. In fact disabling
UEFI but not ACPI will lead to a compilation error:

ld: prelink.o: in function `acpi_os_get_root_pointer':
/builds/xen-project/people/andyhhp/xen/xen/drivers/acpi/osl.c:73:
undefined reference to `efi'
/builds/xen-project/people/andyhhp/xen/xen/drivers/acpi/osl.c:73:(.init.text+0x8ac0):
relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against
undefined symbol `efi'

So make the dependency clear in the Kconfig.

This was spotted by the randconfig build in gitlab CI.

Fixes: 12314be5749e ("xen/arm: make ARM_EFI selectable for Arm64")
Signed-off-by: Julien Grall <jgrall@amazon.com>
---
 xen/arch/arm/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index c30f32457388..439cc94f3344 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -63,11 +63,11 @@ source "arch/Kconfig"
 
 config ACPI
 	bool "ACPI (Advanced Configuration and Power Interface) Support (UNSUPPORTED)" if UNSUPPORTED
-	depends on ARM_64
+	depends on ARM_64 && ARM_EFI
 	---help---
 
 	  Advanced Configuration and Power Interface (ACPI) support for Xen is
-	  an alternative to device tree on ARM64.
+	  an alternative to device tree on ARM64. This requires UEFI.
 
 config ARM_EFI
 	bool "UEFI boot service support"
-- 
2.40.1
Re: [PATCH] xen/arm: Kconfig: ACPI should depend on UEFI
Posted by Bertrand Marquis 10 months ago
Hi Julien,

> On 5 Jul 2023, at 13:55, Julien Grall <julien@xen.org> wrote:
> 
> From: Julien Grall <jgrall@amazon.com>
> 
> On Arm, it is not possible to use ACPI without UEFI. In fact disabling
> UEFI but not ACPI will lead to a compilation error:
> 
> ld: prelink.o: in function `acpi_os_get_root_pointer':
> /builds/xen-project/people/andyhhp/xen/xen/drivers/acpi/osl.c:73:
> undefined reference to `efi'
> /builds/xen-project/people/andyhhp/xen/xen/drivers/acpi/osl.c:73:(.init.text+0x8ac0):
> relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against
> undefined symbol `efi'
> 
> So make the dependency clear in the Kconfig.
> 
> This was spotted by the randconfig build in gitlab CI.
> 
> Fixes: 12314be5749e ("xen/arm: make ARM_EFI selectable for Arm64")
> Signed-off-by: Julien Grall <jgrall@amazon.com>

Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Cheers
Bertrand

> ---
> xen/arch/arm/Kconfig | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index c30f32457388..439cc94f3344 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -63,11 +63,11 @@ source "arch/Kconfig"
> 
> config ACPI
> bool "ACPI (Advanced Configuration and Power Interface) Support (UNSUPPORTED)" if UNSUPPORTED
> - depends on ARM_64
> + depends on ARM_64 && ARM_EFI
> ---help---
> 
>  Advanced Configuration and Power Interface (ACPI) support for Xen is
> -  an alternative to device tree on ARM64.
> +  an alternative to device tree on ARM64. This requires UEFI.
> 
> config ARM_EFI
> bool "UEFI boot service support"
> -- 
> 2.40.1
> 
Re: [PATCH] xen/arm: Kconfig: ACPI should depend on UEFI
Posted by Julien Grall 10 months ago
Hi,

On 05/07/2023 13:51, Bertrand Marquis wrote:
>> On 5 Jul 2023, at 13:55, Julien Grall <julien@xen.org> wrote:
>>
>> From: Julien Grall <jgrall@amazon.com>
>>
>> On Arm, it is not possible to use ACPI without UEFI. In fact disabling
>> UEFI but not ACPI will lead to a compilation error:
>>
>> ld: prelink.o: in function `acpi_os_get_root_pointer':
>> /builds/xen-project/people/andyhhp/xen/xen/drivers/acpi/osl.c:73:
>> undefined reference to `efi'
>> /builds/xen-project/people/andyhhp/xen/xen/drivers/acpi/osl.c:73:(.init.text+0x8ac0):
>> relocation truncated to fit: R_AARCH64_ADR_PREL_PG_HI21 against
>> undefined symbol `efi'
>>
>> So make the dependency clear in the Kconfig.
>>
>> This was spotted by the randconfig build in gitlab CI.
>>
>> Fixes: 12314be5749e ("xen/arm: make ARM_EFI selectable for Arm64")
>> Signed-off-by: Julien Grall <jgrall@amazon.com>
> 
> Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>

Thanks. I have committed the patch.

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/arm: Kconfig: ACPI should depend on UEFI
Posted by Jan Beulich 10 months ago
On 05.07.2023 13:55, Julien Grall wrote:
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -63,11 +63,11 @@ source "arch/Kconfig"
>  
>  config ACPI
>  	bool "ACPI (Advanced Configuration and Power Interface) Support (UNSUPPORTED)" if UNSUPPORTED
> -	depends on ARM_64
> +	depends on ARM_64 && ARM_EFI

Wouldn't it make sense to drop the ARM_64 dependency then? It's now
redundant, and it seems quite likely that if EFI was ever support
for 32-bit, ACPI could then be supported there as well.

Jan
Re: [PATCH] xen/arm: Kconfig: ACPI should depend on UEFI
Posted by Julien Grall 10 months ago
Hi Jan,

On 05/07/2023 13:04, Jan Beulich wrote:
> On 05.07.2023 13:55, Julien Grall wrote:
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -63,11 +63,11 @@ source "arch/Kconfig"
>>   
>>   config ACPI
>>   	bool "ACPI (Advanced Configuration and Power Interface) Support (UNSUPPORTED)" if UNSUPPORTED
>> -	depends on ARM_64
>> +	depends on ARM_64 && ARM_EFI
> 
> Wouldn't it make sense to drop the ARM_64 dependency then? It's now
> redundant, and it seems quite likely that if EFI was ever support
> for 32-bit, ACPI could then be supported there as well.

I think so. I am not planning to resend a new version for that. So I 
will do it on commit and mention it in the commit message.

Cheers,

-- 
Julien Grall
Re: [PATCH] xen/arm: Kconfig: ACPI should depend on UEFI
Posted by Julien Grall 10 months ago

On 05/07/2023 13:30, Julien Grall wrote:
> Hi Jan,
> 
> On 05/07/2023 13:04, Jan Beulich wrote:
>> On 05.07.2023 13:55, Julien Grall wrote:
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -63,11 +63,11 @@ source "arch/Kconfig"
>>>   config ACPI
>>>       bool "ACPI (Advanced Configuration and Power Interface) Support 
>>> (UNSUPPORTED)" if UNSUPPORTED
>>> -    depends on ARM_64
>>> +    depends on ARM_64 && ARM_EFI
>>
>> Wouldn't it make sense to drop the ARM_64 dependency then? It's now
>> redundant, and it seems quite likely that if EFI was ever support
>> for 32-bit, ACPI could then be supported there as well.
> 
> I think so. I am not planning to resend a new version for that. So I 
> will do it on commit and mention it in the commit message.

Actually no. It would be easier to add UEFI on Arm32 compare to adding 
ACPI. So it would be best to keep ARM_64 here at least for the time been.

Cheers,

-- 
Julien Grall

Re: [PATCH] xen/arm: Kconfig: ACPI should depend on UEFI
Posted by Bertrand Marquis 10 months ago
Hi Jan,

> On 5 Jul 2023, at 14:04, Jan Beulich <jbeulich@suse.com> wrote:
> 
> On 05.07.2023 13:55, Julien Grall wrote:
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -63,11 +63,11 @@ source "arch/Kconfig"
>> 
>> config ACPI
>> bool "ACPI (Advanced Configuration and Power Interface) Support (UNSUPPORTED)" if UNSUPPORTED
>> - depends on ARM_64
>> + depends on ARM_64 && ARM_EFI
> 
> Wouldn't it make sense to drop the ARM_64 dependency then? It's now
> redundant, and it seems quite likely that if EFI was ever support
> for 32-bit, ACPI could then be supported there as well.

I think we need to keep it.
If we add one day EFI support on arm32, we will probably not add ACPI support anyway.

Cheers
Bertrand

> 
> Jan
Re: [PATCH] xen/arm: Kconfig: ACPI should depend on UEFI
Posted by Bertrand Marquis 10 months ago

> On 5 Jul 2023, at 14:50, Bertrand Marquis <bertrand.marquis@arm.com> wrote:
> 
> Hi Jan,
> 
>> On 5 Jul 2023, at 14:04, Jan Beulich <jbeulich@suse.com> wrote:
>> 
>> On 05.07.2023 13:55, Julien Grall wrote:
>>> --- a/xen/arch/arm/Kconfig
>>> +++ b/xen/arch/arm/Kconfig
>>> @@ -63,11 +63,11 @@ source "arch/Kconfig"
>>> 
>>> config ACPI
>>> bool "ACPI (Advanced Configuration and Power Interface) Support (UNSUPPORTED)" if UNSUPPORTED
>>> - depends on ARM_64
>>> + depends on ARM_64 && ARM_EFI
>> 
>> Wouldn't it make sense to drop the ARM_64 dependency then? It's now
>> redundant, and it seems quite likely that if EFI was ever support
>> for 32-bit, ACPI could then be supported there as well.
> 
> I think we need to keep it.
> If we add one day EFI support on arm32, we will probably not add ACPI support anyway.

Sorry Julien I answered Jan before seeing your mail but I said the same :-)

Cheers
Bertrand

> 
> Cheers
> Bertrand
> 
>> 
>> Jan