diff mbox series

[v2,3/4] KVM: selftests: Add ucall test support for LoongArch

Message ID 20230807065137.3408970-4-zhaotianrui@loongson.cn (mailing list archive)
State New, archived
Headers show
Series KVM: selftests: Add LoongArch support | expand

Commit Message

zhaotianrui Aug. 7, 2023, 6:51 a.m. UTC
Add ucall test support for LoongArch. A ucall is a "hypercall to
userspace".

Based-on: <20230803022138.2736430-1-zhaotianrui@loongson.cn>
Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
---
 .../selftests/kvm/lib/loongarch/ucall.c       | 43 +++++++++++++++++++
 1 file changed, 43 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/lib/loongarch/ucall.c

Comments

Sean Christopherson Sept. 27, 2023, 8:54 p.m. UTC | #1
On Mon, Aug 07, 2023, Tianrui Zhao wrote:
> Add ucall test support for LoongArch. A ucall is a "hypercall to
> userspace".

Nit, can you explain why LoongArch uses MMIO to trigger ucall, and what alternatives
were considred (if any)?  The main reason for the ask is because we've tossed
around the idea of converting all architectures (except s390) to MMIO-based ucall
in order to reduce the number of "flavors" of ucall we have to worry about it.
If MMIO is the only reasonable choice for LoongArch, that's another reason to
double down on MMIO as the primary choice for ucall.

> Based-on: <20230803022138.2736430-1-zhaotianrui@loongson.cn>
> Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
> ---
>  .../selftests/kvm/lib/loongarch/ucall.c       | 43 +++++++++++++++++++
>  1 file changed, 43 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/lib/loongarch/ucall.c
> 
> diff --git a/tools/testing/selftests/kvm/lib/loongarch/ucall.c b/tools/testing/selftests/kvm/lib/loongarch/ucall.c
> new file mode 100644
> index 000000000000..72868ddec313
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/lib/loongarch/ucall.c
> @@ -0,0 +1,43 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * ucall support. A ucall is a "hypercall to userspace".
> + *
> + */
> +#include "kvm_util.h"
> +
> +/*
> + * ucall_exit_mmio_addr holds per-VM values (global data is duplicated by each
> + * VM), it must not be accessed from host code.
> + */
> +static vm_vaddr_t *ucall_exit_mmio_addr;
> +
> +void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
> +{
> +	vm_vaddr_t mmio_gva = vm_vaddr_unused_gap(vm, vm->page_size, KVM_UTIL_MIN_VADDR);
> +
> +	virt_map(vm, mmio_gva, mmio_gpa, 1);
> +
> +	vm->ucall_mmio_addr = mmio_gpa;
> +
> +	write_guest_global(vm, ucall_exit_mmio_addr, (vm_vaddr_t *)mmio_gva);
> +}
> +
> +void ucall_arch_do_ucall(vm_vaddr_t uc)
> +{
> +	WRITE_ONCE(*ucall_exit_mmio_addr, uc);

Another uber nit, you might want to put this in the header as a static inline to
avoid function calls.  I doubt it'll actually matter, but we've had enough weird,
hard-to-debug issues with ucall that minimizing the amount of generated code might
save some future pain.
zhaotianrui Sept. 28, 2023, 4:04 a.m. UTC | #2
在 2023/9/28 上午4:54, Sean Christopherson 写道:
> On Mon, Aug 07, 2023, Tianrui Zhao wrote:
>> Add ucall test support for LoongArch. A ucall is a "hypercall to
>> userspace".
> Nit, can you explain why LoongArch uses MMIO to trigger ucall, and what alternatives
> were considred (if any)?  The main reason for the ask is because we've tossed
> around the idea of converting all architectures (except s390) to MMIO-based ucall
> in order to reduce the number of "flavors" of ucall we have to worry about it.
> If MMIO is the only reasonable choice for LoongArch, that's another reason to
> double down on MMIO as the primary choice for ucall.
Thanks for your reminding about ucall, and our guest can also use 
hypercall instruction to trigger ucall, so this change will not affect us.

Thanks
Tianrui Zhao
>
>> Based-on: <20230803022138.2736430-1-zhaotianrui@loongson.cn>
>> Signed-off-by: Tianrui Zhao <zhaotianrui@loongson.cn>
>> ---
>>   .../selftests/kvm/lib/loongarch/ucall.c       | 43 +++++++++++++++++++
>>   1 file changed, 43 insertions(+)
>>   create mode 100644 tools/testing/selftests/kvm/lib/loongarch/ucall.c
>>
>> diff --git a/tools/testing/selftests/kvm/lib/loongarch/ucall.c b/tools/testing/selftests/kvm/lib/loongarch/ucall.c
>> new file mode 100644
>> index 000000000000..72868ddec313
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/lib/loongarch/ucall.c
>> @@ -0,0 +1,43 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * ucall support. A ucall is a "hypercall to userspace".
>> + *
>> + */
>> +#include "kvm_util.h"
>> +
>> +/*
>> + * ucall_exit_mmio_addr holds per-VM values (global data is duplicated by each
>> + * VM), it must not be accessed from host code.
>> + */
>> +static vm_vaddr_t *ucall_exit_mmio_addr;
>> +
>> +void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
>> +{
>> +	vm_vaddr_t mmio_gva = vm_vaddr_unused_gap(vm, vm->page_size, KVM_UTIL_MIN_VADDR);
>> +
>> +	virt_map(vm, mmio_gva, mmio_gpa, 1);
>> +
>> +	vm->ucall_mmio_addr = mmio_gpa;
>> +
>> +	write_guest_global(vm, ucall_exit_mmio_addr, (vm_vaddr_t *)mmio_gva);
>> +}
>> +
>> +void ucall_arch_do_ucall(vm_vaddr_t uc)
>> +{
>> +	WRITE_ONCE(*ucall_exit_mmio_addr, uc);
> Another uber nit, you might want to put this in the header as a static inline to
> avoid function calls.  I doubt it'll actually matter, but we've had enough weird,
> hard-to-debug issues with ucall that minimizing the amount of generated code might
> save some future pain.
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/lib/loongarch/ucall.c b/tools/testing/selftests/kvm/lib/loongarch/ucall.c
new file mode 100644
index 000000000000..72868ddec313
--- /dev/null
+++ b/tools/testing/selftests/kvm/lib/loongarch/ucall.c
@@ -0,0 +1,43 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ucall support. A ucall is a "hypercall to userspace".
+ *
+ */
+#include "kvm_util.h"
+
+/*
+ * ucall_exit_mmio_addr holds per-VM values (global data is duplicated by each
+ * VM), it must not be accessed from host code.
+ */
+static vm_vaddr_t *ucall_exit_mmio_addr;
+
+void ucall_arch_init(struct kvm_vm *vm, vm_paddr_t mmio_gpa)
+{
+	vm_vaddr_t mmio_gva = vm_vaddr_unused_gap(vm, vm->page_size, KVM_UTIL_MIN_VADDR);
+
+	virt_map(vm, mmio_gva, mmio_gpa, 1);
+
+	vm->ucall_mmio_addr = mmio_gpa;
+
+	write_guest_global(vm, ucall_exit_mmio_addr, (vm_vaddr_t *)mmio_gva);
+}
+
+void ucall_arch_do_ucall(vm_vaddr_t uc)
+{
+	WRITE_ONCE(*ucall_exit_mmio_addr, uc);
+}
+
+void *ucall_arch_get_ucall(struct kvm_vcpu *vcpu)
+{
+	struct kvm_run *run = vcpu->run;
+
+	if (run->exit_reason == KVM_EXIT_MMIO &&
+	    run->mmio.phys_addr == vcpu->vm->ucall_mmio_addr) {
+		TEST_ASSERT(run->mmio.is_write && run->mmio.len == sizeof(uint64_t),
+			    "Unexpected ucall exit mmio address access");
+
+		return (void *)(*((uint64_t *)run->mmio.data));
+	}
+
+	return NULL;
+}