* [PATCH v2] KVM: x86: Add Intel CPUID.1F cpuid emulation support
@ 2019-04-26 3:17 Like Xu
2019-04-26 14:13 ` Sean Christopherson
0 siblings, 1 reply; 5+ messages in thread
From: Like Xu @ 2019-04-26 3:17 UTC (permalink / raw)
To: kvm
Cc: Sean Christopherson, Xiaoyao Li, Paolo Bonzini,
Konrad Rzeszutek Wilk, linux-kernel
Some new systems have multiple software-visible die within each package.
Add support to expose Intel V2 Extended Topology Enumeration Leaf CPUID.1F.
Co-developed-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
==changelog==
v2:
- Apply cpuid.1f check rule on Intel SDM page 3-222 Vol.2A
- Add comment to handle 0x1f anf 0xb in common code
- Reduce check time in a descending-break style
v1: https://lkml.org/lkml/2019/4/22/28
arch/x86/kvm/cpuid.c | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index fd39516..f9b529e 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -425,6 +425,11 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
switch (function) {
case 0:
+ /* Check if the cpuid leaf 0x1f is actually implemented */
+ if (entry->eax >= 0x1f && (cpuid_ebx(0x1f) & 0x0000ffff)) {
+ entry->eax = 0x1f;
+ break;
+ }
entry->eax = min(entry->eax, (u32)(f_intel_pt ? 0x14 : 0xd));
break;
case 1:
@@ -544,7 +549,12 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
entry->edx = edx.full;
break;
}
- /* function 0xb has additional index. */
+ /*
+ * Intel documentation states that 0x1f and 0xb have
+ * identical formats and thus can be handled by common code.
+ * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
+ */
+ case 0x1f:
case 0xb: {
int i, level_type;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] KVM: x86: Add Intel CPUID.1F cpuid emulation support
2019-04-26 3:17 [PATCH v2] KVM: x86: Add Intel CPUID.1F cpuid emulation support Like Xu
@ 2019-04-26 14:13 ` Sean Christopherson
2019-04-26 14:54 ` Xiaoyao Li
0 siblings, 1 reply; 5+ messages in thread
From: Sean Christopherson @ 2019-04-26 14:13 UTC (permalink / raw)
To: Like Xu
Cc: kvm, Xiaoyao Li, Paolo Bonzini, Konrad Rzeszutek Wilk, linux-kernel
On Fri, Apr 26, 2019 at 11:17:52AM +0800, Like Xu wrote:
> Some new systems have multiple software-visible die within each package.
> Add support to expose Intel V2 Extended Topology Enumeration Leaf CPUID.1F.
>
> Co-developed-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>
> ==changelog==
> v2:
> - Apply cpuid.1f check rule on Intel SDM page 3-222 Vol.2A
> - Add comment to handle 0x1f anf 0xb in common code
> - Reduce check time in a descending-break style
>
> v1: https://lkml.org/lkml/2019/4/22/28
>
> arch/x86/kvm/cpuid.c | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index fd39516..f9b529e 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -425,6 +425,11 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
>
> switch (function) {
> case 0:
> + /* Check if the cpuid leaf 0x1f is actually implemented */
> + if (entry->eax >= 0x1f && (cpuid_ebx(0x1f) & 0x0000ffff)) {
Restricting the check to bits 15:0 is unnecessary, the SDM explicitly
states that EBX will be zero for invalid sub-leaves:
For sub-leaves that return an invalid level-type of 0 in ECX[15:8];
EAX and EBX will return 0.
This code is merely checking for the existence of CPUID.1F, nothing will
break if future CPUs provide additional information, i.e. checking for a
valid sub-leaf is sufficient.
That being said, if you insist on restricting the check to non-reserved
bits then I think the earlier suggestion of "cpuid_ecx(0x1f) & 0x0000ff00"
makes more sense since the SDM clearly intends ECX to be used to detect
valid vs. invalid levels.
> + entry->eax = 0x1f;
> + break;
I find if/else easier to follow than the separate break, but either option
works for me.
> + }
> entry->eax = min(entry->eax, (u32)(f_intel_pt ? 0x14 : 0xd));
> break;
> case 1:
> @@ -544,7 +549,12 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function,
> entry->edx = edx.full;
> break;
> }
> - /* function 0xb has additional index. */
> + /*
> + * Intel documentation states that 0x1f and 0xb have
> + * identical formats and thus can be handled by common code.
> + * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
> + */
It's probably safe to assume anyone reading reading this code is already
all too aware of Intel's propensity for dumping CPUID enumeration into the
CPUID entry in the ISR. Maybe shorten this to something like:
/*
* Per Intel's SDM, 0x1f is a superset of 0xb, thus they can be handled
* by common code.
*/
> + case 0x1f:
> case 0xb: {
> int i, level_type;
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] KVM: x86: Add Intel CPUID.1F cpuid emulation support
2019-04-26 14:13 ` Sean Christopherson
@ 2019-04-26 14:54 ` Xiaoyao Li
2019-04-26 15:04 ` Sean Christopherson
0 siblings, 1 reply; 5+ messages in thread
From: Xiaoyao Li @ 2019-04-26 14:54 UTC (permalink / raw)
To: Sean Christopherson, Like Xu
Cc: kvm, Paolo Bonzini, Konrad Rzeszutek Wilk, linux-kernel
On Fri, 2019-04-26 at 07:13 -0700, Sean Christopherson wrote:
> On Fri, Apr 26, 2019 at 11:17:52AM +0800, Like Xu wrote:
> > Some new systems have multiple software-visible die within each package.
> > Add support to expose Intel V2 Extended Topology Enumeration Leaf CPUID.1F.
> >
> > Co-developed-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> > Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > ---
> >
> > ==changelog==
> > v2:
> > - Apply cpuid.1f check rule on Intel SDM page 3-222 Vol.2A
> > - Add comment to handle 0x1f anf 0xb in common code
> > - Reduce check time in a descending-break style
> >
> > v1: https://lkml.org/lkml/2019/4/22/28
> >
> > arch/x86/kvm/cpuid.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index fd39516..f9b529e 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -425,6 +425,11 @@ static inline int __do_cpuid_ent(struct
> > kvm_cpuid_entry2 *entry, u32 function,
> >
> > switch (function) {
> > case 0:
> > + /* Check if the cpuid leaf 0x1f is actually implemented */
> > + if (entry->eax >= 0x1f && (cpuid_ebx(0x1f) & 0x0000ffff)) {
>
> Restricting the check to bits 15:0 is unnecessary, the SDM explicitly
> states that EBX will be zero for invalid sub-leaves:
>
> For sub-leaves that return an invalid level-type of 0 in ECX[15:8];
> EAX and EBX will return 0.
>
> This code is merely checking for the existence of CPUID.1F, nothing will
> break if future CPUs provide additional information, i.e. checking for a
> valid sub-leaf is sufficient.
>
> That being said, if you insist on restricting the check to non-reserved
> bits then I think the earlier suggestion of "cpuid_ecx(0x1f) & 0x0000ff00"
> makes more sense since the SDM clearly intends ECX to be used to detect
> valid vs. invalid levels.
Here we use CPUID.1F_0:EBX[15:0] to check the existence, not the output of
ECX[15:8], which is following the Intel SDM.
Specifically, in page 3-222 Vol.2A of latest SDM publish on January 2019, there
is such description of Input EAX = 1FH:
When CPUID executes with EAX set to 1FH, the processor returns information
about extended topology enumeration data. Software must detect the presence
of CPUID leaf 1FH by verifying (a) the highest leaf index supported by CPUID
is >= 1FH, and (b) CPUID.1FH:EBX[15:0] reports a non-zero value.
And also, I look up the existing codes about initialising topology using leaf
0xb in kernel. In function detect_extended_topology_early(), it use following
if (ebx == 0 || (LEAFB_SUBTYPE(ecx) != SMT_TYPE))
to verify whether leaf 0xb is invalid. I think any of them is strong enough to
verify the leaf 0xb is invalid, so I don't know why it uses both of them.
> > + entry->eax = 0x1f;
> > + break;
>
> I find if/else easier to follow than the separate break, but either option
> works for me.
>
> > + }
> > entry->eax = min(entry->eax, (u32)(f_intel_pt ? 0x14 : 0xd));
> > break;
> > case 1:
> > @@ -544,7 +549,12 @@ static inline int __do_cpuid_ent(struct
> > kvm_cpuid_entry2 *entry, u32 function,
> > entry->edx = edx.full;
> > break;
> > }
> > - /* function 0xb has additional index. */
> > + /*
> > + * Intel documentation states that 0x1f and 0xb have
> > + * identical formats and thus can be handled by common code.
> > + * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
> > + */
>
> It's probably safe to assume anyone reading reading this code is already
> all too aware of Intel's propensity for dumping CPUID enumeration into the
> CPUID entry in the ISR. Maybe shorten this to something like:
>
> /*
> * Per Intel's SDM, 0x1f is a superset of 0xb, thus they can be handled
> * by common code.
> */
>
> > + case 0x1f:
> > case 0xb: {
> > int i, level_type;
> >
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] KVM: x86: Add Intel CPUID.1F cpuid emulation support
2019-04-26 14:54 ` Xiaoyao Li
@ 2019-04-26 15:04 ` Sean Christopherson
2019-04-26 15:32 ` Like Xu
0 siblings, 1 reply; 5+ messages in thread
From: Sean Christopherson @ 2019-04-26 15:04 UTC (permalink / raw)
To: Xiaoyao Li
Cc: Like Xu, kvm, Paolo Bonzini, Konrad Rzeszutek Wilk, linux-kernel
On Fri, Apr 26, 2019 at 10:54:19PM +0800, Xiaoyao Li wrote:
> On Fri, 2019-04-26 at 07:13 -0700, Sean Christopherson wrote:
> > On Fri, Apr 26, 2019 at 11:17:52AM +0800, Like Xu wrote:
> > > Some new systems have multiple software-visible die within each package.
> > > Add support to expose Intel V2 Extended Topology Enumeration Leaf CPUID.1F.
> > >
> > > Co-developed-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> > > Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
> > > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > > ---
> > >
> > > ==changelog==
> > > v2:
> > > - Apply cpuid.1f check rule on Intel SDM page 3-222 Vol.2A
> > > - Add comment to handle 0x1f anf 0xb in common code
> > > - Reduce check time in a descending-break style
> > >
> > > v1: https://lkml.org/lkml/2019/4/22/28
> > >
> > > arch/x86/kvm/cpuid.c | 12 +++++++++++-
> > > 1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > > index fd39516..f9b529e 100644
> > > --- a/arch/x86/kvm/cpuid.c
> > > +++ b/arch/x86/kvm/cpuid.c
> > > @@ -425,6 +425,11 @@ static inline int __do_cpuid_ent(struct
> > > kvm_cpuid_entry2 *entry, u32 function,
> > >
> > > switch (function) {
> > > case 0:
> > > + /* Check if the cpuid leaf 0x1f is actually implemented */
> > > + if (entry->eax >= 0x1f && (cpuid_ebx(0x1f) & 0x0000ffff)) {
> >
> > Restricting the check to bits 15:0 is unnecessary, the SDM explicitly
> > states that EBX will be zero for invalid sub-leaves:
> >
> > For sub-leaves that return an invalid level-type of 0 in ECX[15:8];
> > EAX and EBX will return 0.
> >
> > This code is merely checking for the existence of CPUID.1F, nothing will
> > break if future CPUs provide additional information, i.e. checking for a
> > valid sub-leaf is sufficient.
> >
> > That being said, if you insist on restricting the check to non-reserved
> > bits then I think the earlier suggestion of "cpuid_ecx(0x1f) & 0x0000ff00"
> > makes more sense since the SDM clearly intends ECX to be used to detect
> > valid vs. invalid levels.
>
> Here we use CPUID.1F_0:EBX[15:0] to check the existence, not the output of
> ECX[15:8], which is following the Intel SDM.
>
> Specifically, in page 3-222 Vol.2A of latest SDM publish on January 2019, there
> is such description of Input EAX = 1FH:
>
> When CPUID executes with EAX set to 1FH, the processor returns information
> about extended topology enumeration data. Software must detect the presence
> of CPUID leaf 1FH by verifying (a) the highest leaf index supported by CPUID
> is >= 1FH, and (b) CPUID.1FH:EBX[15:0] reports a non-zero value.
Ah, perfect. Please put exactly that in the changelog.
>
> And also, I look up the existing codes about initialising topology using leaf
> 0xb in kernel. In function detect_extended_topology_early(), it use following
>
> if (ebx == 0 || (LEAFB_SUBTYPE(ecx) != SMT_TYPE))
>
> to verify whether leaf 0xb is invalid. I think any of them is strong enough to
> verify the leaf 0xb is invalid, so I don't know why it uses both of them.
>
> > > + entry->eax = 0x1f;
> > > + break;
> >
> > I find if/else easier to follow than the separate break, but either option
> > works for me.
> >
> > > + }
> > > entry->eax = min(entry->eax, (u32)(f_intel_pt ? 0x14 : 0xd));
> > > break;
> > > case 1:
> > > @@ -544,7 +549,12 @@ static inline int __do_cpuid_ent(struct
> > > kvm_cpuid_entry2 *entry, u32 function,
> > > entry->edx = edx.full;
> > > break;
> > > }
> > > - /* function 0xb has additional index. */
> > > + /*
> > > + * Intel documentation states that 0x1f and 0xb have
> > > + * identical formats and thus can be handled by common code.
> > > + * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
> > > + */
> >
> > It's probably safe to assume anyone reading reading this code is already
> > all too aware of Intel's propensity for dumping CPUID enumeration into the
> > CPUID entry in the ISR. Maybe shorten this to something like:
> >
> > /*
> > * Per Intel's SDM, 0x1f is a superset of 0xb, thus they can be handled
> > * by common code.
> > */
> >
> > > + case 0x1f:
> > > case 0xb: {
> > > int i, level_type;
> > >
> > > --
> > > 1.8.3.1
> > >
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] KVM: x86: Add Intel CPUID.1F cpuid emulation support
2019-04-26 15:04 ` Sean Christopherson
@ 2019-04-26 15:32 ` Like Xu
0 siblings, 0 replies; 5+ messages in thread
From: Like Xu @ 2019-04-26 15:32 UTC (permalink / raw)
To: Sean Christopherson, Xiaoyao Li
Cc: kvm, Paolo Bonzini, Konrad Rzeszutek Wilk, linux-kernel
On 2019/4/26 23:04, Sean Christopherson wrote:
> On Fri, Apr 26, 2019 at 10:54:19PM +0800, Xiaoyao Li wrote:
>> On Fri, 2019-04-26 at 07:13 -0700, Sean Christopherson wrote:
>>> On Fri, Apr 26, 2019 at 11:17:52AM +0800, Like Xu wrote:
>>>> Some new systems have multiple software-visible die within each package.
>>>> Add support to expose Intel V2 Extended Topology Enumeration Leaf CPUID.1F.
>>>>
>>>> Co-developed-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
>>>> Signed-off-by: Xiaoyao Li <xiaoyao.li@linux.intel.com>
>>>> Signed-off-by: Like Xu <like.xu@linux.intel.com>
>>>> ---
>>>>
>>>> ==changelog==
>>>> v2:
>>>> - Apply cpuid.1f check rule on Intel SDM page 3-222 Vol.2A
>>>> - Add comment to handle 0x1f anf 0xb in common code
>>>> - Reduce check time in a descending-break style
>>>>
>>>> v1: https://lkml.org/lkml/2019/4/22/28
>>>>
>>>> arch/x86/kvm/cpuid.c | 12 +++++++++++-
>>>> 1 file changed, 11 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
>>>> index fd39516..f9b529e 100644
>>>> --- a/arch/x86/kvm/cpuid.c
>>>> +++ b/arch/x86/kvm/cpuid.c
>>>> @@ -425,6 +425,11 @@ static inline int __do_cpuid_ent(struct
>>>> kvm_cpuid_entry2 *entry, u32 function,
>>>>
>>>> switch (function) {
>>>> case 0:
>>>> + /* Check if the cpuid leaf 0x1f is actually implemented */
>>>> + if (entry->eax >= 0x1f && (cpuid_ebx(0x1f) & 0x0000ffff)) {
>>>
>>> Restricting the check to bits 15:0 is unnecessary, the SDM explicitly
>>> states that EBX will be zero for invalid sub-leaves:
>>>
>>> For sub-leaves that return an invalid level-type of 0 in ECX[15:8];
>>> EAX and EBX will return 0.
>>>
>>> This code is merely checking for the existence of CPUID.1F, nothing will
>>> break if future CPUs provide additional information, i.e. checking for a
>>> valid sub-leaf is sufficient.
>>>
>>> That being said, if you insist on restricting the check to non-reserved
>>> bits then I think the earlier suggestion of "cpuid_ecx(0x1f) & 0x0000ff00"
>>> makes more sense since the SDM clearly intends ECX to be used to detect
>>> valid vs. invalid levels.
>>
>> Here we use CPUID.1F_0:EBX[15:0] to check the existence, not the output of
>> ECX[15:8], which is following the Intel SDM.
>>
>> Specifically, in page 3-222 Vol.2A of latest SDM publish on January 2019, there
>> is such description of Input EAX = 1FH:
>>
>> When CPUID executes with EAX set to 1FH, the processor returns information
>> about extended topology enumeration data. Software must detect the presence
>> of CPUID leaf 1FH by verifying (a) the highest leaf index supported by CPUID
>> is >= 1FH, and (b) CPUID.1FH:EBX[15:0] reports a non-zero value.
>
> Ah, perfect. Please put exactly that in the changelog.
Thanks Sean and xiaoyao.
Both of you inspired me a lot and a v3 patch would be released.
>
>>
>> And also, I look up the existing codes about initialising topology using leaf
>> 0xb in kernel. In function detect_extended_topology_early(), it use following
>>
>> if (ebx == 0 || (LEAFB_SUBTYPE(ecx) != SMT_TYPE))
>>
>> to verify whether leaf 0xb is invalid. I think any of them is strong enough to
>> verify the leaf 0xb is invalid, so I don't know why it uses both of them.
>>
>>>> + entry->eax = 0x1f;
>>>> + break;
>>>
>>> I find if/else easier to follow than the separate break, but either option
>>> works for me.
>>>
>>>> + }
>>>> entry->eax = min(entry->eax, (u32)(f_intel_pt ? 0x14 : 0xd));
>>>> break;
>>>> case 1:
>>>> @@ -544,7 +549,12 @@ static inline int __do_cpuid_ent(struct
>>>> kvm_cpuid_entry2 *entry, u32 function,
>>>> entry->edx = edx.full;
>>>> break;
>>>> }
>>>> - /* function 0xb has additional index. */
>>>> + /*
>>>> + * Intel documentation states that 0x1f and 0xb have
>>>> + * identical formats and thus can be handled by common code.
>>>> + * (Intel SDM Vol. 2A - Instruction Set Reference - CPUID)
>>>> + */
>>>
>>> It's probably safe to assume anyone reading reading this code is already
>>> all too aware of Intel's propensity for dumping CPUID enumeration into the
>>> CPUID entry in the ISR. Maybe shorten this to something like:
>>>
>>> /*
>>> * Per Intel's SDM, 0x1f is a superset of 0xb, thus they can be handled
>>> * by common code.
>>> */
>>>
>>>> + case 0x1f:
>>>> case 0xb: {
>>>> int i, level_type;
>>>>
>>>> --
>>>> 1.8.3.1
>>>>
>>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-04-26 15:32 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-26 3:17 [PATCH v2] KVM: x86: Add Intel CPUID.1F cpuid emulation support Like Xu
2019-04-26 14:13 ` Sean Christopherson
2019-04-26 14:54 ` Xiaoyao Li
2019-04-26 15:04 ` Sean Christopherson
2019-04-26 15:32 ` Like Xu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).