Re: [XEN v3] xen/arm: vGICv3: Emulate properly 32-bit access on GICR_PENDBASER

[Julien Grall <julien@xxxxxxx>]

On 26/10/2022 16:06, Ayan Kumar Halder wrote:
> > ... you also need to ensure that the writers are atomically setting 
> > rdist_pendbase. Please correct if I am wrong, but the callers are not 
> > using write_atomic(). So how does that work?
> I think read_atomic()/write_atomic() may not be the correct approach for 
> the following reasons :-
> 1. __vgic_v3_rdistr_rd_mmio_read is a static function. So 'val' has a 
> global lifetime. Thus, all the following three lines need to be 
> protected from concurrent access.
I don't understand this argument. 'static' means the function is not 
exported. The local variables will still reside on the stack.
So why does the use of 'val' needs to be protected with the lock?

>          val = read_atomic(&v->arch.vgic.rdist_pendbase);
>          val &= ~GICR_PENDBASER_PTZ;      /* WO, reads as 0 */
>
>          /* If a context switch happens here, then the 'val' below may 
> potentially be incorrect. */
>          *r = vreg_reg64_extract(val, info);
>
> 2. The same holds true for 'reg' as well in 
> __vgic_v3_rdistr_rd_mmio_write()
>              reg = v->arch.vgic.rdist_pendbase;
>              blah, blah
>              v->arch.vgic.rdist_pendbase = reg;
Same here.

Cheers,

--
Julien Grall