Re: [Xen-devel] [RFC 27/29] build: convert HAS_GICV3 use to Kconfig

[Doug Goldstein <cardoe@xxxxxxxxxx>]

On 10/5/15 5:25 PM, Julien Grall wrote:
> Hi,

Thanks for the quick review! I appreciate it.

> 
> On 05/10/2015 23:03, Doug Goldstein wrote:
>> Use the Kconfig generated CONFIG_HAS_GICV3 defines in the code base.
> 
> If you are going to rename all HAS_* to CONFIG_HAS_, please drop the HAS
> which is now redundant.

So I am treating the CONFIG_HAS_ defines similar to how the Linux kernel
has CONFIG_HAVE_. These are not actually user facing options but instead
they are enforced by the architecture instead of being a user facing option.

For example CONFIG_HAS_KEXEC means the architecture/platform supports
KEXEC but CONFIG_KEXEC would be the user configurable option to turn it
on/off. This would be consistent with the current behavior in the Xen
tree where HAS_KEXEC means the architecture/platform supports KEXEC
while 'kexec' as an env var allows the user to build kexec on or off.


> 
>>
>> Signed-off-by: Doug Goldstein <cardoe@xxxxxxxxxx>
>> ---
>>   xen/arch/arm/Kconfig         | 4 ++++
>>   xen/arch/arm/Makefile        | 2 +-
>>   xen/arch/arm/Rules.mk        | 2 --
>>   xen/arch/arm/vgic.c          | 2 +-
>>   xen/include/asm-arm/domain.h | 3 ++-
>>   xen/include/asm-arm/gic.h    | 4 ++--
>>   xen/include/asm-arm/vgic.h   | 2 +-
>>   7 files changed, 11 insertions(+), 8 deletions(-)
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index f100f17..01744c7 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -28,6 +28,10 @@ config ARCH_DEFCONFIG
>>       default "arch/arm/arm32_defconfig" if ARM_32
>>       default "arch/arm/arm64_defconfig" if ARM_64
>>
>> +# Select HAS_GICV3 if Generic Interrupt Connect (GICv3) is supported
> 
> s/Connect/Controller/ although saying GICv3 is enough. No need to spell
> out the acronym.
> 
> If you really want to spell it it should be Generic Interrupt Controller
> v3.

Thanks. I will update those comments.

> 
>> +config HAS_GICV3
>> +    bool
>> +
> 
> I may have miss something with this change GICv3 is not built anymore
> for ARM64.
> 
> The user should be able to get a Xen with the exactly the same features
> after this series without any changes from his side.


Definitely my goal with this patchset is to make sure everything behaves
the same way it did before.

> 
>>   source "common/Kconfig"
>>
>>   source "drivers/Kconfig"
> 
> [...]
> 
>> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
>> index b89727e..4dd72ed 100644
>> --- a/xen/include/asm-arm/domain.h
>> +++ b/xen/include/asm-arm/domain.h
>> @@ -102,7 +102,8 @@ struct arch_domain
>>           struct pending_irq *pending_irqs;
>>           /* Base address for guest GIC */
>>           paddr_t dbase; /* Distributor base address */
>> -#ifdef HAS_GICV3
>> +        paddr_t cbase; /* CPU base address */
> 
> Can you please make sure that you series don't re-introduce code or
> change it.
> 
> This should be pretty easy to check with grep. I.e any changes in *.c
> and *.h files but in lines containing ifdef/endif are likely wrong.
> 
> Regards,
> 

Apologies, I made a mistake during a rebase.


-- 
Doug Goldstein

Attachment: signature.asc
Description: OpenPGP digital signature

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