Re: [Xen-devel] [PATCH] xen/arm: vgic-v3: Fix the typo of GICD IRQ active status range

[Wei Xu <xuwei5@hisilicon.com>]

Hi Julien,

On 2019/12/28 16:09, Julien Grall wrote:
> Hi,
>
> On 28/12/2019 03:08, Wei Xu wrote:
>> This patch fixes the typo about the active status range of an IRQ
>> via GICD. Otherwise it will be failed to handle the mmio access and
>> inject a data abort.
> I have seen a patch similar from NXP a month ago and I disagreed on 
> the approach.
>
> If you look at the context you modifed, it says that reading ACTIVER 
> is not supported. While I agree the behavior is not consistent accross 
> ACTIVER, injecting a data abort is a perfectly fine behavior to me 
> (though not spec compliant) as we don't implement the registers 
> correctly.
>
> I guess you are sending this patch, because you tried Linux 5.4 (or 
> later) on Xen, right? Linux has recently began to read ACTIVER to 
> check whether an IRQ is active at the HW level during the 
> synchronizing of the IRQS. From my understanding, this is used because 
> there is a window where the interrupt is active at the HW level but 
> the Linux IRQ subsystem is not aware of it.
>
> While the patch below will allow Linux 5.4 to not crash, it is not 
> going to make it fly very far because of the above. So I am rather not 
> happy with persuing with returning 0.
>

Yes, I am using Linux 5.5-rc2 :)
Got it and thanks for the explanation.
I am not insistent on this and OK to wait for the update.
Thanks and have a very happy new year!

Best Regards,
Wei

> @Stefano, you mention you will look at implementing ACTIVER. What's 
> the state?
>
> >
>> Fixes: a2b83f95bfad ("xen/arm: vgic: Properly emulate the full 
>> register")
>>
>> Signed-off-by: Wei Xu <xuwei5@hisilicon.com>
>> ---
>>   xen/arch/arm/vgic-v3.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/arm/vgic-v3.c b/xen/arch/arm/vgic-v3.c
>> index 422b94f..e802f20 100644
>> --- a/xen/arch/arm/vgic-v3.c
>> +++ b/xen/arch/arm/vgic-v3.c
>> @@ -706,7 +706,7 @@ static int __vgic_v3_distr_common_mmio_read(const 
>> char *name, struct vcpu *v,
>>           goto read_as_zero;
>>         /* Read the active status of an IRQ via GICD/GICR is not 
>> supported */
>> -    case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVER):
>> +    case VRANGE32(GICD_ISACTIVER, GICD_ISACTIVERN):
>>       case VRANGE32(GICD_ICACTIVER, GICD_ICACTIVERN):
>>           goto read_as_zero;
>>
>
> Cheers,
>



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel