Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

2017-01-24 Thread Andrew Cooper 
On 24/01/17 15:41, Roger Pau Monné wrote:
> On Tue, Jan 24, 2017 at 08:10:56AM -0700, Jan Beulich wrote:
> On 24.01.17 at 15:38,  wrote:
>>> On Wed, Jan 18, 2017 at 07:40:53PM +, Andrew Cooper wrote:
 The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
 mask, but nothing thusfar has prevented the features being visible in
 HVM-based control domains (where there is no toolstack decision to hide the
 features).

 As a side effect of calling nestedhvm_enabled() earlier during domain
 creation, it needs to cope with the params[] array array not having been
 allocated.

 Reported-by: Roger Pau Monné 
 Signed-off-by: Andrew Cooper 
 ---
 CC: Jan Beulich 
 CC: Roger Pau Monné 
 ---
  xen/arch/x86/cpuid.c | 25 ++---
  xen/arch/x86/hvm/nestedhvm.c |  3 ++-
  2 files changed, 20 insertions(+), 8 deletions(-)

 diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
 index eb829d7..7b9af1b 100644
 --- a/xen/arch/x86/cpuid.c
 +++ b/xen/arch/x86/cpuid.c
 @@ -3,6 +3,7 @@
  #include 
  #include 
  #include 
 +#include 
  #include 
  #include 
  #include 
 @@ -361,14 +362,24 @@ void recalculate_cpuid_policy(struct domain *d)
  cpuid_policy_to_featureset(p, fs);
  cpuid_policy_to_featureset(max, max_fs);
  
 -/*
 - * HVM domains using Shadow paging have further restrictions on their
 - * available paging features.
 - */
 -if ( is_hvm_domain(d) && !hap_enabled(d) )
 +if ( is_hvm_domain(d) )
