On Fri, 27 Jan 2023 11:29:19 +0000
Steven Price <steven.price@xxxxxxx> wrote:
> MMIO emulation for a realm cannot be done directly with the VM's
> registers as they are protected from the host. However the RMM interface
> provides a structure member for providing the read/written value and
More details would be better for helping the review. I can only see the
emulated mmio value from the device model (kvmtool or kvm_io_bus) is put into
the GPRS[0] of the RecEntry object. But the rest of the flow is missing.
I guess RMM copies the value in the RecEntry.GPRS[0] to the target GPR in the
guest context in RMI_REC_ENTER when seeing RMI_EMULATED_MMIO. This is for
the guest MMIO read path.
How about the MMIO write path? I don't see where the RecExit.GPRS[0] is loaded
to a varible and returned to the userspace.
> we can transfer this to the appropriate VCPU's register entry and then
> depend on the generic MMIO handling code in KVM.
>
> Signed-off-by: Steven Price <steven.price@xxxxxxx>
> ---
> arch/arm64/kvm/mmio.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/arch/arm64/kvm/mmio.c b/arch/arm64/kvm/mmio.c
> index 3dd38a151d2a..c4879fa3a8d3 100644
> --- a/arch/arm64/kvm/mmio.c
> +++ b/arch/arm64/kvm/mmio.c
> @@ -6,6 +6,7 @@
>
> #include <linux/kvm_host.h>
> #include <asm/kvm_emulate.h>
> +#include <asm/rmi_smc.h>
> #include <trace/events/kvm.h>
>
> #include "trace.h"
> @@ -109,6 +110,9 @@ int kvm_handle_mmio_return(struct kvm_vcpu *vcpu)
> &data);
> data = vcpu_data_host_to_guest(vcpu, data, len);
> vcpu_set_reg(vcpu, kvm_vcpu_dabt_get_rd(vcpu), data);
> +
> + if (vcpu_is_rec(vcpu))
> + vcpu->arch.rec.run->entry.gprs[0] = data;
I think the guest context is maintained by RMM (while KVM can only touch
Rec{Entry, Exit} object) so that guest context in the legacy VHE mode is
unused.
If yes, I guess here is should be:
if (unlikely(vcpu_is_rec(vcpu)))
vcpu->arch.rec.run->entry.gprs[0] = data;
else
vcpu_set_reg(vcpu, kvm_vcpu_dabt_get_rd(vcpu), data);
> }
>
> /*
> @@ -179,6 +183,9 @@ int io_mem_abort(struct kvm_vcpu *vcpu, phys_addr_t fault_ipa)
> run->mmio.len = len;
> vcpu->mmio_needed = 1;
>
> + if (vcpu_is_rec(vcpu))
> + vcpu->arch.rec.run->entry.flags |= RMI_EMULATED_MMIO;
> +
Wouldn't it be better to set this in the kvm_handle_mmio_return where the MMIO
read emulation has been surely successful?
> if (!ret) {
> /* We handled the access successfully in the kernel. */
> if (!is_write)
