Re: [PATCH] KVM: arm64: Fix smp_processor_id() call in preemptible context

From: Oliver Upton <oliver.upton@linux.dev>
To: Sean Christopherson <seanjc@google.com>
Cc: Sebastian Ott <sebott@redhat.com>,
	kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org,
	Marc Zyngier <maz@kernel.org>
Subject: Re: [PATCH] KVM: arm64: Fix smp_processor_id() call in preemptible context
Date: Tue, 6 Jun 2023 15:18:27 +0000	[thread overview]
Message-ID: <ZH9OQzKIfwxR3q54@linux.dev> (raw)
In-Reply-To: <ZH9CvE54NSVXqjlO@google.com>

On Tue, Jun 06, 2023 at 07:29:16AM -0700, Sean Christopherson wrote:
> On Tue, Jun 06, 2023, Oliver Upton wrote:
> > The call from a preemptible context is intentional, so this really
> > should just be raw_smp_processor_id(). Do you mind if we fix it with the
> > following?
> 
> ...
> 
> > Nonetheless, there's no functional requirement for disabling preemption,
> > as the cpu # is only used to walk the arm_pmus list. Fix it by using
> > raw_smp_processor_id() instead.
> 
> As a partial outsider, that needs an explanation, and the code could really use a
> comment.  I assume KVM's ABI is that it's userspace's responsibility to ensure that
> the CPU(s) used for KVM_RUN is compatible with the CPU used for KVM_ARM_VCPU_PMU_V3_CTRL,
> but neither the original changelog nor the above state that, nor does anything
> explain what happens if userspace doesn't uphold its side of things.

See commit 40e54cad4540 ("KVM: arm64: Document default vPMU behavior on
heterogeneous systems"), which documents the subtlety of vCPU scheduling
with the 'old' ABI at the callsite of this function. I don't want to
bleed any details of this crap into user documentation, since the entire
scheme is irretrievably broken.

See Documentation/virt/kvm/api/devices/vcpu.rst 1.4 for the 'new' ABI
where userspace explicitly selects a vPMU instance.

> That stuff might be covered in documentation somewhere, but for someone
> just looking at git blame, this is all very magical.

Personally, I find any other fix that involves disabling preemption to be
quite a lot more 'magical', as there isn't any percpu data we're working
with in the loop.

-- 
Thanks,
Oliver

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel