* EFER in HVM guests
@ 2006-11-29 13:07 Jan Beulich
2006-11-29 13:09 ` Keir Fraser
2006-11-29 13:11 ` Petersson, Mats
0 siblings, 2 replies; 26+ messages in thread
From: Jan Beulich @ 2006-11-29 13:07 UTC (permalink / raw)
To: xen-devel
Is it intentional that
- under SVM, 32-bit guests can freely set EFER.LME
- under VMX, 32-bit guests can't access EFER at all?
Thanks, Jan
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: EFER in HVM guests
2006-11-29 13:07 Jan Beulich
@ 2006-11-29 13:09 ` Keir Fraser
2006-11-29 14:08 ` Jan Beulich
2006-11-29 13:11 ` Petersson, Mats
1 sibling, 1 reply; 26+ messages in thread
From: Keir Fraser @ 2006-11-29 13:09 UTC (permalink / raw)
To: Jan Beulich, xen-devel
On 29/11/06 13:07, "Jan Beulich" <jbeulich@novell.com> wrote:
> Is it intentional that
> - under SVM, 32-bit guests can freely set EFER.LME
> - under VMX, 32-bit guests can't access EFER at all?
>
> Thanks, Jan
I'm sure any differences are unintentional. There is obviously scope for
making much of the MSR and CPUID code non-vmx/svm specific.
I assume that this particular difference doesn't really matter?
-- Keir
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-11-29 13:07 Jan Beulich
2006-11-29 13:09 ` Keir Fraser
@ 2006-11-29 13:11 ` Petersson, Mats
2006-11-29 14:09 ` Jan Beulich
1 sibling, 1 reply; 26+ messages in thread
From: Petersson, Mats @ 2006-11-29 13:11 UTC (permalink / raw)
To: Jan Beulich, xen-devel
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> Jan Beulich
> Sent: 29 November 2006 13:07
> To: xen-devel@lists.xensource.com
> Subject: [Xen-devel] EFER in HVM guests
>
> Is it intentional that
> - under SVM, 32-bit guests can freely set EFER.LME
Ehm, I guess you mean "on 32-bit hypervisor", as it's impossible to
distinguish a 32-bit and 64-bit guest until the guest is setting LME...
;-).
The correct reaction for a 32-bit hypervisor is to react like a
"non-64-bit capable processor", which means that EFER has the LME as a
mbz-bit. GP-fault if it's written as a one. We do prevent the long-mode
from being advertised by CPUID when HV is in 32-bit mode, so if the
guest is well-behaved, it shouldn't try to set this bit.
--
Mats
> - under VMX, 32-bit guests can't access EFER at all?
>
> Thanks, Jan
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: EFER in HVM guests
2006-11-29 13:09 ` Keir Fraser
@ 2006-11-29 14:08 ` Jan Beulich
2006-11-29 14:22 ` Petersson, Mats
0 siblings, 1 reply; 26+ messages in thread
From: Jan Beulich @ 2006-11-29 14:08 UTC (permalink / raw)
To: xen-devel, Keir Fraser
>>> Keir Fraser <keir@xensource.com> 29.11.06 14:09 >>>
>On 29/11/06 13:07, "Jan Beulich" <jbeulich@novell.com> wrote:
>
>> Is it intentional that
>> - under SVM, 32-bit guests can freely set EFER.LME
>> - under VMX, 32-bit guests can't access EFER at all?
>>
>> Thanks, Jan
>
>I'm sure any differences are unintentional. There is obviously scope for
>making much of the MSR and CPUID code non-vmx/svm specific.
>
>I assume that this particular difference doesn't really matter?
I think it does - allowing a guest to enable EFER.LME when the
hypervisor is a 32-bit one is clearly a security problem: While I
haven't tried it, I would suspect the moment you load a context
with such an EFER the whole system's dead.
Not being able to access EFER is also a potential problem, as a
guest should be allowed to set EFER.NX (at least) - the CPUID
handling code specifically does not suppress this bit if the guest
is allowed to use PAE (which we agreed a few days ago should
be the default anyway).
Jan
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-11-29 13:11 ` Petersson, Mats
@ 2006-11-29 14:09 ` Jan Beulich
0 siblings, 0 replies; 26+ messages in thread
From: Jan Beulich @ 2006-11-29 14:09 UTC (permalink / raw)
To: Mats Petersson, xen-devel
>> Is it intentional that
>> - under SVM, 32-bit guests can freely set EFER.LME
>
>Ehm, I guess you mean "on 32-bit hypervisor", as it's impossible to
>distinguish a 32-bit and 64-bit guest until the guest is setting LME...
>;-).
Yes, sorry for being imprecise. I meant a 32-bit hvm guest on a 32-bit hv.
Jan
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-11-29 14:08 ` Jan Beulich
@ 2006-11-29 14:22 ` Petersson, Mats
0 siblings, 0 replies; 26+ messages in thread
From: Petersson, Mats @ 2006-11-29 14:22 UTC (permalink / raw)
To: Jan Beulich, xen-devel, Keir Fraser
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> Jan Beulich
> Sent: 29 November 2006 14:08
> To: xen-devel@lists.xensource.com; Keir Fraser
> Subject: Re: [Xen-devel] EFER in HVM guests
>
> >>> Keir Fraser <keir@xensource.com> 29.11.06 14:09 >>>
> >On 29/11/06 13:07, "Jan Beulich" <jbeulich@novell.com> wrote:
> >
> >> Is it intentional that
> >> - under SVM, 32-bit guests can freely set EFER.LME
> >> - under VMX, 32-bit guests can't access EFER at all?
> >>
> >> Thanks, Jan
> >
> >I'm sure any differences are unintentional. There is
> obviously scope for
> >making much of the MSR and CPUID code non-vmx/svm specific.
> >
> >I assume that this particular difference doesn't really matter?
>
> I think it does - allowing a guest to enable EFER.LME when the
> hypervisor is a 32-bit one is clearly a security problem: While I
> haven't tried it, I would suspect the moment you load a context
> with such an EFER the whole system's dead.
Actually, it's a bit more complex than that, but assuming the guest has
access to EFER, it also has access to CR0, so it could try to enable
long mode by:
CR0.PG = 0
CR4.PAE = 1
EFER.LME = 1
CR0.PG = 1
If PAE isn't available, it wouldn't be possible to set long-mode
(processor consistency checks for PAE=1 when LME=1). See section 14.6.3
in the AMD Programmers Manual Vol 2.
I think that the setting of EFER.LME in 32-bit Hypervisor should
generate GP-fault, as that's what the real processor does...
> Not being able to access EFER is also a potential problem, as a
> guest should be allowed to set EFER.NX (at least) - the CPUID
> handling code specifically does not suppress this bit if the guest
> is allowed to use PAE (which we agreed a few days ago should
> be the default anyway).
This makes sense to me. As well as EFER.SCE, perhaps?
--
Mats
>
> Jan
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-11-29 16:34 Nakajima, Jun
2006-11-29 17:21 ` Keir Fraser
2006-11-29 17:37 ` Petersson, Mats
0 siblings, 2 replies; 26+ messages in thread
From: Nakajima, Jun @ 2006-11-29 16:34 UTC (permalink / raw)
To: Jan Beulich, xen-devel, Keir Fraser
Jan Beulich wrote:
>>>> Keir Fraser <keir@xensource.com> 29.11.06 14:09 >>>
>> On 29/11/06 13:07, "Jan Beulich" <jbeulich@novell.com> wrote:
>>
>>> Is it intentional that
>>> - under SVM, 32-bit guests can freely set EFER.LME
>>> - under VMX, 32-bit guests can't access EFER at all?
>>>
>>> Thanks, Jan
>>
>> I'm sure any differences are unintentional. There is obviously scope
>> for making much of the MSR and CPUID code non-vmx/svm specific.
>>
>> I assume that this particular difference doesn't really matter?
>
> I think it does - allowing a guest to enable EFER.LME when the
> hypervisor is a 32-bit one is clearly a security problem: While I
> haven't tried it, I would suspect the moment you load a context
> with such an EFER the whole system's dead.
> Not being able to access EFER is also a potential problem, as a
> guest should be allowed to set EFER.NX (at least) - the CPUID
> handling code specifically does not suppress this bit if the guest
> is allowed to use PAE (which we agreed a few days ago should
> be the default anyway).
>
> Jan
>
I agree that we should allow 32-bit guests to set EFER.NX on the PAE
Xen. We'll fix it. EFER.SCE should not be set on IA-32.
Jun
---
Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: EFER in HVM guests
2006-11-29 16:34 EFER in HVM guests Nakajima, Jun
@ 2006-11-29 17:21 ` Keir Fraser
2006-11-29 17:37 ` Petersson, Mats
1 sibling, 0 replies; 26+ messages in thread
From: Keir Fraser @ 2006-11-29 17:21 UTC (permalink / raw)
To: Nakajima, Jun, Jan Beulich, xen-devel, Keir Fraser
*Please* can you make the handling of generic CPUID leaves and architectural
MSRs common HVM code? There is lots of needless code duplication right now
with niggling differences that shouldn't be necessary.
-- Keir
On 29/11/06 16:34, "Nakajima, Jun" <jun.nakajima@intel.com> wrote:
>> I think it does - allowing a guest to enable EFER.LME when the
>> hypervisor is a 32-bit one is clearly a security problem: While I
>> haven't tried it, I would suspect the moment you load a context
>> with such an EFER the whole system's dead.
>> Not being able to access EFER is also a potential problem, as a
>> guest should be allowed to set EFER.NX (at least) - the CPUID
>> handling code specifically does not suppress this bit if the guest
>> is allowed to use PAE (which we agreed a few days ago should
>> be the default anyway).
>>
>> Jan
>>
>
> I agree that we should allow 32-bit guests to set EFER.NX on the PAE
> Xen. We'll fix it. EFER.SCE should not be set on IA-32.
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-11-29 16:34 EFER in HVM guests Nakajima, Jun
2006-11-29 17:21 ` Keir Fraser
@ 2006-11-29 17:37 ` Petersson, Mats
1 sibling, 0 replies; 26+ messages in thread
From: Petersson, Mats @ 2006-11-29 17:37 UTC (permalink / raw)
To: Nakajima, Jun, Jan Beulich, xen-devel, Keir Fraser
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> Nakajima, Jun
> Sent: 29 November 2006 16:35
> To: Jan Beulich; xen-devel@lists.xensource.com; Keir Fraser
> Subject: RE: [Xen-devel] EFER in HVM guests
>
> Jan Beulich wrote:
> >>>> Keir Fraser <keir@xensource.com> 29.11.06 14:09 >>>
> >> On 29/11/06 13:07, "Jan Beulich" <jbeulich@novell.com> wrote:
> >>
> >>> Is it intentional that
> >>> - under SVM, 32-bit guests can freely set EFER.LME
> >>> - under VMX, 32-bit guests can't access EFER at all?
> >>>
> >>> Thanks, Jan
> >>
> >> I'm sure any differences are unintentional. There is
> obviously scope
> >> for making much of the MSR and CPUID code non-vmx/svm specific.
> >>
> >> I assume that this particular difference doesn't really matter?
> >
> > I think it does - allowing a guest to enable EFER.LME when the
> > hypervisor is a 32-bit one is clearly a security problem: While I
> > haven't tried it, I would suspect the moment you load a context
> > with such an EFER the whole system's dead.
> > Not being able to access EFER is also a potential problem, as a
> > guest should be allowed to set EFER.NX (at least) - the CPUID
> > handling code specifically does not suppress this bit if the guest
> > is allowed to use PAE (which we agreed a few days ago should
> > be the default anyway).
> >
> > Jan
> >
>
> I agree that we should allow 32-bit guests to set EFER.NX on the PAE
> Xen. We'll fix it. EFER.SCE should not be set on IA-32.
Why not? If CPUID bits indicate that it's available, it can be used in
32- or 64-bit mode.
--
Mats
>
> Jun
> ---
> Intel Open Source Technology Center
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-11-29 18:24 Nakajima, Jun
0 siblings, 0 replies; 26+ messages in thread
From: Nakajima, Jun @ 2006-11-29 18:24 UTC (permalink / raw)
To: Petersson, Mats, Jan Beulich, xen-devel, Keir Fraser
Petersson, Mats wrote:
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xensource.com
>> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
>> Nakajima, Jun Sent: 29 November 2006 16:35
>> To: Jan Beulich; xen-devel@lists.xensource.com; Keir Fraser
>> Subject: RE: [Xen-devel] EFER in HVM guests
>>
>> Jan Beulich wrote:
>>>>>> Keir Fraser <keir@xensource.com> 29.11.06 14:09 >>>
>>>> On 29/11/06 13:07, "Jan Beulich" <jbeulich@novell.com> wrote:
>>>>
>>>>> Is it intentional that
>>>>> - under SVM, 32-bit guests can freely set EFER.LME
>>>>> - under VMX, 32-bit guests can't access EFER at all?
>>>>>
>>>>> Thanks, Jan
>>>>
>>>> I'm sure any differences are unintentional. There is obviously
>>>> scope for making much of the MSR and CPUID code non-vmx/svm
>>>> specific.
>>>>
>>>> I assume that this particular difference doesn't really matter?
>>>
>>> I think it does - allowing a guest to enable EFER.LME when the
>>> hypervisor is a 32-bit one is clearly a security problem: While I
>>> haven't tried it, I would suspect the moment you load a context
>>> with such an EFER the whole system's dead.
>>> Not being able to access EFER is also a potential problem, as a
>>> guest should be allowed to set EFER.NX (at least) - the CPUID
>>> handling code specifically does not suppress this bit if the guest
>>> is allowed to use PAE (which we agreed a few days ago should
>>> be the default anyway).
>>>
>>> Jan
>>>
>>
>> I agree that we should allow 32-bit guests to set EFER.NX on the PAE
>> Xen. We'll fix it. EFER.SCE should not be set on IA-32.
>
> Why not? If CPUID bits indicate that it's available, it can be used in
> 32- or 64-bit mode.
>
On IA-32 (i.e. I meant Intel), it's not available. The merged HVM code
should use CPUID to handle this kind of differences.
Jun
---
Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-13 12:48 Li, Xin B
2006-12-13 13:08 ` Jan Beulich
2006-12-13 14:37 ` Keir Fraser
0 siblings, 2 replies; 26+ messages in thread
From: Li, Xin B @ 2006-12-13 12:48 UTC (permalink / raw)
To: Keir Fraser, Nakajima, Jun, Jan Beulich, xen-devel
[-- Attachment #1: Type: text/plain, Size: 1559 bytes --]
Is this the framework what you want?
And we still need merge the common cases here.
-Xin
>-----Original Message-----
>From: xen-devel-bounces@lists.xensource.com
>[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
>Sent: 2006年11月30日 1:22
>To: Nakajima, Jun; Jan Beulich; xen-devel@lists.xensource.com;
>Keir Fraser
>Subject: Re: [Xen-devel] EFER in HVM guests
>
>
>*Please* can you make the handling of generic CPUID leaves and
>architectural
>MSRs common HVM code? There is lots of needless code
>duplication right now
>with niggling differences that shouldn't be necessary.
>
> -- Keir
>
>On 29/11/06 16:34, "Nakajima, Jun" <jun.nakajima@intel.com> wrote:
>
>>> I think it does - allowing a guest to enable EFER.LME when the
>>> hypervisor is a 32-bit one is clearly a security problem: While I
>>> haven't tried it, I would suspect the moment you load a context
>>> with such an EFER the whole system's dead.
>>> Not being able to access EFER is also a potential problem, as a
>>> guest should be allowed to set EFER.NX (at least) - the CPUID
>>> handling code specifically does not suppress this bit if the guest
>>> is allowed to use PAE (which we agreed a few days ago should
>>> be the default anyway).
>>>
>>> Jan
>>>
>>
>> I agree that we should allow 32-bit guests to set EFER.NX on the PAE
>> Xen. We'll fix it. EFER.SCE should not be set on IA-32.
>
>
>_______________________________________________
>Xen-devel mailing list
>Xen-devel@lists.xensource.com
>http://lists.xensource.com/xen-devel
>
[-- Attachment #2: cpuid.patch --]
[-- Type: application/octet-stream, Size: 10213 bytes --]
diff -r 478ddc354ccd xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c Wed Dec 13 11:32:04 2006 +0000
+++ b/xen/arch/x86/hvm/hvm.c Wed Dec 13 18:52:08 2006 +0800
@@ -384,6 +384,11 @@ int hvm_copy_from_guest_virt(void *buf,
return __hvm_copy(buf, vaddr, size, 0, 1);
}
+void hvm_cpuid(uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
+{
+ if ( !cpuid_hypervisor_leaves(*eax, eax, ebx, ecx, edx) )
+ return hvm_do_cpuid(eax, ebx, ecx, edx);
+}
/* HVM specific printbuf. Mostly used for hvmloader chit-chat. */
void hvm_print_line(struct vcpu *v, const char c)
diff -r 478ddc354ccd xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Wed Dec 13 11:32:04 2006 +0000
+++ b/xen/arch/x86/hvm/vmx/vmx.c Wed Dec 13 20:22:43 2006 +0800
@@ -640,6 +640,100 @@ static void vmx_set_tsc_offset(struct vc
vmx_vmcs_exit(v);
}
+#define bitmaskof(idx) (1U << ((idx)&31))
+static void vmx_do_cpuid(uint32_t *eax, uint32_t *ebx,
+ uint32_t *ecx, uint32_t *edx)
+{
+ uint32_t input = *eax;
+ uint32_t count = *ecx;
+ struct vcpu *v = current;
+
+ if ( input == CPUID_LEAF_0x4 )
+ {
+ cpuid_count(input, count, eax, ebx, ecx, edx);
+ *eax &= NUM_CORES_RESET_MASK;
+ }
+ else if ( input == 0x40000003 )
+ {
+ /*
+ * NB. Unsupported interface for private use of VMXASSIST only.
+ * Note that this leaf lives at <max-hypervisor-leaf> + 1.
+ */
+ u64 value = (((u64)(*edx)) << 32) | (*ecx);
+ unsigned long mfn = get_mfn_from_gpfn(value >> PAGE_SHIFT);
+ char *p;
+
+ gdprintk(XENLOG_INFO, "Input address is 0x%"PRIx64".\n", value);
+
+ /* 8-byte aligned valid pseudophys address from vmxassist, please. */
+ if ( (value & 7) || (mfn == INVALID_MFN) ||
+ !v->arch.hvm_vmx.vmxassist_enabled )
+ {
+ domain_crash(v->domain);
+ return;
+ }
+
+ p = map_domain_page(mfn);
+ value = *((uint64_t *)(p + (value & (PAGE_SIZE - 1))));
+ unmap_domain_page(p);
+
+ gdprintk(XENLOG_INFO, "Output value is 0x%"PRIx64".\n", value);
+ *ecx = (u32)(value >> 0);
+ *edx = (u32)(value >> 32);
+ }
+ else
+ {
+ cpuid(input, eax, ebx, ecx, edx);
+
+ if ( input == CPUID_LEAF_0x1 )
+ {
+ /* Mask off reserved bits. */
+ *ecx &= ~VMX_VCPU_CPUID_L1_ECX_RESERVED;
+
+ if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
+ clear_bit(X86_FEATURE_APIC, edx);
+
+#if CONFIG_PAGING_LEVELS >= 3
+ if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
+#endif
+ clear_bit(X86_FEATURE_PAE, edx);
+ clear_bit(X86_FEATURE_PSE36, edx);
+
+ *ebx &= NUM_THREADS_RESET_MASK;
+
+ /* Unsupportable for virtualised CPUs. */
+ *ecx &= ~(bitmaskof(X86_FEATURE_VMXE) |
+ bitmaskof(X86_FEATURE_EST) |
+ bitmaskof(X86_FEATURE_TM2) |
+ bitmaskof(X86_FEATURE_CID) |
+ bitmaskof(X86_FEATURE_MWAIT) );
+
+ *edx &= ~( bitmaskof(X86_FEATURE_HT) |
+ bitmaskof(X86_FEATURE_ACPI) |
+ bitmaskof(X86_FEATURE_ACC) );
+ }
+ else if ( ( input == CPUID_LEAF_0x6 )
+ || ( input == CPUID_LEAF_0x9 )
+ || ( input == CPUID_LEAF_0xA ))
+ {
+ *eax = *ebx = *ecx = *edx = 0x0;
+ }
+ else if ( input == CPUID_LEAF_0x80000001 )
+ {
+#if CONFIG_PAGING_LEVELS >= 3
+ if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
+#endif
+ clear_bit(X86_FEATURE_NX & 31, edx);
+#ifdef __i386__
+ clear_bit(X86_FEATURE_LAHF_LM & 31, ecx);
+
+ clear_bit(X86_FEATURE_LM & 31, edx);
+ clear_bit(X86_FEATURE_SYSCALL & 31, edx);
+#endif
+ }
+ }
+}
+
static void vmx_init_ap_context(
struct vcpu_guest_context *ctxt, int vcpuid, int trampoline_vector)
{
@@ -751,6 +845,8 @@ static void vmx_setup_hvm_funcs(void)
hvm_funcs.stts = vmx_stts;
hvm_funcs.set_tsc_offset = vmx_set_tsc_offset;
+ hvm_funcs.do_cpuid = vmx_do_cpuid;
+
hvm_funcs.inject_exception = vmx_inject_exception;
hvm_funcs.init_ap_context = vmx_init_ap_context;
@@ -886,121 +982,6 @@ static void vmx_do_no_device_fault(void)
v->arch.hvm_vmx.cpu_cr0 &= ~X86_CR0_TS;
__vmwrite(GUEST_CR0, v->arch.hvm_vmx.cpu_cr0);
}
-}
-
-#define bitmaskof(idx) (1U << ((idx)&31))
-static void vmx_do_cpuid(struct cpu_user_regs *regs)
-{
- unsigned int input = (unsigned int)regs->eax;
- unsigned int count = (unsigned int)regs->ecx;
- unsigned int eax, ebx, ecx, edx;
- unsigned long eip;
- struct vcpu *v = current;
-
- eip = __vmread(GUEST_RIP);
-
- HVM_DBG_LOG(DBG_LEVEL_3, "(eax) 0x%08lx, (ebx) 0x%08lx, "
- "(ecx) 0x%08lx, (edx) 0x%08lx, (esi) 0x%08lx, (edi) 0x%08lx",
- (unsigned long)regs->eax, (unsigned long)regs->ebx,
- (unsigned long)regs->ecx, (unsigned long)regs->edx,
- (unsigned long)regs->esi, (unsigned long)regs->edi);
-
- if ( input == CPUID_LEAF_0x4 )
- {
- cpuid_count(input, count, &eax, &ebx, &ecx, &edx);
- eax &= NUM_CORES_RESET_MASK;
- }
- else if ( input == 0x40000003 )
- {
- /*
- * NB. Unsupported interface for private use of VMXASSIST only.
- * Note that this leaf lives at <max-hypervisor-leaf> + 1.
- */
- u64 value = ((u64)regs->edx << 32) | (u32)regs->ecx;
- unsigned long mfn = get_mfn_from_gpfn(value >> PAGE_SHIFT);
- char *p;
-
- gdprintk(XENLOG_INFO, "Input address is 0x%"PRIx64".\n", value);
-
- /* 8-byte aligned valid pseudophys address from vmxassist, please. */
- if ( (value & 7) || (mfn == INVALID_MFN) ||
- !v->arch.hvm_vmx.vmxassist_enabled )
- {
- domain_crash(v->domain);
- return;
- }
-
- p = map_domain_page(mfn);
- value = *((uint64_t *)(p + (value & (PAGE_SIZE - 1))));
- unmap_domain_page(p);
-
- gdprintk(XENLOG_INFO, "Output value is 0x%"PRIx64".\n", value);
- ecx = (u32)(value >> 0);
- edx = (u32)(value >> 32);
- }
- else if ( !cpuid_hypervisor_leaves(input, &eax, &ebx, &ecx, &edx) )
- {
- cpuid(input, &eax, &ebx, &ecx, &edx);
-
- if ( input == CPUID_LEAF_0x1 )
- {
- /* Mask off reserved bits. */
- ecx &= ~VMX_VCPU_CPUID_L1_ECX_RESERVED;
-
- if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
- clear_bit(X86_FEATURE_APIC, &edx);
-
-#if CONFIG_PAGING_LEVELS >= 3
- if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
-#endif
- clear_bit(X86_FEATURE_PAE, &edx);
- clear_bit(X86_FEATURE_PSE36, &edx);
-
- ebx &= NUM_THREADS_RESET_MASK;
-
- /* Unsupportable for virtualised CPUs. */
- ecx &= ~(bitmaskof(X86_FEATURE_VMXE) |
- bitmaskof(X86_FEATURE_EST) |
- bitmaskof(X86_FEATURE_TM2) |
- bitmaskof(X86_FEATURE_CID) |
- bitmaskof(X86_FEATURE_MWAIT) );
-
- edx &= ~( bitmaskof(X86_FEATURE_HT) |
- bitmaskof(X86_FEATURE_ACPI) |
- bitmaskof(X86_FEATURE_ACC) );
- }
- else if ( ( input == CPUID_LEAF_0x6 )
- || ( input == CPUID_LEAF_0x9 )
- || ( input == CPUID_LEAF_0xA ))
- {
- eax = ebx = ecx = edx = 0x0;
- }
- else if ( input == CPUID_LEAF_0x80000001 )
- {
-#if CONFIG_PAGING_LEVELS >= 3
- if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
-#endif
- clear_bit(X86_FEATURE_NX & 31, &edx);
-#ifdef __i386__
- clear_bit(X86_FEATURE_LAHF_LM & 31, &ecx);
-
- clear_bit(X86_FEATURE_LM & 31, &edx);
- clear_bit(X86_FEATURE_SYSCALL & 31, &edx);
-#endif
- }
- }
-
- regs->eax = (unsigned long) eax;
- regs->ebx = (unsigned long) ebx;
- regs->ecx = (unsigned long) ecx;
- regs->edx = (unsigned long) edx;
-
- HVM_DBG_LOG(DBG_LEVEL_3, "eip@%lx, input: 0x%lx, "
- "output: eax = 0x%08lx, ebx = 0x%08lx, "
- "ecx = 0x%08lx, edx = 0x%08lx",
- (unsigned long)eip, (unsigned long)input,
- (unsigned long)eax, (unsigned long)ebx,
- (unsigned long)ecx, (unsigned long)edx);
}
#define CASE_GET_REG_P(REG, reg) \
@@ -2459,7 +2440,8 @@ asmlinkage void vmx_vmexit_handler(struc
case EXIT_REASON_CPUID:
inst_len = __get_instruction_length(); /* Safe: CPUID */
__update_guest_eip(inst_len);
- vmx_do_cpuid(regs);
+ hvm_cpuid((uint32_t *)®s->eax, (uint32_t *)®s->ebx,
+ (uint32_t *)®s->ecx, (uint32_t *)®s->edx);
break;
case EXIT_REASON_HLT:
inst_len = __get_instruction_length(); /* Safe: HLT */
diff -r 478ddc354ccd xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h Wed Dec 13 11:32:04 2006 +0000
+++ b/xen/include/asm-x86/hvm/hvm.h Wed Dec 13 19:00:36 2006 +0800
@@ -110,6 +110,8 @@ struct hvm_function_table {
void (*stts)(struct vcpu *v);
void (*set_tsc_offset)(struct vcpu *v, u64 offset);
+ void (*do_cpuid)(uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
+
void (*inject_exception)(unsigned int trapnr, int errcode,
unsigned long cr2);
@@ -218,6 +220,14 @@ void hvm_migrate_timers(struct vcpu *v);
void hvm_migrate_timers(struct vcpu *v);
void hvm_do_resume(struct vcpu *v);
+void hvm_cpuid(uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx);
+
+static inline void
+hvm_do_cpuid(uint32_t *eax, uint32_t *ebx, uint32_t *ecx, uint32_t *edx)
+{
+ return hvm_funcs.do_cpuid(eax, ebx, ecx, edx);
+}
+
static inline void
hvm_init_ap_context(struct vcpu_guest_context *ctxt,
int vcpuid, int trampoline_vector)
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-12-13 12:48 Li, Xin B
@ 2006-12-13 13:08 ` Jan Beulich
2006-12-13 14:37 ` Keir Fraser
1 sibling, 0 replies; 26+ messages in thread
From: Jan Beulich @ 2006-12-13 13:08 UTC (permalink / raw)
To: Xin B Li; +Cc: xen-devel, Keir Fraser, Jun Nakajima
>And we still need merge the common cases here.
But that is the important part. So far you basically only renamed functions.
Jan
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-13 14:09 Li, Xin B
0 siblings, 0 replies; 26+ messages in thread
From: Li, Xin B @ 2006-12-13 14:09 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Nakajima, Jun
I have no SVM env. Can anyone help on that side?
-Xin
>-----Original Message-----
>From: Jan Beulich [mailto:jbeulich@novell.com]
>Sent: 2006年12月13日 21:08
>To: Li, Xin B
>Cc: Nakajima, Jun; xen-devel@lists.xensource.com; Keir Fraser
>Subject: RE: [Xen-devel] EFER in HVM guests
>
>>And we still need merge the common cases here.
>
>But that is the important part. So far you basically only
>renamed functions.
>
>Jan
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-13 14:13 Li, Xin B
0 siblings, 0 replies; 26+ messages in thread
From: Li, Xin B @ 2006-12-13 14:13 UTC (permalink / raw)
To: Jan Beulich; +Cc: xen-devel, Keir Fraser, Nakajima, Jun
>-----Original Message-----
>From: Jan Beulich [mailto:jbeulich@novell.com]
>Sent: 2006年12月13日 21:08
>To: Li, Xin B
>Cc: Nakajima, Jun; xen-devel@lists.xensource.com; Keir Fraser
>Subject: RE: [Xen-devel] EFER in HVM guests
>
>>And we still need merge the common cases here.
>
>But that is the important part. So far you basically only
>renamed functions.
>
That's the start point we can merge.
-Xin
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: EFER in HVM guests
2006-12-13 12:48 Li, Xin B
2006-12-13 13:08 ` Jan Beulich
@ 2006-12-13 14:37 ` Keir Fraser
2006-12-15 16:04 ` Woller, Thomas
1 sibling, 1 reply; 26+ messages in thread
From: Keir Fraser @ 2006-12-13 14:37 UTC (permalink / raw)
To: Li, Xin B, Keir Fraser, Nakajima, Jun, Jan Beulich, xen-devel
Not quite what I had in mind. Control starts from VMX/SVM so we shouldn't
need an extra hvm_ops hook function.
What I envisage is something like:
Vmx_cpuid() {
/* CPU-specific pre-processing goes here. */
hvm_cpuid();
/* CPU-specific post-processing goes here. */
}
So VMX/SVM calls out to HVM, not vice versa. You can see that this also
gives you full flexibility to do pre-processing (before calling hvm-generic
function) as well as post-processing.
As Jan points out, there's little point in doing this without actually
pulling out some common code at the same time. Or hvm_cpuid() will be a
no-op. :-)
-- Keir
On 13/12/06 12:48, "Li, Xin B" <xin.b.li@intel.com> wrote:
> Is this the framework what you want?
> And we still need merge the common cases here.
> -Xin
>
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xensource.com
>> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
>> Sent: 2006年11月30日 1:22
>> To: Nakajima, Jun; Jan Beulich; xen-devel@lists.xensource.com;
>> Keir Fraser
>> Subject: Re: [Xen-devel] EFER in HVM guests
>>
>>
>> *Please* can you make the handling of generic CPUID leaves and
>> architectural
>> MSRs common HVM code? There is lots of needless code
>> duplication right now
>> with niggling differences that shouldn't be necessary.
>>
>> -- Keir
>>
>> On 29/11/06 16:34, "Nakajima, Jun" <jun.nakajima@intel.com> wrote:
>>
>>>> I think it does - allowing a guest to enable EFER.LME when the
>>>> hypervisor is a 32-bit one is clearly a security problem: While I
>>>> haven't tried it, I would suspect the moment you load a context
>>>> with such an EFER the whole system's dead.
>>>> Not being able to access EFER is also a potential problem, as a
>>>> guest should be allowed to set EFER.NX (at least) - the CPUID
>>>> handling code specifically does not suppress this bit if the guest
>>>> is allowed to use PAE (which we agreed a few days ago should
>>>> be the default anyway).
>>>>
>>>> Jan
>>>>
>>>
>>> I agree that we should allow 32-bit guests to set EFER.NX on the PAE
>>> Xen. We'll fix it. EFER.SCE should not be set on IA-32.
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-12-13 14:37 ` Keir Fraser
@ 2006-12-15 16:04 ` Woller, Thomas
0 siblings, 0 replies; 26+ messages in thread
From: Woller, Thomas @ 2006-12-15 16:04 UTC (permalink / raw)
To: Keir Fraser, Li, Xin B, Nakajima, Jun, Jan Beulich, xen-devel
Xin, when you have a tested hvm/vmx functional patch, we can run thru SVM platforms for testing and determine if any AMD specific modifications are needed. Just post to the list the patch/changeset tested, (CC me directly if you remember). I'll have our test group run thru the boot tests first, then our usual ltp/cerberus and windows testing over a few days.
Cheers,
-- Tom
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> Keir Fraser
> Sent: Wednesday, December 13, 2006 8:37 AM
> To: Li, Xin B; Keir Fraser; Nakajima, Jun; Jan Beulich;
> xen-devel@lists.xensource.com
> Subject: Re: [Xen-devel] EFER in HVM guests
>
> Not quite what I had in mind. Control starts from VMX/SVM so
> we shouldn't need an extra hvm_ops hook function.
>
> What I envisage is something like:
>
> Vmx_cpuid() {
> /* CPU-specific pre-processing goes here. */
> hvm_cpuid();
> /* CPU-specific post-processing goes here. */ }
>
> So VMX/SVM calls out to HVM, not vice versa. You can see that
> this also gives you full flexibility to do pre-processing
> (before calling hvm-generic
> function) as well as post-processing.
>
> As Jan points out, there's little point in doing this without
> actually pulling out some common code at the same time. Or
> hvm_cpuid() will be a no-op. :-)
>
> -- Keir
>
>
>
> On 13/12/06 12:48, "Li, Xin B" <xin.b.li@intel.com> wrote:
>
> > Is this the framework what you want?
> > And we still need merge the common cases here.
> > -Xin
> >
> >> -----Original Message-----
> >> From: xen-devel-bounces@lists.xensource.com
> >> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir
> >> Fraser
> >> Sent: 2006年11月30日 1:22
> >> To: Nakajima, Jun; Jan Beulich;
> xen-devel@lists.xensource.com; Keir
> >> Fraser
> >> Subject: Re: [Xen-devel] EFER in HVM guests
> >>
> >>
> >> *Please* can you make the handling of generic CPUID leaves and
> >> architectural MSRs common HVM code? There is lots of needless code
> >> duplication right now with niggling differences that shouldn't be
> >> necessary.
> >>
> >> -- Keir
> >>
> >> On 29/11/06 16:34, "Nakajima, Jun" <jun.nakajima@intel.com> wrote:
> >>
> >>>> I think it does - allowing a guest to enable EFER.LME when the
> >>>> hypervisor is a 32-bit one is clearly a security
> problem: While I
> >>>> haven't tried it, I would suspect the moment you load a context
> >>>> with such an EFER the whole system's dead.
> >>>> Not being able to access EFER is also a potential problem, as a
> >>>> guest should be allowed to set EFER.NX (at least) - the CPUID
> >>>> handling code specifically does not suppress this bit if
> the guest
> >>>> is allowed to use PAE (which we agreed a few days ago
> should be the
> >>>> default anyway).
> >>>>
> >>>> Jan
> >>>>
> >>>
> >>> I agree that we should allow 32-bit guests to set EFER.NX
> on the PAE
> >>> Xen. We'll fix it. EFER.SCE should not be set on IA-32.
> >>
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xensource.com
> >> http://lists.xensource.com/xen-devel
> >>
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-18 15:10 Li, Xin B
2006-12-18 15:47 ` Petersson, Mats
0 siblings, 1 reply; 26+ messages in thread
From: Li, Xin B @ 2006-12-18 15:10 UTC (permalink / raw)
To: Woller, Thomas, Keir Fraser, Nakajima, Jun, Jan Beulich,
xen-devel
[-- Attachment #1: Type: text/plain, Size: 3883 bytes --]
>Xin, when you have a tested hvm/vmx functional patch, we can
>run thru SVM platforms for testing and determine if any AMD
>specific modifications are needed. Just post to the list the
>patch/changeset tested, (CC me directly if you remember). I'll
>have our test group run thru the boot tests first, then our
>usual ltp/cerberus and windows testing over a few days.
>Cheers,
> -- Tom
Hi Tom, the patch is attached, and I've tested it on my side, pls test your side.
One thing strange to me, why your leaf 0x80000001 needs to handle PAE and APIC feature bits, seems it's same as 0x00000001, is this intended?
thanks
-Xin
>
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xensource.com
>> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
>> Keir Fraser
>> Sent: Wednesday, December 13, 2006 8:37 AM
>> To: Li, Xin B; Keir Fraser; Nakajima, Jun; Jan Beulich;
>> xen-devel@lists.xensource.com
>> Subject: Re: [Xen-devel] EFER in HVM guests
>>
>> Not quite what I had in mind. Control starts from VMX/SVM so
>> we shouldn't need an extra hvm_ops hook function.
>>
>> What I envisage is something like:
>>
>> Vmx_cpuid() {
>> /* CPU-specific pre-processing goes here. */
>> hvm_cpuid();
>> /* CPU-specific post-processing goes here. */ }
>>
>> So VMX/SVM calls out to HVM, not vice versa. You can see that
>> this also gives you full flexibility to do pre-processing
>> (before calling hvm-generic
>> function) as well as post-processing.
>>
>> As Jan points out, there's little point in doing this without
>> actually pulling out some common code at the same time. Or
>> hvm_cpuid() will be a no-op. :-)
>>
>> -- Keir
>>
>>
>>
>> On 13/12/06 12:48, "Li, Xin B" <xin.b.li@intel.com> wrote:
>>
>> > Is this the framework what you want?
>> > And we still need merge the common cases here.
>> > -Xin
>> >
>> >> -----Original Message-----
>> >> From: xen-devel-bounces@lists.xensource.com
>> >> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir
>> >> Fraser
>> >> Sent: 2006年11月30日 1:22
>> >> To: Nakajima, Jun; Jan Beulich;
>> xen-devel@lists.xensource.com; Keir
>> >> Fraser
>> >> Subject: Re: [Xen-devel] EFER in HVM guests
>> >>
>> >>
>> >> *Please* can you make the handling of generic CPUID leaves and
>> >> architectural MSRs common HVM code? There is lots of
>needless code
>> >> duplication right now with niggling differences that shouldn't be
>> >> necessary.
>> >>
>> >> -- Keir
>> >>
>> >> On 29/11/06 16:34, "Nakajima, Jun" <jun.nakajima@intel.com> wrote:
>> >>
>> >>>> I think it does - allowing a guest to enable EFER.LME when the
>> >>>> hypervisor is a 32-bit one is clearly a security
>> problem: While I
>> >>>> haven't tried it, I would suspect the moment you load a context
>> >>>> with such an EFER the whole system's dead.
>> >>>> Not being able to access EFER is also a potential problem, as a
>> >>>> guest should be allowed to set EFER.NX (at least) - the CPUID
>> >>>> handling code specifically does not suppress this bit if
>> the guest
>> >>>> is allowed to use PAE (which we agreed a few days ago
>> should be the
>> >>>> default anyway).
>> >>>>
>> >>>> Jan
>> >>>>
>> >>>
>> >>> I agree that we should allow 32-bit guests to set EFER.NX
>> on the PAE
>> >>> Xen. We'll fix it. EFER.SCE should not be set on IA-32.
>> >>
>> >>
>> >> _______________________________________________
>> >> Xen-devel mailing list
>> >> Xen-devel@lists.xensource.com
>> >> http://lists.xensource.com/xen-devel
>> >>
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@lists.xensource.com
>> > http://lists.xensource.com/xen-devel
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com
>> http://lists.xensource.com/xen-devel
>>
>>
>>
>
[-- Attachment #2: hvm_cpuid.patch --]
[-- Type: application/octet-stream, Size: 14781 bytes --]
diff -r 469478194aef xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c Mon Dec 18 00:14:40 2006 +0000
+++ b/xen/arch/x86/hvm/hvm.c Mon Dec 18 21:33:48 2006 +0800
@@ -400,6 +400,45 @@ void hvm_print_line(struct vcpu *v, cons
hd->pbuf_idx = 0;
}
spin_unlock(&hd->pbuf_lock);
+}
+
+void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx)
+{
+ if ( !cpuid_hypervisor_leaves(input, eax, ebx, ecx, edx) )
+ {
+ cpuid(input, eax, ebx, ecx, edx);
+
+ if ( input == 0x00000001 )
+ {
+ struct vcpu *v = current;
+
+ if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
+ clear_bit(X86_FEATURE_APIC, edx);
+
+#if CONFIG_PAGING_LEVELS >= 3
+ if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
+#endif
+ clear_bit(X86_FEATURE_PAE, edx);
+
+ clear_bit(X86_FEATURE_PSE36, edx);
+ }
+ else if ( input == 0x80000001 )
+ {
+#if CONFIG_PAGING_LEVELS >= 3
+ struct vcpu *v = current;
+ if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
+#endif
+ clear_bit(X86_FEATURE_NX & 31, edx);
+#ifdef __i386__
+ /* Mask feature for Intel ia32e or AMD long mode. */
+ clear_bit(X86_FEATURE_LAHF_LM & 31, ecx);
+
+ clear_bit(X86_FEATURE_LM & 31, edx);
+ clear_bit(X86_FEATURE_SYSCALL & 31, edx);
+#endif
+ }
+ }
}
typedef unsigned long hvm_hypercall_t(
diff -r 469478194aef xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c Mon Dec 18 00:14:40 2006 +0000
+++ b/xen/arch/x86/hvm/svm/svm.c Mon Dec 18 21:38:08 2006 +0800
@@ -999,91 +999,67 @@ static void svm_do_general_protection_fa
/* Reserved bits EDX: [31:29], [27], [22:20], [18], [10] */
#define SVM_VCPU_CPUID_L1_EDX_RESERVED 0xe8740400
-static void svm_vmexit_do_cpuid(struct vmcb_struct *vmcb, unsigned long input,
- struct cpu_user_regs *regs)
-{
+static void svm_vmexit_do_cpuid(struct cpu_user_regs *regs)
+{
+ unsigned long input = regs->eax;
unsigned int eax, ebx, ecx, edx;
- unsigned long eip;
struct vcpu *v = current;
+ struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
int inst_len;
ASSERT(vmcb);
- eip = vmcb->rip;
-
- HVM_DBG_LOG(DBG_LEVEL_1,
- "do_cpuid: (eax) %lx, (ebx) %lx, (ecx) %lx, (edx) %lx,"
- " (esi) %lx, (edi) %lx",
- (unsigned long)regs->eax, (unsigned long)regs->ebx,
- (unsigned long)regs->ecx, (unsigned long)regs->edx,
- (unsigned long)regs->esi, (unsigned long)regs->edi);
-
- if ( !cpuid_hypervisor_leaves(input, &eax, &ebx, &ecx, &edx) )
- {
- cpuid(input, &eax, &ebx, &ecx, &edx);
- if (input == 0x00000001 || input == 0x80000001 )
- {
- if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
- {
- /* Since the apic is disabled, avoid any confusion
- about SMP cpus being available */
- clear_bit(X86_FEATURE_APIC, &edx);
- }
+ hvm_cpuid(input, &eax, &ebx, &ecx, &edx);
+
+ if ( input == 0x00000001 )
+ {
+ /* Clear out reserved bits. */
+ ecx &= ~SVM_VCPU_CPUID_L1_ECX_RESERVED;
+ edx &= ~SVM_VCPU_CPUID_L1_EDX_RESERVED;
+
+ clear_bit(X86_FEATURE_MWAIT & 31, &ecx);
+
+ /* Guest should only see one logical processor.
+ * See details on page 23 of AMD CPUID Specification.
+ */
+ clear_bit(X86_FEATURE_HT, &edx); /* clear the hyperthread bit */
+ ebx &= 0xFF00FFFF; /* clear the logical processor count when HTT=0 */
+ ebx |= 0x00010000; /* set to 1 just for precaution */
+ }
+ else if ( input == 0x80000001 )
+ {
+ if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
+ clear_bit(X86_FEATURE_APIC, &edx);
+
#if CONFIG_PAGING_LEVELS >= 3
- if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
+ if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
#endif
- {
- clear_bit(X86_FEATURE_PAE, &edx);
- if (input == 0x80000001 )
- clear_bit(X86_FEATURE_NX & 31, &edx);
- }
- clear_bit(X86_FEATURE_PSE36, &edx);
- if (input == 0x00000001 )
- {
- /* Clear out reserved bits. */
- ecx &= ~SVM_VCPU_CPUID_L1_ECX_RESERVED;
- edx &= ~SVM_VCPU_CPUID_L1_EDX_RESERVED;
-
- clear_bit(X86_FEATURE_MWAIT & 31, &ecx);
-
- /* Guest should only see one logical processor.
- * See details on page 23 of AMD CPUID Specification.
- */
- clear_bit(X86_FEATURE_HT, &edx); /* clear the hyperthread bit */
- ebx &= 0xFF00FFFF; /* clear the logical processor count when HTT=0 */
- ebx |= 0x00010000; /* set to 1 just for precaution */
- }
- else
- {
- /* Clear the Cmp_Legacy bit
- * This bit is supposed to be zero when HTT = 0.
- * See details on page 23 of AMD CPUID Specification.
- */
- clear_bit(X86_FEATURE_CMP_LEGACY & 31, &ecx);
- /* Make SVM feature invisible to the guest. */
- clear_bit(X86_FEATURE_SVME & 31, &ecx);
-#ifdef __i386__
- /* Mask feature for Intel ia32e or AMD long mode. */
- clear_bit(X86_FEATURE_LAHF_LM & 31, &ecx);
-
- clear_bit(X86_FEATURE_LM & 31, &edx);
- clear_bit(X86_FEATURE_SYSCALL & 31, &edx);
-#endif
- /* So far, we do not support 3DNow for the guest. */
- clear_bit(X86_FEATURE_3DNOW & 31, &edx);
- clear_bit(X86_FEATURE_3DNOWEXT & 31, &edx);
- }
- }
- else if ( ( input == 0x80000007 ) || ( input == 0x8000000A ) )
- {
- /* Mask out features of power management and SVM extension. */
- eax = ebx = ecx = edx = 0;
- }
- else if ( input == 0x80000008 )
- {
- /* Make sure Number of CPU core is 1 when HTT=0 */
- ecx &= 0xFFFFFF00;
- }
+ clear_bit(X86_FEATURE_PAE, &edx);
+
+ clear_bit(X86_FEATURE_PSE36, &edx);
+
+ /* Clear the Cmp_Legacy bit
+ * This bit is supposed to be zero when HTT = 0.
+ * See details on page 23 of AMD CPUID Specification.
+ */
+ clear_bit(X86_FEATURE_CMP_LEGACY & 31, &ecx);
+
+ /* Make SVM feature invisible to the guest. */
+ clear_bit(X86_FEATURE_SVME & 31, &ecx);
+
+ /* So far, we do not support 3DNow for the guest. */
+ clear_bit(X86_FEATURE_3DNOW & 31, &edx);
+ clear_bit(X86_FEATURE_3DNOWEXT & 31, &edx);
+ }
+ else if ( input == 0x80000007 || input == 0x8000000A )
+ {
+ /* Mask out features of power management and SVM extension. */
+ eax = ebx = ecx = edx = 0;
+ }
+ else if ( input == 0x80000008 )
+ {
+ /* Make sure Number of CPU core is 1 when HTT=0 */
+ ecx &= 0xFFFFFF00;
}
regs->eax = (unsigned long)eax;
@@ -1091,16 +1067,10 @@ static void svm_vmexit_do_cpuid(struct v
regs->ecx = (unsigned long)ecx;
regs->edx = (unsigned long)edx;
- HVM_DBG_LOG(DBG_LEVEL_1,
- "svm_vmexit_do_cpuid: eip: %lx, input: %lx, out:eax=%x, "
- "ebx=%x, ecx=%x, edx=%x",
- eip, input, eax, ebx, ecx, edx);
-
inst_len = __get_instruction_length(vmcb, INSTR_CPUID, NULL);
ASSERT(inst_len > 0);
__update_guest_eip(vmcb, inst_len);
}
-
static inline unsigned long *get_reg_p(unsigned int gpreg,
struct cpu_user_regs *regs, struct vmcb_struct *vmcb)
@@ -2828,7 +2798,7 @@ asmlinkage void svm_vmexit_handler(struc
goto exit_and_crash;
case VMEXIT_CPUID:
- svm_vmexit_do_cpuid(vmcb, regs->eax, regs);
+ svm_vmexit_do_cpuid(regs);
break;
case VMEXIT_HLT:
diff -r 469478194aef xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Mon Dec 18 00:14:40 2006 +0000
+++ b/xen/arch/x86/hvm/vmx/vmx.c Mon Dec 18 21:34:52 2006 +0800
@@ -899,24 +899,14 @@ static void vmx_do_no_device_fault(void)
}
}
-#define bitmaskof(idx) (1U << ((idx)&31))
+#define bitmaskof(idx) (1U << ((idx) & 31))
static void vmx_do_cpuid(struct cpu_user_regs *regs)
{
unsigned int input = (unsigned int)regs->eax;
unsigned int count = (unsigned int)regs->ecx;
unsigned int eax, ebx, ecx, edx;
- unsigned long eip;
- struct vcpu *v = current;
-
- eip = __vmread(GUEST_RIP);
-
- HVM_DBG_LOG(DBG_LEVEL_3, "(eax) 0x%08lx, (ebx) 0x%08lx, "
- "(ecx) 0x%08lx, (edx) 0x%08lx, (esi) 0x%08lx, (edi) 0x%08lx",
- (unsigned long)regs->eax, (unsigned long)regs->ebx,
- (unsigned long)regs->ecx, (unsigned long)regs->edx,
- (unsigned long)regs->esi, (unsigned long)regs->edi);
-
- if ( input == CPUID_LEAF_0x4 )
+
+ if ( input == 0x00000004 )
{
cpuid_count(input, count, &eax, &ebx, &ecx, &edx);
eax &= NUM_CORES_RESET_MASK;
@@ -929,6 +919,7 @@ static void vmx_do_cpuid(struct cpu_user
*/
u64 value = ((u64)regs->edx << 32) | (u32)regs->ecx;
unsigned long mfn = get_mfn_from_gpfn(value >> PAGE_SHIFT);
+ struct vcpu *v = current;
char *p;
gdprintk(XENLOG_INFO, "Input address is 0x%"PRIx64".\n", value);
@@ -946,72 +937,38 @@ static void vmx_do_cpuid(struct cpu_user
unmap_domain_page(p);
gdprintk(XENLOG_INFO, "Output value is 0x%"PRIx64".\n", value);
- ecx = (u32)(value >> 0);
+ ecx = (u32)value;
edx = (u32)(value >> 32);
- }
- else if ( !cpuid_hypervisor_leaves(input, &eax, &ebx, &ecx, &edx) )
- {
- cpuid(input, &eax, &ebx, &ecx, &edx);
-
- if ( input == CPUID_LEAF_0x1 )
+ } else {
+ hvm_cpuid(input, &eax, &ebx, &ecx, &edx);
+
+ if ( input == 0x00000001 )
{
/* Mask off reserved bits. */
ecx &= ~VMX_VCPU_CPUID_L1_ECX_RESERVED;
- if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
- clear_bit(X86_FEATURE_APIC, &edx);
-
-#if CONFIG_PAGING_LEVELS >= 3
- if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
-#endif
- clear_bit(X86_FEATURE_PAE, &edx);
- clear_bit(X86_FEATURE_PSE36, &edx);
-
ebx &= NUM_THREADS_RESET_MASK;
/* Unsupportable for virtualised CPUs. */
- ecx &= ~(bitmaskof(X86_FEATURE_VMXE) |
- bitmaskof(X86_FEATURE_EST) |
- bitmaskof(X86_FEATURE_TM2) |
- bitmaskof(X86_FEATURE_CID) |
- bitmaskof(X86_FEATURE_MWAIT) );
-
- edx &= ~( bitmaskof(X86_FEATURE_HT) |
- bitmaskof(X86_FEATURE_ACPI) |
- bitmaskof(X86_FEATURE_ACC) );
- }
- else if ( ( input == CPUID_LEAF_0x6 )
- || ( input == CPUID_LEAF_0x9 )
- || ( input == CPUID_LEAF_0xA ))
- {
+ ecx &= ~(bitmaskof(X86_FEATURE_VMXE) |
+ bitmaskof(X86_FEATURE_EST) |
+ bitmaskof(X86_FEATURE_TM2) |
+ bitmaskof(X86_FEATURE_CID) |
+ bitmaskof(X86_FEATURE_MWAIT));
+
+ edx &= ~(bitmaskof(X86_FEATURE_HT) |
+ bitmaskof(X86_FEATURE_ACPI) |
+ bitmaskof(X86_FEATURE_ACC));
+ }
+
+ if ( input == 0x00000006 || input == 0x00000009 || input == 0x0000000A )
eax = ebx = ecx = edx = 0x0;
- }
- else if ( input == CPUID_LEAF_0x80000001 )
- {
-#if CONFIG_PAGING_LEVELS >= 3
- if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
-#endif
- clear_bit(X86_FEATURE_NX & 31, &edx);
-#ifdef __i386__
- clear_bit(X86_FEATURE_LAHF_LM & 31, &ecx);
-
- clear_bit(X86_FEATURE_LM & 31, &edx);
- clear_bit(X86_FEATURE_SYSCALL & 31, &edx);
-#endif
- }
- }
-
- regs->eax = (unsigned long) eax;
- regs->ebx = (unsigned long) ebx;
- regs->ecx = (unsigned long) ecx;
- regs->edx = (unsigned long) edx;
-
- HVM_DBG_LOG(DBG_LEVEL_3, "eip@%lx, input: 0x%lx, "
- "output: eax = 0x%08lx, ebx = 0x%08lx, "
- "ecx = 0x%08lx, edx = 0x%08lx",
- (unsigned long)eip, (unsigned long)input,
- (unsigned long)eax, (unsigned long)ebx,
- (unsigned long)ecx, (unsigned long)edx);
+ }
+
+ regs->eax = (unsigned long)eax;
+ regs->ebx = (unsigned long)ebx;
+ regs->ecx = (unsigned long)ecx;
+ regs->edx = (unsigned long)edx;
}
#define CASE_GET_REG_P(REG, reg) \
diff -r 469478194aef xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h Mon Dec 18 00:14:40 2006 +0000
+++ b/xen/include/asm-x86/hvm/hvm.h Mon Dec 18 16:00:53 2006 +0800
@@ -219,6 +219,8 @@ hvm_get_segment_register(struct vcpu *v,
hvm_funcs.get_segment_register(v, seg, reg);
}
+void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx);
void hvm_stts(struct vcpu *v);
void hvm_set_guest_time(struct vcpu *v, u64 gtime);
void hvm_freeze_time(struct vcpu *v);
diff -r 469478194aef xen/include/asm-x86/hvm/vmx/cpu.h
--- a/xen/include/asm-x86/hvm/vmx/cpu.h Mon Dec 18 00:14:40 2006 +0000
+++ b/xen/include/asm-x86/hvm/vmx/cpu.h Mon Dec 18 20:58:08 2006 +0800
@@ -32,21 +32,14 @@ struct arch_state_struct {
#define VMX_MF_32 1
#define VMX_MF_64 2
-#define CPUID_LEAF_0x1 0x1
-#define CPUID_LEAF_0x4 0x4
-#define CPUID_LEAF_0x6 0x6
-#define CPUID_LEAF_0x9 0x9
-#define CPUID_LEAF_0xA 0xA
-#define CPUID_LEAF_0x80000001 0x80000001
-
#define NUM_CORES_RESET_MASK 0x00003FFF
#define NUM_THREADS_RESET_MASK 0xFF00FFFF
#define VMX_VCPU_CPUID_L1_ECX_RESERVED_18 0x00040000
#define VMX_VCPU_CPUID_L1_ECX_RESERVED_6 0x00000040
-#define VMX_VCPU_CPUID_L1_ECX_RESERVED \
- ( VMX_VCPU_CPUID_L1_ECX_RESERVED_18 | \
- VMX_VCPU_CPUID_L1_ECX_RESERVED_6 )
+#define VMX_VCPU_CPUID_L1_ECX_RESERVED \
+ ( VMX_VCPU_CPUID_L1_ECX_RESERVED_18 | \
+ VMX_VCPU_CPUID_L1_ECX_RESERVED_6 )
#endif /* __ASM_X86_HVM_VMX_CPU_H__ */
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-12-18 15:10 Li, Xin B
@ 2006-12-18 15:47 ` Petersson, Mats
0 siblings, 0 replies; 26+ messages in thread
From: Petersson, Mats @ 2006-12-18 15:47 UTC (permalink / raw)
To: Li, Xin B, Woller, Thomas, Keir Fraser, Nakajima, Jun,
Jan Beulich, xen-devel
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Li, Xin B
> Sent: 18 December 2006 15:11
> To: Woller, Thomas; Keir Fraser; Nakajima, Jun; Jan Beulich;
> xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] EFER in HVM guests
>
> >Xin, when you have a tested hvm/vmx functional patch, we can
> >run thru SVM platforms for testing and determine if any AMD
> >specific modifications are needed. Just post to the list the
> >patch/changeset tested, (CC me directly if you remember). I'll
> >have our test group run thru the boot tests first, then our
> >usual ltp/cerberus and windows testing over a few days.
> >Cheers,
> > -- Tom
>
> Hi Tom, the patch is attached, and I've tested it on my side,
> pls test your side.
> One thing strange to me, why your leaf 0x80000001 needs to
> handle PAE and APIC feature bits, seems it's same as
> 0x00000001, is this intended?
> thanks
> -Xin
Xin,
Good job - thanks for the patch.
A few notes:
The 0x80000001 leaf was originally an "AMD only" leaf for adding new "non-Intel compatible" features, such as 3DNow! and long-mode, but since x86_64 was adopted by Intel, it's available on Intel processors too. It should be done the same on both AMD and Intel, and since 0x80000001 contains another copy of the APIC and PAE bits, they should be masked for both processors on both 1 and 0x80000001. [Of course, I doubt that anyone would "prefer" to use 0x80000001 from using 1 as the index for the leaf unless the coder is already reading 0x800000001 for some other reason - to detect LM for example].
I would like to see the handling of 0x80000001 in the common case cover PAE/PSE36/APIC features, as that's nor arch-specific. The fact that no-one actually uses it currently isn't a good argument for not getting it right at this time rather than fixing hard-to-find bugs later on... ;-)
Clearing MWAIT bit should also be made common - I doubt anyone will notice the single instruction saved by combining it with a bunch of other bits, compared to the overall benefit of trivially seeing that it's dealt with the same way on both architectures.
Just out of curiosity, why did you change the parameters passed to svm_do_cpuid - I can see why you wouldn't need to pass regs->eax when it's available in regs, but digging out the vmcb again can't be better than passing the already existing one? [Don't worry about it, I'm just curious about why the change was made].
--
Mats
>
> >
> >> -----Original Message-----
> >> From: xen-devel-bounces@lists.xensource.com
> >> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> >> Keir Fraser
> >> Sent: Wednesday, December 13, 2006 8:37 AM
> >> To: Li, Xin B; Keir Fraser; Nakajima, Jun; Jan Beulich;
> >> xen-devel@lists.xensource.com
> >> Subject: Re: [Xen-devel] EFER in HVM guests
> >>
> >> Not quite what I had in mind. Control starts from VMX/SVM so
> >> we shouldn't need an extra hvm_ops hook function.
> >>
> >> What I envisage is something like:
> >>
> >> Vmx_cpuid() {
> >> /* CPU-specific pre-processing goes here. */
> >> hvm_cpuid();
> >> /* CPU-specific post-processing goes here. */ }
> >>
> >> So VMX/SVM calls out to HVM, not vice versa. You can see that
> >> this also gives you full flexibility to do pre-processing
> >> (before calling hvm-generic
> >> function) as well as post-processing.
> >>
> >> As Jan points out, there's little point in doing this without
> >> actually pulling out some common code at the same time. Or
> >> hvm_cpuid() will be a no-op. :-)
> >>
> >> -- Keir
> >>
> >>
> >>
> >> On 13/12/06 12:48, "Li, Xin B" <xin.b.li@intel.com> wrote:
> >>
> >> > Is this the framework what you want?
> >> > And we still need merge the common cases here.
> >> > -Xin
> >> >
> >> >> -----Original Message-----
> >> >> From: xen-devel-bounces@lists.xensource.com
> >> >> [mailto:xen-devel-bounces@lists.xensource.com] On
> Behalf Of Keir
> >> >> Fraser
> >> >> Sent: 2006年11月30日 1:22
> >> >> To: Nakajima, Jun; Jan Beulich;
> >> xen-devel@lists.xensource.com; Keir
> >> >> Fraser
> >> >> Subject: Re: [Xen-devel] EFER in HVM guests
> >> >>
> >> >>
> >> >> *Please* can you make the handling of generic CPUID leaves and
> >> >> architectural MSRs common HVM code? There is lots of
> >needless code
> >> >> duplication right now with niggling differences that
> shouldn't be
> >> >> necessary.
> >> >>
> >> >> -- Keir
> >> >>
> >> >> On 29/11/06 16:34, "Nakajima, Jun"
> <jun.nakajima@intel.com> wrote:
> >> >>
> >> >>>> I think it does - allowing a guest to enable EFER.LME
> when the
> >> >>>> hypervisor is a 32-bit one is clearly a security
> >> problem: While I
> >> >>>> haven't tried it, I would suspect the moment you load
> a context
> >> >>>> with such an EFER the whole system's dead.
> >> >>>> Not being able to access EFER is also a potential
> problem, as a
> >> >>>> guest should be allowed to set EFER.NX (at least) - the CPUID
> >> >>>> handling code specifically does not suppress this bit if
> >> the guest
> >> >>>> is allowed to use PAE (which we agreed a few days ago
> >> should be the
> >> >>>> default anyway).
> >> >>>>
> >> >>>> Jan
> >> >>>>
> >> >>>
> >> >>> I agree that we should allow 32-bit guests to set EFER.NX
> >> on the PAE
> >> >>> Xen. We'll fix it. EFER.SCE should not be set on IA-32.
> >> >>
> >> >>
> >> >> _______________________________________________
> >> >> Xen-devel mailing list
> >> >> Xen-devel@lists.xensource.com
> >> >> http://lists.xensource.com/xen-devel
> >> >>
> >> > _______________________________________________
> >> > Xen-devel mailing list
> >> > Xen-devel@lists.xensource.com
> >> > http://lists.xensource.com/xen-devel
> >>
> >>
> >> _______________________________________________
> >> Xen-devel mailing list
> >> Xen-devel@lists.xensource.com
> >> http://lists.xensource.com/xen-devel
> >>
> >>
> >>
> >
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-19 6:44 Li, Xin B
0 siblings, 0 replies; 26+ messages in thread
From: Li, Xin B @ 2006-12-19 6:44 UTC (permalink / raw)
To: Petersson, Mats, Woller, Thomas, Keir Fraser, Nakajima, Jun,
Jan Beulich, xen-devel
[-- Attachment #1: Type: text/plain, Size: 2113 bytes --]
>
>The 0x80000001 leaf was originally an "AMD only" leaf for
>adding new "non-Intel compatible" features, such as 3DNow! and
>long-mode, but since x86_64 was adopted by Intel, it's
>available on Intel processors too. It should be done the same
>on both AMD and Intel, and since 0x80000001 contains another
>copy of the APIC and PAE bits, they should be masked for both
>processors on both 1 and 0x80000001. [Of course, I doubt that
>anyone would "prefer" to use 0x80000001 from using 1 as the
>index for the leaf unless the coder is already reading
>0x800000001 for some other reason - to detect LM for example].
>
>I would like to see the handling of 0x80000001 in the common
>case cover PAE/PSE36/APIC features, as that's nor
>arch-specific. The fact that no-one actually uses it currently
>isn't a good argument for not getting it right at this time
>rather than fixing hard-to-find bugs later on... ;-)
>
Mats,
Leaf 0x80000001 on Intel processors only uses 4 bits in ECX and EDX,
they are:
LAHF/SAHF: bit 0 of ECX
SYSCALL/SYSRET: bit 11 of EDX
Execution Disable bit: bit 20 of EDX
LM bit: bit 29 of EDX
All other bits are reserved to 0.
>Clearing MWAIT bit should also be made common - I doubt anyone
>will notice the single instruction saved by combining it with
>a bunch of other bits, compared to the overall benefit of
>trivially seeing that it's dealt with the same way on both
>architectures.
I did have this in mind when creating this patch, but I'm not sure if
MWAIT virtualization is common on both sides, so just keep it there, and
The patch attached has this fixed.
>
>Just out of curiosity, why did you change the parameters
>passed to svm_do_cpuid - I can see why you wouldn't need to
>pass regs->eax when it's available in regs, but digging out
>the vmcb again can't be better than passing the already
>existing one? [Don't worry about it, I'm just curious about
>why the change was made].
In my mind, just pass parameters you don't have in hand. And yes,
actually vmcb should be a parameter here :-)
-Xin
[-- Attachment #2: hvm_cpuid.patch --]
[-- Type: application/octet-stream, Size: 14776 bytes --]
diff -r 4ef0dbe95eac xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c Mon Dec 18 16:11:19 2006 +0000
+++ b/xen/arch/x86/hvm/hvm.c Tue Dec 19 14:36:42 2006 +0800
@@ -400,6 +400,47 @@ void hvm_print_line(struct vcpu *v, cons
hd->pbuf_idx = 0;
}
spin_unlock(&hd->pbuf_lock);
+}
+
+void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx)
+{
+ if ( !cpuid_hypervisor_leaves(input, eax, ebx, ecx, edx) )
+ {
+ cpuid(input, eax, ebx, ecx, edx);
+
+ if ( input == 0x00000001 )
+ {
+ struct vcpu *v = current;
+
+ clear_bit(X86_FEATURE_MWAIT & 31, ecx);
+
+ if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
+ clear_bit(X86_FEATURE_APIC & 31, edx);
+
+#if CONFIG_PAGING_LEVELS >= 3
+ if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
+#endif
+ clear_bit(X86_FEATURE_PAE & 31, edx);
+
+ clear_bit(X86_FEATURE_PSE36 & 31, edx);
+ }
+ else if ( input == 0x80000001 )
+ {
+#if CONFIG_PAGING_LEVELS >= 3
+ struct vcpu *v = current;
+ if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
+#endif
+ clear_bit(X86_FEATURE_NX & 31, edx);
+#ifdef __i386__
+ /* Mask feature for Intel ia32e or AMD long mode. */
+ clear_bit(X86_FEATURE_LAHF_LM & 31, ecx);
+
+ clear_bit(X86_FEATURE_LM & 31, edx);
+ clear_bit(X86_FEATURE_SYSCALL & 31, edx);
+#endif
+ }
+ }
}
typedef unsigned long hvm_hypercall_t(
diff -r 4ef0dbe95eac xen/arch/x86/hvm/svm/svm.c
--- a/xen/arch/x86/hvm/svm/svm.c Mon Dec 18 16:11:19 2006 +0000
+++ b/xen/arch/x86/hvm/svm/svm.c Tue Dec 19 14:38:34 2006 +0800
@@ -999,91 +999,65 @@ static void svm_do_general_protection_fa
/* Reserved bits EDX: [31:29], [27], [22:20], [18], [10] */
#define SVM_VCPU_CPUID_L1_EDX_RESERVED 0xe8740400
-static void svm_vmexit_do_cpuid(struct vmcb_struct *vmcb, unsigned long input,
- struct cpu_user_regs *regs)
-{
+static void svm_vmexit_do_cpuid(struct vmcb_struct *vmcb,
+ struct cpu_user_regs *regs)
+{
+ unsigned long input = regs->eax;
unsigned int eax, ebx, ecx, edx;
- unsigned long eip;
struct vcpu *v = current;
int inst_len;
ASSERT(vmcb);
- eip = vmcb->rip;
-
- HVM_DBG_LOG(DBG_LEVEL_1,
- "do_cpuid: (eax) %lx, (ebx) %lx, (ecx) %lx, (edx) %lx,"
- " (esi) %lx, (edi) %lx",
- (unsigned long)regs->eax, (unsigned long)regs->ebx,
- (unsigned long)regs->ecx, (unsigned long)regs->edx,
- (unsigned long)regs->esi, (unsigned long)regs->edi);
-
- if ( !cpuid_hypervisor_leaves(input, &eax, &ebx, &ecx, &edx) )
- {
- cpuid(input, &eax, &ebx, &ecx, &edx);
- if (input == 0x00000001 || input == 0x80000001 )
- {
- if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
- {
- /* Since the apic is disabled, avoid any confusion
- about SMP cpus being available */
- clear_bit(X86_FEATURE_APIC, &edx);
- }
+ hvm_cpuid(input, &eax, &ebx, &ecx, &edx);
+
+ if ( input == 0x00000001 )
+ {
+ /* Clear out reserved bits. */
+ ecx &= ~SVM_VCPU_CPUID_L1_ECX_RESERVED;
+ edx &= ~SVM_VCPU_CPUID_L1_EDX_RESERVED;
+
+ /* Guest should only see one logical processor.
+ * See details on page 23 of AMD CPUID Specification.
+ */
+ clear_bit(X86_FEATURE_HT & 31, &edx); /* clear the hyperthread bit */
+ ebx &= 0xFF00FFFF; /* clear the logical processor count when HTT=0 */
+ ebx |= 0x00010000; /* set to 1 just for precaution */
+ }
+ else if ( input == 0x80000001 )
+ {
+ if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
+ clear_bit(X86_FEATURE_APIC & 31, &edx);
+
#if CONFIG_PAGING_LEVELS >= 3
- if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
+ if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
#endif
- {
- clear_bit(X86_FEATURE_PAE, &edx);
- if (input == 0x80000001 )
- clear_bit(X86_FEATURE_NX & 31, &edx);
- }
- clear_bit(X86_FEATURE_PSE36, &edx);
- if (input == 0x00000001 )
- {
- /* Clear out reserved bits. */
- ecx &= ~SVM_VCPU_CPUID_L1_ECX_RESERVED;
- edx &= ~SVM_VCPU_CPUID_L1_EDX_RESERVED;
-
- clear_bit(X86_FEATURE_MWAIT & 31, &ecx);
-
- /* Guest should only see one logical processor.
- * See details on page 23 of AMD CPUID Specification.
- */
- clear_bit(X86_FEATURE_HT, &edx); /* clear the hyperthread bit */
- ebx &= 0xFF00FFFF; /* clear the logical processor count when HTT=0 */
- ebx |= 0x00010000; /* set to 1 just for precaution */
- }
- else
- {
- /* Clear the Cmp_Legacy bit
- * This bit is supposed to be zero when HTT = 0.
- * See details on page 23 of AMD CPUID Specification.
- */
- clear_bit(X86_FEATURE_CMP_LEGACY & 31, &ecx);
- /* Make SVM feature invisible to the guest. */
- clear_bit(X86_FEATURE_SVME & 31, &ecx);
-#ifdef __i386__
- /* Mask feature for Intel ia32e or AMD long mode. */
- clear_bit(X86_FEATURE_LAHF_LM & 31, &ecx);
-
- clear_bit(X86_FEATURE_LM & 31, &edx);
- clear_bit(X86_FEATURE_SYSCALL & 31, &edx);
-#endif
- /* So far, we do not support 3DNow for the guest. */
- clear_bit(X86_FEATURE_3DNOW & 31, &edx);
- clear_bit(X86_FEATURE_3DNOWEXT & 31, &edx);
- }
- }
- else if ( ( input == 0x80000007 ) || ( input == 0x8000000A ) )
- {
- /* Mask out features of power management and SVM extension. */
- eax = ebx = ecx = edx = 0;
- }
- else if ( input == 0x80000008 )
- {
- /* Make sure Number of CPU core is 1 when HTT=0 */
- ecx &= 0xFFFFFF00;
- }
+ clear_bit(X86_FEATURE_PAE & 31, &edx);
+
+ clear_bit(X86_FEATURE_PSE36 & 31, &edx);
+
+ /* Clear the Cmp_Legacy bit
+ * This bit is supposed to be zero when HTT = 0.
+ * See details on page 23 of AMD CPUID Specification.
+ */
+ clear_bit(X86_FEATURE_CMP_LEGACY & 31, &ecx);
+
+ /* Make SVM feature invisible to the guest. */
+ clear_bit(X86_FEATURE_SVME & 31, &ecx);
+
+ /* So far, we do not support 3DNow for the guest. */
+ clear_bit(X86_FEATURE_3DNOW & 31, &edx);
+ clear_bit(X86_FEATURE_3DNOWEXT & 31, &edx);
+ }
+ else if ( input == 0x80000007 || input == 0x8000000A )
+ {
+ /* Mask out features of power management and SVM extension. */
+ eax = ebx = ecx = edx = 0;
+ }
+ else if ( input == 0x80000008 )
+ {
+ /* Make sure Number of CPU core is 1 when HTT=0 */
+ ecx &= 0xFFFFFF00;
}
regs->eax = (unsigned long)eax;
@@ -1091,16 +1065,10 @@ static void svm_vmexit_do_cpuid(struct v
regs->ecx = (unsigned long)ecx;
regs->edx = (unsigned long)edx;
- HVM_DBG_LOG(DBG_LEVEL_1,
- "svm_vmexit_do_cpuid: eip: %lx, input: %lx, out:eax=%x, "
- "ebx=%x, ecx=%x, edx=%x",
- eip, input, eax, ebx, ecx, edx);
-
inst_len = __get_instruction_length(vmcb, INSTR_CPUID, NULL);
ASSERT(inst_len > 0);
__update_guest_eip(vmcb, inst_len);
}
-
static inline unsigned long *get_reg_p(unsigned int gpreg,
struct cpu_user_regs *regs, struct vmcb_struct *vmcb)
@@ -2828,7 +2796,7 @@ asmlinkage void svm_vmexit_handler(struc
goto exit_and_crash;
case VMEXIT_CPUID:
- svm_vmexit_do_cpuid(vmcb, regs->eax, regs);
+ svm_vmexit_do_cpuid(vmcb, regs);
break;
case VMEXIT_HLT:
diff -r 4ef0dbe95eac xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c Mon Dec 18 16:11:19 2006 +0000
+++ b/xen/arch/x86/hvm/vmx/vmx.c Tue Dec 19 14:34:24 2006 +0800
@@ -899,24 +899,14 @@ static void vmx_do_no_device_fault(void)
}
}
-#define bitmaskof(idx) (1U << ((idx)&31))
+#define bitmaskof(idx) (1U << ((idx) & 31))
static void vmx_do_cpuid(struct cpu_user_regs *regs)
{
unsigned int input = (unsigned int)regs->eax;
unsigned int count = (unsigned int)regs->ecx;
unsigned int eax, ebx, ecx, edx;
- unsigned long eip;
- struct vcpu *v = current;
-
- eip = __vmread(GUEST_RIP);
-
- HVM_DBG_LOG(DBG_LEVEL_3, "(eax) 0x%08lx, (ebx) 0x%08lx, "
- "(ecx) 0x%08lx, (edx) 0x%08lx, (esi) 0x%08lx, (edi) 0x%08lx",
- (unsigned long)regs->eax, (unsigned long)regs->ebx,
- (unsigned long)regs->ecx, (unsigned long)regs->edx,
- (unsigned long)regs->esi, (unsigned long)regs->edi);
-
- if ( input == CPUID_LEAF_0x4 )
+
+ if ( input == 0x00000004 )
{
cpuid_count(input, count, &eax, &ebx, &ecx, &edx);
eax &= NUM_CORES_RESET_MASK;
@@ -929,6 +919,7 @@ static void vmx_do_cpuid(struct cpu_user
*/
u64 value = ((u64)regs->edx << 32) | (u32)regs->ecx;
unsigned long mfn = get_mfn_from_gpfn(value >> PAGE_SHIFT);
+ struct vcpu *v = current;
char *p;
gdprintk(XENLOG_INFO, "Input address is 0x%"PRIx64".\n", value);
@@ -946,72 +937,37 @@ static void vmx_do_cpuid(struct cpu_user
unmap_domain_page(p);
gdprintk(XENLOG_INFO, "Output value is 0x%"PRIx64".\n", value);
- ecx = (u32)(value >> 0);
+ ecx = (u32)value;
edx = (u32)(value >> 32);
- }
- else if ( !cpuid_hypervisor_leaves(input, &eax, &ebx, &ecx, &edx) )
- {
- cpuid(input, &eax, &ebx, &ecx, &edx);
-
- if ( input == CPUID_LEAF_0x1 )
+ } else {
+ hvm_cpuid(input, &eax, &ebx, &ecx, &edx);
+
+ if ( input == 0x00000001 )
{
/* Mask off reserved bits. */
ecx &= ~VMX_VCPU_CPUID_L1_ECX_RESERVED;
- if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
- clear_bit(X86_FEATURE_APIC, &edx);
-
-#if CONFIG_PAGING_LEVELS >= 3
- if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
-#endif
- clear_bit(X86_FEATURE_PAE, &edx);
- clear_bit(X86_FEATURE_PSE36, &edx);
-
ebx &= NUM_THREADS_RESET_MASK;
/* Unsupportable for virtualised CPUs. */
- ecx &= ~(bitmaskof(X86_FEATURE_VMXE) |
- bitmaskof(X86_FEATURE_EST) |
- bitmaskof(X86_FEATURE_TM2) |
- bitmaskof(X86_FEATURE_CID) |
- bitmaskof(X86_FEATURE_MWAIT) );
-
- edx &= ~( bitmaskof(X86_FEATURE_HT) |
- bitmaskof(X86_FEATURE_ACPI) |
- bitmaskof(X86_FEATURE_ACC) );
- }
- else if ( ( input == CPUID_LEAF_0x6 )
- || ( input == CPUID_LEAF_0x9 )
- || ( input == CPUID_LEAF_0xA ))
- {
+ ecx &= ~(bitmaskof(X86_FEATURE_VMXE) |
+ bitmaskof(X86_FEATURE_EST) |
+ bitmaskof(X86_FEATURE_TM2) |
+ bitmaskof(X86_FEATURE_CID));
+
+ edx &= ~(bitmaskof(X86_FEATURE_HT) |
+ bitmaskof(X86_FEATURE_ACPI) |
+ bitmaskof(X86_FEATURE_ACC));
+ }
+
+ if ( input == 0x00000006 || input == 0x00000009 || input == 0x0000000A )
eax = ebx = ecx = edx = 0x0;
- }
- else if ( input == CPUID_LEAF_0x80000001 )
- {
-#if CONFIG_PAGING_LEVELS >= 3
- if ( !v->domain->arch.hvm_domain.params[HVM_PARAM_PAE_ENABLED] )
-#endif
- clear_bit(X86_FEATURE_NX & 31, &edx);
-#ifdef __i386__
- clear_bit(X86_FEATURE_LAHF_LM & 31, &ecx);
-
- clear_bit(X86_FEATURE_LM & 31, &edx);
- clear_bit(X86_FEATURE_SYSCALL & 31, &edx);
-#endif
- }
- }
-
- regs->eax = (unsigned long) eax;
- regs->ebx = (unsigned long) ebx;
- regs->ecx = (unsigned long) ecx;
- regs->edx = (unsigned long) edx;
-
- HVM_DBG_LOG(DBG_LEVEL_3, "eip@%lx, input: 0x%lx, "
- "output: eax = 0x%08lx, ebx = 0x%08lx, "
- "ecx = 0x%08lx, edx = 0x%08lx",
- (unsigned long)eip, (unsigned long)input,
- (unsigned long)eax, (unsigned long)ebx,
- (unsigned long)ecx, (unsigned long)edx);
+ }
+
+ regs->eax = (unsigned long)eax;
+ regs->ebx = (unsigned long)ebx;
+ regs->ecx = (unsigned long)ecx;
+ regs->edx = (unsigned long)edx;
}
#define CASE_GET_REG_P(REG, reg) \
diff -r 4ef0dbe95eac xen/include/asm-x86/hvm/hvm.h
--- a/xen/include/asm-x86/hvm/hvm.h Mon Dec 18 16:11:19 2006 +0000
+++ b/xen/include/asm-x86/hvm/hvm.h Mon Dec 18 16:00:53 2006 +0800
@@ -219,6 +219,8 @@ hvm_get_segment_register(struct vcpu *v,
hvm_funcs.get_segment_register(v, seg, reg);
}
+void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
+ unsigned int *ecx, unsigned int *edx);
void hvm_stts(struct vcpu *v);
void hvm_set_guest_time(struct vcpu *v, u64 gtime);
void hvm_freeze_time(struct vcpu *v);
diff -r 4ef0dbe95eac xen/include/asm-x86/hvm/vmx/cpu.h
--- a/xen/include/asm-x86/hvm/vmx/cpu.h Mon Dec 18 16:11:19 2006 +0000
+++ b/xen/include/asm-x86/hvm/vmx/cpu.h Mon Dec 18 20:58:08 2006 +0800
@@ -32,21 +32,14 @@ struct arch_state_struct {
#define VMX_MF_32 1
#define VMX_MF_64 2
-#define CPUID_LEAF_0x1 0x1
-#define CPUID_LEAF_0x4 0x4
-#define CPUID_LEAF_0x6 0x6
-#define CPUID_LEAF_0x9 0x9
-#define CPUID_LEAF_0xA 0xA
-#define CPUID_LEAF_0x80000001 0x80000001
-
#define NUM_CORES_RESET_MASK 0x00003FFF
#define NUM_THREADS_RESET_MASK 0xFF00FFFF
#define VMX_VCPU_CPUID_L1_ECX_RESERVED_18 0x00040000
#define VMX_VCPU_CPUID_L1_ECX_RESERVED_6 0x00000040
-#define VMX_VCPU_CPUID_L1_ECX_RESERVED \
- ( VMX_VCPU_CPUID_L1_ECX_RESERVED_18 | \
- VMX_VCPU_CPUID_L1_ECX_RESERVED_6 )
+#define VMX_VCPU_CPUID_L1_ECX_RESERVED \
+ ( VMX_VCPU_CPUID_L1_ECX_RESERVED_18 | \
+ VMX_VCPU_CPUID_L1_ECX_RESERVED_6 )
#endif /* __ASM_X86_HVM_VMX_CPU_H__ */
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-19 7:04 Li, Xin B
0 siblings, 0 replies; 26+ messages in thread
From: Li, Xin B @ 2006-12-19 7:04 UTC (permalink / raw)
To: Li, Xin B, Petersson, Mats, Woller, Thomas, Keir Fraser,
Nakajima, Jun, Jan Beulich, xen-devel
>>
>>Just out of curiosity, why did you change the parameters
>>passed to svm_do_cpuid - I can see why you wouldn't need to
>>pass regs->eax when it's available in regs, but digging out
>>the vmcb again can't be better than passing the already
>>existing one? [Don't worry about it, I'm just curious about
>>why the change was made].
>
In svm code, you don't always pass vmcb as input paramter, for example,
svm_do_msr_access, it's a little bit confusing, I think you need clean
it up.
-Xin
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-19 9:12 Li, Xin B
2006-12-19 9:46 ` Petersson, Mats
0 siblings, 1 reply; 26+ messages in thread
From: Li, Xin B @ 2006-12-19 9:12 UTC (permalink / raw)
To: Keir Fraser, Nakajima, Jun, Woller, Thomas, xen-devel,
Petersson, Mats
When I'm trying to merge VMX/SVM MSR access code, I'm confused by some
SVM MSR access code, since quite a few MSRs have corresponding fields in
vmcb, why still the access will cause VMExits?
-Xin
>-----Original Message-----
>From: xen-devel-bounces@lists.xensource.com
>[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Keir Fraser
>Sent: Thursday, November 30, 2006 1:22 AM
>To: Nakajima, Jun; Jan Beulich; xen-devel@lists.xensource.com;
>Keir Fraser
>Subject: Re: [Xen-devel] EFER in HVM guests
>
>
>*Please* can you make the handling of generic CPUID leaves and
>architectural
>MSRs common HVM code? There is lots of needless code
>duplication right now
>with niggling differences that shouldn't be necessary.
>
> -- Keir
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-12-19 9:12 Li, Xin B
@ 2006-12-19 9:46 ` Petersson, Mats
0 siblings, 0 replies; 26+ messages in thread
From: Petersson, Mats @ 2006-12-19 9:46 UTC (permalink / raw)
To: Li, Xin B, Keir Fraser, Nakajima, Jun, Woller, Thomas, xen-devel
> -----Original Message-----
> From: Li, Xin B [mailto:xin.b.li@intel.com]
> Sent: 19 December 2006 09:13
> To: Keir Fraser; Nakajima, Jun; Woller, Thomas;
> xen-devel@lists.xensource.com; Petersson, Mats
> Subject: RE: [Xen-devel] EFER in HVM guests
>
> When I'm trying to merge VMX/SVM MSR access code, I'm confused by some
> SVM MSR access code, since quite a few MSRs have
> corresponding fields in
> vmcb, why still the access will cause VMExits?
All MSR read/write operations are set to exit according to the line
which sets "msrpm" to all ones around line 142 [I'm on an older
changeset, so that may be quite a long while ago] in .../hvm/svm/vmcb.c.
Certain MSR writes need to be tracked in the hypervisor to understand
what's going on, and most other ones should be "ignored".
--
Mats
> -Xin
>
> >-----Original Message-----
> >From: xen-devel-bounces@lists.xensource.com
> >[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> Keir Fraser
> >Sent: Thursday, November 30, 2006 1:22 AM
> >To: Nakajima, Jun; Jan Beulich; xen-devel@lists.xensource.com;
> >Keir Fraser
> >Subject: Re: [Xen-devel] EFER in HVM guests
> >
> >
> >*Please* can you make the handling of generic CPUID leaves and
> >architectural
> >MSRs common HVM code? There is lots of needless code
> >duplication right now
> >with niggling differences that shouldn't be necessary.
> >
> > -- Keir
> >
>
>
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-19 14:25 Li, Xin B
2006-12-19 20:38 ` Woller, Thomas
0 siblings, 1 reply; 26+ messages in thread
From: Li, Xin B @ 2006-12-19 14:25 UTC (permalink / raw)
To: Li, Xin B, Petersson, Mats, Woller, Thomas, Keir Fraser,
Nakajima, Jun, Jan Beulich, xen-devel
Mats, Did you find any issue on your side?
-Xin
>-----Original Message-----
>From: xen-devel-bounces@lists.xensource.com
>[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Li, Xin B
>Sent: Tuesday, December 19, 2006 2:45 PM
>To: Petersson, Mats; Woller, Thomas; Keir Fraser; Nakajima,
>Jun; Jan Beulich; xen-devel@lists.xensource.com
>Subject: RE: [Xen-devel] EFER in HVM guests
>
>>
>>The 0x80000001 leaf was originally an "AMD only" leaf for
>>adding new "non-Intel compatible" features, such as 3DNow! and
>>long-mode, but since x86_64 was adopted by Intel, it's
>>available on Intel processors too. It should be done the same
>>on both AMD and Intel, and since 0x80000001 contains another
>>copy of the APIC and PAE bits, they should be masked for both
>>processors on both 1 and 0x80000001. [Of course, I doubt that
>>anyone would "prefer" to use 0x80000001 from using 1 as the
>>index for the leaf unless the coder is already reading
>>0x800000001 for some other reason - to detect LM for example].
>>
>>I would like to see the handling of 0x80000001 in the common
>>case cover PAE/PSE36/APIC features, as that's nor
>>arch-specific. The fact that no-one actually uses it currently
>>isn't a good argument for not getting it right at this time
>>rather than fixing hard-to-find bugs later on... ;-)
>>
>
>Mats,
>Leaf 0x80000001 on Intel processors only uses 4 bits in ECX and EDX,
>they are:
>LAHF/SAHF: bit 0 of ECX
>SYSCALL/SYSRET: bit 11 of EDX
>Execution Disable bit: bit 20 of EDX
>LM bit: bit 29 of EDX
>All other bits are reserved to 0.
>
>
>>Clearing MWAIT bit should also be made common - I doubt anyone
>>will notice the single instruction saved by combining it with
>>a bunch of other bits, compared to the overall benefit of
>>trivially seeing that it's dealt with the same way on both
>>architectures.
>
>I did have this in mind when creating this patch, but I'm not sure if
>MWAIT virtualization is common on both sides, so just keep it
>there, and
>The patch attached has this fixed.
>
>>
>>Just out of curiosity, why did you change the parameters
>>passed to svm_do_cpuid - I can see why you wouldn't need to
>>pass regs->eax when it's available in regs, but digging out
>>the vmcb again can't be better than passing the already
>>existing one? [Don't worry about it, I'm just curious about
>>why the change was made].
>
>In my mind, just pass parameters you don't have in hand. And yes,
>actually vmcb should be a parameter here :-)
>
>-Xin
>
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-12-19 14:25 Li, Xin B
@ 2006-12-19 20:38 ` Woller, Thomas
2006-12-20 11:29 ` Petersson, Mats
0 siblings, 1 reply; 26+ messages in thread
From: Woller, Thomas @ 2006-12-19 20:38 UTC (permalink / raw)
To: Li, Xin B, Petersson, Mats, Keir Fraser, Nakajima, Jun,
Jan Beulich, xen-devel
[-- Attachment #1: Type: text/plain, Size: 3686 bytes --]
Mats, which configurations did you test? Can you post those results?
Xin,
Attached is overnight testing on patch you sent for 32bit hv (xls
spreadsheet) on top of 13078, so far looks ok, we have planned to do
some 32bit PAE and 64bit hv with HVM AMD-V guest testing. With the
interaction/comments from Mats, do you anticipate another patch with
more consolidation of SVM/VMX code?
Thanks
Tom
> -----Original Message-----
> From: xen-devel-bounces@lists.xensource.com
> [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Li, Xin B
> Sent: Tuesday, December 19, 2006 8:25 AM
> To: Li, Xin B; Petersson, Mats; Woller, Thomas; Keir Fraser;
> Nakajima, Jun; Jan Beulich; xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] EFER in HVM guests
>
> Mats, Did you find any issue on your side?
>
> -Xin
>
> >-----Original Message-----
> >From: xen-devel-bounces@lists.xensource.com
> >[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Li, Xin B
> >Sent: Tuesday, December 19, 2006 2:45 PM
> >To: Petersson, Mats; Woller, Thomas; Keir Fraser; Nakajima, Jun; Jan
> >Beulich; xen-devel@lists.xensource.com
> >Subject: RE: [Xen-devel] EFER in HVM guests
> >
> >>
> >>The 0x80000001 leaf was originally an "AMD only" leaf for
> adding new
> >>"non-Intel compatible" features, such as 3DNow! and long-mode, but
> >>since x86_64 was adopted by Intel, it's available on Intel
> processors
> >>too. It should be done the same on both AMD and Intel, and since
> >>0x80000001 contains another copy of the APIC and PAE bits,
> they should
> >>be masked for both processors on both 1 and 0x80000001. [Of
> course, I
> >>doubt that anyone would "prefer" to use 0x80000001 from
> using 1 as the
> >>index for the leaf unless the coder is already reading
> >>0x800000001 for some other reason - to detect LM for example].
> >>
> >>I would like to see the handling of 0x80000001 in the common case
> >>cover PAE/PSE36/APIC features, as that's nor arch-specific.
> The fact
> >>that no-one actually uses it currently isn't a good
> argument for not
> >>getting it right at this time rather than fixing hard-to-find bugs
> >>later on... ;-)
> >>
> >
> >Mats,
> >Leaf 0x80000001 on Intel processors only uses 4 bits in ECX and EDX,
> >they are:
> >LAHF/SAHF: bit 0 of ECX
> >SYSCALL/SYSRET: bit 11 of EDX
> >Execution Disable bit: bit 20 of EDX
> >LM bit: bit 29 of EDX
> >All other bits are reserved to 0.
> >
> >
> >>Clearing MWAIT bit should also be made common - I doubt anyone will
> >>notice the single instruction saved by combining it with a bunch of
> >>other bits, compared to the overall benefit of trivially
> seeing that
> >>it's dealt with the same way on both architectures.
> >
> >I did have this in mind when creating this patch, but I'm
> not sure if
> >MWAIT virtualization is common on both sides, so just keep it there,
> >and The patch attached has this fixed.
> >
> >>
> >>Just out of curiosity, why did you change the parameters passed to
> >>svm_do_cpuid - I can see why you wouldn't need to pass
> regs->eax when
> >>it's available in regs, but digging out the vmcb again
> can't be better
> >>than passing the already existing one? [Don't worry about
> it, I'm just
> >>curious about why the change was made].
> >
> >In my mind, just pass parameters you don't have in hand. And yes,
> >actually vmcb should be a parameter here :-)
> >
> >-Xin
> >
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
>
>
>
[-- Attachment #2: cpuid_boot_test_12_19_2006.xls --]
[-- Type: application/vnd.ms-excel, Size: 21504 bytes --]
[-- Attachment #3: Type: text/plain, Size: 138 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
@ 2006-12-20 1:27 Li, Xin B
0 siblings, 0 replies; 26+ messages in thread
From: Li, Xin B @ 2006-12-20 1:27 UTC (permalink / raw)
To: Woller, Thomas, Petersson, Mats, Keir Fraser, Nakajima, Jun,
Jan Beulich, xen-devel
>Xin,
>Attached is overnight testing on patch you sent for 32bit hv (xls
>spreadsheet) on top of 13078, so far looks ok, we have planned to do
>some 32bit PAE and 64bit hv with HVM AMD-V guest testing. With the
>interaction/comments from Mats, do you anticipate another patch with
>more consolidation of SVM/VMX code?
I think it's enough for now.
-Xin
^ permalink raw reply [flat|nested] 26+ messages in thread
* RE: EFER in HVM guests
2006-12-19 20:38 ` Woller, Thomas
@ 2006-12-20 11:29 ` Petersson, Mats
0 siblings, 0 replies; 26+ messages in thread
From: Petersson, Mats @ 2006-12-20 11:29 UTC (permalink / raw)
To: Woller, Thomas, Li, Xin B, Keir Fraser, Nakajima, Jun,
Jan Beulich, xen-devel
I haven'd done any testing - I had a quick look at the patch in source
form, and wrote my comments based on that.
If Tom's tests don't show up any problems, I'm happy with the latests
patch-set.
--
Mats
> -----Original Message-----
> From: Woller, Thomas
> Sent: 19 December 2006 20:38
> To: Li, Xin B; Petersson, Mats; Keir Fraser; Nakajima, Jun;
> Jan Beulich; xen-devel@lists.xensource.com
> Subject: RE: [Xen-devel] EFER in HVM guests
>
> Mats, which configurations did you test? Can you post those results?
>
> Xin,
> Attached is overnight testing on patch you sent for 32bit hv
> (xls spreadsheet) on top of 13078, so far looks ok, we have
> planned to do some 32bit PAE and 64bit hv with HVM AMD-V
> guest testing. With the interaction/comments from Mats, do
> you anticipate another patch with more consolidation of SVM/VMX code?
>
> Thanks
> Tom
>
>
>
> > -----Original Message-----
> > From: xen-devel-bounces@lists.xensource.com
> > [mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of
> Li, Xin B
> > Sent: Tuesday, December 19, 2006 8:25 AM
> > To: Li, Xin B; Petersson, Mats; Woller, Thomas; Keir Fraser;
> > Nakajima, Jun; Jan Beulich; xen-devel@lists.xensource.com
> > Subject: RE: [Xen-devel] EFER in HVM guests
> >
> > Mats, Did you find any issue on your side?
> >
> > -Xin
> >
> > >-----Original Message-----
> > >From: xen-devel-bounces@lists.xensource.com
> > >[mailto:xen-devel-bounces@lists.xensource.com] On Behalf
> Of Li, Xin B
> > >Sent: Tuesday, December 19, 2006 2:45 PM
> > >To: Petersson, Mats; Woller, Thomas; Keir Fraser;
> Nakajima, Jun; Jan
> > >Beulich; xen-devel@lists.xensource.com
> > >Subject: RE: [Xen-devel] EFER in HVM guests
> > >
> > >>
> > >>The 0x80000001 leaf was originally an "AMD only" leaf for
> > adding new
> > >>"non-Intel compatible" features, such as 3DNow! and
> long-mode, but
> > >>since x86_64 was adopted by Intel, it's available on Intel
> > processors
> > >>too. It should be done the same on both AMD and Intel, and since
> > >>0x80000001 contains another copy of the APIC and PAE bits,
> > they should
> > >>be masked for both processors on both 1 and 0x80000001. [Of
> > course, I
> > >>doubt that anyone would "prefer" to use 0x80000001 from
> > using 1 as the
> > >>index for the leaf unless the coder is already reading
> > >>0x800000001 for some other reason - to detect LM for example].
> > >>
> > >>I would like to see the handling of 0x80000001 in the common case
> > >>cover PAE/PSE36/APIC features, as that's nor arch-specific.
> > The fact
> > >>that no-one actually uses it currently isn't a good
> > argument for not
> > >>getting it right at this time rather than fixing
> hard-to-find bugs
> > >>later on... ;-)
> > >>
> > >
> > >Mats,
> > >Leaf 0x80000001 on Intel processors only uses 4 bits in
> ECX and EDX,
> > >they are:
> > >LAHF/SAHF: bit 0 of ECX
> > >SYSCALL/SYSRET: bit 11 of EDX
> > >Execution Disable bit: bit 20 of EDX
> > >LM bit: bit 29 of EDX
> > >All other bits are reserved to 0.
> > >
> > >
> > >>Clearing MWAIT bit should also be made common - I doubt
> anyone will
> > >>notice the single instruction saved by combining it with
> a bunch of
> > >>other bits, compared to the overall benefit of trivially
> > seeing that
> > >>it's dealt with the same way on both architectures.
> > >
> > >I did have this in mind when creating this patch, but I'm
> > not sure if
> > >MWAIT virtualization is common on both sides, so just keep
> it there,
> > >and The patch attached has this fixed.
> > >
> > >>
> > >>Just out of curiosity, why did you change the parameters
> passed to
> > >>svm_do_cpuid - I can see why you wouldn't need to pass
> > regs->eax when
> > >>it's available in regs, but digging out the vmcb again
> > can't be better
> > >>than passing the already existing one? [Don't worry about
> > it, I'm just
> > >>curious about why the change was made].
> > >
> > >In my mind, just pass parameters you don't have in hand. And yes,
> > >actually vmcb should be a parameter here :-)
> > >
> > >-Xin
> > >
> >
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xensource.com
> > http://lists.xensource.com/xen-devel
> >
> >
> >
>
^ permalink raw reply [flat|nested] 26+ messages in thread
end of thread, other threads:[~2006-12-20 11:29 UTC | newest]
Thread overview: 26+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-29 16:34 EFER in HVM guests Nakajima, Jun
2006-11-29 17:21 ` Keir Fraser
2006-11-29 17:37 ` Petersson, Mats
-- strict thread matches above, loose matches on Subject: below --
2006-12-20 1:27 Li, Xin B
2006-12-19 14:25 Li, Xin B
2006-12-19 20:38 ` Woller, Thomas
2006-12-20 11:29 ` Petersson, Mats
2006-12-19 9:12 Li, Xin B
2006-12-19 9:46 ` Petersson, Mats
2006-12-19 7:04 Li, Xin B
2006-12-19 6:44 Li, Xin B
2006-12-18 15:10 Li, Xin B
2006-12-18 15:47 ` Petersson, Mats
2006-12-13 14:13 Li, Xin B
2006-12-13 14:09 Li, Xin B
2006-12-13 12:48 Li, Xin B
2006-12-13 13:08 ` Jan Beulich
2006-12-13 14:37 ` Keir Fraser
2006-12-15 16:04 ` Woller, Thomas
2006-11-29 18:24 Nakajima, Jun
2006-11-29 13:07 Jan Beulich
2006-11-29 13:09 ` Keir Fraser
2006-11-29 14:08 ` Jan Beulich
2006-11-29 14:22 ` Petersson, Mats
2006-11-29 13:11 ` Petersson, Mats
2006-11-29 14:09 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.