[v3] libxc: Expose the 1GB pages cpuid flag to guest

[v3] libxc: Expose the 1GB pages cpuid flag to guest

[Liang Li]

If hardware support the 1GB pages, expose the feature to guest by
default. Users don't have to use a 'cpuid= ' option in config fil
e to turn it on.

Signed-off-by: Liang Li <liang.z.li@intel.com>
Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
---
 tools/libxc/xc_cpuid_x86.c | 3 +++
 xen/arch/x86/hvm/hvm.c     | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index a18b1ff..c97f91a 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -109,6 +109,7 @@ static void amd_xc_cpuid_policy(
         regs[3] &= (0x0183f3ff | /* features shared with 0x00000001:EDX */
                     bitmaskof(X86_FEATURE_NX) |
                     bitmaskof(X86_FEATURE_LM) |
+                    bitmaskof(X86_FEATURE_PAGE1GB) |
                     bitmaskof(X86_FEATURE_SYSCALL) |
                     bitmaskof(X86_FEATURE_MP) |
                     bitmaskof(X86_FEATURE_MMXEXT) |
@@ -192,6 +193,7 @@ static void intel_xc_cpuid_policy(
                     bitmaskof(X86_FEATURE_ABM));
         regs[3] &= (bitmaskof(X86_FEATURE_NX) |
                     bitmaskof(X86_FEATURE_LM) |
+                    bitmaskof(X86_FEATURE_PAGE1GB) |
                     bitmaskof(X86_FEATURE_SYSCALL) |
                     bitmaskof(X86_FEATURE_RDTSCP));
         break;
@@ -386,6 +388,7 @@ static void xc_cpuid_hvm_policy(
             clear_bit(X86_FEATURE_LM, regs[3]);
             clear_bit(X86_FEATURE_NX, regs[3]);
             clear_bit(X86_FEATURE_PSE36, regs[3]);
+            clear_bit(X86_FEATURE_PAGE1GB, regs[3]);
         }
         break;
 
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 8f49b44..c825618 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4287,7 +4287,7 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
              !host_tsc_is_safe() )
             *edx &= ~cpufeat_mask(X86_FEATURE_RDTSCP);
         /* Hide 1GB-superpage feature if we can't emulate it. */
-        if (!hvm_pse1gb_supported(d))
+        if (!hvm_pse1gb_supported(d) || paging_mode_shadow(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)) )
-- 
1.9.1

Re: [v3] libxc: Expose the 1GB pages cpuid flag to guest

[Jan Beulich]

>>> On 28.11.14 at 04:28, <liang.z.li@intel.com> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4287,7 +4287,7 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx,
>               !host_tsc_is_safe() )
>              *edx &= ~cpufeat_mask(X86_FEATURE_RDTSCP);
>          /* Hide 1GB-superpage feature if we can't emulate it. */
> -        if (!hvm_pse1gb_supported(d))
> +        if (!hvm_pse1gb_supported(d) || paging_mode_shadow(d))
>              *edx &= ~cpufeat_mask(X86_FEATURE_PAGE1GB);

With

#define hvm_pse1gb_supported(d) \
    (cpu_has_page1gb && paging_mode_hap(d))

the change above is pointless. While, considering this, comments on
v2 may have been misleading, you should have simply updated the
patch description instead to clarify why the v2 change was okay
even for the shadow mode case.

Jan

Re: [v3] libxc: Expose the 1GB pages cpuid flag to guest

[Li, Liang Z]

>> -        if (!hvm_pse1gb_supported(d))
>> +        if (!hvm_pse1gb_supported(d) || paging_mode_shadow(d))
>>              *edx &= ~cpufeat_mask(X86_FEATURE_PAGE1GB);
>
>With
>
>#define hvm_pse1gb_supported(d) \
>    (cpu_has_page1gb && paging_mode_hap(d))

>the change above is pointless. While, considering this, comments on
>v2 may have been misleading, you should have simply updated the patch description instead to clarify why the v2 change was okay even for the shadow mode case.

I checked the code and found that for a normal guest can only be in hap mode or shadow mode before I sending the patch, but I am not share if it's true for dom0. 

Liang

Re: [v3] libxc: Expose the 1GB pages cpuid flag to guest

[Andrew Cooper]

On 28/11/14 10:29, Li, Liang Z wrote:
>>> -        if (!hvm_pse1gb_supported(d))
>>> +        if (!hvm_pse1gb_supported(d) || paging_mode_shadow(d))
>>>              *edx &= ~cpufeat_mask(X86_FEATURE_PAGE1GB);
>> With
>>
>> #define hvm_pse1gb_supported(d) \
>>    (cpu_has_page1gb && paging_mode_hap(d))
>> the change above is pointless. While, considering this, comments on
>> v2 may have been misleading, you should have simply updated the patch description instead to clarify why the v2 change was okay even for the shadow mode case.
> I checked the code and found that for a normal guest can only be in hap mode or shadow mode before I sending the patch, but I am not share if it's true for dom0. 
>
> Liang

Dom0 may either be PV (in which case neither hap nor shadow, and cant
use 1GB pages anyway), or experimentally PVH which is currently
restricted to hap.

~Andrew

Re: [v3] libxc: Expose the 1GB pages cpuid flag to guest

[Li, Liang Z]

>>>    (cpu_has_page1gb && paging_mode_hap(d)) the change above is 
>>> pointless. While, considering this, comments on
>>> v2 may have been misleading, you should have simply updated the patch description instead to clarify why the v2 change was okay even for the shadow mode case.
>> I checked the code and found that for a normal guest can only be in hap mode or shadow mode before I sending the patch, but I am not share if it's true for dom0. 
>>
>> Liang
>
> Dom0 may either be PV (in which case neither hap nor shadow, and cant use 1GB pages anyway), or experimentally PVH which is currently restricted to hap.
>
>~Andrew

Thanks for clarification.  I will resend the v2 patch.

Liang

Re: [v3] libxc: Expose the 1GB pages cpuid flag to guest

[Jan Beulich]

>>> On 28.11.14 at 11:29, <liang.z.li@intel.com> wrote:
>> > -        if (!hvm_pse1gb_supported(d))
>>> +        if (!hvm_pse1gb_supported(d) || paging_mode_shadow(d))
>>>              *edx &= ~cpufeat_mask(X86_FEATURE_PAGE1GB);
>>
>>With
>>
>>#define hvm_pse1gb_supported(d) \
>>    (cpu_has_page1gb && paging_mode_hap(d))
> 
>>the change above is pointless. While, considering this, comments on
>>v2 may have been misleading, you should have simply updated the patch 
> description instead to clarify why the v2 change was okay even for the shadow 
> mode case.
> 
> I checked the code and found that for a normal guest can only be in hap mode 
> or shadow mode before I sending the patch, but I am not share if it's true 
> for dom0. 

The CPUID code in libxc doesn't apply to Dom0 at all, and CPUID
handling is also special cased in the hypervisor for Dom0. Plus
finally Dom0 only possibly being PV or PVH, PVH requiring HAP
and PV generally not allowing large pages anyway, your concern
is unnecessary.

Jan