diff mbox series

[v2,08/13] KVM: arm64: Add support for KVM_EXIT_HYPERCALL

Message ID 20230330154918.4014761-9-oliver.upton@linux.dev (mailing list archive)
State New, archived
Headers show
Series [v2,01/13] KVM: x86: Redefine 'longmode' as a flag for KVM_EXIT_HYPERCALL | expand

Commit Message

Oliver Upton March 30, 2023, 3:49 p.m. UTC
In anticipation of user hypercall filters, add the necessary plumbing to
get SMCCC calls out to userspace. Even though the exit structure has
space for KVM to pass register arguments, let's just avoid it altogether
and let userspace poke at the registers via KVM_GET_ONE_REG.

This deliberately stretches the definition of a 'hypercall' to cover
SMCs from EL1 in addition to the HVCs we know and love. KVM doesn't
support EL1 calls into secure services, but now we can paint that as a
userspace problem and be done with it.

Finally, we need a flag to let userspace know what conduit instruction
was used (i.e. SMC vs. HVC).

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 Documentation/virt/kvm/api.rst    | 22 ++++++++++++++++++++--
 arch/arm64/include/uapi/asm/kvm.h |  4 ++++
 arch/arm64/kvm/handle_exit.c      |  4 +++-
 arch/arm64/kvm/hypercalls.c       | 17 +++++++++++++++++
 4 files changed, 44 insertions(+), 3 deletions(-)

Comments

Marc Zyngier March 31, 2023, 5:10 p.m. UTC | #1
On Thu, 30 Mar 2023 16:49:13 +0100,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> In anticipation of user hypercall filters, add the necessary plumbing to
> get SMCCC calls out to userspace. Even though the exit structure has
> space for KVM to pass register arguments, let's just avoid it altogether
> and let userspace poke at the registers via KVM_GET_ONE_REG.
> 
> This deliberately stretches the definition of a 'hypercall' to cover
> SMCs from EL1 in addition to the HVCs we know and love. KVM doesn't
> support EL1 calls into secure services, but now we can paint that as a
> userspace problem and be done with it.
> 
> Finally, we need a flag to let userspace know what conduit instruction
> was used (i.e. SMC vs. HVC).
> 
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  Documentation/virt/kvm/api.rst    | 22 ++++++++++++++++++++--
>  arch/arm64/include/uapi/asm/kvm.h |  4 ++++
>  arch/arm64/kvm/handle_exit.c      |  4 +++-
>  arch/arm64/kvm/hypercalls.c       | 17 +++++++++++++++++
>  4 files changed, 44 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index 9b01e3d0e757..c8ab2f730945 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -6221,11 +6221,29 @@ to the byte array.
>  			__u64 flags;
>  		} hypercall;
>  
> -Unused.  This was once used for 'hypercall to userspace'.  To implement
> -such functionality, use KVM_EXIT_IO (x86) or KVM_EXIT_MMIO (all except s390).
> +
> +It is strongly recommended that userspace use ``KVM_EXIT_IO`` (x86) or
> +``KVM_EXIT_MMIO`` (all except s390) to implement functionality that
> +requires a guest to interact with host userpace.
>  
>  .. note:: KVM_EXIT_IO is significantly faster than KVM_EXIT_MMIO.
>  
> +For arm64:
> +----------
> +
> +SMCCC exits can be enabled depending on the configuration of the SMCCC
> +filter. See the Documentation/virt/kvm/devices/vm.rst
> +``KVM_ARM_SMCCC_FILTER`` for more details.

Maybe this hunk should come with the following patch which actually
adds that doc.

