All of lore.kernel.org
 help / color / mirror / Atom feed
* [v4] libxc: Expose the 1GB pages cpuid flag to guest
@ 2014-11-28 10:52 Liang Li
  2014-11-28 11:31 ` Jan Beulich
  2014-11-28 11:50 ` Ian Campbell
  0 siblings, 2 replies; 10+ messages in thread
From: Liang Li @ 2014-11-28 10:52 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, ian.campbell, stefano.stabellini, tim, Liang Li,
	ian.jackson, JBeulich, andrew.cooper3, yang.z.zhang

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.

If guest use shadow mode, the 1GB pages feature will be hidden from
guest, this is done in the function hvm_cpuid(). So the change is
okay for shadow mode case.

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 +++
 1 file changed, 3 insertions(+)

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;
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-11-28 10:52 [v4] libxc: Expose the 1GB pages cpuid flag to guest Liang Li
@ 2014-11-28 11:31 ` Jan Beulich
  2015-01-08 14:58   ` Ian Campbell
  2014-11-28 11:50 ` Ian Campbell
  1 sibling, 1 reply; 10+ messages in thread
From: Jan Beulich @ 2014-11-28 11:31 UTC (permalink / raw)
  To: Liang Li
  Cc: tim, wei.liu2, ian.campbell, stefano.stabellini, andrew.cooper3,
	ian.jackson, xen-devel, yang.z.zhang

>>> On 28.11.14 at 11:52, <liang.z.li@intel.com> wrote:
> 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.
> 
> If guest use shadow mode, the 1GB pages feature will be hidden from
> guest, this is done in the function hvm_cpuid(). So the change is
> okay for shadow mode case.
> 
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-11-28 10:52 [v4] libxc: Expose the 1GB pages cpuid flag to guest Liang Li
  2014-11-28 11:31 ` Jan Beulich
@ 2014-11-28 11:50 ` Ian Campbell
  2014-12-02 21:09   ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2014-11-28 11:50 UTC (permalink / raw)
  To: Liang Li
  Cc: wei.liu2, stefano.stabellini, tim, ian.jackson, xen-devel,
	JBeulich, andrew.cooper3, yang.z.zhang

On Fri, 2014-11-28 at 18:52 +0800, Liang Li wrote:
> 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.
> 
> If guest use shadow mode, the 1GB pages feature will be hidden from
> guest, this is done in the function hvm_cpuid(). So the change is
> okay for shadow mode case.
> 
> Signed-off-by: Liang Li <liang.z.li@intel.com>
> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>

FTR although this is strictly speaking a toolstack patch I think the
main ack required should be from the x86 hypervisor guys...

> ---
>  tools/libxc/xc_cpuid_x86.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> 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;
>  

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-11-28 11:50 ` Ian Campbell
@ 2014-12-02 21:09   ` Konrad Rzeszutek Wilk
  2014-12-03  9:38     ` Ian Campbell
  0 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-12-02 21:09 UTC (permalink / raw)
  To: Ian Campbell
  Cc: wei.liu2, stefano.stabellini, ian.jackson, Liang Li, tim,
	xen-devel, JBeulich, andrew.cooper3, yang.z.zhang

On Fri, Nov 28, 2014 at 11:50:43AM +0000, Ian Campbell wrote:
> On Fri, 2014-11-28 at 18:52 +0800, Liang Li wrote:
> > 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.
> > 
> > If guest use shadow mode, the 1GB pages feature will be hidden from
> > guest, this is done in the function hvm_cpuid(). So the change is
> > okay for shadow mode case.
> > 
> > Signed-off-by: Liang Li <liang.z.li@intel.com>
> > Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> 
> FTR although this is strictly speaking a toolstack patch I think the
> main ack required should be from the x86 hypervisor guys...

Jan acked it.
> 
> > ---
> >  tools/libxc/xc_cpuid_x86.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > 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;
> >  
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-12-02 21:09   ` Konrad Rzeszutek Wilk
@ 2014-12-03  9:38     ` Ian Campbell
  2014-12-03 15:57       ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 10+ messages in thread
