All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
@ 2014-11-17  5:16 Liang Li
  2014-11-17 15:39 ` Ian Jackson
  0 siblings, 1 reply; 18+ messages in thread
From: Liang Li @ 2014-11-17  5:16 UTC (permalink / raw)
  To: xen-devel
  Cc: wei.liu2, ian.campbell, stefano.stabellini, Liang Li, ian.jackson,
	yang.z.zhang

If hardware support the pdpe1gb flag, expose it to guest by default.
Users don't have to use a 'cpuid= ' option in config file to turn
it on.

Signed-off-by: Liang Li <liang.z.li@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] 18+ messages in thread

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-17  5:16 [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest Liang Li
@ 2014-11-17 15:39 ` Ian Jackson
  2014-11-17 16:24   ` Jan Beulich
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Jackson @ 2014-11-17 15:39 UTC (permalink / raw)
  To: Liang Li
  Cc: wei.liu2, ian.campbell, stefano.stabellini, xen-devel, JBeulich,
	yang.z.zhang

Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag to guest"):
> If hardware support the pdpe1gb flag, expose it to guest by default.
> Users don't have to use a 'cpuid= ' option in config file to turn
> it on.

I don't understand what this flag does.  I guess from the name it
turns on 1G pages.  I guess these are supported ?

I would like to see comment from an x86 hypervisor maintainer.

Thanks,
Ian.

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-17 15:39 ` Ian Jackson
@ 2014-11-17 16:24   ` Jan Beulich
  2014-11-17 16:30     ` Tim Deegan
  0 siblings, 1 reply; 18+ messages in thread
From: Jan Beulich @ 2014-11-17 16:24 UTC (permalink / raw)
  To: Ian Jackson, Liang Li, Tim Deegan
  Cc: yang.z.zhang, xen-devel, wei.liu2, ian.campbell,
	stefano.stabellini

>>> On 17.11.14 at 16:39, <Ian.Jackson@eu.citrix.com> wrote:
> Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag to guest"):
>> If hardware support the pdpe1gb flag, expose it to guest by default.
>> Users don't have to use a 'cpuid= ' option in config file to turn
>> it on.
> 
> I don't understand what this flag does.  I guess from the name it
> turns on 1G pages.  I guess these are supported ?
> 
> I would like to see comment from an x86 hypervisor maintainer.

