[PATCH] xen: provide pse36 cpuid bit

[PATCH] xen: provide pse36 cpuid bit

[Christoph Egger]

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]


Provide pse36 cpuid bit if guest runs in 32bit PAE
or in long mode. Hyper-V refuses to start as
the "cpu does not provide required hw features"
if it does not find the pse36 cpuid bits.

Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

[-- Attachment #2: xen_pse36.diff --]
[-- Type: text/plain, Size: 2706 bytes --]

diff -r 0d092359d86f tools/libxc/xc_cpuid_x86.c
--- a/tools/libxc/xc_cpuid_x86.c	Wed Oct 26 10:32:36 2011 +0200
+++ b/tools/libxc/xc_cpuid_x86.c	Thu Oct 27 14:02:29 2011 +0200
@@ -98,7 +98,6 @@ static void amd_xc_cpuid_policy(
 
         if ( !is_pae )
             clear_bit(X86_FEATURE_PAE, regs[3]);
-        clear_bit(X86_FEATURE_PSE36, regs[3]);
 
         /* Filter all other features according to a whitelist. */
         regs[2] &= ((is_64bit ? bitmaskof(X86_FEATURE_LAHF_LM) : 0) |
@@ -339,6 +338,7 @@ static void xc_cpuid_hvm_policy(
                     bitmaskof(X86_FEATURE_CMOV) |
                     bitmaskof(X86_FEATURE_PAT) |
                     bitmaskof(X86_FEATURE_CLFLSH) |
+                    bitmaskof(X86_FEATURE_PSE36) |
                     bitmaskof(X86_FEATURE_MMX) |
                     bitmaskof(X86_FEATURE_FXSR) |
                     bitmaskof(X86_FEATURE_XMM) |
@@ -348,8 +348,10 @@ static void xc_cpuid_hvm_policy(
         /* We always support MTRR MSRs. */
         regs[3] |= bitmaskof(X86_FEATURE_MTRR);
 
-        if ( !is_pae )
+        if ( !is_pae ) {
             clear_bit(X86_FEATURE_PAE, regs[3]);
+            clear_bit(X86_FEATURE_PSE36, regs[3]);
+        }
         break;
 
     case 0x00000007: /* Intel-defined CPU features */
@@ -371,8 +373,10 @@ static void xc_cpuid_hvm_policy(
         break;
 
     case 0x80000001:
-        if ( !is_pae )
+        if ( !is_pae ) {
             clear_bit(X86_FEATURE_NX, regs[3]);
+            clear_bit(X86_FEATURE_PSE36, regs[3]);
+        }
         break;
 
     case 0x80000007:
diff -r 0d092359d86f xen/arch/x86/hvm/hvm.c
--- a/xen/arch/x86/hvm/hvm.c	Wed Oct 26 10:32:36 2011 +0200
+++ b/xen/arch/x86/hvm/hvm.c	Thu Oct 27 14:02:29 2011 +0200
@@ -2413,6 +2414,10 @@ void hvm_cpuid(unsigned int input, unsig
         if ( xsave_enabled(v) )
             *ecx |= (v->arch.hvm_vcpu.guest_cr[4] & X86_CR4_OSXSAVE) ?
                      cpufeat_mask(X86_FEATURE_OSXSAVE) : 0;
+
+        /* Only provide PSE36 when guest runs in 32bit PAE or in long mode */
+        if ( !(hvm_pae_enabled(v) || hvm_long_mode_enabled(v)) )
+            *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
         break;
     case 0x7:
         if ( (count == 0) && !cpu_has_smep )
@@ -2451,6 +2456,9 @@ void hvm_cpuid(unsigned int input, unsig
         /* Hide 1GB-superpage feature if we can't emulate it. */
         if (!hvm_pse1gb_supported(d))
             *edx &= ~cpufeat_mask(X86_FEATURE_PAGE1GB);
+        /* Only provide PSE36 when guest runs in 32bit PAE or in long mode */
+        if ( !(hvm_pae_enabled(v) || hvm_long_mode_enabled(v)) )
+            *edx &= ~cpufeat_mask(X86_FEATURE_PSE36);
         break;
     }
 }

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

Re: [PATCH] xen: provide pse36 cpuid bit

[Tim Deegan]

At 14:10 +0200 on 27 Oct (1319724631), Christoph Egger wrote:
> 
> Provide pse36 cpuid bit if guest runs in 32bit PAE
> or in long mode. Hyper-V refuses to start as
> the "cpu does not provide required hw features"
> if it does not find the pse36 cpuid bits.
> 
> Signed-off-by: Christoph Egger <Christoph.Egger@amd.com>

This patch appears to advertise PSE36 support to guests without actually
supporting PSE36.  Or am I missing something?

Cheers,

Tim.

Re: [PATCH] xen: provide pse36 cpuid bit

[Christoph Egger]

On 10/27/11 16:06, Tim Deegan wrote:
> At 14:10 +0200 on 27 Oct (1319724631), Christoph Egger wrote:
>>
>> Provide pse36 cpuid bit if guest runs in 32bit PAE
>> or in long mode. Hyper-V refuses to start as
>> the "cpu does not provide required hw features"
>> if it does not find the pse36 cpuid bits.
>>
>> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>
>
> This patch appears to advertise PSE36 support to guests without actually
> supporting PSE36.  Or am I missing something?

That's right. The paging format differs only in 32bit legacy mode.
Since Hyper-V is not running in 32bit legacy mode but insists on having
these cpuid bits present it is sufficient to just populate them to the
guest when guest paging mode != 32bit legacy mode.

Christoph


-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] xen: provide pse36 cpuid bit

[Keir Fraser]

On 27/10/2011 15:30, "Christoph Egger" <Christoph.Egger@amd.com> wrote:

> On 10/27/11 16:06, Tim Deegan wrote:
>> At 14:10 +0200 on 27 Oct (1319724631), Christoph Egger wrote:
>>> 
>>> Provide pse36 cpuid bit if guest runs in 32bit PAE
>>> or in long mode. Hyper-V refuses to start as
>>> the "cpu does not provide required hw features"
>>> if it does not find the pse36 cpuid bits.
>>> 
>>> Signed-off-by: Christoph Egger<Christoph.Egger@amd.com>
>> 
>> This patch appears to advertise PSE36 support to guests without actually
>> supporting PSE36.  Or am I missing something?
> 
> That's right. The paging format differs only in 32bit legacy mode.
> Since Hyper-V is not running in 32bit legacy mode but insists on having
> these cpuid bits present it is sufficient to just populate them to the
> guest when guest paging mode != 32bit legacy mode.

It would be nice if we didn't have to toggle CPUID.PSE36 based on current
guest mode. How hard would it be to pull out bits 35..32 of a physical
address from bits 16..13 of a legacy 32-bit PDE whose PS flag = 1?

I'm actually surprised we don't do it already, it's so trivial! The code
explicitly says we don't though, and for a reason that makes no sense to
me...

 -- Keir

> Christoph
> 

Re: [PATCH] xen: provide pse36 cpuid bit

[Tim Deegan]

At 15:59 +0100 on 27 Oct (1319731185), Keir Fraser wrote:
> >> This patch appears to advertise PSE36 support to guests without actually
> >> supporting PSE36.  Or am I missing something?
> > 
> > That's right. The paging format differs only in 32bit legacy mode.
> > Since Hyper-V is not running in 32bit legacy mode but insists on having
> > these cpuid bits present it is sufficient to just populate them to the
> > guest when guest paging mode != 32bit legacy mode.
> 
> It would be nice if we didn't have to toggle CPUID.PSE36 based on current
> guest mode. How hard would it be to pull out bits 35..32 of a physical
> address from bits 16..13 of a legacy 32-bit PDE whose PS flag = 1?

Should be easy.  It's another bit of logic on the pagetable walker, but
nothing too expensive, and we already handle other simlar special cases.

> I'm actually surprised we don't do it already, it's so trivial! The code
> explicitly says we don't though, and for a reason that makes no sense to
> me...

If you mean this:

 * PSE disabled / PSE36
 * We don't support any modes other than PSE enabled, PSE36 disabled.
 * Neither of those would be hard to change, but we'd need to be able to
 * deal with shadows made in one mode and used in another.

the worry was that we'd need a whole nother shadow mode to handle the
case where one VCPU was in normal 32-bit and another was in PSE36 (since
they can't share shadows).

As it happens the current code does detect PSE-disabled in shadow mode
but just DTRT for the current VCPU, so a mix of PSE-enabled and
PSE-disabled VCPUs will get unpredicatble results from shadow
pagetables. :(

Which means that supporting PSE36 to the same degree (i.e. assuming all
VCPUs behave the same, or if they don't they don't share pagetables)
would be OK too. :)

Cheers,

Tim.

Re: [PATCH] xen: provide pse36 cpuid bit

[Keir Fraser]

On 27/10/2011 16:15, "Tim Deegan" <tim@xen.org> wrote:

> If you mean this:
> 
>  * PSE disabled / PSE36
>  * We don't support any modes other than PSE enabled, PSE36 disabled.
>  * Neither of those would be hard to change, but we'd need to be able to
>  * deal with shadows made in one mode and used in another.
>
> the worry was that we'd need a whole nother shadow mode to handle the
> case where one VCPU was in normal 32-bit and another was in PSE36 (since
> they can't share shadows).
> 
> As it happens the current code does detect PSE-disabled in shadow mode
> but just DTRT for the current VCPU, so a mix of PSE-enabled and
> PSE-disabled VCPUs will get unpredicatble results from shadow
> pagetables. :(
> 
> Which means that supporting PSE36 to the same degree (i.e. assuming all
> VCPUs behave the same, or if they don't they don't share pagetables)
> would be OK too. :)

Ah, I see. Yes, I guessed it would be supported to just the same degree as
'basic' PSE. The likelihood of pagetables being shared across different
pagetable-related CR4 settings? Not great, we hope. :-)

 -- Keir

Re: [PATCH] xen: provide pse36 cpuid bit

[Christoph Egger]

On 10/27/11 17:30, Keir Fraser wrote:
> On 27/10/2011 16:15, "Tim Deegan"<tim@xen.org>  wrote:
>
>> If you mean this:
>>
>>   * PSE disabled / PSE36
>>   * We don't support any modes other than PSE enabled, PSE36 disabled.
>>   * Neither of those would be hard to change, but we'd need to be able to
>>   * deal with shadows made in one mode and used in another.
>>
>> the worry was that we'd need a whole nother shadow mode to handle the
>> case where one VCPU was in normal 32-bit and another was in PSE36 (since
>> they can't share shadows).
>>
>> As it happens the current code does detect PSE-disabled in shadow mode
>> but just DTRT for the current VCPU, so a mix of PSE-enabled and
>> PSE-disabled VCPUs will get unpredicatble results from shadow
>> pagetables. :(
>>
>> Which means that supporting PSE36 to the same degree (i.e. assuming all
>> VCPUs behave the same, or if they don't they don't share pagetables)
>> would be OK too. :)
>
> Ah, I see. Yes, I guessed it would be supported to just the same degree as
> 'basic' PSE. The likelihood of pagetables being shared across different
> pagetable-related CR4 settings? Not great, we hope. :-)

Is the patch acceptable as it is ?  PSE36 support for 32bit legacy mode
can be done in a seperate patch.

Christoph



-- 
---to satisfy European Law for business letters:
Advanced Micro Devices GmbH
Einsteinring 24, 85689 Dornach b. Muenchen
Geschaeftsfuehrer: Alberto Bozzo, Andrew Bowd
Sitz: Dornach, Gemeinde Aschheim, Landkreis Muenchen
Registergericht Muenchen, HRB Nr. 43632

Re: [PATCH] xen: provide pse36 cpuid bit

[Tim Deegan]

At 16:20 +0200 on 28 Oct (1319818819), Christoph Egger wrote:
> On 10/27/11 17:30, Keir Fraser wrote:
> >On 27/10/2011 16:15, "Tim Deegan"<tim@xen.org>  wrote:
> >
> >>If you mean this:
> >>
> >>  * PSE disabled / PSE36
> >>  * We don't support any modes other than PSE enabled, PSE36 disabled.
> >>  * Neither of those would be hard to change, but we'd need to be able to
> >>  * deal with shadows made in one mode and used in another.
> >>
> >>the worry was that we'd need a whole nother shadow mode to handle the
> >>case where one VCPU was in normal 32-bit and another was in PSE36 (since
> >>they can't share shadows).
> >>
> >>As it happens the current code does detect PSE-disabled in shadow mode
> >>but just DTRT for the current VCPU, so a mix of PSE-enabled and
> >>PSE-disabled VCPUs will get unpredicatble results from shadow
> >>pagetables. :(
> >>
> >>Which means that supporting PSE36 to the same degree (i.e. assuming all
> >>VCPUs behave the same, or if they don't they don't share pagetables)
> >>would be OK too. :)
> >
> >Ah, I see. Yes, I guessed it would be supported to just the same degree as
> >'basic' PSE. The likelihood of pagetables being shared across different
> >pagetable-related CR4 settings? Not great, we hope. :-)
> 
> Is the patch acceptable as it is ? 

I'll put together a patch with basic PSE36 support next week.  I don't
think we're in a terrible hurry to check in the CPUID change before then.

Tim.

Re: [PATCH] xen: provide pse36 cpuid bit

[Tim Deegan]

At 15:53 +0100 on 28 Oct (1319817196), Tim Deegan wrote:
> At 16:20 +0200 on 28 Oct (1319818819), Christoph Egger wrote:
> > On 10/27/11 17:30, Keir Fraser wrote:
> > >On 27/10/2011 16:15, "Tim Deegan"<tim@xen.org>  wrote:
> > >
> > >>If you mean this:
> > >>
> > >>  * PSE disabled / PSE36
> > >>  * We don't support any modes other than PSE enabled, PSE36 disabled.
> > >>  * Neither of those would be hard to change, but we'd need to be able to
> > >>  * deal with shadows made in one mode and used in another.
> > >>
> > >>the worry was that we'd need a whole nother shadow mode to handle the
> > >>case where one VCPU was in normal 32-bit and another was in PSE36 (since
> > >>they can't share shadows).
> > >>
> > >>As it happens the current code does detect PSE-disabled in shadow mode
> > >>but just DTRT for the current VCPU, so a mix of PSE-enabled and
> > >>PSE-disabled VCPUs will get unpredicatble results from shadow
> > >>pagetables. :(
> > >>
> > >>Which means that supporting PSE36 to the same degree (i.e. assuming all
> > >>VCPUs behave the same, or if they don't they don't share pagetables)
> > >>would be OK too. :)
> > >
> > >Ah, I see. Yes, I guessed it would be supported to just the same degree as
> > >'basic' PSE. The likelihood of pagetables being shared across different
> > >pagetable-related CR4 settings? Not great, we hope. :-)
> > 
> > Is the patch acceptable as it is ? 
> 
> I'll put together a patch with basic PSE36 support next week.  I don't
> think we're in a terrible hurry to check in the CPUID change before then.

Supporting PSE36 turned out to be trickier than I had thought - the
trick in the guest-pagetable walker of making up an l1e with the
equivalent semantics to the PSE l2e doesn't work for PSE36 because there
isn't space for the extra address bits in a 32bit l1e. :(  I didn't fancy
changing that interface right now because it would need a fair bit of
fixing up in the shadow code, but I might come back to it when I have
more time. 

In the meantime I've applied Christoph's patch.

Cheers,

Tim.