From: Ian Campbell @ 2014-12-03  9:38 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: wei.liu2, stefano.stabellini, ian.jackson, Liang Li, tim,
	xen-devel, JBeulich, andrew.cooper3, yang.z.zhang

On Tue, 2014-12-02 at 16:09 -0500, Konrad Rzeszutek Wilk wrote:
> On Fri, Nov 28, 2014 at 11:50:43AM +0000, Ian Campbell wrote:
> > On Fri, 2014-11-28 at 18:52 +0800, Liang Li wrote:
> > > 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.
> > > 
> > > If guest use shadow mode, the 1GB pages feature will be hidden from
> > > guest, this is done in the function hvm_cpuid(). So the change is
> > > okay for shadow mode case.
> > > 
> > > Signed-off-by: Liang Li <liang.z.li@intel.com>
> > > Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> > 
> > FTR although this is strictly speaking a toolstack patch I think the
> > main ack required should be from the x86 hypervisor guys...
> 
> Jan acked it.

For 4.5?

Have you release acked it?

This seemed like 4.6 material to me, or at least I've not seen any
mention/argument to the contrary.

Ian.

> > 
> > > ---
> > >  tools/libxc/xc_cpuid_x86.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > 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;
> > >  
> > 
> > 
> > 
> > _______________________________________________
> > Xen-devel mailing list
> > Xen-devel@lists.xen.org
> > http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-12-03  9:38     ` Ian Campbell
@ 2014-12-03 15:57       ` Konrad Rzeszutek Wilk
  2014-12-04  1:50         ` Zhang, Yang Z
  0 siblings, 1 reply; 10+ messages in thread
From: Konrad Rzeszutek Wilk @ 2014-12-03 15:57 UTC (permalink / raw)
  To: Ian Campbell
  Cc: wei.liu2, stefano.stabellini, ian.jackson, Liang Li, tim,
	xen-devel, JBeulich, andrew.cooper3, yang.z.zhang

On Wed, Dec 03, 2014 at 09:38:49AM +0000, Ian Campbell wrote:
> On Tue, 2014-12-02 at 16:09 -0500, Konrad Rzeszutek Wilk wrote:
> > On Fri, Nov 28, 2014 at 11:50:43AM +0000, Ian Campbell wrote:
> > > On Fri, 2014-11-28 at 18:52 +0800, Liang Li wrote:
> > > > 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.
> > > > 
> > > > If guest use shadow mode, the 1GB pages feature will be hidden from
> > > > guest, this is done in the function hvm_cpuid(). So the change is
> > > > okay for shadow mode case.
> > > > 
> > > > Signed-off-by: Liang Li <liang.z.li@intel.com>
> > > > Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> > > 
> > > FTR although this is strictly speaking a toolstack patch I think the
> > > main ack required should be from the x86 hypervisor guys...
> > 
> > Jan acked it.
> 
> For 4.5?

Probably not.
> 
> Have you release acked it?

No.
> 
> This seemed like 4.6 material to me, or at least I've not seen any
> mention/argument to the contrary.

Correct. 4.6 please.
> 
> Ian.
> 
> > > 
> > > > ---
> > > >  tools/libxc/xc_cpuid_x86.c | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > > 
> > > > 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;
> > > >  
> > > 
> > > 
> > > 
> > > _______________________________________________
> > > Xen-devel mailing list
> > > Xen-devel@lists.xen.org
> > > http://lists.xen.org/xen-devel
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-12-03 15:57       ` Konrad Rzeszutek Wilk
@ 2014-12-04  1:50         ` Zhang, Yang Z
  2014-12-04 10:21           ` Andrew Cooper
  0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Yang Z @ 2014-12-04  1:50 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, Ian Campbell
  Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com,
	tim@xen.org, Li, Liang Z, ian.jackson@eu.citrix.com,
	xen-devel@lists.xen.org, JBeulich@suse.com,
	andrew.cooper3@citrix.com

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