> +
> +``nr`` contains the function ID of the guest's SMCCC call. Userspace is
> +expected to use the ``KVM_GET_ONE_REG`` ioctl to retrieve the call
> +parameters from the vCPU's GPRs.
> +
> +Definition of ``flags``:
> + - ``KVM_HYPERCALL_EXIT_SMC``: Indicates that the guest used the SMC
> +   conduit to initiate the SMCCC call. If this bit is 0 then the guest
> +   used the HVC conduit for the SMCCC call.
> +
>  ::
>  
>  		/* KVM_EXIT_TPR_ACCESS */
> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> index bbab92402510..1dabb7d05514 100644
> --- a/arch/arm64/include/uapi/asm/kvm.h
> +++ b/arch/arm64/include/uapi/asm/kvm.h
> @@ -472,12 +472,16 @@ enum {
>  enum kvm_smccc_filter_action {
>  	KVM_SMCCC_FILTER_ALLOW = 0,
>  	KVM_SMCCC_FILTER_DENY,
> +	KVM_SMCCC_FILTER_FWD_TO_USER,
>  
>  #ifdef __KERNEL__
>  	NR_SMCCC_FILTER_ACTIONS
>  #endif
>  };
>  
> +/* arm64-specific KVM_EXIT_HYPERCALL flags */
> +#define KVM_HYPERCALL_EXIT_SMC	(1U << 0)
> +
>  #endif
>  
>  #endif /* __ARM_KVM_H__ */
> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
> index 68f95dcd41a1..3f43e20c48b6 100644
> --- a/arch/arm64/kvm/handle_exit.c
> +++ b/arch/arm64/kvm/handle_exit.c
> @@ -71,7 +71,9 @@ static int handle_smc(struct kvm_vcpu *vcpu)
>  	 * Trap exception, not a Secure Monitor Call exception [...]"
>  	 *
>  	 * We need to advance the PC after the trap, as it would
> -	 * otherwise return to the same address...
> +	 * otherwise return to the same address. Furthermore, pre-incrementing
> +	 * the PC before potentially exiting to userspace maintains the same
> +	 * abstraction for both SMCs and HVCs.
>  	 */
>  	kvm_incr_pc(vcpu);
>  
> diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> index 73b218ddd1a5..7e8c850847c1 100644
> --- a/arch/arm64/kvm/hypercalls.c
> +++ b/arch/arm64/kvm/hypercalls.c
> @@ -180,6 +180,19 @@ static u8 kvm_smccc_get_action(struct kvm_vcpu *vcpu, u32 func_id)
>  	return KVM_SMCCC_FILTER_DENY;
>  }
>  
> +static void kvm_prepare_hypercall_exit(struct kvm_vcpu *vcpu, u32 func_id)
> +{
> +	u8 ec = ESR_ELx_EC(kvm_vcpu_get_esr(vcpu));
> +	struct kvm_run *run = vcpu->run;
> +
> +	run->exit_reason = KVM_EXIT_HYPERCALL;
> +	run->hypercall.nr = func_id;
> +	run->hypercall.flags = 0;
> +
> +	if (ec == ESR_ELx_EC_SMC32 || ec == ESR_ELx_EC_SMC64)
> +		run->hypercall.flags |= KVM_HYPERCALL_EXIT_SMC;
> +}
> +
>  int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
> @@ -192,6 +205,10 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
>  	action = kvm_smccc_get_action(vcpu, func_id);
>  	if (action == KVM_SMCCC_FILTER_DENY)
>  		goto out;
> +	if (action == KVM_SMCCC_FILTER_FWD_TO_USER) {
> +		kvm_prepare_hypercall_exit(vcpu, func_id);
> +		return 0;
> +	}

nit: maybe write this as a switch statement?

