Re: [Xen-devel] [PATCH] arch: arm: vgic-v3: fix GICD_ISACTIVER range

[Peng Fan <peng.fan@xxxxxxx>]

Hi Julien,

Inline marked with [Peng Fan]

From: Julien Grall <julien.grall.oss@xxxxxxxxx> 
Sent: 2019年11月9日 6:44
To: Stefano Stabellini <sstabellini@xxxxxxxxxx>; Andre Przywara 
<andre.przywara@xxxxxxx>
Cc: Peng Fan <peng.fan@xxxxxxx>; Jürgen Groß <jgross@xxxxxxxx>; 
julien.grall@xxxxxxx; xen-devel@xxxxxxxxxxxxx
Subject: Re: [Xen-devel] [PATCH] arch: arm: vgic-v3: fix GICD_ISACTIVER range

Hi,

Sorry for the formatting.
On Sat, 9 Nov 2019, 04:27 Stefano Stabellini, <mailto:sstabellini@xxxxxxxxxx> 
wrote:
On Thu, 7 Nov 2019, Peng Fan wrote:
> The end should be GICD_ISACTIVERN not GICD_ISACTIVER.
> 
> Signed-off-by: Peng Fan <mailto:peng.fan@xxxxxxx>

Reviewed-by: Stefano Stabellini <mailto:sstabellini@xxxxxxxxxx>

To be honest, I am not sure the code is correct. A read to those registers 
should tell you the list of interrupts active. As we always return 0, this will 
not return the correct state of the GIC.

I know that returning the list of actives interrupts is complicated with the 
old vGIC, but I don't think silently ignoring it is a good idea.

The question here is why the guest accessed those registers? What is it trying 
to figure out?

[Peng Fan] I am running Linux 5.4 kernel dom0, gic_peek_irq triggers abort.



Juergen, I think this fix should be in the release (and also
backported to stable trees.)

Without an understanding of the problem, I disagree with this request (see 
above).

As an aside, the range ISPENDR  has the same issue.

[Peng Fan] Should I include this change in v2? Or develop new method to fix the 
issue?
But at least dom0 abort when boot.

Thanks,
Peng.

Cheers,






> ---
>  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 422b94f902..e802f2055a 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;
>  
> -- 
> 2.16.4
> 

_______________________________________________
Xen-devel mailing list
mailto:Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.xenproject.org%2Fmailman%2Flistinfo%2Fxen-devel&data=02%7C01%7Cpeng.fan%40nxp.com%7C33f2e907cdc84ed0a48608d7649d359e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637088498678782239&sdata=G3FA2vefr56FeUX5QVZQwSzG22nfv1m%2F0fKIDOnfuFQ%3D&reserved=0
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel