Re: [Qemu-devel] [PATCH] WHPX Add signature CPUID
On 16/05/2018 16:18, Justin Terry (VM) via Qemu-devel wrote: > Hey Paolo, > > I agree that in the future it would be great if the Windows > Hypervisor Platform supported that and if that happens there is no > reason to differentiate the two. > > However, today WHPX actually doesn’t support any of the synthetic > enlightenments that Hyper-V does. We are really trying to make the > Windows Hypervisor Platform a generic hypervisor where as Hyper-V is > a specific implementation on top of that hypervisor (and QEMU is > another such implementation). For example, VMBus was mentioned but as > it stands today, VMBus is not supported in WHPX due to lack of > support for hypercalls. Additionally, timer enlightenments that you > mentioned would not be supported through synthetic paths and instead > run emulated as devices (hpet etc.). Sorry I missed this email. Note that I didn't mention timer enlightenments, I mentioned relaxed timing, CPUID[0x4004].EAX[bit 5]. These are the main reason to implement Hyper-V interfaces in non-Hyper-V hypervisors, otherwise Windows will bugcheck with STOP 0x101 under load. The question is why you need to implement a CPUID signature. The guest OS should not care about the hypervisor that hosts it, and indeed Windows works well without detecting Xen, KVM, etc. It only detects the Hyper-V interface that is presented by those hypervisors. Implementing and documenting a QEMU-specific hypervisor interface has a potentially large future cost, and it would be preferrable to implement an existing hypervisor interface for QEMU with WHPX. > I hope this adds some clarity here for why WHPX doesn’t implement > hyperv-proto.h as Hyper-V != Windows Hypervisor Platform in many > aspects. Please let me know if there is anything else I can explain > to help clarify the two. Non-Hyper-V hypervisors can provide the Hyper-V interfaces with no hypercalls and the three synthetic MSRs HV_X64_MSR_GUEST_OS_ID, HV_X64_MSR_HYPERCALL and HV_X64_MSR_VP_INDEX. This should not be hard. For example at least KVM and Xen don't make the hypercall page an overlay; they just replace the old contents with a vmcall+ret sequence, which simplifies noticeably the implementation. Does WHPX allow trapping hypercalls? If not, a stopgap implementation could be to put something like this in the hypercall page xor eax, eax // The next byte sets eax=1 and zf=0 in 32-bit mode // In 64-bit mode it is decoded together with the JZ db 0x40 jz is_64_bit // Zero edx in 32-bit mode cdq is_64_bit: // HV_STATUS_INVALID_HYPERCALL_CODE mov al,2 ret It would not satisfy the requirement of trapping in real mode or at CPL=3, and it would clobber the zero flag, but perhaps it's better than nothing. Paolo > >> -Original Message- From: Paolo Bonzini >> Sent: Wednesday, May 16, 2018 12:35 AM To: >> apilotti Cc: >> petrutlucia...@gmail.com; Lucian Petrut >> ; Eduardo Habkost >> ; open list:All patches CC here > de...@nongnu.org>; Justin Terry (VM) ; >> Richard Henderson Subject: Re: [Qemu-devel] >> [PATCH] WHPX Add signature CPUID >> >> On 16/05/2018 01:55, Alessandro Pilotti wrote: >>> Hi Paolo, >>> >>> The main reason for different signatures is to allow guest >>> workloads to be aware of the differences between the two >>> platforms (eg VirtIO vs VMBus). >> >> Why does it matter? CPUID tells you about the enlightenments that >> the hypervisor provides, not the availability of the VMBus. >> >> VMBus requires some of the enlightenments, mostly related to the >> synthetic interrupt controllers, but the opposite is not true---you >> can have enlightenments without VMBus, and in fact you probably >> want WHPX to enable the relaxed timing enlightenment. >> >> Thanks, >> >> Paolo
Re: [Qemu-devel] [PATCH] WHPX Add signature CPUID
Hey Paolo, I agree that in the future it would be great if the Windows Hypervisor Platform supported that and if that happens there is no reason to differentiate the two. However, today WHPX actually doesn’t support any of the synthetic enlightenments that Hyper-V does. We are really trying to make the Windows Hypervisor Platform a generic hypervisor where as Hyper-V is a specific implementation on top of that hypervisor (and QEMU is another such implementation). For example, VMBus was mentioned but as it stands today, VMBus is not supported in WHPX due to lack of support for hypercalls. Additionally, timer enlightenments that you mentioned would not be supported through synthetic paths and instead run emulated as devices (hpet etc.). I hope this adds some clarity here for why WHPX doesn’t implement hyperv-proto.h as Hyper-V != Windows Hypervisor Platform in many aspects. Please let me know if there is anything else I can explain to help clarify the two. Thanks, Justin > -Original Message- > From: Paolo Bonzini > Sent: Wednesday, May 16, 2018 12:35 AM > To: apilotti > Cc: petrutlucia...@gmail.com; Lucian Petrut > ; Eduardo Habkost > ; open list:All patches CC here de...@nongnu.org>; Justin Terry (VM) ; Richard > Henderson > Subject: Re: [Qemu-devel] [PATCH] WHPX Add signature CPUID > > On 16/05/2018 01:55, Alessandro Pilotti wrote: > > Hi Paolo, > > > > The main reason for different signatures is to allow guest workloads > > to be aware of the differences between the two platforms (eg VirtIO vs > > VMBus). > > Why does it matter? CPUID tells you about the enlightenments that the > hypervisor provides, not the availability of the VMBus. > > VMBus requires some of the enlightenments, mostly related to the synthetic > interrupt controllers, but the opposite is not true---you can have > enlightenments without VMBus, and in fact you probably want WHPX to > enable the relaxed timing enlightenment. > > Thanks, > > Paolo
Re: [Qemu-devel] [PATCH] WHPX Add signature CPUID
On 16/05/2018 01:55, Alessandro Pilotti wrote: > Hi Paolo, > > The main reason for different signatures is to allow guest workloads > to be aware of the differences between the two platforms (eg VirtIO > vs VMBus). Why does it matter? CPUID tells you about the enlightenments that the hypervisor provides, not the availability of the VMBus. VMBus requires some of the enlightenments, mostly related to the synthetic interrupt controllers, but the opposite is not true---you can have enlightenments without VMBus, and in fact you probably want WHPX to enable the relaxed timing enlightenment. Thanks, Paolo
Re: [Qemu-devel] [PATCH] WHPX Add signature CPUID
Hi Paolo, The main reason for different signatures is to allow guest workloads to be aware of the differences between the two platforms (eg VirtIO vs VMBus). Thanks, Alessandro > On 15 May 2018, at 16:44, Paolo Bonzini wrote: > >> On 15/05/2018 13:37, petrutlucia...@gmail.com wrote: >> From: Lucian Petrut >> >> Adds the CPUID trap for CPUID 0x4000, sending the WHPX signature >> to the guest upon request. This is consistent with other QEMU >> accelerators (KVM). >> >> Signed-off-by: Alessandro Pilotti >> Signed-off-by: Justin Terry (VM) >> Signed-off-by: Lucian Petrut > > Is it worth defining a different signature? Can WHPX implement part of > the Hyper-V spec, and if so would it be better to return the Hv > signature ("Hv#1") instead? > > Thanks, > > Paolo > >> --- >> As opposed to the previous patch, this one will set the result of >> this specific CPUID leaf when initializing the accelerator so that >> we avoid a vcpu exit. >> >> target/i386/whpx-all.c | 23 +++ >> 1 file changed, 23 insertions(+) >> >> diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c >> index 5843517..c8310de 100644 >> --- a/target/i386/whpx-all.c >> +++ b/target/i386/whpx-all.c >> @@ -29,6 +29,8 @@ >> #include >> #include >> >> +#define WHPX_CPUID_SIGNATURE 0x4000 >> + >> struct whpx_state { >> uint64_t mem_quota; >> WHV_PARTITION_HANDLE partition; >> @@ -1342,6 +1344,27 @@ static int whpx_accel_init(MachineState *ms) >> cpuidExitList, >> RTL_NUMBER_OF(cpuidExitList) * >> sizeof(UINT32)); >> >> +UINT32 signature[3] = {0}; >> +memcpy(signature, "WHPXWHPXWHPX", 12); >> + >> +WHV_X64_CPUID_RESULT cpuidResultList[1] = {0}; >> +cpuidResultList[0].Function = WHPX_CPUID_SIGNATURE; >> +cpuidResultList[0].Eax = 0; >> +cpuidResultList[0].Ebx = signature[0]; >> +cpuidResultList[0].Ecx = signature[1]; >> +cpuidResultList[0].Edx = signature[2]; >> +hr = WHvSetPartitionProperty(whpx->partition, >> + WHvPartitionPropertyCodeCpuidResultList, >> + cpuidResultList, >> + RTL_NUMBER_OF(cpuidResultList) * >> +sizeof(WHV_X64_CPUID_RESULT)); >> +if (FAILED(hr)) { >> +error_report("WHPX: Failed to set partition CpuidResultList >> hr=%08lx", >> + hr); >> +ret = -EINVAL; >> +goto error; >> +} >> + >> if (FAILED(hr)) { >> error_report("WHPX: Failed to set partition CpuidExitList hr=%08lx", >> hr); >> >
Re: [Qemu-devel] [PATCH] WHPX Add signature CPUID
On 15/05/2018 13:37, petrutlucia...@gmail.com wrote: > From: Lucian Petrut > > Adds the CPUID trap for CPUID 0x4000, sending the WHPX signature > to the guest upon request. This is consistent with other QEMU > accelerators (KVM). > > Signed-off-by: Alessandro Pilotti > Signed-off-by: Justin Terry (VM) > Signed-off-by: Lucian Petrut Is it worth defining a different signature? Can WHPX implement part of the Hyper-V spec, and if so would it be better to return the Hv signature ("Hv#1") instead? Thanks, Paolo > --- > As opposed to the previous patch, this one will set the result of > this specific CPUID leaf when initializing the accelerator so that > we avoid a vcpu exit. > > target/i386/whpx-all.c | 23 +++ > 1 file changed, 23 insertions(+) > > diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c > index 5843517..c8310de 100644 > --- a/target/i386/whpx-all.c > +++ b/target/i386/whpx-all.c > @@ -29,6 +29,8 @@ > #include > #include > > +#define WHPX_CPUID_SIGNATURE 0x4000 > + > struct whpx_state { > uint64_t mem_quota; > WHV_PARTITION_HANDLE partition; > @@ -1342,6 +1344,27 @@ static int whpx_accel_init(MachineState *ms) > cpuidExitList, > RTL_NUMBER_OF(cpuidExitList) * > sizeof(UINT32)); > > +UINT32 signature[3] = {0}; > +memcpy(signature, "WHPXWHPXWHPX", 12); > + > +WHV_X64_CPUID_RESULT cpuidResultList[1] = {0}; > +cpuidResultList[0].Function = WHPX_CPUID_SIGNATURE; > +cpuidResultList[0].Eax = 0; > +cpuidResultList[0].Ebx = signature[0]; > +cpuidResultList[0].Ecx = signature[1]; > +cpuidResultList[0].Edx = signature[2]; > +hr = WHvSetPartitionProperty(whpx->partition, > + WHvPartitionPropertyCodeCpuidResultList, > + cpuidResultList, > + RTL_NUMBER_OF(cpuidResultList) * > +sizeof(WHV_X64_CPUID_RESULT)); > +if (FAILED(hr)) { > +error_report("WHPX: Failed to set partition CpuidResultList > hr=%08lx", > + hr); > +ret = -EINVAL; > +goto error; > +} > + > if (FAILED(hr)) { > error_report("WHPX: Failed to set partition CpuidExitList hr=%08lx", > hr); >
Re: [Qemu-devel] [PATCH] WHPX Add signature CPUID
Thanks for the patch Lucian. Looks good to me! -Justin > -Original Message- > From: petrutlucia...@gmail.com > Sent: Tuesday, May 15, 2018 4:38 AM > Cc: Lucian Petrut ; apilotti > ; Justin Terry (VM) > ; Paolo Bonzini ; Richard > Henderson ; Eduardo Habkost ; > open list:All patches CC here > Subject: [PATCH] WHPX Add signature CPUID > > From: Lucian Petrut > > Adds the CPUID trap for CPUID 0x4000, sending the WHPX signature to > the guest upon request. This is consistent with other QEMU accelerators > (KVM). > > Signed-off-by: Alessandro Pilotti > Signed-off-by: Justin Terry (VM) > Signed-off-by: Lucian Petrut > --- > As opposed to the previous patch, this one will set the result of this > specific > CPUID leaf when initializing the accelerator so that we avoid a vcpu exit. > > target/i386/whpx-all.c | 23 +++ > 1 file changed, 23 insertions(+) > > diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c index > 5843517..c8310de 100644 > --- a/target/i386/whpx-all.c > +++ b/target/i386/whpx-all.c > @@ -29,6 +29,8 @@ > #include > #include > > +#define WHPX_CPUID_SIGNATURE 0x4000 > + > struct whpx_state { > uint64_t mem_quota; > WHV_PARTITION_HANDLE partition; > @@ -1342,6 +1344,27 @@ static int whpx_accel_init(MachineState *ms) > cpuidExitList, > RTL_NUMBER_OF(cpuidExitList) * > sizeof(UINT32)); > > +UINT32 signature[3] = {0}; > +memcpy(signature, "WHPXWHPXWHPX", 12); > + > +WHV_X64_CPUID_RESULT cpuidResultList[1] = {0}; > +cpuidResultList[0].Function = WHPX_CPUID_SIGNATURE; > +cpuidResultList[0].Eax = 0; > +cpuidResultList[0].Ebx = signature[0]; > +cpuidResultList[0].Ecx = signature[1]; > +cpuidResultList[0].Edx = signature[2]; > +hr = WHvSetPartitionProperty(whpx->partition, > + WHvPartitionPropertyCodeCpuidResultList, > + cpuidResultList, > + RTL_NUMBER_OF(cpuidResultList) * > +sizeof(WHV_X64_CPUID_RESULT)); > +if (FAILED(hr)) { > +error_report("WHPX: Failed to set partition CpuidResultList > hr=%08lx", > + hr); > +ret = -EINVAL; > +goto error; > +} > + > if (FAILED(hr)) { > error_report("WHPX: Failed to set partition CpuidExitList hr=%08lx", > hr); > -- > 2.7.4
[Qemu-devel] [PATCH] WHPX Add signature CPUID
From: Lucian Petrut Adds the CPUID trap for CPUID 0x4000, sending the WHPX signature to the guest upon request. This is consistent with other QEMU accelerators (KVM). Signed-off-by: Alessandro Pilotti Signed-off-by: Justin Terry (VM) Signed-off-by: Lucian Petrut --- As opposed to the previous patch, this one will set the result of this specific CPUID leaf when initializing the accelerator so that we avoid a vcpu exit. target/i386/whpx-all.c | 23 +++ 1 file changed, 23 insertions(+) diff --git a/target/i386/whpx-all.c b/target/i386/whpx-all.c index 5843517..c8310de 100644 --- a/target/i386/whpx-all.c +++ b/target/i386/whpx-all.c @@ -29,6 +29,8 @@ #include #include +#define WHPX_CPUID_SIGNATURE 0x4000 + struct whpx_state { uint64_t mem_quota; WHV_PARTITION_HANDLE partition; @@ -1342,6 +1344,27 @@ static int whpx_accel_init(MachineState *ms) cpuidExitList, RTL_NUMBER_OF(cpuidExitList) * sizeof(UINT32)); +UINT32 signature[3] = {0}; +memcpy(signature, "WHPXWHPXWHPX", 12); + +WHV_X64_CPUID_RESULT cpuidResultList[1] = {0}; +cpuidResultList[0].Function = WHPX_CPUID_SIGNATURE; +cpuidResultList[0].Eax = 0; +cpuidResultList[0].Ebx = signature[0]; +cpuidResultList[0].Ecx = signature[1]; +cpuidResultList[0].Edx = signature[2]; +hr = WHvSetPartitionProperty(whpx->partition, + WHvPartitionPropertyCodeCpuidResultList, + cpuidResultList, + RTL_NUMBER_OF(cpuidResultList) * +sizeof(WHV_X64_CPUID_RESULT)); +if (FAILED(hr)) { +error_report("WHPX: Failed to set partition CpuidResultList hr=%08lx", + hr); +ret = -EINVAL; +goto error; +} + if (FAILED(hr)) { error_report("WHPX: Failed to set partition CpuidExitList hr=%08lx", hr); -- 2.7.4