lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAAhV-H5TVwX2T=ccT4o0btJEbL+gwBESBD6Y_Z2BEqNuhom3iw@mail.gmail.com>
Date: Tue, 31 Dec 2024 16:41:30 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: bibo mao <maobibo@...ngson.cn>
Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>, WANG Xuerui <kernel@...0n.name>, kvm@...r.kernel.org, 
	loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] LoongArch: KVM: Add hypercall service support for
 usermode VMM

On Mon, Dec 30, 2024 at 10:13 AM bibo mao <maobibo@...ngson.cn> 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.

And we may also need to set vcpu->run->exit_reason for all cases,
which is done in my version.

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@...ngson.cn> 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@...ngson.cn> 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@...ngson.cn>
> >>>> ---
> >>>>    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
> >>>>
> >>
> >>
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