>>> This should be has_hvm_container_domain or else classic PVH is broken, but I
>>> don't know how much we care about classic PVH any longer.
>> The old check excluded PVHv1 (due to it depending on HAP), as
>> does the new check (in a more explicit way), so I don't see what's
>> wrong here.
> Right, I guess this is caused by e94ce5, which did:
>
>  case EXIT_REASON_CPUID:
>  {
> -int rc;
> -
> -if ( is_pvh_vcpu(v) )
> -{
> -pv_cpuid(regs);
> -rc = 0;
> -}
> -else
> -rc = vmx_do_cpuid(regs);
> +int rc = vmx_do_cpuid(regs);
>
> Which removed the special casing for the PVH CPUID, and I assume pv_cpuid used
> to remove the VT-x extensions from the output of CPUID?

PVH guests still enter pv_cpuid() via the legacy path in guest_cpuid().

However, PVH cpuid handling was quite broken to start with.  I am not
deliberately trying to make it worse, so your original suggestion should
probably be made (if anyone actually cares).

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

2017-01-24 Thread Roger Pau Monné 
On Tue, Jan 24, 2017 at 08:10:56AM -0700, Jan Beulich wrote:
> >>> On 24.01.17 at 15:38,  wrote:
> > On Wed, Jan 18, 2017 at 07:40:53PM +, Andrew Cooper wrote:
> >> The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
> >> mask, but nothing thusfar has prevented the features being visible in
> >> HVM-based control domains (where there is no toolstack decision to hide the
> >> features).
> >> 
> >> As a side effect of calling nestedhvm_enabled() earlier during domain
> >> creation, it needs to cope with the params[] array array not having been
> >> allocated.
> >> 
> >> Reported-by: Roger Pau Monné 
> >> Signed-off-by: Andrew Cooper 
> >> ---
> >> CC: Jan Beulich 
> >> CC: Roger Pau Monné 
> >> ---
> >>  xen/arch/x86/cpuid.c | 25 ++---
> >>  xen/arch/x86/hvm/nestedhvm.c |  3 ++-
> >>  2 files changed, 20 insertions(+), 8 deletions(-)
> >> 
> >> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> >> index eb829d7..7b9af1b 100644
> >> --- a/xen/arch/x86/cpuid.c
> >> +++ b/xen/arch/x86/cpuid.c
> >> @@ -3,6 +3,7 @@
> >>  #include 
> >>  #include 
> >>  #include 
> >> +#include 
> >>  #include 
> >>  #include 
> >>  #include 
> >> @@ -361,14 +362,24 @@ void recalculate_cpuid_policy(struct domain *d)
> >>  cpuid_policy_to_featureset(p, fs);
> >>  cpuid_policy_to_featureset(max, max_fs);
> >>  
> >> -/*
> >> - * HVM domains using Shadow paging have further restrictions on their
> >> - * available paging features.
> >> - */
> >> -if ( is_hvm_domain(d) && !hap_enabled(d) )
> >> +if ( is_hvm_domain(d) )
> > 
> > This should be has_hvm_container_domain or else classic PVH is broken, but I
> > don't know how much we care about classic PVH any longer.
> 
> The old check excluded PVHv1 (due to it depending on HAP), as
> does the new check (in a more explicit way), so I don't see what's
> wrong here.

Right, I guess this is caused by e94ce5, which did:

 case EXIT_REASON_CPUID:
 {
-int rc;
-
-if ( is_pvh_vcpu(v) )
-{
-pv_cpuid(regs);
-rc = 0;
-}
-else
-rc = vmx_do_cpuid(regs);
+int rc = vmx_do_cpuid(regs);

Which removed the special casing for the PVH CPUID, and I assume pv_cpuid used
to remove the VT-x extensions from the output of CPUID?

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

2017-01-24 Thread Jan Beulich 
>>> On 24.01.17 at 15:38,  wrote:
> On Wed, Jan 18, 2017 at 07:40:53PM +, Andrew Cooper wrote:
>> The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
>> mask, but nothing thusfar has prevented the features being visible in
>> HVM-based control domains (where there is no toolstack decision to hide the
>> features).
>> 
>> As a side effect of calling nestedhvm_enabled() earlier during domain
>> creation, it needs to cope with the params[] array array not having been
>> allocated.
>> 
>> Reported-by: Roger Pau Monné 
>> Signed-off-by: Andrew Cooper 
>> ---
>> CC: Jan Beulich 
>> CC: Roger Pau Monné 
>> ---
>>  xen/arch/x86/cpuid.c | 25 ++---
>>  xen/arch/x86/hvm/nestedhvm.c |  3 ++-
>>  2 files changed, 20 insertions(+), 8 deletions(-)
>> 
>> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
>> index eb829d7..7b9af1b 100644
>> --- a/xen/arch/x86/cpuid.c
>> +++ b/xen/arch/x86/cpuid.c
>> @@ -3,6 +3,7 @@
>>  #include 
>>  #include 
>>  #include 
>> +#include 
>>  #include 
>>  #include 
>>  #include 
>> @@ -361,14 +362,24 @@ void recalculate_cpuid_policy(struct domain *d)
>>  cpuid_policy_to_featureset(p, fs);
>>  cpuid_policy_to_featureset(max, max_fs);
>>  
>> -/*
>> - * HVM domains using Shadow paging have further restrictions on their
>> - * available paging features.
>> - */
>> -if ( is_hvm_domain(d) && !hap_enabled(d) )
>> +if ( is_hvm_domain(d) )
> 
> This should be has_hvm_container_domain or else classic PVH is broken, but I
> don't know how much we care about classic PVH any longer.

The old check excluded PVHv1 (due to it depending on HAP), as
does the new check (in a more explicit way), so I don't see what's
wrong here.

Jan

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

2017-01-24 Thread Roger Pau Monné 
On Wed, Jan 18, 2017 at 07:40:53PM +, Andrew Cooper wrote:
> The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
> mask, but nothing thusfar has prevented the features being visible in
> HVM-based control domains (where there is no toolstack decision to hide the
> features).
> 
> As a side effect of calling nestedhvm_enabled() earlier during domain
> creation, it needs to cope with the params[] array array not having been
> allocated.
> 
> Reported-by: Roger Pau Monné 
> Signed-off-by: Andrew Cooper 
> ---
> CC: Jan Beulich 
> CC: Roger Pau Monné 
> ---
>  xen/arch/x86/cpuid.c | 25 ++---
>  xen/arch/x86/hvm/nestedhvm.c |  3 ++-
>  2 files changed, 20 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
> index eb829d7..7b9af1b 100644
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -3,6 +3,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -361,14 +362,24 @@ void recalculate_cpuid_policy(struct domain *d)
>  cpuid_policy_to_featureset(p, fs);
>  cpuid_policy_to_featureset(max, max_fs);
>  
> -/*
> - * HVM domains using Shadow paging have further restrictions on their
> - * available paging features.
> - */
> -if ( is_hvm_domain(d) && !hap_enabled(d) )
> +if ( is_hvm_domain(d) )

This should be has_hvm_container_domain or else classic PVH is broken, but I
don't know how much we care about classic PVH any longer.

Roger.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

2017-01-20 Thread Jan Beulich 
>>> On 18.01.17 at 20:40,  wrote:
> The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
> mask, but nothing thusfar has prevented the features being visible in
> HVM-based control domains (where there is no toolstack decision to hide the
> features).
> 
> As a side effect of calling nestedhvm_enabled() earlier during domain
> creation, it needs to cope with the params[] array array not having been
> allocated.
> 
> Reported-by: Roger Pau Monné 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Jan Beulich 


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

2017-01-19 Thread Andrew Cooper 
On 19/01/17 03:56, Doug Goldstein wrote:
> On 1/18/17 2:40 PM, Andrew Cooper wrote:
>> The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
>> mask, but nothing thusfar has prevented the features being visible in
> thus far? Could be the difference between British English and American
> English.

Just a lack of a space on my behalf.

>
>> HVM-based control domains (where there is no toolstack decision to hide the
>> features).
>>
>> As a side effect of calling nestedhvm_enabled() earlier during domain
>> creation, it needs to cope with the params[] array array not having been
> array array?

Both fixed.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

2017-01-18 Thread Doug Goldstein 
On 1/18/17 2:40 PM, Andrew Cooper wrote:
> The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
> mask, but nothing thusfar has prevented the features being visible in

thus far? Could be the difference between British English and American
English.

> HVM-based control domains (where there is no toolstack decision to hide the
> features).
> 
> As a side effect of calling nestedhvm_enabled() earlier during domain
> creation, it needs to cope with the params[] array array not having been

array array?

> allocated.
> 
> Reported-by: Roger Pau Monné 
> Signed-off-by: Andrew Cooper 

Reviewed-by: Doug Goldstein 

-- 
Doug Goldstein



signature.asc
Description: OpenPGP digital signature
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

[Xen-devel] [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains

2017-01-18 Thread Andrew Cooper 
The VT-x/SVM features are hidden from PV dom0 by the pv_featureset[] upper
mask, but nothing thusfar has prevented the features being visible in
HVM-based control domains (where there is no toolstack decision to hide the
features).

As a side effect of calling nestedhvm_enabled() earlier during domain
creation, it needs to cope with the params[] array array not having been
allocated.

Reported-by: Roger Pau Monné 
Signed-off-by: Andrew Cooper 
---
CC: Jan Beulich 
CC: Roger Pau Monné 
---
 xen/arch/x86/cpuid.c | 25 ++---
 xen/arch/x86/hvm/nestedhvm.c |  3 ++-
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index eb829d7..7b9af1b 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -3,6 +3,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -361,14 +362,24 @@ void recalculate_cpuid_policy(struct domain *d)
 cpuid_policy_to_featureset(p, fs);
 cpuid_policy_to_featureset(max, max_fs);
 
-/*
- * HVM domains using Shadow paging have further restrictions on their
- * available paging features.
- */
-if ( is_hvm_domain(d) && !hap_enabled(d) )
+if ( is_hvm_domain(d) )
 {
-for ( i = 0; i < ARRAY_SIZE(max_fs); i++ )
-max_fs[i] &= hvm_shadow_featuremask[i];
+/*
+ * HVM domains using Shadow paging have further restrictions on their
+ * available paging features.
+ */
+if ( !hap_enabled(d) )
+{
+for ( i = 0; i < ARRAY_SIZE(max_fs); i++ )
+max_fs[i] &= hvm_shadow_featuremask[i];
+}
+
+/* Hide nested-virt if it hasn't been explicitly configured. */
+if ( !nestedhvm_enabled(d) )
+{
+__clear_bit(X86_FEATURE_VMX, max_fs);
+__clear_bit(X86_FEATURE_SVM, max_fs);
+}
 }
 
 /*
diff --git a/xen/arch/x86/hvm/nestedhvm.c b/xen/arch/x86/hvm/nestedhvm.c
index a400d55..f2f7469 100644
--- a/xen/arch/x86/hvm/nestedhvm.c
+++ b/xen/arch/x86/hvm/nestedhvm.c
@@ -29,7 +29,8 @@ static unsigned long *shadow_io_bitmap[3];
 /* Nested HVM on/off per domain */
 bool nestedhvm_enabled(const struct domain *d)
 {
-return is_hvm_domain(d) && d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM];
+return is_hvm_domain(d) && d->arch.hvm_domain.params &&
+d->arch.hvm_domain.params[HVM_PARAM_NESTEDHVM];
 }
 
 /* Nested VCPU */
-- 
2.1.4


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel