Re: [Xen-devel] [PATCH] arm/gic-v3: Fix driver probe fail on GICv4 hardware

[Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>]

On 05/27/2016 09:07 AM, Julien Grall wrote:
>
>
> On 27/05/16 14:48, Shanker Donthineni wrote:
>> Hi Julien,
>
> Hello Shanker,
>
>> On 05/27/2016 07:35 AM, Julien Grall wrote:
>>> On 27/05/16 00:59, Shanker Donthineni wrote:
>>>> The current driver probe fails on hardware which has GICv4 version,
>>>> even though it is fully compatible to GICv3. This patch fixes the
>>>> issue by registering the same probe function for GICv4 hardware.
>>>>
>>>> Signed-off-by: Shanker Donthineni <shankerd@xxxxxxxxxxxxxx>
>>>> ---
>>>>    xen/arch/arm/gic-v3.c     | 13 +++++++++++++
>>>>    xen/include/asm-arm/gic.h |  1 +
>>>>    2 files changed, 14 insertions(+)
>>>>
>>>> diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c
>>>> index a095064..594cf6e 100644
>>>> --- a/xen/arch/arm/gic-v3.c
>>>> +++ b/xen/arch/arm/gic-v3.c
>>>> @@ -1604,10 +1604,23 @@ static int __init gicv3_acpi_preinit(const void 
>>>> *data)
>>>>        return 0;
>>>>    }
>>>>
>>>> +static int __init gicv4_acpi_preinit(const void *data)
>>>> +{
>>>> +    gicv3_info.hw_version = GIC_V4;
>>>
>>> It will crash Xen as soon as DOM0 is created (see the BUG() in 
>>> arch_domain_create). Please test any patch before sending on the ML.
>>>
>>> Anyway, there is no support of GICv4 in Xen. Instead Xen will drive it 
>>> using the GICv3 driver. So the hardware version should be GIC_V3 here.
>>>
>> Yes, I know I am going to fix in a separate to fix dom0 boot issue some 
>> thing like below.
>
> This should have been in the same patch or before. There is no point to have 
> a patch adding support for GICv4 with ACPI which will lead to an obscure 
> crash (a BUG() rather than a panic with a nice message).
>
Sure, I'll do it in a single patch.
>>   diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
>> index 1365b4a..56a47f5 100644
>> --- a/xen/arch/arm/domain.c
>> +++ b/xen/arch/arm/domain.c
>> @@ -571,6 +571,11 @@ int arch_domain_create(struct domain *d, unsigned int 
>> domcr_flags,
>>               d->arch.vgic.version = GIC_V3;
>>               break;
>>
>> +        case GIC_V4:
>> +            config->gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
>> +            d->arch.vgic.version = GIC_V3;
>> +            break;
>
> As mentioned in my previous mail, there is no support of GICv4 in Xen. 
> Although the GICv3 driver is supporting this hardware.So we should not 
> advertise GIC_V4 outside of the driver until Xen will get enough knowledge of 
> GICv4 which will require sensible change in the generic code.
>
GICv4 hardware is fully compatible to GICv3 and has an additional feature vLPI. 
 We don't need any special driver or changes to current driver to support GICv4 
for SPIs/LPIs in Xen just like Linux kernel.

Confused, you are expecting gic_hw_version() should return GIC version number 
'3' on on GICv4 hardware, right?

        switch ( gic_hw_version () )
        {
        case GIC_V2:
            config->gic_version = XEN_DOMCTL_CONFIG_GIC_V2;
            d->arch.vgic.version = GIC_V2;
            break;
        case GIC_V3:
            config->gic_version = XEN_DOMCTL_CONFIG_GIC_V3;
            d->arch.vgic.version = GIC_V3;
            break;

> Regards,
>

-- 
Shanker Donthineni
Qualcomm Technologies, Inc. on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux 
Foundation Collaborative Project


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