On 4/8/25 2:34 AM, Steven Price wrote:
On 04/03/2025 01:03, Gavin Shan wrote:
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@xxxxxxx>
---
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@xxxxxxxxxx>
[...]
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.
While I get this is duplication, I'm a little reluctant to move it to a
header file because this is completely internal to each C file (the
xxx_exit_handler[] arrays are both static). If either side wants to e.g.
add an extra argument there shouldn't be a requirement to reflect that
change in the other.
Specifically I'm wondering if we're going to ever need to pass an RMI
return status into the rme-exit callbacks at some point.
Ok, thanks for explaining it in details. In that case, I think it's fine
to keep exit_handler_fn in rme-exit.c.
+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.
It shouldn't matter, but like you say it's not technically the correct
condition so I'll fix this up.
Ok.
Thanks,
Steve
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
