| 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
| ||
|
Message-ID: <CAAhV-H5UufM7RdsoOxqdzGt=KrRyLM_FV7vwL3u4BC0ugwt+TA@mail.gmail.com>
Date: Tue, 31 Dec 2024 17:37:08 +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 Tue, Dec 31, 2024 at 5:27 PM bibo mao <maobibo@...ngson.cn> wrote:
>
>
>
> On 2024/12/31 下午4:41, Huacai Chen wrote:
> > 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.
> 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@...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