* [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
2017-01-18 19:40 [PATCH 0/6] x86/cpuid: Handling of simple leaves in guest_cpuid() Andrew Cooper
@ 2017-01-18 19:40 ` Andrew Cooper
2017-01-19 3:56 ` Doug Goldstein
` (2 more replies)
2017-01-18 19:40 ` [PATCH 2/6] x86/cpuid: Remove BUG_ON() condition from guest_cpuid() Andrew Cooper
` (4 subsequent siblings)
5 siblings, 3 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-18 19:40 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich, Roger Pau Monné
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é <roger.pau@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
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 <xen/sched.h>
#include <asm/cpuid.h>
#include <asm/hvm/hvm.h>
+#include <asm/hvm/nestedhvm.h>
#include <asm/hvm/vmx/vmcs.h>
#include <asm/processor.h>
#include <asm/xstate.h>
@@ -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
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
2017-01-18 19:40 ` [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains Andrew Cooper
@ 2017-01-19 3:56 ` Doug Goldstein
2017-01-19 14:13 ` Andrew Cooper
2017-01-20 15:44 ` Jan Beulich
2017-01-24 14:38 ` Roger Pau Monné
2 siblings, 1 reply; 27+ messages in thread
From: Doug Goldstein @ 2017-01-19 3:56 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich, Roger Pau Monné
[-- Attachment #1.1.1: Type: text/plain, Size: 753 bytes --]
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é <roger.pau@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
2017-01-19 3:56 ` Doug Goldstein
@ 2017-01-19 14:13 ` Andrew Cooper
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-19 14:13 UTC (permalink / raw)
To: Doug Goldstein, Xen-devel; +Cc: Jan Beulich, Roger Pau Monné
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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
2017-01-18 19:40 ` [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains Andrew Cooper
2017-01-19 3:56 ` Doug Goldstein
@ 2017-01-20 15:44 ` Jan Beulich
2017-01-24 14:38 ` Roger Pau Monné
2 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2017-01-20 15:44 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel, roger.pau
>>> On 18.01.17 at 20:40, <andrew.cooper3@citrix.com> 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é <roger.pau@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
2017-01-18 19:40 ` [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains Andrew Cooper
2017-01-19 3:56 ` Doug Goldstein
2017-01-20 15:44 ` Jan Beulich
@ 2017-01-24 14:38 ` Roger Pau Monné
2017-01-24 15:10 ` Jan Beulich
2 siblings, 1 reply; 27+ messages in thread
From: Roger Pau Monné @ 2017-01-24 14:38 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Jan Beulich, Xen-devel
On Wed, Jan 18, 2017 at 07:40:53PM +0000, 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é <roger.pau@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> ---
> 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 <xen/sched.h>
> #include <asm/cpuid.h>
> #include <asm/hvm/hvm.h>
> +#include <asm/hvm/nestedhvm.h>
> #include <asm/hvm/vmx/vmcs.h>
> #include <asm/processor.h>
> #include <asm/xstate.h>
> @@ -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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
2017-01-24 14:38 ` Roger Pau Monné
@ 2017-01-24 15:10 ` Jan Beulich
2017-01-24 15:41 ` Roger Pau Monné
0 siblings, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2017-01-24 15:10 UTC (permalink / raw)
To: Roger Pau Monné; +Cc: Andrew Cooper, Xen-devel
>>> On 24.01.17 at 15:38, <roger.pau@citrix.com> wrote:
> On Wed, Jan 18, 2017 at 07:40:53PM +0000, 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é <roger.pau@citrix.com>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>> 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 <xen/sched.h>
>> #include <asm/cpuid.h>
>> #include <asm/hvm/hvm.h>
>> +#include <asm/hvm/nestedhvm.h>
>> #include <asm/hvm/vmx/vmcs.h>
>> #include <asm/processor.h>
>> #include <asm/xstate.h>
>> @@ -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
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
2017-01-24 15:10 ` Jan Beulich
@ 2017-01-24 15:41 ` Roger Pau Monné
2017-01-24 16:17 ` Andrew Cooper
0 siblings, 1 reply; 27+ messages in thread
From: Roger Pau Monné @ 2017-01-24 15:41 UTC (permalink / raw)
To: Jan Beulich; +Cc: Andrew Cooper, Xen-devel
On Tue, Jan 24, 2017 at 08:10:56AM -0700, Jan Beulich wrote:
> >>> On 24.01.17 at 15:38, <roger.pau@citrix.com> wrote:
> > On Wed, Jan 18, 2017 at 07:40:53PM +0000, 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é <roger.pau@citrix.com>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Jan Beulich <JBeulich@suse.com>
> >> CC: Roger Pau Monné <roger.pau@citrix.com>
> >> ---
> >> 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 <xen/sched.h>
> >> #include <asm/cpuid.h>
> >> #include <asm/hvm/hvm.h>
> >> +#include <asm/hvm/nestedhvm.h>
> >> #include <asm/hvm/vmx/vmcs.h>
> >> #include <asm/processor.h>
> >> #include <asm/xstate.h>
> >> @@ -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
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains
2017-01-24 15:41 ` Roger Pau Monné
@ 2017-01-24 16:17 ` Andrew Cooper
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-24 16:17 UTC (permalink / raw)
To: Roger Pau Monné, Jan Beulich; +Cc: Xen-devel
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, <roger.pau@citrix.com> wrote:
>>> On Wed, Jan 18, 2017 at 07:40:53PM +0000, 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é <roger.pau@citrix.com>
>>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> ---
>>>> CC: Jan Beulich <JBeulich@suse.com>
>>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>>> ---
>>>> 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 <xen/sched.h>
>>>> #include <asm/cpuid.h>
>>>> #include <asm/hvm/hvm.h>
>>>> +#include <asm/hvm/nestedhvm.h>
>>>> #include <asm/hvm/vmx/vmcs.h>
>>>> #include <asm/processor.h>
>>>> #include <asm/xstate.h>
>>>> @@ -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
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 2/6] x86/cpuid: Remove BUG_ON() condition from guest_cpuid()
2017-01-18 19:40 [PATCH 0/6] x86/cpuid: Handling of simple leaves in guest_cpuid() Andrew Cooper
2017-01-18 19:40 ` [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains Andrew Cooper
@ 2017-01-18 19:40 ` Andrew Cooper
2017-01-19 3:57 ` Doug Goldstein
2017-01-20 15:45 ` Jan Beulich
2017-01-18 19:40 ` [PATCH 3/6] x86/cpuid: Handle leaf 0 in guest_cpuid() Andrew Cooper
` (3 subsequent siblings)
5 siblings, 2 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-18 19:40 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Include a min() against the appropriate ARRAY_SIZE(), and ASSERT() that
max_subleaf is within ARRAY_SIZE().
This is more robust to unexpected problems in a release build of Xen.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/arch/x86/cpuid.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 7b9af1b..076fab3 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -856,10 +856,11 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
switch ( leaf )
{
case 0x7:
- if ( subleaf > p->feat.max_subleaf )
+ ASSERT(p->feat.max_subleaf < ARRAY_SIZE(p->feat.raw));
+ if ( subleaf > min_t(uint32_t, p->feat.max_subleaf,
+ ARRAY_SIZE(p->feat.raw) - 1) )
return;
- BUG_ON(subleaf >= ARRAY_SIZE(p->feat.raw));
*res = p->feat.raw[subleaf];
break;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 2/6] x86/cpuid: Remove BUG_ON() condition from guest_cpuid()
2017-01-18 19:40 ` [PATCH 2/6] x86/cpuid: Remove BUG_ON() condition from guest_cpuid() Andrew Cooper
@ 2017-01-19 3:57 ` Doug Goldstein
2017-01-20 15:45 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Doug Goldstein @ 2017-01-19 3:57 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich
[-- Attachment #1.1.1: Type: text/plain, Size: 373 bytes --]
On 1/18/17 2:40 PM, Andrew Cooper wrote:
> Include a min() against the appropriate ARRAY_SIZE(), and ASSERT() that
> max_subleaf is within ARRAY_SIZE().
>
> This is more robust to unexpected problems in a release build of Xen.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 2/6] x86/cpuid: Remove BUG_ON() condition from guest_cpuid()
2017-01-18 19:40 ` [PATCH 2/6] x86/cpuid: Remove BUG_ON() condition from guest_cpuid() Andrew Cooper
2017-01-19 3:57 ` Doug Goldstein
@ 2017-01-20 15:45 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2017-01-20 15:45 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 18.01.17 at 20:40, <andrew.cooper3@citrix.com> wrote:
> Include a min() against the appropriate ARRAY_SIZE(), and ASSERT() that
> max_subleaf is within ARRAY_SIZE().
>
> This is more robust to unexpected problems in a release build of Xen.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 3/6] x86/cpuid: Handle leaf 0 in guest_cpuid()
2017-01-18 19:40 [PATCH 0/6] x86/cpuid: Handling of simple leaves in guest_cpuid() Andrew Cooper
2017-01-18 19:40 ` [PATCH 1/6] x86/cpuid: Hide VT-x/SVM from HVM-based control domains Andrew Cooper
2017-01-18 19:40 ` [PATCH 2/6] x86/cpuid: Remove BUG_ON() condition from guest_cpuid() Andrew Cooper
@ 2017-01-18 19:40 ` Andrew Cooper
2017-01-19 4:02 ` Doug Goldstein
2017-01-20 15:48 ` Jan Beulich
2017-01-18 19:40 ` [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 " Andrew Cooper
` (2 subsequent siblings)
5 siblings, 2 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-18 19:40 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Calculate a domains x86_vendor early in recalculate_cpuid_policy(); subsequent
patches need to make other recalculation decisions based on it.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/arch/x86/cpuid.c | 13 ++++++++++++-
xen/arch/x86/domctl.c | 10 ++--------
xen/include/asm-x86/cpuid.h | 4 +++-
3 files changed, 17 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 076fab3..85c829d 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -355,6 +355,9 @@ void recalculate_cpuid_policy(struct domain *d)
uint32_t fs[FSCAPINTS], max_fs[FSCAPINTS];
unsigned int i;
+ p->x86_vendor = get_cpu_vendor(p->basic.vendor_ebx, p->basic.vendor_ecx,
+ p->basic.vendor_edx, gcv_guest);
+
p->basic.max_leaf = min(p->basic.max_leaf, max->basic.max_leaf);
p->feat.max_subleaf = min(p->feat.max_subleaf, max->feat.max_subleaf);
p->extd.max_leaf = min(p->extd.max_leaf, max->extd.max_leaf);
@@ -677,6 +680,7 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res)
*res = EMPTY_LEAF;
break;
+ case 0x0:
case 0x7:
case XSTATE_CPUID:
ASSERT_UNREACHABLE();
@@ -825,6 +829,7 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res)
res->a = 0;
break;
+ case 0x0:
case 0x7:
case XSTATE_CPUID:
ASSERT_UNREACHABLE();
@@ -850,7 +855,9 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
switch ( leaf )
{
case 0 ... CPUID_GUEST_NR_BASIC - 1:
- if ( leaf > p->basic.max_leaf )
+ ASSERT(p->basic.max_leaf < ARRAY_SIZE(p->basic.raw));
+ if ( leaf > min_t(uint32_t, p->basic.max_leaf,
+ ARRAY_SIZE(p->basic.raw) - 1) )
return;
switch ( leaf )
@@ -873,6 +880,10 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
default:
goto legacy;
+
+ case 0x0:
+ *res = p->basic.raw[leaf];
+ break;
}
break;
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index a3f51f3..8e5259f 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -92,7 +92,7 @@ static int update_domain_cpuid_info(struct domain *d,
{
struct cpuid_policy *p = d->arch.cpuid;
const struct cpuid_leaf leaf = { ctl->eax, ctl->ebx, ctl->ecx, ctl->edx };
- int rc;
+ int rc, old_vendor = p->x86_vendor;
/*
* Skip update for leaves we don't care about. This avoids the overhead
@@ -155,11 +155,7 @@ static int update_domain_cpuid_info(struct domain *d,
switch ( ctl->input[0] )
{
- case 0: {
- int old_vendor = p->x86_vendor;
-
- p->x86_vendor = get_cpu_vendor(ctl->ebx, ctl->ecx, ctl->edx, gcv_guest);
-
+ case 0:
if ( is_hvm_domain(d) && (p->x86_vendor != old_vendor) )
{
struct vcpu *v;
@@ -167,9 +163,7 @@ static int update_domain_cpuid_info(struct domain *d,
for_each_vcpu( d, v )
hvm_update_guest_vendor(v);
}
-
break;
- }
case 1:
if ( is_pv_domain(d) && ((levelling_caps & LCAP_1cd) == LCAP_1cd) )
diff --git a/xen/include/asm-x86/cpuid.h b/xen/include/asm-x86/cpuid.h
index e5140ca..299a026 100644
--- a/xen/include/asm-x86/cpuid.h
+++ b/xen/include/asm-x86/cpuid.h
@@ -81,6 +81,7 @@ struct cpuid_policy
* - All of the feat and xstate unions
* - max_{,sub}leaf
* - All FEATURESET_* words
+ * - Low short vendor infomation
*
* Per-domain objects:
*
@@ -88,6 +89,7 @@ struct cpuid_policy
* - All of the feat and xstate unions
* - max_{,sub}leaf
* - All FEATURESET_* words
+ * - Low short vendor infomation
*
* Everything else should be considered inaccurate, and not necesserily 0.
*/
@@ -101,7 +103,7 @@ struct cpuid_policy
struct cpuid_leaf raw[CPUID_GUEST_NR_BASIC];
struct {
/* Leaf 0x0 - Max and vendor. */
- uint32_t max_leaf, /* b */:32, /* c */:32, /* d */:32;
+ uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
/* Leaf 0x1 - Family/model/stepping and features. */
uint32_t raw_fms, /* b */:32;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 in guest_cpuid()
2017-01-18 19:40 [PATCH 0/6] x86/cpuid: Handling of simple leaves in guest_cpuid() Andrew Cooper
` (2 preceding siblings ...)
2017-01-18 19:40 ` [PATCH 3/6] x86/cpuid: Handle leaf 0 in guest_cpuid() Andrew Cooper
@ 2017-01-18 19:40 ` Andrew Cooper
2017-01-19 4:03 ` Doug Goldstein
2017-01-20 15:58 ` Jan Beulich
2017-01-18 19:40 ` [PATCH 5/6] x86/cpuid: Handle the long vendor string " Andrew Cooper
2017-01-18 19:40 ` [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once Andrew Cooper
5 siblings, 2 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-18 19:40 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
The calculations for p->extd.max_leaf are reworked to force a value of at
least 0x80000000, and to take the domains chosen vendor into account when
clamping maximum value.
The high short vendor information is clobbered or duplicated according to the
chosen vendor.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/arch/x86/cpuid.c | 48 +++++++++++++++++++++++++++++++++++++++------
xen/include/asm-x86/cpuid.h | 6 +++---
2 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 85c829d..dc76cf4 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -163,6 +163,24 @@ static void recalculate_xstate(struct cpuid_policy *p)
}
}
+static void recalculate_common(struct cpuid_policy *p)
+{
+ switch ( p->x86_vendor )
+ {
+ case X86_VENDOR_INTEL:
+ p->extd.vendor_ebx = 0;
+ p->extd.vendor_ecx = 0;
+ p->extd.vendor_edx = 0;
+ break;
+
+ case X86_VENDOR_AMD:
+ p->extd.vendor_ebx = p->basic.vendor_ebx;
+ p->extd.vendor_ecx = p->basic.vendor_ecx;
+ p->extd.vendor_edx = p->basic.vendor_edx;
+ break;
+ }
+}
+
static void __init calculate_raw_policy(void)
{
struct cpuid_policy *p = &raw_policy;
@@ -227,12 +245,12 @@ static void __init calculate_host_policy(void)
min_t(uint32_t, p->basic.max_leaf, ARRAY_SIZE(p->basic.raw) - 1);
p->feat.max_subleaf =
min_t(uint32_t, p->feat.max_subleaf, ARRAY_SIZE(p->feat.raw) - 1);
- p->extd.max_leaf =
- min_t(uint32_t, p->extd.max_leaf,
- 0x80000000u + ARRAY_SIZE(p->extd.raw) - 1);
+ p->extd.max_leaf = 0x80000000 | min_t(uint32_t, p->extd.max_leaf & 0xffff,
+ ARRAY_SIZE(p->extd.raw) - 1);
cpuid_featureset_to_policy(boot_cpu_data.x86_capability, p);
recalculate_xstate(p);
+ recalculate_common(p);
}
static void __init calculate_pv_max_policy(void)
@@ -360,7 +378,10 @@ void recalculate_cpuid_policy(struct domain *d)
p->basic.max_leaf = min(p->basic.max_leaf, max->basic.max_leaf);
p->feat.max_subleaf = min(p->feat.max_subleaf, max->feat.max_subleaf);
- p->extd.max_leaf = min(p->extd.max_leaf, max->extd.max_leaf);
+ p->extd.max_leaf = 0x80000000 | min(p->extd.max_leaf & 0xffff,
+ (p->x86_vendor == X86_VENDOR_AMD
+ ? CPUID_GUEST_NR_EXTD_AMD
+ : CPUID_GUEST_NR_EXTD_INTEL) - 1);
cpuid_policy_to_featureset(p, fs);
cpuid_policy_to_featureset(max, max_fs);
@@ -428,6 +449,7 @@ void recalculate_cpuid_policy(struct domain *d)
cpuid_featureset_to_policy(fs, p);
recalculate_xstate(p);
+ recalculate_common(p);
}
int init_domain_cpuid_policy(struct domain *d)
@@ -683,6 +705,7 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res)
case 0x0:
case 0x7:
case XSTATE_CPUID:
+ case 0x80000000:
ASSERT_UNREACHABLE();
/* Now handled in guest_cpuid(). */
}
@@ -832,6 +855,7 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res)
case 0x0:
case 0x7:
case XSTATE_CPUID:
+ case 0x80000000:
ASSERT_UNREACHABLE();
/* Now handled in guest_cpuid(). */
}
@@ -901,9 +925,21 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
return cpuid_hypervisor_leaves(v, leaf, subleaf, res);
case 0x80000000 ... 0x80000000 + CPUID_GUEST_NR_EXTD - 1:
- if ( leaf > p->extd.max_leaf )
+ ASSERT((p->extd.max_leaf & 0xffff) < ARRAY_SIZE(p->extd.raw));
+ if ( (leaf & 0xffff) > min_t(uint32_t, p->extd.max_leaf & 0xffff,
+ ARRAY_SIZE(p->extd.raw) - 1) )
return;
- goto legacy;
+
+ switch ( leaf )
+ {
+ default:
+ goto legacy;
+
+ case 0x80000000:
+ *res = p->extd.raw[leaf & 0xffff];
+ break;
+ }
+ break;
default:
return;
diff --git a/xen/include/asm-x86/cpuid.h b/xen/include/asm-x86/cpuid.h
index 299a026..4712b73 100644
--- a/xen/include/asm-x86/cpuid.h
+++ b/xen/include/asm-x86/cpuid.h
@@ -81,7 +81,7 @@ struct cpuid_policy
* - All of the feat and xstate unions
* - max_{,sub}leaf
* - All FEATURESET_* words
- * - Low short vendor infomation
+ * - Short vendor infomation
*
* Per-domain objects:
*
@@ -89,7 +89,7 @@ struct cpuid_policy
* - All of the feat and xstate unions
* - max_{,sub}leaf
* - All FEATURESET_* words
- * - Low short vendor infomation
+ * - Short vendor infomation
*
* Everything else should be considered inaccurate, and not necesserily 0.
*/
@@ -168,7 +168,7 @@ struct cpuid_policy
struct cpuid_leaf raw[CPUID_GUEST_NR_EXTD];
struct {
/* Leaf 0x80000000 - Max and vendor. */
- uint32_t max_leaf, /* b */:32, /* c */:32, /* d */:32;
+ uint32_t max_leaf, vendor_ebx, vendor_ecx, vendor_edx;
/* Leaf 0x80000001 - Family/model/stepping and features. */
uint32_t /* a */:32, /* b */:32;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 in guest_cpuid()
2017-01-18 19:40 ` [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 " Andrew Cooper
@ 2017-01-19 4:03 ` Doug Goldstein
2017-01-20 15:58 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Doug Goldstein @ 2017-01-19 4:03 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich
[-- Attachment #1.1.1: Type: text/plain, Size: 466 bytes --]
On 1/18/17 2:40 PM, Andrew Cooper wrote:
> The calculations for p->extd.max_leaf are reworked to force a value of at
> least 0x80000000, and to take the domains chosen vendor into account when
> clamping maximum value.
>
> The high short vendor information is clobbered or duplicated according to the
> chosen vendor.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 in guest_cpuid()
2017-01-18 19:40 ` [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 " Andrew Cooper
2017-01-19 4:03 ` Doug Goldstein
@ 2017-01-20 15:58 ` Jan Beulich
2017-01-20 16:02 ` Andrew Cooper
1 sibling, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2017-01-20 15:58 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 18.01.17 at 20:40, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/cpuid.c
> +++ b/xen/arch/x86/cpuid.c
> @@ -163,6 +163,24 @@ static void recalculate_xstate(struct cpuid_policy *p)
> }
> }
>
> +static void recalculate_common(struct cpuid_policy *p)
> +{
> + switch ( p->x86_vendor )
> + {
> + case X86_VENDOR_INTEL:
> + p->extd.vendor_ebx = 0;
> + p->extd.vendor_ecx = 0;
> + p->extd.vendor_edx = 0;
> + break;
> +
> + case X86_VENDOR_AMD:
> + p->extd.vendor_ebx = p->basic.vendor_ebx;
> + p->extd.vendor_ecx = p->basic.vendor_ecx;
> + p->extd.vendor_edx = p->basic.vendor_edx;
> + break;
> + }
> +}
I find the word "common" in the name here not very indicative
of what the function does, especially with ...
> @@ -227,12 +245,12 @@ static void __init calculate_host_policy(void)
> min_t(uint32_t, p->basic.max_leaf, ARRAY_SIZE(p->basic.raw) - 1);
> p->feat.max_subleaf =
> min_t(uint32_t, p->feat.max_subleaf, ARRAY_SIZE(p->feat.raw) - 1);
> - p->extd.max_leaf =
> - min_t(uint32_t, p->extd.max_leaf,
> - 0x80000000u + ARRAY_SIZE(p->extd.raw) - 1);
> + p->extd.max_leaf = 0x80000000 | min_t(uint32_t, p->extd.max_leaf & 0xffff,
> + ARRAY_SIZE(p->extd.raw) - 1);
>
> cpuid_featureset_to_policy(boot_cpu_data.x86_capability, p);
> recalculate_xstate(p);
> + recalculate_common(p);
> }
... the neighboring call here (which is quite a bit more specific).
Of course possible alternatives depend on what further uses of
this function you do (or do not) plan, but by the name of the
other function here it could be recalculate_extd_vendor().
> @@ -901,9 +925,21 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
> return cpuid_hypervisor_leaves(v, leaf, subleaf, res);
>
> case 0x80000000 ... 0x80000000 + CPUID_GUEST_NR_EXTD - 1:
> - if ( leaf > p->extd.max_leaf )
> + ASSERT((p->extd.max_leaf & 0xffff) < ARRAY_SIZE(p->extd.raw));
> + if ( (leaf & 0xffff) > min_t(uint32_t, p->extd.max_leaf & 0xffff,
> + ARRAY_SIZE(p->extd.raw) - 1) )
> return;
> - goto legacy;
> +
> + switch ( leaf )
> + {
> + default:
> + goto legacy;
> +
> + case 0x80000000:
> + *res = p->extd.raw[leaf & 0xffff];
I take it that the plan is to have further leaves and up here, or else
the array index could simply be literal zero.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 in guest_cpuid()
2017-01-20 15:58 ` Jan Beulich
@ 2017-01-20 16:02 ` Andrew Cooper
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-20 16:02 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel
On 20/01/17 15:58, Jan Beulich wrote:
>>>> On 18.01.17 at 20:40, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/cpuid.c
>> +++ b/xen/arch/x86/cpuid.c
>> @@ -163,6 +163,24 @@ static void recalculate_xstate(struct cpuid_policy *p)
>> }
>> }
>>
>> +static void recalculate_common(struct cpuid_policy *p)
>> +{
>> + switch ( p->x86_vendor )
>> + {
>> + case X86_VENDOR_INTEL:
>> + p->extd.vendor_ebx = 0;
>> + p->extd.vendor_ecx = 0;
>> + p->extd.vendor_edx = 0;
>> + break;
>> +
>> + case X86_VENDOR_AMD:
>> + p->extd.vendor_ebx = p->basic.vendor_ebx;
>> + p->extd.vendor_ecx = p->basic.vendor_ecx;
>> + p->extd.vendor_edx = p->basic.vendor_edx;
>> + break;
>> + }
>> +}
> I find the word "common" in the name here not very indicative
> of what the function does, especially with ...
>
>> @@ -227,12 +245,12 @@ static void __init calculate_host_policy(void)
>> min_t(uint32_t, p->basic.max_leaf, ARRAY_SIZE(p->basic.raw) - 1);
>> p->feat.max_subleaf =
>> min_t(uint32_t, p->feat.max_subleaf, ARRAY_SIZE(p->feat.raw) - 1);
>> - p->extd.max_leaf =
>> - min_t(uint32_t, p->extd.max_leaf,
>> - 0x80000000u + ARRAY_SIZE(p->extd.raw) - 1);
>> + p->extd.max_leaf = 0x80000000 | min_t(uint32_t, p->extd.max_leaf & 0xffff,
>> + ARRAY_SIZE(p->extd.raw) - 1);
>>
>> cpuid_featureset_to_policy(boot_cpu_data.x86_capability, p);
>> recalculate_xstate(p);
>> + recalculate_common(p);
>> }
> ... the neighboring call here (which is quite a bit more specific).
> Of course possible alternatives depend on what further uses of
> this function you do (or do not) plan, but by the name of the
> other function here it could be recalculate_extd_vendor().
It adjustments common to the global calculations, and the per-domain
recalculations.
It isn't however just per-vendor adjustments; there are global
adjustments (like clobbering the reserved leaves).
I am open to any naming suggestions.
>
>> @@ -901,9 +925,21 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>> return cpuid_hypervisor_leaves(v, leaf, subleaf, res);
>>
>> case 0x80000000 ... 0x80000000 + CPUID_GUEST_NR_EXTD - 1:
>> - if ( leaf > p->extd.max_leaf )
>> + ASSERT((p->extd.max_leaf & 0xffff) < ARRAY_SIZE(p->extd.raw));
>> + if ( (leaf & 0xffff) > min_t(uint32_t, p->extd.max_leaf & 0xffff,
>> + ARRAY_SIZE(p->extd.raw) - 1) )
>> return;
>> - goto legacy;
>> +
>> + switch ( leaf )
>> + {
>> + default:
>> + goto legacy;
>> +
>> + case 0x80000000:
>> + *res = p->extd.raw[leaf & 0xffff];
> I take it that the plan is to have further leaves and up here, or else
> the array index could simply be literal zero.
I believe you found your answer in the following patch? Eventually
(when the legacy path disappears), this entire switch statement will go.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 5/6] x86/cpuid: Handle the long vendor string in guest_cpuid()
2017-01-18 19:40 [PATCH 0/6] x86/cpuid: Handling of simple leaves in guest_cpuid() Andrew Cooper
` (3 preceding siblings ...)
2017-01-18 19:40 ` [PATCH 4/6] x86/cpuid: Handle leaf 0x80000000 " Andrew Cooper
@ 2017-01-18 19:40 ` Andrew Cooper
2017-01-19 4:03 ` Doug Goldstein
2017-01-20 16:00 ` Jan Beulich
2017-01-18 19:40 ` [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once Andrew Cooper
5 siblings, 2 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-18 19:40 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Leaves 0x80000002 through 0x80000004 are plain ASCII text, and require no
specific recalculation.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/arch/x86/cpuid.c | 3 +++
xen/include/asm-x86/cpuid.h | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index dc76cf4..7926d0b 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -706,6 +706,7 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res)
case 0x7:
case XSTATE_CPUID:
case 0x80000000:
+ case 0x80000002 ... 0x80000004:
ASSERT_UNREACHABLE();
/* Now handled in guest_cpuid(). */
}
@@ -856,6 +857,7 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res)
case 0x7:
case XSTATE_CPUID:
case 0x80000000:
+ case 0x80000002 ... 0x80000004:
ASSERT_UNREACHABLE();
/* Now handled in guest_cpuid(). */
}
@@ -936,6 +938,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
goto legacy;
case 0x80000000:
+ case 0x80000002 ... 0x80000004:
*res = p->extd.raw[leaf & 0xffff];
break;
}
diff --git a/xen/include/asm-x86/cpuid.h b/xen/include/asm-x86/cpuid.h
index 4712b73..d181e9a 100644
--- a/xen/include/asm-x86/cpuid.h
+++ b/xen/include/asm-x86/cpuid.h
@@ -81,7 +81,7 @@ struct cpuid_policy
* - All of the feat and xstate unions
* - max_{,sub}leaf
* - All FEATURESET_* words
- * - Short vendor infomation
+ * - Short and long vendor infomation
*
* Per-domain objects:
*
@@ -89,7 +89,7 @@ struct cpuid_policy
* - All of the feat and xstate unions
* - max_{,sub}leaf
* - All FEATURESET_* words
- * - Short vendor infomation
+ * - Short and long vendor infomation
*
* Everything else should be considered inaccurate, and not necesserily 0.
*/
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 5/6] x86/cpuid: Handle the long vendor string in guest_cpuid()
2017-01-18 19:40 ` [PATCH 5/6] x86/cpuid: Handle the long vendor string " Andrew Cooper
@ 2017-01-19 4:03 ` Doug Goldstein
2017-01-20 16:00 ` Jan Beulich
1 sibling, 0 replies; 27+ messages in thread
From: Doug Goldstein @ 2017-01-19 4:03 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Jan Beulich
[-- Attachment #1.1.1: Type: text/plain, Size: 286 bytes --]
On 1/18/17 2:40 PM, Andrew Cooper wrote:
> Leaves 0x80000002 through 0x80000004 are plain ASCII text, and require no
> specific recalculation.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/6] x86/cpuid: Handle the long vendor string in guest_cpuid()
2017-01-18 19:40 ` [PATCH 5/6] x86/cpuid: Handle the long vendor string " Andrew Cooper
2017-01-19 4:03 ` Doug Goldstein
@ 2017-01-20 16:00 ` Jan Beulich
2017-01-20 16:03 ` Andrew Cooper
1 sibling, 1 reply; 27+ messages in thread
From: Jan Beulich @ 2017-01-20 16:00 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 18.01.17 at 20:40, <andrew.cooper3@citrix.com> wrote:
> Leaves 0x80000002 through 0x80000004 are plain ASCII text, and require no
> specific recalculation.
Do they not? We don't currently get them in line with a perhaps
overridden vendor, but maybe we should?
> @@ -936,6 +938,7 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
> goto legacy;
>
> case 0x80000000:
> + case 0x80000002 ... 0x80000004:
> *res = p->extd.raw[leaf & 0xffff];
> break;
> }
Ah yes, here we go.
In any event
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 5/6] x86/cpuid: Handle the long vendor string in guest_cpuid()
2017-01-20 16:00 ` Jan Beulich
@ 2017-01-20 16:03 ` Andrew Cooper
0 siblings, 0 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-20 16:03 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel
On 20/01/17 16:00, Jan Beulich wrote:
>>>> On 18.01.17 at 20:40, <andrew.cooper3@citrix.com> wrote:
>> Leaves 0x80000002 through 0x80000004 are plain ASCII text, and require no
>> specific recalculation.
> Do they not? We don't currently get them in line with a perhaps
> overridden vendor, but maybe we should?
There is no existing logic doing this. My point here is we don't need
to make any adjustments to the values provided by the toolstack.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once
2017-01-18 19:40 [PATCH 0/6] x86/cpuid: Handling of simple leaves in guest_cpuid() Andrew Cooper
` (4 preceding siblings ...)
2017-01-18 19:40 ` [PATCH 5/6] x86/cpuid: Handle the long vendor string " Andrew Cooper
@ 2017-01-18 19:40 ` Andrew Cooper
2017-01-19 4:03 ` Doug Goldstein
` (2 more replies)
5 siblings, 3 replies; 27+ messages in thread
From: Andrew Cooper @ 2017-01-18 19:40 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Ian Jackson, Wei Liu, Jan Beulich
With accurate vendor information available, the shared bits can be sorted out
during recalculation, rather than at query time in the legacy cpuid path.
This means that:
* Duplication can be dropped from the automatically generated cpuid data.
* The toolstack need not worry about setting them appropriately.
* They can be dropped from the system maximum featuresets.
While editing gen-cpuid.py, reflow some comments which exceeded the expected
line length.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
tools/libxc/xc_cpuid_x86.c | 19 -------------------
xen/arch/x86/cpuid.c | 25 +++++--------------------
xen/tools/gen-cpuid.py | 29 ++++++++++-------------------
3 files changed, 15 insertions(+), 58 deletions(-)
diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 96d6025..918590f 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -41,7 +41,6 @@ enum {
#define DEF_MAX_BASE 0x0000000du
#define DEF_MAX_INTELEXT 0x80000008u
#define DEF_MAX_AMDEXT 0x8000001cu
-#define COMMON_1D CPUID_COMMON_1D_FEATURES
int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps)
{
@@ -712,24 +711,6 @@ static void sanitise_featureset(struct cpuid_domain_info *info)
disabled_features[i] &= ~dfs[i];
}
}
-
- switch ( info->vendor )
- {
- case VENDOR_INTEL:
- /* Intel clears the common bits in e1d. */
- info->featureset[featureword_of(X86_FEATURE_SYSCALL)] &= ~COMMON_1D;
- break;
-
- case VENDOR_AMD:
- /* AMD duplicates the common bits between 1d and e1d. */
- info->featureset[featureword_of(X86_FEATURE_SYSCALL)] =
- ((info->featureset[featureword_of(X86_FEATURE_FPU)] & COMMON_1D) |
- (info->featureset[featureword_of(X86_FEATURE_SYSCALL)] & ~COMMON_1D));
- break;
-
- default:
- break;
- }
}
int xc_cpuid_apply_policy(xc_interface *xch, domid_t domid,
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index 7926d0b..a27a8d6 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -67,18 +67,6 @@ static void sanitise_featureset(uint32_t *fs)
disabled_features[j] &= ~dfs[j];
}
}
-
- /*
- * Sort out shared bits. We are constructing a featureset which needs to
- * be applicable to a cross-vendor case. Intel strictly clears the common
- * bits in e1d, while AMD strictly duplicates them.
- *
- * We duplicate them here to be compatible with AMD while on Intel, and
- * rely on logic closer to the guest to make the featureset stricter if
- * emulating Intel.
- */
- fs[FEATURESET_e1d] = ((fs[FEATURESET_1d] & CPUID_COMMON_1D_FEATURES) |
- (fs[FEATURESET_e1d] & ~CPUID_COMMON_1D_FEATURES));
}
static void recalculate_xstate(struct cpuid_policy *p)
@@ -165,6 +153,8 @@ static void recalculate_xstate(struct cpuid_policy *p)
static void recalculate_common(struct cpuid_policy *p)
{
+ p->extd.e1d &= ~CPUID_COMMON_1D_FEATURES;
+
switch ( p->x86_vendor )
{
case X86_VENDOR_INTEL:
@@ -177,6 +167,8 @@ static void recalculate_common(struct cpuid_policy *p)
p->extd.vendor_ebx = p->basic.vendor_ebx;
p->extd.vendor_ecx = p->basic.vendor_ecx;
p->extd.vendor_edx = p->basic.vendor_edx;
+
+ p->extd.e1d |= p->basic._1d & CPUID_COMMON_1D_FEATURES;
break;
}
}
@@ -665,10 +657,6 @@ static void pv_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res)
res->c = p->extd.e1c;
res->d = p->extd.e1d;
- /* If not emulating AMD, clear the duplicated features in e1d. */
- if ( p->x86_vendor != X86_VENDOR_AMD )
- res->d &= ~CPUID_COMMON_1D_FEATURES;
-
/*
* MTRR used to unconditionally leak into PV guests. They cannot MTRR
* infrastructure at all, and shouldn't be able to see the feature.
@@ -790,11 +778,8 @@ static void hvm_cpuid(uint32_t leaf, uint32_t subleaf, struct cpuid_leaf *res)
res->c = p->extd.e1c;
res->d = p->extd.e1d;
- /* If not emulating AMD, clear the duplicated features in e1d. */
- if ( p->x86_vendor != X86_VENDOR_AMD )
- res->d &= ~CPUID_COMMON_1D_FEATURES;
/* fast-forward MSR_APIC_BASE.EN if it hasn't already been clobbered. */
- else if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
+ if ( vlapic_hw_disabled(vcpu_vlapic(v)) )
res->d &= ~cpufeat_bit(X86_FEATURE_APIC);
/*
diff --git a/xen/tools/gen-cpuid.py b/xen/tools/gen-cpuid.py
index 6212e4f..5cab6db 100755
--- a/xen/tools/gen-cpuid.py
+++ b/xen/tools/gen-cpuid.py
@@ -129,16 +129,7 @@ def crunch_numbers(state):
common_1d = (FPU, VME, DE, PSE, TSC, MSR, PAE, MCE, CX8, APIC,
MTRR, PGE, MCA, CMOV, PAT, PSE36, MMX, FXSR)
- # All known features. Duplicate the common features in e1d
- e1d_base = SYSCALL & ~31
- state.known = featureset_to_uint32s(
- state.names.keys() + [ e1d_base + (x % 32) for x in common_1d ],
- nr_entries)
-
- # Fold common back into names
- for f in common_1d:
- state.names[e1d_base + (f % 32)] = "E1D_" + state.names[f]
-
+ state.known = featureset_to_uint32s(state.names.keys(), nr_entries)
state.common_1d = featureset_to_uint32s(common_1d, 1)[0]
state.special = featureset_to_uint32s(state.raw_special, nr_entries)
state.pv = featureset_to_uint32s(state.raw_pv, nr_entries)
@@ -248,13 +239,14 @@ def crunch_numbers(state):
# standard 3DNow in the earlier K6 processors.
_3DNOW: [_3DNOWEXT],
- # This is just the dependency between AVX512 and AVX2 of XSTATE feature flags.
- # If want to use AVX512, AVX2 must be supported and enabled.
+ # This is just the dependency between AVX512 and AVX2 of XSTATE
+ # feature flags. If want to use AVX512, AVX2 must be supported and
+ # enabled.
AVX2: [AVX512F],
- # AVX512F is taken to mean hardware support for EVEX encoded instructions,
- # 512bit registers, and the instructions themselves. All further AVX512 features
- # are built on top of AVX512F
+ # AVX512F is taken to mean hardware support for EVEX encoded
+ # instructions, 512bit registers, and the instructions themselves. All
+ # further AVX512 features are built on top of AVX512F
AVX512F: [AVX512DQ, AVX512IFMA, AVX512PF, AVX512ER, AVX512CD,
AVX512BW, AVX512VL, AVX512VBMI, AVX512_4VNNIW,
AVX512_4FMAPS, AVX512_VPOPCNTDQ],
@@ -305,10 +297,9 @@ def crunch_numbers(state):
if name and name[0] in "0123456789":
name = "_" + name
- # Don't generate names for the duplicate features, or ones
- # fast-forwarded from other state
- if (name.startswith("E1D_") or
- name in ("APIC", "OSXSAVE", "OSPKE")):
+ # Don't generate names for features fast-forwarded from other
+ # state
+ if name in ("APIC", "OSXSAVE", "OSPKE"):
name = ""
names.append(name.lower())
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once
2017-01-18 19:40 ` [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once Andrew Cooper
@ 2017-01-19 4:03 ` Doug Goldstein
2017-01-19 11:01 ` Wei Liu
2017-01-20 16:06 ` Jan Beulich
2 siblings, 0 replies; 27+ messages in thread
From: Doug Goldstein @ 2017-01-19 4:03 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel; +Cc: Wei Liu, Ian Jackson, Jan Beulich
[-- Attachment #1.1.1: Type: text/plain, Size: 674 bytes --]
On 1/18/17 2:40 PM, Andrew Cooper wrote:
> With accurate vendor information available, the shared bits can be sorted out
> during recalculation, rather than at query time in the legacy cpuid path.
>
> This means that:
> * Duplication can be dropped from the automatically generated cpuid data.
> * The toolstack need not worry about setting them appropriately.
> * They can be dropped from the system maximum featuresets.
>
> While editing gen-cpuid.py, reflow some comments which exceeded the expected
> line length.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Doug Goldstein <cardoe@cardoe.com>
--
Doug Goldstein
[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 959 bytes --]
[-- Attachment #2: Type: text/plain, Size: 127 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once
2017-01-18 19:40 ` [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once Andrew Cooper
2017-01-19 4:03 ` Doug Goldstein
@ 2017-01-19 11:01 ` Wei Liu
2017-01-20 16:06 ` Jan Beulich
2 siblings, 0 replies; 27+ messages in thread
From: Wei Liu @ 2017-01-19 11:01 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Wei Liu, Ian Jackson, Jan Beulich, Xen-devel
On Wed, Jan 18, 2017 at 07:40:58PM +0000, Andrew Cooper wrote:
> With accurate vendor information available, the shared bits can be sorted out
> during recalculation, rather than at query time in the legacy cpuid path.
>
> This means that:
> * Duplication can be dropped from the automatically generated cpuid data.
> * The toolstack need not worry about setting them appropriately.
> * They can be dropped from the system maximum featuresets.
>
> While editing gen-cpuid.py, reflow some comments which exceeded the expected
> line length.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> CC: Wei Liu <wei.liu2@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once
2017-01-18 19:40 ` [PATCH 6/6] x86/cpuid: Only recalculate the shared feature bits once Andrew Cooper
2017-01-19 4:03 ` Doug Goldstein
2017-01-19 11:01 ` Wei Liu
@ 2017-01-20 16:06 ` Jan Beulich
2 siblings, 0 replies; 27+ messages in thread
From: Jan Beulich @ 2017-01-20 16:06 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Ian Jackson, Wei Liu, Xen-devel
>>> On 18.01.17 at 20:40, <andrew.cooper3@citrix.com> wrote:
> @@ -165,6 +153,8 @@ static void recalculate_xstate(struct cpuid_policy *p)
>
> static void recalculate_common(struct cpuid_policy *p)
> {
> + p->extd.e1d &= ~CPUID_COMMON_1D_FEATURES;
> +
> switch ( p->x86_vendor )
> {
> case X86_VENDOR_INTEL:
> @@ -177,6 +167,8 @@ static void recalculate_common(struct cpuid_policy *p)
> p->extd.vendor_ebx = p->basic.vendor_ebx;
> p->extd.vendor_ecx = p->basic.vendor_ecx;
> p->extd.vendor_edx = p->basic.vendor_edx;
> +
> + p->extd.e1d |= p->basic._1d & CPUID_COMMON_1D_FEATURES;
> break;
> }
> }
Coming back to the name of this function, with this new addition in
mind, perhaps recalculate_extd_common()?
Independent of this the patch here is
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 27+ messages in thread