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

[Julien Grall <julien@xxxxxxx>]

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.
@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@xxxxxxxxxxxxx>
> ---
>   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,

--
Julien Grall

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