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