Konrad Rzeszutek Wilk wrote on 2014-12-03:
> On Wed, Dec 03, 2014 at 09:38:49AM +0000, Ian Campbell wrote:
>> On Tue, 2014-12-02 at 16:09 -0500, Konrad Rzeszutek Wilk wrote:
>>> On Fri, Nov 28, 2014 at 11:50:43AM +0000, Ian Campbell wrote:
>>>> On Fri, 2014-11-28 at 18:52 +0800, Liang Li wrote:
>>>>> 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.
>>>>> 
>>>>> If guest use shadow mode, the 1GB pages feature will be hidden from
>>>>> guest, this is done in the function hvm_cpuid(). So the change is
>>>>> okay for shadow mode case.
>>>>> 
>>>>> Signed-off-by: Liang Li <liang.z.li@intel.com>
>>>>> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
>>>> 
>>>> FTR although this is strictly speaking a toolstack patch I think the
>>>> main ack required should be from the x86 hypervisor guys...
>>> 
>>> Jan acked it.
>> 
>> For 4.5?
> 
> Probably not.
>> 
>> Have you release acked it?
> 
> No.
>> 
>> This seemed like 4.6 material to me, or at least I've not seen any
>> mention/argument to the contrary.
> 
> Correct. 4.6 please.

I think this more like a bug fixing than a feature. See our previous discussion.

>> 
>> Ian.
>> 
>>>> 
>>>>> ---
>>>>>  tools/libxc/xc_cpuid_x86.c | 3 +++
>>>>>  1 file changed, 3 insertions(+)
>>>>> 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;
>>>> 
>>>> 
>>>> 
>>>> _______________________________________________
>>>> Xen-devel mailing list
>>>> Xen-devel@lists.xen.org
>>>> http://lists.xen.org/xen-devel
>> 
>> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


Best regards,
Yang



