Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.35 as permitted sender) client-ip=23.128.96.35;
Date:   Fri, 1 Dec 2023 09:30:18 -0800
In-Reply-To: <20231116133628.5976-1-clopez@suse.de>
Mime-Version: 1.0
References: <20231116133628.5976-1-clopez@suse.de>
Message-ID: <ZWoYKs0OKdSOLOLR@google.com>
Subject: Re: [PATCH v2] KVM: X86: improve documentation for KVM_CAP_X86_BUS_LOCK_EXIT
From:   Sean Christopherson <seanjc@google.com>
To:     "Carlos =?utf-8?B?TMOzcGV6?=" <clopez@suse.de>
Cc:     linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
        linux-doc@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        Jonathan Corbet <corbet@lwn.net>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Nov 16, 2023, Carlos L=C3=B3pez wrote:
> Improve the description for the KVM_CAP_X86_BUS_LOCK_EXIT capability,
> fixing a few typos and improving grammar for overall clarity.
>=20
> Signed-off-by: Carlos L=C3=B3pez <clopez@suse.de>
> ---
> v2: Corrected the name of the KVM_RUN_X86_BUS_LOCK flag
>=20
>  Documentation/virt/kvm/api.rst | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>=20
> 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::
> =20
>    /* 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)
> =20
> @@ -7582,20 +7582,20 @@ KVM_BUS_LOCK_DETECTION_OFF and KVM_BUS_LOCK_DETEC=
TION_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 v=
m exits
> +With KVM_BUS_LOCK_DETECTION_OFF set, bus locks in guest will not cause V=
M exits
>  so that no additional actions are needed. This is the default mode.

Heh, this is technically wrong.  KVM intercepts #AC when the non-virtualize=
d
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 dete=
cted
> -in VM. KVM just exits to userspace when handling them. Userspace can enf=
orce
> -its own throttling or other policy based mitigations.
> -
> -This capability is aimed to address the thread that VM can exploit bus l=
ocks to
> -degree the performance of the whole system. Once the userspace enable th=
is
> -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. Co=
ncerning
> -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. Userspac=
e can
> +enforce its own throttling or other policy based mitigations.
> +
> +This capability is aimed to address the fact that a VM can exploit bus l=
ocks to

I liked the attempt to capture that this is a threat mitigation, although t=
he
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 se=
t the
> +KVM_RUN_X86_BUS_LOCK flag in the vcpu->run->flags field and exit to user=
space.

Oof, the existing wording is nonsensical.  This reads like KVM unconditiona=
lly 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 (w=
hich
triggers an #AC exception that KVM intercepts).  This capability is intende=
d to
mitigate attacks where a malicious/buggy guest can exploit bus locks to deg=
rade
the performance of the whole system.=20

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 enabl=
ed.

If bus lock detection is set to EXIT, KVM enables a CPU feature that ensure=
s
bus locks in the guest trigger a VM exit, and KVM exits to userspace for al=
l
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 s=
ets
KVM_RUN_X86_BUS_LOCK in vcpu-run->flags, and conditionally sets the exit_re=
ason
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 reaso=
n if
userspace wants to take action on all detected bus locks.