[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [v4] libxc: Expose the 1GB pages cpuid flag to guest



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@xxxxxxxxx>
>>>>> Signed-off-by: Yang Zhang <yang.z.zhang@xxxxxxxxx>
>>>> 
>>>> 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@xxxxxxxxxxxxx
>>>> http://lists.xen.org/xen-devel
>> 
>> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel


Best regards,
Yang


--- Begin Message ---
On Mon, Jan 13, 2014 at 11:51:28AM +0000, Jan Beulich wrote:
> >>> On 13.01.14 at 12:38, Ian Campbell <Ian.Campbell@xxxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.