Message-ID: <ZWoYKs0OKdSOLOLR@google.com>
Date:   Fri, 1 Dec 2023 09:30:18 -0800
From:   Sean Christopherson <seanjc@...gle.com>
To:     "Carlos López" <clopez@...e.de>
Cc:     linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-doc@...r.kernel.org, Paolo Bonzini <pbonzini@...hat.com>,
        Jonathan Corbet <corbet@....net>
Subject: Re: [PATCH v2] KVM: X86: improve documentation for KVM_CAP_X86_BUS_LOCK_EXIT

On Thu, Nov 16, 2023, Carlos López wrote:
> Improve the description for the KVM_CAP_X86_BUS_LOCK_EXIT capability,
> fixing a few typos and improving grammar for overall clarity.
> 
> Signed-off-by: Carlos López <clopez@...e.de>
> ---
> v2: Corrected the name of the KVM_RUN_X86_BUS_LOCK flag
> 
>  Documentation/virt/kvm/api.rst | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 7025b3751027..4701370bf46f 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6256,9 +6256,9 @@ More architecture-specific flags detailing state of the VCPU that may
>  affect the device's behavior. Current defined flags::
>  
>    /* x86, set if the VCPU is in system management mode */
> -  #define KVM_RUN_X86_SMM     (1 << 0)
> +  #define KVM_RUN_X86_SMM          (1 << 0)
>    /* x86, set if bus lock detected in VM */
> -  #define KVM_RUN_BUS_LOCK    (1 << 1)
> +  #define KVM_RUN_X86_BUS_LOCK     (1 << 1)
>    /* arm64, set for KVM_EXIT_DEBUG */
>    #define KVM_DEBUG_ARCH_HSR_HIGH_VALID  (1 << 0)
>  
> @@ -7582,20 +7582,20 @@ KVM_BUS_LOCK_DETECTION_OFF and KVM_BUS_LOCK_DETECTION_EXIT are supported
>  currently and mutually exclusive with each other. More bits can be added in
>  the future.

Meh, there's a lot of pointless boilerplate in here.  These statements hold true
for almost every KVM capability.

> -With KVM_BUS_LOCK_DETECTION_OFF set, bus locks in guest will not cause vm exits
> +With KVM_BUS_LOCK_DETECTION_OFF set, bus locks in guest will not cause VM exits
>  so that no additional actions are needed. This is the default mode.

Heh, this is technically wrong.  KVM intercepts #AC when the non-virtualized
split-lock detection is enabled, i.e. bus locks will cause VM-Exit, though they
won't be forwarded to userspace.

> -With KVM_BUS_LOCK_DETECTION_EXIT set, vm exits happen when bus lock detected
> -in VM. KVM just exits to userspace when handling them. Userspace can enforce
> -its own throttling or other policy based mitigations.
> -
> -This capability is aimed to address the thread that VM can exploit bus locks to
> -degree the performance of the whole system. Once the userspace enable this
> -capability and select the KVM_BUS_LOCK_DETECTION_EXIT mode, KVM will set the
> -KVM_RUN_BUS_LOCK flag in vcpu-run->flags field and exit to userspace. Concerning
> -the bus lock vm exit can be preempted by a higher priority VM exit, the exit
> -notifications to userspace can be KVM_EXIT_BUS_LOCK or other reasons.
> -KVM_RUN_BUS_LOCK flag is used to distinguish between them.
> +With KVM_BUS_LOCK_DETECTION_EXIT set, VM exits happen when a bus lock is
> +detected in VM. KVM just exits to userspace when handling them. Userspace can
> +enforce its own throttling or other policy based mitigations.
> +
> +This capability is aimed to address the fact that a VM can exploit bus locks to

I liked the attempt to capture that this is a threat mitigation, although the
typo probably made that hard to see.

> +impact the performance of the whole system. Once userspace enables this
> +capability and selects the KVM_BUS_LOCK_DETECTION_EXIT mode, KVM will set the
> +KVM_RUN_X86_BUS_LOCK flag in the vcpu->run->flags field and exit to userspace.

Oof, the existing wording is nonsensical.  This reads like KVM unconditionally sets
KVM_RUN_X86_BUS_LOCK whenever the capability is enabled.

> +Concerning the bus lock, a VM exit can be preempted by a higher priority VM
> +exit, so the exit notification to userspace can be KVM_EXIT_BUS_LOCK or another
> +reason. KVM_RUN_X86_BUS_LOCK flag is used to distinguish between them.

This is also worded rather weirdly.  It's not so much that VM exits can be
preempted, it's that the CPU can detect a bus lock while also delivering an
unrelated VM exit.

Something like this?

---

This capability allows userspace to force VM exits on bus locks detected in the
guest, irrespective whether or not host has enabled split-lock detection (which
triggers an #AC exception that KVM intercepts).  This capability is intended to
mitigate attacks where a malicious/buggy guest can exploit bus locks to degrade
the performance of the whole system. 

If bus lock detection is OFF, KVM doesn't force guest bus locks to VM exit,
although the host kernel's split-lock #AC detection still applies, if enabled.

If bus lock detection is set to EXIT, KVM enables a CPU feature that ensures
bus locks in the guest trigger a VM exit, and KVM exits to userspace for all
such VM exits, e.g. to allow userspace to throttle the offending guest and/or
apply some other policy-based mitigation.  When exiting to userspace, KVM sets
KVM_RUN_X86_BUS_LOCK in vcpu-run->flags, and conditionally sets the exit_reason
to KVM_EXIT_X86_BUS_LOCK.

Note!  Detected bus locks may be coincident with other exits to userspace, i.e.
KVM_RUN_X86_BUS_LOCK should be checked regardless of the primary exit reason if
userspace wants to take action on all detected bus locks.

Powered by blists - more mailing lists