From: Oliver Upton <oliver.upton@linux.dev>
To: Fuad Tabba <tabba@google.com>
Cc: kvmarm@lists.linux.dev, maz@kernel.org, will@kernel.org,
qperret@google.com, seanjc@google.com, alexandru.elisei@arm.com,
catalin.marinas@arm.com, philmd@linaro.org, james.morse@arm.com,
suzuki.poulose@arm.com, mark.rutland@arm.com, broonie@kernel.org,
joey.gouly@arm.com, rananta@google.com, smostafa@google.com
Subject: Re: [PATCH v3 31/31] KVM: arm64: Force injection of a data abort on NISV MMIO exit
Date: Fri, 19 Apr 2024 20:28:27 +0000 [thread overview]
Message-ID: <ZiLT60yvCMcJmK5T@linux.dev> (raw)
In-Reply-To: <20240419075941.4085061-32-tabba@google.com>
On Fri, Apr 19, 2024 at 08:59:41AM +0100, Fuad Tabba wrote:
> From: Marc Zyngier <maz@kernel.org>
>
> If a vcpu exits for a data abort with an invalid syndrome, the
> expectations are that userspace has a chance to save the day if
> it has requested to see such exits.
>
> However, this is completely futile in the case of a protected VM,
> as none of the state is available. In this particular case, inject
> a data abort directly into the vcpu, consistent with what userspace
> could do.
>
> This also helps with pKVM, which discards all syndrome information when
> forwarding data aborts that are not known to be MMIO.
>
> Finally, hide the RETURN_NISV_IO_ABORT_TO_USER cap from userspace on
> protected VMs, and document this tweak to the API.
Are there going to be more KVM caps that we hide from protected VMs in
the future? If so, it might be better to have a common helper that
enforces a denylist of capabilities not supported by pKVM.
That way we can avoid adding kvm_vm_is_protected() checks all over the
shop.
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 66301131d5a9..750386a84968 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -80,9 +80,13 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>
> switch (cap->cap) {
> case KVM_CAP_ARM_NISV_TO_USER:
> - r = 0;
> - set_bit(KVM_ARCH_FLAG_RETURN_NISV_IO_ABORT_TO_USER,
> - &kvm->arch.flags);
> + if (kvm_vm_is_protected(kvm)) {
> + r = -EINVAL;
> + } else {
> + r = 0;
> + set_bit(KVM_ARCH_FLAG_RETURN_NISV_IO_ABORT_TO_USER,
> + &kvm->arch.flags);
> + }
nitpick: initialize r = -EINVAL and get rid of all the error-path
initializations in kvm_vm_ioctl_enable_cap()
> break;
> case KVM_CAP_ARM_MTE:
> mutex_lock(&kvm->lock);
> @@ -237,7 +241,6 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_IMMEDIATE_EXIT:
> case KVM_CAP_VCPU_EVENTS:
> case KVM_CAP_ARM_IRQ_LINE_LAYOUT_2:
> - case KVM_CAP_ARM_NISV_TO_USER:
> case KVM_CAP_ARM_INJECT_EXT_DABT:
> case KVM_CAP_SET_GUEST_DEBUG:
> case KVM_CAP_VCPU_ATTRIBUTES:
> @@ -247,6 +250,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> case KVM_CAP_COUNTER_OFFSET:
> r = 1;
> break;
> + case KVM_CAP_ARM_NISV_TO_USER:
> + r = !kvm || !kvm_vm_is_protected(kvm);
> + break;
> case KVM_CAP_SET_GUEST_DEBUG2:
> return KVM_GUESTDBG_VALID_MASK;
> case KVM_CAP_ARM_SET_DEVICE_ADDR:
> diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c
> index 5e1ffb0d5363..75e1072948cd 100644
> --- a/arch/arm64/kvm/mmio.c
> +++ b/arch/arm64/kvm/mmio.c
> @@ -133,11 +133,19 @@ int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa)
> /*
> * No valid syndrome? Ask userspace for help if it has
> * volunteered to do so, and bail out otherwise.
> + *
> + * In the protected VM case, there isn't much userspace can do
> + * though, so directly deliver an exception to the guest.
> */
> if (!kvm_vcpu_dabt_isvalid(vcpu)) {
> trace_kvm_mmio_nisv(*vcpu_pc(vcpu), kvm_vcpu_get_esr(vcpu),
> kvm_vcpu_get_hfar(vcpu), fault_ipa);
>
> + if (is_protected_kvm_enabled() && vcpu_is_protected(vcpu)) {
You can drop the is_protected_kvm_enabled() now that it got folded in to
kvm_vm_is_protected().
--
Thanks,
Oliver
next prev parent reply other threads:[~2024-04-19 20:28 UTC|newest]
Thread overview: 44+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-19 7:59 [PATCH v3 00/31] KVM: arm64: Preamble for pKVM Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 01/31] KVM: arm64: Initialize the kvm host data's fpsimd_state pointer in pKVM Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 02/31] KVM: arm64: Move guest_owns_fp_regs() to increase its scope Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 03/31] KVM: arm64: Refactor checks for FP state ownership Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 04/31] KVM: arm64: Do not re-initialize the KVM lock Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 05/31] KVM: arm64: Issue CMOs when tearing down guest s2 pages Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 06/31] KVM: arm64: Avoid BUG-ing from the host abort path Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 07/31] KVM: arm64: Check for PTE validity when checking for executable/cacheable Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 08/31] KVM: arm64: Avoid BBM when changing only s/w bits in Stage-2 PTE Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 09/31] KVM: arm64: Support TLB invalidation in guest context Fuad Tabba
2024-04-19 20:54 ` Oliver Upton
2024-04-22 8:11 ` Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 10/31] KVM: arm64: Do not map the host fpsimd state to hyp in pKVM Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 11/31] KVM: arm64: Remove locking from EL2 allocation fast-paths Fuad Tabba
2024-04-19 20:42 ` Oliver Upton
2024-04-22 8:09 ` Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 12/31] KVM: arm64: Prevent kmemleak from accessing .hyp.data Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 13/31] KVM: arm64: Fix comment for __pkvm_vcpu_init_traps() Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 14/31] KVM: arm64: Change kvm_handle_mmio_return() return polarity Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 15/31] KVM: arm64: Move setting the page as dirty out of the critical section Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 16/31] KVM: arm64: Simplify vgic-v3 hypercalls Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 17/31] KVM: arm64: Add is_pkvm_initialized() helper Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 18/31] KVM: arm64: Introduce and use predicates that check for protected VMs Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 19/31] KVM: arm64: Move pstate reset value definitions to kvm_arm.h Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 20/31] KVM: arm64: Clarify rationale for ZCR_EL1 value restored on guest exit Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 21/31] KVM: arm64: Refactor calculating SVE state size to use helpers Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 22/31] KVM: arm64: Move some kvm_psci functions to a shared header Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 23/31] KVM: arm64: Refactor reset_mpidr() to extract its computation Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 24/31] KVM: arm64: Refactor kvm_vcpu_enable_ptrauth() for hyp use Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 25/31] KVM: arm64: Introduce hyp_rwlock_t Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 26/31] KVM: arm64: Add atomics-based checking refcount implementation at EL2 Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 27/31] KVM: arm64: Use atomic refcount helpers for 'struct hyp_page::refcount' Fuad Tabba
2024-04-19 20:52 ` Oliver Upton
2024-04-22 8:10 ` Fuad Tabba
2024-04-22 13:08 ` Fuad Tabba
2024-04-22 20:46 ` Oliver Upton
2024-04-22 23:44 ` Will Deacon
2024-04-23 1:15 ` Oliver Upton
2024-04-19 7:59 ` [PATCH v3 28/31] KVM: arm64: Reformat/beautify PTP hypercall documentation Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 29/31] KVM: arm64: Rename firmware pseudo-register documentation file Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 30/31] KVM: arm64: Document the KVM/arm64-specific calls in hypercalls.rst Fuad Tabba
2024-04-19 7:59 ` [PATCH v3 31/31] KVM: arm64: Force injection of a data abort on NISV MMIO exit Fuad Tabba
2024-04-19 20:28 ` Oliver Upton [this message]
2024-04-22 8:07 ` Fuad Tabba
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZiLT60yvCMcJmK5T@linux.dev \
--to=oliver.upton@linux.dev \
--cc=alexandru.elisei@arm.com \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=james.morse@arm.com \
--cc=joey.gouly@arm.com \
--cc=kvmarm@lists.linux.dev \
--cc=mark.rutland@arm.com \
--cc=maz@kernel.org \
--cc=philmd@linaro.org \
--cc=qperret@google.com \
--cc=rananta@google.com \
--cc=seanjc@google.com \
--cc=smostafa@google.com \
--cc=suzuki.poulose@arm.com \
--cc=tabba@google.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.