[PATCH v7 2/4] KVM: arm64: PMU: Protect the list of PMUs with RCU

[Akihiko Odaki]

On 2026/04/20 16:01, Marc Zyngier wrote:
> On Mon, 20 Apr 2026 07:21:45 +0100,
> Akihiko Odaki <odaki at rsg.ci.i.u-tokyo.ac.jp> wrote:
>>
>> On 2026/04/19 23:34, Marc Zyngier wrote:
>>> On Sat, 18 Apr 2026 09:14:24 +0100,
>>> Akihiko Odaki <odaki at rsg.ci.i.u-tokyo.ac.jp> wrote:
>>>>
>>>> Convert the list of PMUs to a RCU-protected list that has primitives to
>>>> avoid read-side contention.
>>>>
>>>> Signed-off-by: Akihiko Odaki <odaki at rsg.ci.i.u-tokyo.ac.jp>
>>>> ---
>>>>    arch/arm64/kvm/pmu-emul.c | 14 ++++++--------
>>>>    1 file changed, 6 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
>>>> index 59ec96e09321..ef5140bbfe28 100644
>>>> --- a/arch/arm64/kvm/pmu-emul.c
>>>> +++ b/arch/arm64/kvm/pmu-emul.c
>>>> @@ -7,9 +7,9 @@
>>>>    #include <linux/cpu.h>
>>>>    #include <linux/kvm.h>
>>>>    #include <linux/kvm_host.h>
>>>> -#include <linux/list.h>
>>>>    #include <linux/perf_event.h>
>>>>    #include <linux/perf/arm_pmu.h>
>>>> +#include <linux/rculist.h>
>>>>    #include <linux/uaccess.h>
>>>>    #include <asm/kvm_emulate.h>
>>>>    #include <kvm/arm_pmu.h>
>>>> @@ -26,7 +26,6 @@ static bool kvm_pmu_counter_is_enabled(struct kvm_pmc *pmc);
>>>>      bool kvm_supports_guest_pmuv3(void)
>>>>    {
>>>> -	guard(mutex)(&arm_pmus_lock);
>>>>    	return !list_empty(&arm_pmus);
>>>
>>> Please read include/linux/rculist.h and the discussion about the
>>> interaction of list_empty() with RCU-protected lists. How about using
>>> list_first_or_null_rcu() for peace of mind?
>>
>> list_first_or_null_rcu() is useful to replace a sequence of
>> list_empty() and list_first_entry() that is protected by a lock, but
>> this function instead requires the invariant that nobody deletes an
>> element from the list, and list_first_or_null_rcu() does not allow
>> removing the requirement.
>>
>> The header file says:
>>> Where are list_empty_rcu() and list_first_entry_rcu()?
>>>
>>> They do not exist because they would lead to subtle race conditions:
>>>
>>> if (!list_empty_rcu(mylist)) {
>>> 	struct foo *bar = list_first_entry_rcu(mylist, struct foo,
>>> 					       list_member);
>>> 	do_something(bar);
>>> }
>>>
>>> The list might be non-empty when list_empty_rcu() checks it, but it
>>> might have become empty by the time that list_first_entry_rcu()
>>> rereads the ->next pointer, which would result in a SEGV.
>>>
>>> When not using RCU, it is OK for list_first_entry() to re-read that
>>> pointer because both functions should be protected by some lock that
>>> blocks writers.
>>>
>>> When using RCU, list_empty() uses READ_ONCE() to fetch the
>>> RCU-protected ->next pointer and then compares it to the address of
>>> the  list head.  However, it neither dereferences this pointer nor
>>> provides  this pointer to its caller.  Thus, READ_ONCE() suffices
>>> (that is,  rcu_dereference() is not needed), which means that
>>> list_empty() can be used anywhere you would want to use
>>> list_empty_rcu().  Just don't expect anything useful to happen if you
>>> do a subsequent lockless call to list_first_entry_rcu()!!!
>>>
>>> See list_first_or_null_rcu for an alternative.
>>
>> However, kvm_supports_guest_pmuv3() locked a mutex when calling
>> list_empty() and unlocked it immediately after that, instead of
>> re-reading list_first_entry(). This construct inherently had a race
>> condition with code that deletes an element; when the caller of
>> kvm_supports_guest_pmuv3() decides to enable guest PMUv3, the host PMU
>> may have been gone. But it was still safe because no one deletes an
>> element.
>>
>> The same logic also applies when using RCU. As the comment says, we
>> can use list_empty() instead of the hypothetical list_empty_rcu()
>> macro because we don't expect it to magically enable something like
>> list_first_entry_rcu(). This function instead keep relying on the fact
>> that no one deletes an element of the list.
> 
> And that's exactly the sort of thing I am trying to plan for. *Should*
> we introduce a way to remove PMUs from the list, this predicate
> becomes unsafe.

Perhaps so. In regards to this series, I'd rather like to keep it out of 
scope as the requirement is not new.

> 
> So I want at least a comment explaining this to the unsuspecting
> reader, as this is rather subtle.

I agree. I had to put some effort to understand the previous 
mutex-protected implementation and to design the new RCU-protected one. 
I'll add one with the next version.

Regards,
Akihiko Odaki