Yes, we support 1Gb pages. The purpose of the patch is to not
unconditionally hide the flag from guests. (I had commented on
v1, but sadly this one wasn't tagged as v2, nor was I included on
the Cc list, hence I didn't spot the new version.)

The one thing I'm not certain about is shadow mode: Only 2Mb
pages may be supported there. Tim?

Jan

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-17 16:24   ` Jan Beulich
@ 2014-11-17 16:30     ` Tim Deegan
  2014-11-17 16:40       ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Tim Deegan @ 2014-11-17 16:30 UTC (permalink / raw)
  To: Jan Beulich
  Cc: wei.liu2, ian.campbell, stefano.stabellini, Liang Li, Ian Jackson,
	xen-devel, yang.z.zhang

At 16:24 +0000 on 17 Nov (1416237888), Jan Beulich wrote:
> >>> On 17.11.14 at 16:39, <Ian.Jackson@eu.citrix.com> wrote:
> > Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag to guest"):
> >> If hardware support the pdpe1gb flag, expose it to guest by default.
> >> Users don't have to use a 'cpuid= ' option in config file to turn
> >> it on.
> > 
> > I don't understand what this flag does.  I guess from the name it
> > turns on 1G pages.  I guess these are supported ?
> > 
> > I would like to see comment from an x86 hypervisor maintainer.
> 
> Yes, we support 1Gb pages. The purpose of the patch is to not
> unconditionally hide the flag from guests. (I had commented on
> v1, but sadly this one wasn't tagged as v2, nor was I included on
> the Cc list, hence I didn't spot the new version.)
> 
> The one thing I'm not certain about is shadow mode: Only 2Mb
> pages may be supported there. Tim?

Indeed, only 2MiB pages are supported in shadow mode.  See, e.g.
guest_supports_1G_superpages()->hvm_pse1gb_supported()->paging_mode_hap()

Tim.

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-17 16:30     ` Tim Deegan
@ 2014-11-17 16:40       ` Andrew Cooper
  2014-11-17 17:00         ` Tim Deegan
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2014-11-17 16:40 UTC (permalink / raw)
  To: Tim Deegan, Jan Beulich
  Cc: wei.liu2, ian.campbell, stefano.stabellini, Liang Li, Ian Jackson,
	xen-devel, yang.z.zhang

On 17/11/14 16:30, Tim Deegan wrote:
> At 16:24 +0000 on 17 Nov (1416237888), Jan Beulich wrote:
>>>>> On 17.11.14 at 16:39, <Ian.Jackson@eu.citrix.com> wrote:
>>> Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag to guest"):
>>>> If hardware support the pdpe1gb flag, expose it to guest by default.
>>>> Users don't have to use a 'cpuid= ' option in config file to turn
>>>> it on.
>>> I don't understand what this flag does.  I guess from the name it
>>> turns on 1G pages.  I guess these are supported ?
>>>
>>> I would like to see comment from an x86 hypervisor maintainer.
>> Yes, we support 1Gb pages. The purpose of the patch is to not
>> unconditionally hide the flag from guests. (I had commented on
>> v1, but sadly this one wasn't tagged as v2, nor was I included on
>> the Cc list, hence I didn't spot the new version.)
>>
>> The one thing I'm not certain about is shadow mode: Only 2Mb
>> pages may be supported there. Tim?
> Indeed, only 2MiB pages are supported in shadow mode.  See, e.g.
> guest_supports_1G_superpages()->hvm_pse1gb_supported()->paging_mode_hap()

This is yet another case which proves that libxc is the wrong place to
be choosing the cpuid flags exposed to a domain.

Furthermore, guest_supports_1G_superpages() is insufficient as it only
checks whether the host is capable of providing 1G superpages, not
whether the guest has been permitted to use it.

This causes a problem when migrating between hap-capable and
hap-incapable systems.

I do hope to fix all of this with my planned changes to the cpuid
infrastructure.

~Andrew

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-17 16:40       ` Andrew Cooper
@ 2014-11-17 17:00         ` Tim Deegan
  2014-11-17 17:25           ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Tim Deegan @ 2014-11-17 17:00 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: wei.liu2, ian.campbell, stefano.stabellini, Liang Li, Ian Jackson,
	xen-devel, Jan Beulich, yang.z.zhang

At 16:40 +0000 on 17 Nov (1416238835), Andrew Cooper wrote:
> On 17/11/14 16:30, Tim Deegan wrote:
> > At 16:24 +0000 on 17 Nov (1416237888), Jan Beulich wrote:
> >>>>> On 17.11.14 at 16:39, <Ian.Jackson@eu.citrix.com> wrote:
> >>> Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag to guest"):
> >>>> If hardware support the pdpe1gb flag, expose it to guest by default.
> >>>> Users don't have to use a 'cpuid= ' option in config file to turn
> >>>> it on.
> >>> I don't understand what this flag does.  I guess from the name it
> >>> turns on 1G pages.  I guess these are supported ?
> >>>
> >>> I would like to see comment from an x86 hypervisor maintainer.
> >> Yes, we support 1Gb pages. The purpose of the patch is to not
> >> unconditionally hide the flag from guests. (I had commented on
> >> v1, but sadly this one wasn't tagged as v2, nor was I included on
> >> the Cc list, hence I didn't spot the new version.)
> >>
> >> The one thing I'm not certain about is shadow mode: Only 2Mb
> >> pages may be supported there. Tim?
> > Indeed, only 2MiB pages are supported in shadow mode.  See, e.g.
> > guest_supports_1G_superpages()->hvm_pse1gb_supported()->paging_mode_hap()
> 
> This is yet another case which proves that libxc is the wrong place to
> be choosing the cpuid flags exposed to a domain.
> 
> Furthermore, guest_supports_1G_superpages() is insufficient as it only
> checks whether the host is capable of providing 1G superpages, not
> whether the guest has been permitted to use it.
> 
> This causes a problem when migrating between hap-capable and
> hap-incapable systems.

There's no notion of 'permitted' to use 1G pages, AFAICS, much like
other CPU features.  But of course a _well-behaved_ guest that has
been told in cpuid not to use 1G superpages will have no problems. :)

> I do hope to fix all of this with my planned changes to the cpuid
> infrastructure.

Good luck!

Tim.

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-17 17:00         ` Tim Deegan
@ 2014-11-17 17:25           ` Andrew Cooper
  2014-11-18 10:14             ` Tim Deegan
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2014-11-17 17:25 UTC (permalink / raw)
  To: Tim Deegan
  Cc: wei.liu2, ian.campbell, stefano.stabellini, Liang Li, Ian Jackson,
	xen-devel, Jan Beulich, yang.z.zhang

On 17/11/14 17:00, Tim Deegan wrote:
> At 16:40 +0000 on 17 Nov (1416238835), Andrew Cooper wrote:
>> On 17/11/14 16:30, Tim Deegan wrote:
>>> At 16:24 +0000 on 17 Nov (1416237888), Jan Beulich wrote:
>>>>>>> On 17.11.14 at 16:39, <Ian.Jackson@eu.citrix.com> wrote:
>>>>> Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag to guest"):
>>>>>> If hardware support the pdpe1gb flag, expose it to guest by default.
>>>>>> Users don't have to use a 'cpuid= ' option in config file to turn
>>>>>> it on.
>>>>> I don't understand what this flag does.  I guess from the name it
>>>>> turns on 1G pages.  I guess these are supported ?
>>>>>
>>>>> I would like to see comment from an x86 hypervisor maintainer.
>>>> Yes, we support 1Gb pages. The purpose of the patch is to not
>>>> unconditionally hide the flag from guests. (I had commented on
>>>> v1, but sadly this one wasn't tagged as v2, nor was I included on
>>>> the Cc list, hence I didn't spot the new version.)
>>>>
>>>> The one thing I'm not certain about is shadow mode: Only 2Mb
>>>> pages may be supported there. Tim?
>>> Indeed, only 2MiB pages are supported in shadow mode.  See, e.g.
>>> guest_supports_1G_superpages()->hvm_pse1gb_supported()->paging_mode_hap()
>> This is yet another case which proves that libxc is the wrong place to
>> be choosing the cpuid flags exposed to a domain.
>>
>> Furthermore, guest_supports_1G_superpages() is insufficient as it only
>> checks whether the host is capable of providing 1G superpages, not
>> whether the guest has been permitted to use it.
>>
>> This causes a problem when migrating between hap-capable and
>> hap-incapable systems.
> There's no notion of 'permitted' to use 1G pages, AFAICS, much like
> other CPU features.  But of course a _well-behaved_ guest that has
> been told in cpuid not to use 1G superpages will have no problems. :)

That is my point.

If 1GB pages are not supported (i.e. not present in cpuid), then the PS
bit in an L3 is reserved, must be 0, and cause a pagefault if used.

Nothing in Xen currently enforces this because
guest_supports_1G_superpages() doesn't check domain_cpuid().

It does however make me wonder how VMX/SVM non-root mode would enforce
this as it would see the host cpuid, not guest cpuid when performing
paging internally.

~Andrew

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-17 17:25           ` Andrew Cooper
@ 2014-11-18 10:14             ` Tim Deegan
  2014-11-18 10:43               ` Andrew Cooper
  0 siblings, 1 reply; 18+ messages in thread
From: Tim Deegan @ 2014-11-18 10:14 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: wei.liu2, ian.campbell, stefano.stabellini, Liang Li, Ian Jackson,
	xen-devel, Jan Beulich, yang.z.zhang

At 17:25 +0000 on 17 Nov (1416241517), Andrew Cooper wrote:
> On 17/11/14 17:00, Tim Deegan wrote:
> > At 16:40 +0000 on 17 Nov (1416238835), Andrew Cooper wrote:
> >> On 17/11/14 16:30, Tim Deegan wrote:
> >>> At 16:24 +0000 on 17 Nov (1416237888), Jan Beulich wrote:
> >>>>>>> On 17.11.14 at 16:39, <Ian.Jackson@eu.citrix.com> wrote:
> >>>>> Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag to guest"):
> >>>>>> If hardware support the pdpe1gb flag, expose it to guest by default.
> >>>>>> Users don't have to use a 'cpuid= ' option in config file to turn
> >>>>>> it on.
> >>>>> I don't understand what this flag does.  I guess from the name it
> >>>>> turns on 1G pages.  I guess these are supported ?
> >>>>>
> >>>>> I would like to see comment from an x86 hypervisor maintainer.
> >>>> Yes, we support 1Gb pages. The purpose of the patch is to not
> >>>> unconditionally hide the flag from guests. (I had commented on
> >>>> v1, but sadly this one wasn't tagged as v2, nor was I included on
> >>>> the Cc list, hence I didn't spot the new version.)
> >>>>
> >>>> The one thing I'm not certain about is shadow mode: Only 2Mb
> >>>> pages may be supported there. Tim?
> >>> Indeed, only 2MiB pages are supported in shadow mode.  See, e.g.
> >>> guest_supports_1G_superpages()->hvm_pse1gb_supported()->paging_mode_hap()
> >> This is yet another case which proves that libxc is the wrong place to
> >> be choosing the cpuid flags exposed to a domain.
> >>
> >> Furthermore, guest_supports_1G_superpages() is insufficient as it only
> >> checks whether the host is capable of providing 1G superpages, not
> >> whether the guest has been permitted to use it.
> >>
> >> This causes a problem when migrating between hap-capable and
> >> hap-incapable systems.
> > There's no notion of 'permitted' to use 1G pages, AFAICS, much like
> > other CPU features.  But of course a _well-behaved_ guest that has
> > been told in cpuid not to use 1G superpages will have no problems. :)
> 
> That is my point.
> 
> If 1GB pages are not supported (i.e. not present in cpuid), then the PS
> bit in an L3 is reserved, must be 0, and cause a pagefault if used.
> 
> Nothing in Xen currently enforces this because
> guest_supports_1G_superpages() doesn't check domain_cpuid().

For shadow mode, Xen DTRT by checking hvm_pse1gb_supported() in the
HVM cpuid handler, so we don't advertise a feature we can't support.

For HAP mode, we _could_ add a cpuid check to the pagetable walker
but...

> It does however make me wonder how VMX/SVM non-root mode would enforce
> this as it would see the host cpuid, not guest cpuid when performing
> paging internally.

...non-emulated PT walks will get to use 1GB superpages anyway.
This is the same for other features (new instructions &c).  We
can mask them out of CPUID but by and large we can't make them fault.

Tim.

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-18 10:14             ` Tim Deegan
@ 2014-11-18 10:43               ` Andrew Cooper
  2014-11-18 11:41                 ` Zhang, Yang Z
  0 siblings, 1 reply; 18+ messages in thread
From: Andrew Cooper @ 2014-11-18 10:43 UTC (permalink / raw)
  To: Tim Deegan
  Cc: wei.liu2, ian.campbell, stefano.stabellini, Liang Li, Ian Jackson,
	xen-devel, Jan Beulich, yang.z.zhang

On 18/11/14 10:14, Tim Deegan wrote:
> At 17:25 +0000 on 17 Nov (1416241517), Andrew Cooper wrote:
>> On 17/11/14 17:00, Tim Deegan wrote:
>>> At 16:40 +0000 on 17 Nov (1416238835), Andrew Cooper wrote:
>>>> On 17/11/14 16:30, Tim Deegan wrote:
>>>>> At 16:24 +0000 on 17 Nov (1416237888), Jan Beulich wrote:
>>>>>>>>> On 17.11.14 at 16:39, <Ian.Jackson@eu.citrix.com> wrote:
>>>>>>> Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag to guest"):
>>>>>>>> If hardware support the pdpe1gb flag, expose it to guest by default.
>>>>>>>> Users don't have to use a 'cpuid= ' option in config file to turn
>>>>>>>> it on.
>>>>>>> I don't understand what this flag does.  I guess from the name it
>>>>>>> turns on 1G pages.  I guess these are supported ?
>>>>>>>
>>>>>>> I would like to see comment from an x86 hypervisor maintainer.
>>>>>> Yes, we support 1Gb pages. The purpose of the patch is to not
>>>>>> unconditionally hide the flag from guests. (I had commented on
>>>>>> v1, but sadly this one wasn't tagged as v2, nor was I included on
>>>>>> the Cc list, hence I didn't spot the new version.)
>>>>>>
>>>>>> The one thing I'm not certain about is shadow mode: Only 2Mb
>>>>>> pages may be supported there. Tim?
>>>>> Indeed, only 2MiB pages are supported in shadow mode.  See, e.g.
>>>>> guest_supports_1G_superpages()->hvm_pse1gb_supported()->paging_mode_hap()
>>>> This is yet another case which proves that libxc is the wrong place to
>>>> be choosing the cpuid flags exposed to a domain.
>>>>
>>>> Furthermore, guest_supports_1G_superpages() is insufficient as it only
>>>> checks whether the host is capable of providing 1G superpages, not
>>>> whether the guest has been permitted to use it.
>>>>
>>>> This causes a problem when migrating between hap-capable and
>>>> hap-incapable systems.
>>> There's no notion of 'permitted' to use 1G pages, AFAICS, much like
>>> other CPU features.  But of course a _well-behaved_ guest that has
>>> been told in cpuid not to use 1G superpages will have no problems. :)
>> That is my point.
>>
>> If 1GB pages are not supported (i.e. not present in cpuid), then the PS
>> bit in an L3 is reserved, must be 0, and cause a pagefault if used.
>>
>> Nothing in Xen currently enforces this because
>> guest_supports_1G_superpages() doesn't check domain_cpuid().
> For shadow mode, Xen DTRT by checking hvm_pse1gb_supported() in the
> HVM cpuid handler, so we don't advertise a feature we can't support.
>
> For HAP mode, we _could_ add a cpuid check to the pagetable walker
> but...
>
>> It does however make me wonder how VMX/SVM non-root mode would enforce
>> this as it would see the host cpuid, not guest cpuid when performing
>> paging internally.
> ...non-emulated PT walks will get to use 1GB superpages anyway.
> This is the same for other features (new instructions &c).  We
> can mask them out of CPUID but by and large we can't make them fault.

Hmm - this is a pitfall waiting to happen.

In the case that there is a heterogeneous setup with one 1G capable and
one 1G incapable server, Xen cannot forcibly prevent the use of 1G pages
on the capable hardware.  Any VM which guesses at hardware support by
means other than cpuid features is liable to explode on migrate.

I suspect this will just have to fall into the category of "here be yet
more dragons with heterogeneous migration"

~Andrew

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-18 10:43               ` Andrew Cooper
@ 2014-11-18 11:41                 ` Zhang, Yang Z
  2014-11-18 14:26                   ` Ian Campbell
  0 siblings, 1 reply; 18+ messages in thread
From: Zhang, Yang Z @ 2014-11-18 11:41 UTC (permalink / raw)
  To: Andrew Cooper, Tim Deegan
  Cc: wei.liu2@citrix.com, ian.campbell@citrix.com,
	stefano.stabellini@eu.citrix.com, Li, Liang Z, Ian Jackson,
	xen-devel@lists.xen.org, Jan Beulich

Andrew Cooper wrote on 2014-11-18:
> On 18/11/14 10:14, Tim Deegan wrote:
>> At 17:25 +0000 on 17 Nov (1416241517), Andrew Cooper wrote:
>>> On 17/11/14 17:00, Tim Deegan wrote:
>>>> At 16:40 +0000 on 17 Nov (1416238835), Andrew Cooper wrote:
>>>>> On 17/11/14 16:30, Tim Deegan wrote:
>>>>>> At 16:24 +0000 on 17 Nov (1416237888), Jan Beulich wrote:
>>>>>>>>>> On 17.11.14 at 16:39, <Ian.Jackson@eu.citrix.com> wrote:
>>>>>>>> Liang Li writes ("[PATCH] libxc: Expose the pdpe1gb cpuid flag
>>>>>>>> to
> guest"):
>>>>>>>>> If hardware support the pdpe1gb flag, expose it to guest by default.
>>>>>>>>> Users don't have to use a 'cpuid= ' option in config file to
>>>>>>>>> turn it on.
>>>>>>>> I don't understand what this flag does.  I guess from the name
>>>>>>>> it turns on 1G pages.  I guess these are supported ?
>>>>>>>> 
>>>>>>>> I would like to see comment from an x86 hypervisor maintainer.
>>>>>>> Yes, we support 1Gb pages. The purpose of the patch is to not
>>>>>>> unconditionally hide the flag from guests. (I had commented on
>>>>>>> v1, but sadly this one wasn't tagged as v2, nor was I included
>>>>>>> on the Cc list, hence I didn't spot the new version.)
>>>>>>> 
>>>>>>> The one thing I'm not certain about is shadow mode: Only 2Mb
>>>>>>> pages may be supported there. Tim?
>>>>>> Indeed, only 2MiB pages are supported in shadow mode.  See, e.g.
>>>>>> 
>>>>>> guest_supports_1G_superpages()->hvm_pse1gb_supported()->paging_mod
>>>>>> e_hap()
>>>>> This is yet another case which proves that libxc is the wrong
>>>>> place to be choosing the cpuid flags exposed to a domain.
>>>>> 
>>>>> Furthermore, guest_supports_1G_superpages() is insufficient as it
>>>>> only checks whether the host is capable of providing 1G
>>>>> superpages, not whether the guest has been permitted to use it.
>>>>> 
>>>>> This causes a problem when migrating between hap-capable and
>>>>> hap-incapable systems.
>>>> There's no notion of 'permitted' to use 1G pages, AFAICS, much
>>>> like other CPU features.  But of course a _well-behaved_ guest
>>>> that has been told in cpuid not to use 1G superpages will have no problems.
>>>> :)
>>> That is my point.
>>> 
>>> If 1GB pages are not supported (i.e. not present in cpuid), then
>>> the PS bit in an L3 is reserved, must be 0, and cause a pagefault if used.
>>> 
>>> Nothing in Xen currently enforces this because
>>> guest_supports_1G_superpages() doesn't check domain_cpuid().
>> For shadow mode, Xen DTRT by checking hvm_pse1gb_supported() in the
>> HVM cpuid handler, so we don't advertise a feature we can't support.
>> 
>> For HAP mode, we _could_ add a cpuid check to the pagetable walker
>> but...
>> 
>>> It does however make me wonder how VMX/SVM non-root mode would
>>> enforce this as it would see the host cpuid, not guest cpuid when
>>> performing paging internally.
>> ...non-emulated PT walks will get to use 1GB superpages anyway.
>> This is the same for other features (new instructions &c).  We can
>> mask them out of CPUID but by and large we can't make them fault.

Agree. I will forward this question to internally to see whether they aware of this problem.

> 
> Hmm - this is a pitfall waiting to happen.
> 
> In the case that there is a heterogeneous setup with one 1G capable
> and one 1G incapable server, Xen cannot forcibly prevent the use of 1G
> pages on the capable hardware.  Any VM which guesses at hardware
> support by means other than cpuid features is liable to explode on migrate.

But a normal guest shouldn't to guess it, right?

> 
> I suspect this will just have to fall into the category of "here be
> yet more dragons with heterogeneous migration"
> 
> ~Andrew


Best regards,
Yang

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-18 11:41                 ` Zhang, Yang Z
@ 2014-11-18 14:26                   ` Ian Campbell
  2014-11-18 15:15                     ` Tim Deegan
  0 siblings, 1 reply; 18+ messages in thread
From: Ian Campbell @ 2014-11-18 14:26 UTC (permalink / raw)
  To: Zhang, Yang Z
  Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com,
	Andrew Cooper, Li, Liang Z, Ian Jackson, Tim Deegan, Jan Beulich,
	xen-devel@lists.xen.org

On Tue, 2014-11-18 at 11:41 +0000, Zhang, Yang Z wrote:
> > Hmm - this is a pitfall waiting to happen.
> > 
> > In the case that there is a heterogeneous setup with one 1G capable
> > and one 1G incapable server, Xen cannot forcibly prevent the use of 1G
> > pages on the capable hardware.  Any VM which guesses at hardware
> > support by means other than cpuid features is liable to explode on migrate.
> 
> But a normal guest shouldn't to guess it, right?

IMHO any guest which does not use the mechanism explicitly provided for
feature detection deserves to break randomly.

It's basically equivalent to a physical system where the BIOS masks this
cpuid bit because of some hardware errata or something, the underlying
feature might appear to be present but will have more or less subtle
issues.

Ian.

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-18 14:26                   ` Ian Campbell
@ 2014-11-18 15:15                     ` Tim Deegan
  2014-11-19  1:29                       ` Zhang, Yang Z
  0 siblings, 1 reply; 18+ messages in thread
From: Tim Deegan @ 2014-11-18 15:15 UTC (permalink / raw)
  To: Ian Campbell
  Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com,
	Andrew Cooper, Li, Liang Z, Ian Jackson, xen-devel@lists.xen.org,
	Jan Beulich, Zhang, Yang Z

At 14:26 +0000 on 18 Nov (1416317209), Ian Campbell wrote:
> On Tue, 2014-11-18 at 11:41 +0000, Zhang, Yang Z wrote:
> > > Hmm - this is a pitfall waiting to happen.
> > > 
> > > In the case that there is a heterogeneous setup with one 1G capable
> > > and one 1G incapable server, Xen cannot forcibly prevent the use of 1G
> > > pages on the capable hardware.  Any VM which guesses at hardware
> > > support by means other than cpuid features is liable to explode on migrate.
> > 
> > But a normal guest shouldn't to guess it, right?
> 
> IMHO any guest which does not use the mechanism explicitly provided for
> feature detection deserves to break randomly.

Yes, that's a reasonable position (notwithstanding that we know such
software exists).

In this case, the guest is entitled to _expect_ pagefaults on 1GB
mappings if CPUID claims they are not supported.  That sounds like an
unlikely thing for the guest to be relying on, but Xen itself does
something similar for the SHOPT_FAST_FAULT_PATH (and now also for
IOMMU entries for the deferred caching attribute updates).

Cheers,

Tim.

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-18 15:15                     ` Tim Deegan
@ 2014-11-19  1:29                       ` Zhang, Yang Z
  2014-11-19  9:54                         ` Tim Deegan
  0 siblings, 1 reply; 18+ messages in thread
From: Zhang, Yang Z @ 2014-11-19  1:29 UTC (permalink / raw)
  To: Tim Deegan, Ian Campbell
  Cc: wei.liu2@citrix.com, stefano.stabellini@eu.citrix.com,
	Andrew Cooper, Li, Liang Z, Ian Jackson, xen-devel@lists.xen.org,
	Jan Beulich

Tim Deegan wrote on 2014-11-18:
> At 14:26 +0000 on 18 Nov (1416317209), Ian Campbell wrote:
>> On Tue, 2014-11-18 at 11:41 +0000, Zhang, Yang Z wrote:
>>>> Hmm - this is a pitfall waiting to happen.
>>>> 
>>>> In the case that there is a heterogeneous setup with one 1G
>>>> capable and one 1G incapable server, Xen cannot forcibly prevent
>>>> the use of 1G pages on the capable hardware.  Any VM which
>>>> guesses at hardware support by means other than cpuid features
>>>> is liable to
> explode on migrate.
>>> 
>>> But a normal guest shouldn't to guess it, right?
>> 
>> IMHO any guest which does not use the mechanism explicitly provided
>> for feature detection deserves to break randomly.
> 
> Yes, that's a reasonable position (notwithstanding that we know such
> software exists).
> 

Agree.

> In this case, the guest is entitled to _expect_ pagefaults on 1GB
> mappings if CPUID claims they are not supported.  That sounds like an
> unlikely thing for the guest to be relying on, but Xen itself does
> something similar for the SHOPT_FAST_FAULT_PATH (and now also for
> IOMMU entries for the deferred caching attribute updates).

Indeed. How about adding the software check (as Andrew mentioned) firstly and leave the hardware problem (Actually, I don't think we can solve it currently).

> 
> Cheers,
> 
> Tim.


Best regards,
Yang

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-19  1:29                       ` Zhang, Yang Z
@ 2014-11-19  9:54                         ` Tim Deegan
  2014-11-25  1:47                           ` Zhang, Yang Z
  0 siblings, 1 reply; 18+ messages in thread
From: Tim Deegan @ 2014-11-19  9:54 UTC (permalink / raw)
  To: Zhang, Yang Z
  Cc: wei.liu2@citrix.com, Ian Campbell,
	stefano.stabellini@eu.citrix.com, Andrew Cooper, Li, Liang Z,
	Ian Jackson, xen-devel@lists.xen.org, Jan Beulich

At 01:29 +0000 on 19 Nov (1416356943), Zhang, Yang Z wrote:
> Tim Deegan wrote on 2014-11-18:
> > In this case, the guest is entitled to _expect_ pagefaults on 1GB
> > mappings if CPUID claims they are not supported.  That sounds like an
> > unlikely thing for the guest to be relying on, but Xen itself does
> > something similar for the SHOPT_FAST_FAULT_PATH (and now also for
> > IOMMU entries for the deferred caching attribute updates).
> 
> Indeed. How about adding the software check (as Andrew mentioned)
> firstly and leave the hardware problem (Actually, I don't think we
> can solve it currently).

I don't think we should change the software path unless we can change
the hardware behaviour too.  It's better to be consistent, and it
saves us some cycles in the pt walker.

Cheers,

Tim.

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-19  9:54                         ` Tim Deegan
@ 2014-11-25  1:47                           ` Zhang, Yang Z
  2014-11-25  8:20                             ` Jan Beulich
  0 siblings, 1 reply; 18+ messages in thread
From: Zhang, Yang Z @ 2014-11-25  1:47 UTC (permalink / raw)
  To: Tim Deegan
  Cc: wei.liu2@citrix.com, Ian Campbell,
	stefano.stabellini@eu.citrix.com, Andrew Cooper, Li, Liang Z,
	Ian Jackson, xen-devel@lists.xen.org, Jan Beulich

Tim Deegan wrote on 2014-11-19:
> At 01:29 +0000 on 19 Nov (1416356943), Zhang, Yang Z wrote:
>> Tim Deegan wrote on 2014-11-18:
>>> In this case, the guest is entitled to _expect_ pagefaults on 1GB
>>> mappings if CPUID claims they are not supported.  That sounds like
>>> an unlikely thing for the guest to be relying on, but Xen itself
>>> does something similar for the SHOPT_FAST_FAULT_PATH (and now also
>>> for IOMMU entries for the deferred caching attribute updates).
>> 
>> Indeed. How about adding the software check (as Andrew mentioned)
>> firstly and leave the hardware problem (Actually, I don't think we
>> can solve it currently).
> 
> I don't think we should change the software path unless we can change
> the hardware behaviour too.  It's better to be consistent, and it
> saves us some cycles in the pt walker.

So if we don't need to add the software check, does this mean current patch is ok?

> 
> Cheers,
> 
> Tim.


Best regards,
Yang

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-25  1:47                           ` Zhang, Yang Z
@ 2014-11-25  8:20                             ` Jan Beulich
  2014-11-26  0:40                               ` Zhang, Yang Z
  0 siblings, 1 reply; 18+ messages in thread
From: Jan Beulich @ 2014-11-25  8:20 UTC (permalink / raw)
  To: Yang Z Zhang
  Cc: Tim Deegan, wei.liu2@citrix.com, Ian Campbell,
	stefano.stabellini@eu.citrix.com, Andrew Cooper, Liang Z Li,
	Ian Jackson, xen-devel@lists.xen.org

>>> On 25.11.14 at 02:47, <yang.z.zhang@intel.com> wrote:
> Tim Deegan wrote on 2014-11-19:
>> At 01:29 +0000 on 19 Nov (1416356943), Zhang, Yang Z wrote:
>>> Tim Deegan wrote on 2014-11-18:
>>>> In this case, the guest is entitled to _expect_ pagefaults on 1GB
>>>> mappings if CPUID claims they are not supported.  That sounds like
>>>> an unlikely thing for the guest to be relying on, but Xen itself
>>>> does something similar for the SHOPT_FAST_FAULT_PATH (and now also
>>>> for IOMMU entries for the deferred caching attribute updates).
>>> 
>>> Indeed. How about adding the software check (as Andrew mentioned)
>>> firstly and leave the hardware problem (Actually, I don't think we
>>> can solve it currently).
>> 
>> I don't think we should change the software path unless we can change
>> the hardware behaviour too.  It's better to be consistent, and it
>> saves us some cycles in the pt walker.
> 
> So if we don't need to add the software check, does this mean current patch 
> is ok?

No - Tim having confirmed that shadow mode doesn't support 1Gb
pages, the feature clearly must not be made visible for shadow
mode guest.

Jan

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-25  8:20                             ` Jan Beulich
@ 2014-11-26  0:40                               ` Zhang, Yang Z
  2014-11-28  0:34                                 ` Li, Liang Z
  0 siblings, 1 reply; 18+ messages in thread
From: Zhang, Yang Z @ 2014-11-26  0:40 UTC (permalink / raw)
  To: Jan Beulich
  Cc: Tim Deegan, wei.liu2@citrix.com, Ian Campbell,
	stefano.stabellini@eu.citrix.com, Andrew Cooper, Li, Liang Z,
	Ian Jackson, xen-devel@lists.xen.org

Jan Beulich wrote on 2014-11-25:
>>>> On 25.11.14 at 02:47, <yang.z.zhang@intel.com> wrote:
>> Tim Deegan wrote on 2014-11-19:
>>> At 01:29 +0000 on 19 Nov (1416356943), Zhang, Yang Z wrote:
>>>> Tim Deegan wrote on 2014-11-18:
>>>>> In this case, the guest is entitled to _expect_ pagefaults on 1GB
>>>>> mappings if CPUID claims they are not supported.  That sounds
>>>>> like an unlikely thing for the guest to be relying on, but Xen
>>>>> itself does something similar for the SHOPT_FAST_FAULT_PATH (and
>>>>> now also for IOMMU entries for the deferred caching attribute updates).
>>>> 
>>>> Indeed. How about adding the software check (as Andrew mentioned)
>>>> firstly and leave the hardware problem (Actually, I don't think we
>>>> can solve it currently).
>>> 
>>> I don't think we should change the software path unless we can
>>> change the hardware behaviour too.  It's better to be consistent,
>>> and it saves us some cycles in the pt walker.
>> 
>> So if we don't need to add the software check, does this mean
>> current patch is ok?
> 
> No - Tim having confirmed that shadow mode doesn't support 1Gb pages,
> the feature clearly must not be made visible for shadow mode guest.

Indeed. Liang, Can you add the shadow mode check in the next version?

> 
> Jan


Best regards,
Yang

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

* Re: [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest
  2014-11-26  0:40                               ` Zhang, Yang Z
@ 2014-11-28  0:34                                 ` Li, Liang Z
  0 siblings, 0 replies; 18+ messages in thread
From: Li, Liang Z @ 2014-11-28  0:34 UTC (permalink / raw)
  To: Zhang, Yang Z, Jan Beulich
  Cc: Tim Deegan, wei.liu2@citrix.com, Ian Campbell,
	stefano.stabellini@eu.citrix.com, Andrew Cooper, Ian Jackson,
	xen-devel@lists.xen.org

>>> patch is ok?
>> 
>> No - Tim having confirmed that shadow mode doesn't support 1Gb pages, 
> the feature clearly must not be made visible for shadow mode guest.

>Indeed. Liang, Can you add the shadow mode check in the next version?

Ok , I will do it and resend the patch.

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

end of thread, other threads:[~2014-11-28  0:34 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-11-17  5:16 [PATCH] libxc: Expose the pdpe1gb cpuid flag to guest Liang Li
2014-11-17 15:39 ` Ian Jackson
2014-11-17 16:24   ` Jan Beulich
2014-11-17 16:30     ` Tim Deegan
2014-11-17 16:40       ` Andrew Cooper
2014-11-17 17:00         ` Tim Deegan
2014-11-17 17:25           ` Andrew Cooper
2014-11-18 10:14             ` Tim Deegan
2014-11-18 10:43               ` Andrew Cooper
2014-11-18 11:41                 ` Zhang, Yang Z
2014-11-18 14:26                   ` Ian Campbell
2014-11-18 15:15                     ` Tim Deegan
2014-11-19  1:29                       ` Zhang, Yang Z
2014-11-19  9:54                         ` Tim Deegan
2014-11-25  1:47                           ` Zhang, Yang Z
2014-11-25  8:20                             ` Jan Beulich
2014-11-26  0:40                               ` Zhang, Yang Z
2014-11-28  0:34                                 ` Li, Liang 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.