Re: [PATCH v7 17/45] arm64: RME: Handle realm enter/exit

Gavin Shan
2025-03-04T01:03:35Z

On 2/14/25 2:13 AM, Steven Price wrote:
> Entering a realm is done using a SMC call to the RMM. On exit the
> exit-codes need to be handled slightly differently to the normal KVM
> path so define our own functions for realm enter/exit and hook them
> in if the guest is a realm guest.
>
> Signed-off-by: Steven Price <steven.price@arm.com>
> ---
> Changes since v6:
> * Use vcpu_err() rather than pr_err/kvm_err when there is an associated
> vcpu to the error.
> * Return -EFAULT for KVM_EXIT_MEMORY_FAULT as per the documentation for
> this exit type.
> * Split code handling a RIPAS change triggered by the guest to the
> following patch.
> Changes since v5:
> * For a RIPAS_CHANGE request from the guest perform the actual RIPAS
> change on next entry rather than immediately on the exit. This allows
> the VMM to 'reject' a RIPAS change by refusing to continue
> scheduling.
> Changes since v4:
> * Rename handle_rme_exit() to handle_rec_exit()
> * Move the loop to copy registers into the REC enter structure from the
> to rec_exit_handlers callbacks to kvm_rec_enter(). This fixes a bug
> where the handler exits to user space and user space wants to modify
> the GPRS.
> * Some code rearrangement in rec_exit_ripas_change().
> Changes since v2:
> * realm_set_ipa_state() now provides an output parameter for the
> top_iap that was changed. Use this to signal the VMM with the correct
> range that has been transitioned.
> * Adapt to previous patch changes.
> ---
> arch/arm64/include/asm/kvm_rme.h | 3 +
> arch/arm64/kvm/Makefile | 2 +-
> arch/arm64/kvm/arm.c | 19 +++-
> arch/arm64/kvm/rme-exit.c | 171 +++++++++++++++++++++++++++++++
> arch/arm64/kvm/rme.c | 19 ++++
> 5 files changed, 208 insertions(+), 6 deletions(-)
> create mode 100644 arch/arm64/kvm/rme-exit.c
>

With below nitpicks addressed:

Reviewed-by: Gavin Shan <gshan@redhat.com>

[...]

> diff --git a/arch/arm64/kvm/rme-exit.c b/arch/arm64/kvm/rme-exit.c
> new file mode 100644
> index 000000000000..aae1adefe1a3
> --- /dev/null
> +++ b/arch/arm64/kvm/rme-exit.c
> @@ -0,0 +1,171 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2023 ARM Ltd.
> + */
> +
> +#include <linux/kvm_host.h>
> +#include <kvm/arm_hypercalls.h>
> +#include <kvm/arm_psci.h>
> +
> +#include <asm/rmi_smc.h>
> +#include <asm/kvm_emulate.h>
> +#include <asm/kvm_rme.h>
> +#include <asm/kvm_mmu.h>
> +
> +typedef int (*exit_handler_fn)(struct kvm_vcpu *vcpu);
> +

Duplicated to exit_handler_fn, defined in handle_exit.c, need move the
definition to header file.

> +static int rec_exit_reason_notimpl(struct kvm_vcpu *vcpu)
> +{
> + struct realm_rec *rec = &vcpu->arch.rec;
> +
> + vcpu_err(vcpu, "Unhandled exit reason from realm (ESR: %#llx)\n",
> + rec->run->exit.esr);
> + return -ENXIO;
> +}
> +
> +static int rec_exit_sync_dabt(struct kvm_vcpu *vcpu)
> +{
> + return kvm_handle_guest_abort(vcpu);
> +}
> +
> +static int rec_exit_sync_iabt(struct kvm_vcpu *vcpu)
> +{
> + struct realm_rec *rec = &vcpu->arch.rec;
> +
> + vcpu_err(vcpu, "Unhandled instruction abort (ESR: %#llx).\n",
> + rec->run->exit.esr);
> + return -ENXIO;
> +}
> +
> +static int rec_exit_sys_reg(struct kvm_vcpu *vcpu)
> +{
> + struct realm_rec *rec = &vcpu->arch.rec;
> + unsigned long esr = kvm_vcpu_get_esr(vcpu);
> + int rt = kvm_vcpu_sys_get_rt(vcpu);
> + bool is_write = !(esr & 1);
> + int ret;
> +
> + if (is_write)
> + vcpu_set_reg(vcpu, rt, rec->run->exit.gprs[0]);
> +
> + ret = kvm_handle_sys_reg(vcpu);
> +
> + if (ret >= 0 && !is_write)
> + rec->run->enter.gprs[0] = vcpu_get_reg(vcpu, rt);
> +

Unncessary blank line and the conditon isn't completely correct: kvm_handle_sys_reg()
should return 0 if the requested emulation fails, even it always returns 1 for now.

ret = kvm_handle_sys_reg(vcpu);
if (ret > 0 && !is_write)
rec->run->enter.gprs[0] = vcpu_get_reg(vcpu, rt);

> + return ret;
> +}
> +

[...]

Thanks,
Gavin