[-- Attachment #2: Type: message/rfc822, Size: 7715 bytes --]

From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>, "ian.jackson@eu.citrix.com" <ian.jackson@eu.citrix.com>, Ian Campbell <Ian.Campbell@citrix.com>, "Nakajima, Jun" <jun.nakajima@intel.com>, "stefano.stabellini@eu.citrix.com" <stefano.stabellini@eu.citrix.com>
Subject: Re: [Xen-devel] 1GB hugepages and intel_xc_cpuid_policy by default disables it.
Date: Wed, 15 Jan 2014 20:07:36 +0000
Message-ID: <20140115200736.GB5201@phenom.dumpdata.com>

On Mon, Jan 13, 2014 at 11:51:28AM +0000, Jan Beulich wrote:
> >>> On 13.01.14 at 12:38, Ian Campbell <Ian.Campbell@citrix.com> wrote:
> > On Mon, 2014-01-13 at 11:30 +0000, Jan Beulich wrote:
> >> In fact I can't see where this would be forced off: xc_cpuid_x86.c
> >> only does so in the PV case, and all hvm_pse1gb_supported() is
> >> that the CPU supports it and the domain uses HAP.
> >
> > Took me a while to spot it too:
> > static void intel_xc_cpuid_policy(
> > [...]
> >             case 0x80000001: {
> >                 int is_64bit = hypervisor_is_64bit(xch) && is_pae;
> >
> >                 /* Only a few features are advertised in Intel's 0x80000001. */
> >                 regs[2] &= (is_64bit ? bitmaskof(X86_FEATURE_LAHF_LM) : 0) |
> >                                        bitmaskof(X86_FEATURE_ABM);
> >                 regs[3] &= ((is_pae ? bitmaskof(X86_FEATURE_NX) : 0) |
> >                             (is_64bit ? bitmaskof(X86_FEATURE_LM) : 0) |
> >                             (is_64bit ? bitmaskof(X86_FEATURE_SYSCALL) : 0) |
> >                             (is_64bit ? bitmaskof(X86_FEATURE_RDTSCP) : 0));
> >                 break;
> >             }
> >
> >
> > Which masks anything which is not explicitly mentioned. (PAGE1GB is in
> > regs[3], I think).
>
> Ah, okay. The funs of white listing on HVM vs black listing on PV
> again.
>
> > The AMD version is more permissive:
> >
> >         regs[3] &= (0x0183f3ff | /* features shared with 0x00000001:EDX */
> >                     (is_pae ? bitmaskof(X86_FEATURE_NX) : 0) |
> >                     (is_64bit ? bitmaskof(X86_FEATURE_LM) : 0) |
> >                     bitmaskof(X86_FEATURE_SYSCALL) |
> >                     bitmaskof(X86_FEATURE_MP) |
> >                     bitmaskof(X86_FEATURE_MMXEXT) |
> >                     bitmaskof(X86_FEATURE_FFXSR) |
> >                     bitmaskof(X86_FEATURE_3DNOW) |
> >                     bitmaskof(X86_FEATURE_3DNOWEXT));
> >
> > (but I didn't check if PAGE1GB is in that magic number...)
>
> It's not - it's bit 26.

So.. it sounds to me like everybody is in the agreement that this is the
right thing to do (enable it if the hypervisor has it enabled)?

And the next thing is actually come up with a patch to do some of this
plumbing - naturally for Xen 4.5?
>
> Jan
>

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

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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-12-04  1:50         ` Zhang, Yang Z
@ 2014-12-04 10:21           ` Andrew Cooper
  2014-12-08  2:11             ` Zhang, Yang Z
  0 siblings, 1 reply; 10+ messages in thread
From: Andrew Cooper @ 2014-12-04 10:21 UTC (permalink / raw)
  To: Zhang, Yang Z, Konrad Rzeszutek Wilk, Ian Campbell
  Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com,
	tim@xen.org, Li, Liang Z, ian.jackson@eu.citrix.com,
	xen-devel@lists.xen.org, JBeulich@suse.com

On 04/12/14 01:50, Zhang, Yang Z wrote:
> Konrad Rzeszutek Wilk wrote on 2014-12-03:
>> On Wed, Dec 03, 2014 at 09:38:49AM +0000, Ian Campbell wrote:
>>> On Tue, 2014-12-02 at 16:09 -0500, Konrad Rzeszutek Wilk wrote:
>>>> On Fri, Nov 28, 2014 at 11:50:43AM +0000, Ian Campbell wrote:
>>>>> On Fri, 2014-11-28 at 18:52 +0800, Liang Li wrote:
>>>>>> 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.
>>>>>>
>>>>>> If guest use shadow mode, the 1GB pages feature will be hidden from
>>>>>> guest, this is done in the function hvm_cpuid(). So the change is
>>>>>> okay for shadow mode case.
>>>>>>
>>>>>> Signed-off-by: Liang Li <liang.z.li@intel.com>
>>>>>> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
>>>>> FTR although this is strictly speaking a toolstack patch I think the
>>>>> main ack required should be from the x86 hypervisor guys...
>>>> Jan acked it.
>>> For 4.5?
>> Probably not.
>>> Have you release acked it?
>> No.
>>> This seemed like 4.6 material to me, or at least I've not seen any
>>> mention/argument to the contrary.
>> Correct. 4.6 please.
> I think this more like a bug fixing than a feature. See our previous discussion.

It is allowing HVM guests to use a brand new hardware feature which was
previously unavailable to them.

It is absolutely not a bugfix, and not appropriate for 4.5 at this
point, but a good candidate for acceptance as soon as the 4.6 dev window
opens.

~Andrew

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-12-04 10:21           ` Andrew Cooper
@ 2014-12-08  2:11             ` Zhang, Yang Z
  0 siblings, 0 replies; 10+ messages in thread
From: Zhang, Yang Z @ 2014-12-08  2:11 UTC (permalink / raw)
  To: Andrew Cooper, Konrad Rzeszutek Wilk, Ian Campbell
  Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com,
	tim@xen.org, Li, Liang Z, ian.jackson@eu.citrix.com,
	xen-devel@lists.xen.org, JBeulich@suse.com

Andrew Cooper wrote on 2014-12-04:
> On 04/12/14 01:50, Zhang, Yang Z wrote:
>> Konrad Rzeszutek Wilk wrote on 2014-12-03:
>>> On Wed, Dec 03, 2014 at 09:38:49AM +0000, Ian Campbell wrote:
>>>> On Tue, 2014-12-02 at 16:09 -0500, Konrad Rzeszutek Wilk wrote:
>>>>> On Fri, Nov 28, 2014 at 11:50:43AM +0000, Ian Campbell wrote:
>>>>>> On Fri, 2014-11-28 at 18:52 +0800, Liang Li wrote:
>>>>>>> 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.
>>>>>>> 
>>>>>>> If guest use shadow mode, the 1GB pages feature will be hidden
>>>>>>> from guest, this is done in the function hvm_cpuid(). So the
>>>>>>> change is okay for shadow mode case.
>>>>>>> 
>>>>>>> Signed-off-by: Liang Li <liang.z.li@intel.com>
>>>>>>> Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
>>>>>> FTR although this is strictly speaking a toolstack patch I think
>>>>>> the main ack required should be from the x86 hypervisor guys...
>>>>> Jan acked it.
>>>> For 4.5? Probably not. Have you release acked it? No. This seemed
>>>> like 4.6 material to me, or at least I've not seen any
>>>> mention/argument to the contrary.
>>> Correct. 4.6 please.
>> I think this more like a bug fixing than a feature. See our previous discussion.
> 
> It is allowing HVM guests to use a brand new hardware feature which
> was previously unavailable to them.

Actually, we have regular test case to cover 1G page size which exposing the 1G feature manually through config file. And we also have a bug to track this issue. So it more likes a bugfix to me. But you are right. It is a new feature from guest's point.

> 
> It is absolutely not a bugfix, and not appropriate for 4.5 at this
> point, but a good candidate for acceptance as soon as the 4.6 dev window opens.

Agree.

> 
> ~Andrew


Best regards,
Yang

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [v4] libxc: Expose the 1GB pages cpuid flag to guest
  2014-11-28 11:31 ` Jan Beulich
@ 2015-01-08 14:58   ` Ian Campbell
  0 siblings, 0 replies; 10+ messages in thread
From: Ian Campbell @ 2015-01-08 14:58 UTC (permalink / raw)
  To: Jan Beulich
  Cc: tim, wei.liu2, stefano.stabellini, andrew.cooper3, Liang Li,
	ian.jackson, xen-devel, yang.z.zhang

On Fri, 2014-11-28 at 11:31 +0000, Jan Beulich wrote:
> >>> On 28.11.14 at 11:52, <liang.z.li@intel.com> wrote:
> > 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.
> > 
> > If guest use shadow mode, the 1GB pages feature will be hidden from
> > guest, this is done in the function hvm_cpuid(). So the change is
> > okay for shadow mode case.
> > 
> > Signed-off-by: Liang Li <liang.z.li@intel.com>
> > Signed-off-by: Yang Zhang <yang.z.zhang@intel.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Acked + applied (to 4.6), thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2015-01-08 14:58 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-28 10:52 [v4] libxc: Expose the 1GB pages cpuid flag to guest Liang Li
2014-11-28 11:31 ` Jan Beulich
2015-01-08 14:58   ` Ian Campbell
2014-11-28 11:50 ` Ian Campbell
2014-12-02 21:09   ` Konrad Rzeszutek Wilk
2014-12-03  9:38     ` Ian Campbell
2014-12-03 15:57       ` Konrad Rzeszutek Wilk
2014-12-04  1:50         ` Zhang, Yang Z
2014-12-04 10:21           ` Andrew Cooper
2014-12-08  2:11             ` Zhang, Yang Z

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.