>  
>  	switch (func_id) {
>  	case ARM_SMCCC_VERSION_FUNC_ID:

Thanks,

	M.
Oliver Upton March 31, 2023, 6 p.m. UTC | #2
On Fri, Mar 31, 2023 at 06:10:52PM +0100, Marc Zyngier wrote:
> On Thu, 30 Mar 2023 16:49:13 +0100,
> Oliver Upton <oliver.upton@linux.dev> wrote:
> > 
> > In anticipation of user hypercall filters, add the necessary plumbing to
> > get SMCCC calls out to userspace. Even though the exit structure has
> > space for KVM to pass register arguments, let's just avoid it altogether
> > and let userspace poke at the registers via KVM_GET_ONE_REG.
> > 
> > This deliberately stretches the definition of a 'hypercall' to cover
> > SMCs from EL1 in addition to the HVCs we know and love. KVM doesn't
> > support EL1 calls into secure services, but now we can paint that as a
> > userspace problem and be done with it.
> > 
> > Finally, we need a flag to let userspace know what conduit instruction
> > was used (i.e. SMC vs. HVC).
> > 
> > Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> > ---
> >  Documentation/virt/kvm/api.rst    | 22 ++++++++++++++++++++--
> >  arch/arm64/include/uapi/asm/kvm.h |  4 ++++
> >  arch/arm64/kvm/handle_exit.c      |  4 +++-
> >  arch/arm64/kvm/hypercalls.c       | 17 +++++++++++++++++
> >  4 files changed, 44 insertions(+), 3 deletions(-)
> > 
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index 9b01e3d0e757..c8ab2f730945 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -6221,11 +6221,29 @@ to the byte array.
> >  			__u64 flags;
> >  		} hypercall;
> >  
> > -Unused.  This was once used for 'hypercall to userspace'.  To implement
> > -such functionality, use KVM_EXIT_IO (x86) or KVM_EXIT_MMIO (all except s390).
> > +
> > +It is strongly recommended that userspace use ``KVM_EXIT_IO`` (x86) or
> > +``KVM_EXIT_MMIO`` (all except s390) to implement functionality that
> > +requires a guest to interact with host userpace.
> >  
> >  .. note:: KVM_EXIT_IO is significantly faster than KVM_EXIT_MMIO.
> >  
> > +For arm64:
> > +----------
> > +
> > +SMCCC exits can be enabled depending on the configuration of the SMCCC
> > +filter. See the Documentation/virt/kvm/devices/vm.rst
> > +``KVM_ARM_SMCCC_FILTER`` for more details.
> 
> Maybe this hunk should come with the following patch which actually
> adds that doc.

Heh, you caught me being lazy :) Can do.

> > diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
> > index 73b218ddd1a5..7e8c850847c1 100644
> > --- a/arch/arm64/kvm/hypercalls.c
> > +++ b/arch/arm64/kvm/hypercalls.c
> > @@ -180,6 +180,19 @@ static u8 kvm_smccc_get_action(struct kvm_vcpu *vcpu, u32 func_id)
> >  	return KVM_SMCCC_FILTER_DENY;
> >  }
> >  
> > +static void kvm_prepare_hypercall_exit(struct kvm_vcpu *vcpu, u32 func_id)
> > +{
> > +	u8 ec = ESR_ELx_EC(kvm_vcpu_get_esr(vcpu));
> > +	struct kvm_run *run = vcpu->run;
> > +
> > +	run->exit_reason = KVM_EXIT_HYPERCALL;
> > +	run->hypercall.nr = func_id;
> > +	run->hypercall.flags = 0;
> > +
> > +	if (ec == ESR_ELx_EC_SMC32 || ec == ESR_ELx_EC_SMC64)
> > +		run->hypercall.flags |= KVM_HYPERCALL_EXIT_SMC;
> > +}
> > +
> >  int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
> >  {
> >  	struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
> > @@ -192,6 +205,10 @@ int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
> >  	action = kvm_smccc_get_action(vcpu, func_id);
> >  	if (action == KVM_SMCCC_FILTER_DENY)
> >  		goto out;
> > +	if (action == KVM_SMCCC_FILTER_FWD_TO_USER) {
> > +		kvm_prepare_hypercall_exit(vcpu, func_id);
> > +		return 0;
> > +	}
> 
> nit: maybe write this as a switch statement?

Sure thing. I'll get a new spin on the list sometime in the next day or
two that addresses your feedback. Appreciate the review
diff mbox series

Patch

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 9b01e3d0e757..c8ab2f730945 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -6221,11 +6221,29 @@  to the byte array.
 			__u64 flags;
 		} hypercall;
 
-Unused.  This was once used for 'hypercall to userspace'.  To implement
-such functionality, use KVM_EXIT_IO (x86) or KVM_EXIT_MMIO (all except s390).
+
+It is strongly recommended that userspace use ``KVM_EXIT_IO`` (x86) or
+``KVM_EXIT_MMIO`` (all except s390) to implement functionality that
+requires a guest to interact with host userpace.
 
 .. note:: KVM_EXIT_IO is significantly faster than KVM_EXIT_MMIO.
 
