Re: [PATCH v2] arm/vgic-v3: fix virq offset in the rank when storing irouter

[Julien Grall <julien@xxxxxxx>]

On 03/08/2022 09:19, Julien Grall wrote:
> Hi Hongda,
>
> On 29/07/2022 09:36, Hongda Deng wrote:
> > When vGIC performs irouter registers emulation, to get the target vCPU
> > via virq conveniently, Xen doesn't store the irouter value directly,
> > instead it will use the value (affinities) in irouter to calculate the
> > target vCPU, and then save the target vCPU in irq rank->vcpu[offset].
> >
> > When vGIC tries to get the target vCPU, it first calculates the target
> > vCPU index via
> >    int target = read_atomic(&rank->vcpu[virq & INTERRUPT_RANK_MASK]);
> > and then it gets the target vCPU via
> >    v->domain->vcpu[target];
> >
> > When vGIC tries to store irouter for one virq, the target vCPU index
> > in the rank is computed as
> >    offset &= virq & INTERRUPT_RANK_MASK;
> > finally it gets the target vCPU via
> >    d->vcpu[read_atomic(&rank->vcpu[offset])];
> >
> > There is a difference between them while getting the target vCPU index
> > in the rank. Actually (virq & INTERRUPT_RANK_MASK) would already get
> > the target vCPU index in the rank, it's wrong to add '&' before '=' when
> > calculate the offset.
> >
> > For example, the target vCPU index in the rank should be 6 for virq 38,
> > but vGIC will get offset=0 when vGIC stores the irouter for this virq,
> > and finally vGIC will access the wrong target vCPU index in the rank
> > when updating the irouter.
> >
> > Fixes: 5d495f4349b5 ("xen/arm: vgic: Optimize the way to store the 
> > target vCPU in the rank")
> > Signed-off-by: Hongda Deng <Hongda.Deng@xxxxxxx>
> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
Committed.

Cheers,

--
Julien Grall