On Tue, Dec 31, 2024 at 5:27 PM bibo mao <maobibo@xxxxxxxxxxx> wrote: > > > > On 2024/12/31 下午4:41, Huacai Chen wrote: > > On Mon, Dec 30, 2024 at 10:13 AM bibo mao <maobibo@xxxxxxxxxxx> wrote: > >> > >> Hi Huacai, > >> > >> On 2024/12/23 下午5:05, Huacai Chen wrote: > >>> I also tried to port an untested version, but I think your version is > >>> a tested one. > >>> https://github.com/chenhuacai/linux/commit/e6596b0e45c80756794aba74ac086c5c0e0306eb > >>> > >>> And I have some questions: > >>> 1, "user service" is not only for syscall, so you rename it? > >>> 2, Why 4.19 doesn't need something like "vcpu->run->hypercall.args[0] > >>> = kvm_read_reg(vcpu, LOONGARCH_GPR_A0);" > >>> 3, I think my version about "vcpu->run->exit_reason = > >>> KVM_EXIT_HYPERCALL;" and "update_pc()" is a little better than yours, > >>> so you can improve them. > >> After a second thought, update_pc() before return to user may be not > >> strictly right, since user VMM can dump registers including pc which is > >> advanced already. > > Agree, and we can see how others do. > > > >> > >> How about adding function kvm_complete_hypercall() like > >> kvm_complete_mmio_read(), such as: > > kvm_complete_user_service() maybe better? Since the "classic > > hypercall" doesn't come here. > sure, will do rename it with kvm_complete_user_service. > > > > > And we may also need to set vcpu->run->exit_reason for all cases, > > which is done in my version. > By my understanding, exit_reason need be set only for RESUME_HOST. > it is unnecessary for RESUME_GUEST. Unset means KVM_EXIT_UNKNOWN. Though we only check exit_reason for RESUME_HOST now, let a classic hypercall with KVM_EXIT_UNKNOWN may be error-prone. Huacai > > Regards > Bibo Mao > > > > Huacai > > > >> --- a/arch/loongarch/kvm/exit.c > >> +++ b/arch/loongarch/kvm/exit.c > >> +int kvm_complete_hypercall(struct kvm_vcpu *vcpu, struct kvm_run *run) > >> +{ > >> + update_pc(&vcpu->arch); > >> + kvm_write_reg(vcpu, LOONGARCH_GPR_A0, run->hypercall.ret); > >> +} > >> + > >> --- a/arch/loongarch/kvm/vcpu.c > >> +++ b/arch/loongarch/kvm/vcpu.c > >> @@ -1736,8 +1736,8 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > >> if (!run->iocsr_io.is_write) > >> kvm_complete_iocsr_read(vcpu, run); > >> } else if (run->exit_reason == KVM_EXIT_HYPERCALL) { > >> - kvm_write_reg(vcpu, LOONGARCH_GPR_A0, run->hypercall.ret); > >> - update_pc(&vcpu->arch); > >> + kvm_complete_hypercall(vcpu, run); > >> + run->exit_reason = KVM_EXIT_UNKNOWN; > >> } > >> > >> Regards > >> Bibo Mao > >>> > >>> Huacai > >>> > >>> On Mon, Dec 23, 2024 at 4:54 PM bibo mao <maobibo@xxxxxxxxxxx> wrote: > >>>> > >>>> > >>>> > >>>> On 2024/12/23 下午4:50, Huacai Chen wrote: > >>>>> Hi, Bibo, > >>>>> > >>>>> Is this patch trying to do the same thing as "LoongArch: add hypcall > >>>>> to emulate syscall in kvm" in 4.19? > >>>> yes, it is to do so -:) > >>>> > >>>> Regards > >>>> Bibo Mao > >>>>> > >>>>> Huacai > >>>>> > >>>>> On Mon, Dec 23, 2024 at 4:42 PM Bibo Mao <maobibo@xxxxxxxxxxx> wrote: > >>>>>> > >>>>>> Some VMMs provides special hypercall service in usermode, KVM need > >>>>>> not handle the usermode hypercall service and pass it to VMM and > >>>>>> let VMM handle it. > >>>>>> > >>>>>> Here new code KVM_HCALL_CODE_USER is added for user-mode hypercall > >>>>>> service, KVM loads all six registers to VMM. > >>>>>> > >>>>>> Signed-off-by: Bibo Mao <maobibo@xxxxxxxxxxx> > >>>>>> --- > >>>>>> arch/loongarch/include/asm/kvm_host.h | 1 + > >>>>>> arch/loongarch/include/asm/kvm_para.h | 2 ++ > >>>>>> arch/loongarch/include/uapi/asm/kvm_para.h | 1 + > >>>>>> arch/loongarch/kvm/exit.c | 22 ++++++++++++++++++++++ > >>>>>> arch/loongarch/kvm/vcpu.c | 3 +++ > >>>>>> 5 files changed, 29 insertions(+) > >>>>>> > >>>>>> diff --git a/arch/loongarch/include/asm/kvm_host.h b/arch/loongarch/include/asm/kvm_host.h > >>>>>> index 7b8367c39da8..590982cd986e 100644 > >>>>>> --- a/arch/loongarch/include/asm/kvm_host.h > >>>>>> +++ b/arch/loongarch/include/asm/kvm_host.h > >>>>>> @@ -162,6 +162,7 @@ enum emulation_result { > >>>>>> #define LOONGARCH_PV_FEAT_UPDATED BIT_ULL(63) > >>>>>> #define LOONGARCH_PV_FEAT_MASK (BIT(KVM_FEATURE_IPI) | \ > >>>>>> BIT(KVM_FEATURE_STEAL_TIME) | \ > >>>>>> + BIT(KVM_FEATURE_USER_HCALL) | \ > >>>>>> BIT(KVM_FEATURE_VIRT_EXTIOI)) > >>>>>> > >>>>>> struct kvm_vcpu_arch { > >>>>>> diff --git a/arch/loongarch/include/asm/kvm_para.h b/arch/loongarch/include/asm/kvm_para.h > >>>>>> index c4e84227280d..d3c00de484f6 100644 > >>>>>> --- a/arch/loongarch/include/asm/kvm_para.h > >>>>>> +++ b/arch/loongarch/include/asm/kvm_para.h > >>>>>> @@ -13,12 +13,14 @@ > >>>>>> > >>>>>> #define KVM_HCALL_CODE_SERVICE 0 > >>>>>> #define KVM_HCALL_CODE_SWDBG 1 > >>>>>> +#define KVM_HCALL_CODE_USER 2 > >>>>>> > >>>>>> #define KVM_HCALL_SERVICE HYPERCALL_ENCODE(HYPERVISOR_KVM, KVM_HCALL_CODE_SERVICE) > >>>>>> #define KVM_HCALL_FUNC_IPI 1 > >>>>>> #define KVM_HCALL_FUNC_NOTIFY 2 > >>>>>> > >>>>>> #define KVM_HCALL_SWDBG HYPERCALL_ENCODE(HYPERVISOR_KVM, KVM_HCALL_CODE_SWDBG) > >>>>>> +#define KVM_HCALL_USER_SERVICE HYPERCALL_ENCODE(HYPERVISOR_KVM, KVM_HCALL_CODE_USER) > >>>>>> > >>>>>> /* > >>>>>> * LoongArch hypercall return code > >>>>>> diff --git a/arch/loongarch/include/uapi/asm/kvm_para.h b/arch/loongarch/include/uapi/asm/kvm_para.h > >>>>>> index b0604aa9b4bb..76d802ef01ce 100644 > >>>>>> --- a/arch/loongarch/include/uapi/asm/kvm_para.h > >>>>>> +++ b/arch/loongarch/include/uapi/asm/kvm_para.h > >>>>>> @@ -17,5 +17,6 @@ > >>>>>> #define KVM_FEATURE_STEAL_TIME 2 > >>>>>> /* BIT 24 - 31 are features configurable by user space vmm */ > >>>>>> #define KVM_FEATURE_VIRT_EXTIOI 24 > >>>>>> +#define KVM_FEATURE_USER_HCALL 25 > >>>>>> > >>>>>> #endif /* _UAPI_ASM_KVM_PARA_H */ > >>>>>> diff --git a/arch/loongarch/kvm/exit.c b/arch/loongarch/kvm/exit.c > >>>>>> index a7893bd01e73..1a85cd4fb6a5 100644 > >>>>>> --- a/arch/loongarch/kvm/exit.c > >>>>>> +++ b/arch/loongarch/kvm/exit.c > >>>>>> @@ -873,6 +873,28 @@ static int kvm_handle_hypercall(struct kvm_vcpu *vcpu) > >>>>>> vcpu->stat.hypercall_exits++; > >>>>>> kvm_handle_service(vcpu); > >>>>>> break; > >>>>>> + case KVM_HCALL_USER_SERVICE: > >>>>>> + if (!kvm_guest_has_pv_feature(vcpu, KVM_FEATURE_USER_HCALL)) { > >>>>>> + kvm_write_reg(vcpu, LOONGARCH_GPR_A0, KVM_HCALL_INVALID_CODE); > >>>>>> + break; > >>>>>> + } > >>>>>> + > >>>>>> + vcpu->run->exit_reason = KVM_EXIT_HYPERCALL; > >>>>>> + vcpu->run->hypercall.nr = KVM_HCALL_USER_SERVICE; > >>>>>> + vcpu->run->hypercall.args[0] = kvm_read_reg(vcpu, LOONGARCH_GPR_A0); > >>>>>> + vcpu->run->hypercall.args[1] = kvm_read_reg(vcpu, LOONGARCH_GPR_A1); > >>>>>> + vcpu->run->hypercall.args[2] = kvm_read_reg(vcpu, LOONGARCH_GPR_A2); > >>>>>> + vcpu->run->hypercall.args[3] = kvm_read_reg(vcpu, LOONGARCH_GPR_A3); > >>>>>> + vcpu->run->hypercall.args[4] = kvm_read_reg(vcpu, LOONGARCH_GPR_A4); > >>>>>> + vcpu->run->hypercall.args[5] = kvm_read_reg(vcpu, LOONGARCH_GPR_A5); > >>>>>> + vcpu->run->hypercall.flags = 0; > >>>>>> + /* > >>>>>> + * Set invalid return value by default > >>>>>> + * Need user-mode VMM modify it > >>>>>> + */ > >>>>>> + vcpu->run->hypercall.ret = KVM_HCALL_INVALID_CODE; > >>>>>> + ret = RESUME_HOST; > >>>>>> + break; > >>>>>> case KVM_HCALL_SWDBG: > >>>>>> /* KVM_HCALL_SWDBG only in effective when SW_BP is enabled */ > >>>>>> if (vcpu->guest_debug & KVM_GUESTDBG_SW_BP_MASK) { > >>>>>> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c > >>>>>> index d18a4a270415..8c46ad1872ee 100644 > >>>>>> --- a/arch/loongarch/kvm/vcpu.c > >>>>>> +++ b/arch/loongarch/kvm/vcpu.c > >>>>>> @@ -1735,6 +1735,9 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu) > >>>>>> if (run->exit_reason == KVM_EXIT_LOONGARCH_IOCSR) { > >>>>>> if (!run->iocsr_io.is_write) > >>>>>> kvm_complete_iocsr_read(vcpu, run); > >>>>>> + } else if (run->exit_reason == KVM_EXIT_HYPERCALL) { > >>>>>> + kvm_write_reg(vcpu, LOONGARCH_GPR_A0, run->hypercall.ret); > >>>>>> + update_pc(&vcpu->arch); > >>>>>> } > >>>>>> > >>>>>> if (!vcpu->wants_to_run) > >>>>>> > >>>>>> base-commit: 48f506ad0b683d3e7e794efa60c5785c4fdc86fa > >>>>>> -- > >>>>>> 2.39.3 > >>>>>> > >>>> > >>>> > >> > >