+For arm64:
+----------
+
+SMCCC exits can be enabled depending on the configuration of the SMCCC
+filter. See the Documentation/virt/kvm/devices/vm.rst
+``KVM_ARM_SMCCC_FILTER`` for more details.
+
+``nr`` contains the function ID of the guest's SMCCC call. Userspace is
+expected to use the ``KVM_GET_ONE_REG`` ioctl to retrieve the call
+parameters from the vCPU's GPRs.
+
+Definition of ``flags``:
+ - ``KVM_HYPERCALL_EXIT_SMC``: Indicates that the guest used the SMC
+   conduit to initiate the SMCCC call. If this bit is 0 then the guest
+   used the HVC conduit for the SMCCC call.
+
 ::
 
 		/* KVM_EXIT_TPR_ACCESS */
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
index bbab92402510..1dabb7d05514 100644
--- a/arch/arm64/include/uapi/asm/kvm.h
+++ b/arch/arm64/include/uapi/asm/kvm.h
@@ -472,12 +472,16 @@  enum {
 enum kvm_smccc_filter_action {
 	KVM_SMCCC_FILTER_ALLOW = 0,
 	KVM_SMCCC_FILTER_DENY,
+	KVM_SMCCC_FILTER_FWD_TO_USER,
 
 #ifdef __KERNEL__
 	NR_SMCCC_FILTER_ACTIONS
 #endif
 };
 
+/* arm64-specific KVM_EXIT_HYPERCALL flags */
+#define KVM_HYPERCALL_EXIT_SMC	(1U << 0)
+
 #endif
 
 #endif /* __ARM_KVM_H__ */
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index 68f95dcd41a1..3f43e20c48b6 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -71,7 +71,9 @@  static int handle_smc(struct kvm_vcpu *vcpu)
 	 * Trap exception, not a Secure Monitor Call exception [...]"
 	 *
 	 * We need to advance the PC after the trap, as it would
-	 * otherwise return to the same address...
+	 * otherwise return to the same address. Furthermore, pre-incrementing
+	 * the PC before potentially exiting to userspace maintains the same
+	 * abstraction for both SMCs and HVCs.
 	 */
 	kvm_incr_pc(vcpu);
 
diff --git a/arch/arm64/kvm/hypercalls.c b/arch/arm64/kvm/hypercalls.c
index 73b218ddd1a5..7e8c850847c1 100644
--- a/arch/arm64/kvm/hypercalls.c
+++ b/arch/arm64/kvm/hypercalls.c
@@ -180,6 +180,19 @@  static u8 kvm_smccc_get_action(struct kvm_vcpu *vcpu, u32 func_id)
 	return KVM_SMCCC_FILTER_DENY;
 }
 
+static void kvm_prepare_hypercall_exit(struct kvm_vcpu *vcpu, u32 func_id)
+{
+	u8 ec = ESR_ELx_EC(kvm_vcpu_get_esr(vcpu));
+	struct kvm_run *run = vcpu->run;
+
+	run->exit_reason = KVM_EXIT_HYPERCALL;
+	run->hypercall.nr = func_id;
+	run->hypercall.flags = 0;
+
+	if (ec == ESR_ELx_EC_SMC32 || ec == ESR_ELx_EC_SMC64)
+		run->hypercall.flags |= KVM_HYPERCALL_EXIT_SMC;
+}
+
 int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
 {
 	struct kvm_smccc_features *smccc_feat = &vcpu->kvm->arch.smccc_feat;
@@ -192,6 +205,10 @@  int kvm_smccc_call_handler(struct kvm_vcpu *vcpu)
 	action = kvm_smccc_get_action(vcpu, func_id);
 	if (action == KVM_SMCCC_FILTER_DENY)
 		goto out;
+	if (action == KVM_SMCCC_FILTER_FWD_TO_USER) {
+		kvm_prepare_hypercall_exit(vcpu, func_id);
+		return 0;
+	}
 
 	switch (func_id) {
 	case ARM_SMCCC_VERSION_FUNC_ID: