[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aL8yoZLK73svpYv7@google.com>
Date: Mon, 8 Sep 2025 12:46:41 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Keir Fraser <keirf@...gle.com>
Cc: linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
kvm@...r.kernel.org, Eric Auger <eric.auger@...hat.com>,
Oliver Upton <oliver.upton@...ux.dev>, Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>,
Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v3 3/4] KVM: Implement barriers before accessing
kvm->buses[] on SRCU read paths
On Tue, Aug 19, 2025, Keir Fraser wrote:
> This ensures that, if a VCPU has "observed" that an IO registration has
> occurred, the instruction currently being trapped or emulated will also
> observe the IO registration.
>
> At the same time, enforce that kvm_get_bus() is used only on the
> update side, ensuring that a long-term reference cannot be obtained by
> an SRCU reader.
>
> Signed-off-by: Keir Fraser <keirf@...gle.com>
> ---
> arch/x86/kvm/vmx/vmx.c | 7 +++++++
> include/linux/kvm_host.h | 10 +++++++---
> virt/kvm/kvm_main.c | 33 +++++++++++++++++++++++++++------
> 3 files changed, 41 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index aa157fe5b7b3..2d3c8cb4f860 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -5785,6 +5785,13 @@ static int handle_invalid_guest_state(struct kvm_vcpu *vcpu)
> if (kvm_test_request(KVM_REQ_EVENT, vcpu))
> return 1;
>
> + /*
> + * Ensure that any updates to kvm->buses[] observed by the
> + * previous instruction (emulated or otherwise) are also
> + * visible to the instruction we are about to emulate.
Please avoid pronouns, e.g.
* visible to the instruction KVM is about to emulate.
> + */
> + smp_rmb();
...
> static inline struct kvm_vcpu *kvm_get_vcpu(struct kvm *kvm, int i)
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 6c07dd423458..4f35ae23ee5a 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1103,6 +1103,15 @@ void __weak kvm_arch_create_vm_debugfs(struct kvm *kvm)
> {
> }
>
> +/* Called only on cleanup and destruction paths when there are no users. */
> +static inline struct kvm_io_bus *kvm_get_bus_for_destruction(struct kvm *kvm,
> + enum kvm_bus idx)
> +{
> + return rcu_dereference_protected(kvm->buses[idx],
> + !refcount_read(&kvm->users_count));
> +}
> +
Extra newline.
> +
> static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
> {
> struct kvm *kvm = kvm_arch_alloc_vm();
> @@ -1228,7 +1237,7 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
> out_err_no_arch_destroy_vm:
> WARN_ON_ONCE(!refcount_dec_and_test(&kvm->users_count));
> for (i = 0; i < KVM_NR_BUSES; i++)
> - kfree(kvm_get_bus(kvm, i));
> + kfree(kvm_get_bus_for_destruction(kvm, i));
> kvm_free_irq_routing(kvm);
> out_err_no_irq_routing:
> cleanup_srcu_struct(&kvm->irq_srcu);
> @@ -1276,7 +1285,7 @@ static void kvm_destroy_vm(struct kvm *kvm)
>
> kvm_free_irq_routing(kvm);
> for (i = 0; i < KVM_NR_BUSES; i++) {
> - struct kvm_io_bus *bus = kvm_get_bus(kvm, i);
> + struct kvm_io_bus *bus = kvm_get_bus_for_destruction(kvm, i);
>
> if (bus)
> kvm_io_bus_destroy(bus);
> @@ -5843,6 +5852,18 @@ static int __kvm_io_bus_write(struct kvm_vcpu *vcpu, struct kvm_io_bus *bus,
> return -EOPNOTSUPP;
> }
>
> +static struct kvm_io_bus *kvm_get_bus_srcu(struct kvm *kvm, enum kvm_bus idx)
> +{
> + /*
> + * Ensure that any updates to kvm_buses[] observed by the previous VCPU
s/VCPU/vCPU to match KVM's preferred/typical style.
> + * machine instruction are also visible to the VCPU machine instruction
> + * that triggered this call.
> + */
> + smp_mb__after_srcu_read_lock();
> +
> + return srcu_dereference(kvm->buses[idx], &kvm->srcu);
> +}
Powered by blists - more mailing lists