Re: Question regarding Locking in the MMIO Handling Framework on Arm

[Julien Grall <julien@xxxxxxx>]

Hi Oleksii,

Thanks for the report.

On 09/07/2026 11:49, Oleksii Kurochko wrote:
> I have a question regarding the locking in the MMIO handling framework 
> on Arm.
> Is it sufficient to have read_lock() only in find_mmio_handler()? If
> register_mmio_handler() is executed in parallel with find_mmio_handler() 
> (which I assume was the reason for introducing the rwlock), aren't we 
> still exposed to a race condition?
When the read-write lock was introduced, we didn't sort the entries. So 
it was fine at the time. This bug was introduced by commit 8047e090f4 
("xen/arm: io: Use binary search for mmio handler lookup").
With this change, then we...

> find_mmio_handler() returns a pointer to a handler, but the object it 
> points to could be changed by a subsequent call to register_mmio_handler().
> If register_mmio_handler() runs between find_mmio_handler() and 
> handle_{write,read}(), we could end up operating on a different handler 
> than the one that was originally found.
> In other words, shouldn't we acquire the read_lock() in 
> try_handle_mmio() and keep it held for the entire duration of 
> try_handle_mmio()?
... either need to keep the lock for longer or rework the code to 
allocate the handler structure.
> It seems this is not an issue at the moment because 
> register_mmio_handler() is only called during domain creation. If that's 
> the case, do we really need the rwlock at all?
If you are removing the lock, you will build an assumption that 
register_mmio_handler() can only be called only at domain creation.
We would want to add a check in the code to catch any misuse.

> One more thing: it looks like
>    BUG_ON(vmmio->num_entries >= vmmio->max_num_entries);
> in register_mmio_handler() would be better placed inside the 
> write_lock() section. As again in the case if register_mmio_handler() 
> will start to be called in parallel we will have an issue.
Indeed.

--
Julien Grall