* [Qemu-devel] [PATCH v2 0/5] Enable TOPOEXT to support hyperthreading on AMD CPU
@ 2018-02-24 2:30 ` Babu Moger
0 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: mtosatti, qemu-devel, kvm, pixo, Gary.Hook, babu.moger
These series enable TOPOEXT feature on AMD CPU. These are required to support
hyperthreading on kvm guests. Addresses the issues reported by these bugs
https://bugzilla.redhat.com/show_bug.cgi?id=1481253
https://bugs.launchpad.net/qemu/+bug/1703506
v2:
Fixed few more minor issues per Gary Hooks comments. Thank you Gary.
Removed the patch#1. We need to handle the instruction cache associativity
seperately. It varies based on the cpu family. I will comeback to that later.
Added two more typo corrections in patch#1 and patch#5.
v1:
Stanislav Lanci posted few patches earlier.
https://patchwork.kernel.org/patch/10040903/
Rebased his patches with few changes.
1. Spit the patches into two, separating cpuid functions
0x8000001D and 0x8000001E (Patch 2 and 3).
2. Removed the generic non-intel check and made a separate patch
with some changes(Patch 5).
3. Fixed L3_N_SETS_AMD(from 4096 to 8192) based on CPUID_Fn8000001D_ECX_x03.
Added 2 more patches.
Patch 1. Fixes cache associativity.
Patch 4. Adds TOPOEXT feature on AMD EPYC CPU.
Babu Moger (3):
target/i386: Fix a minor typo found while reviwing
target/i386: Enable TOPOEXT feature on AMD EPYC CPU
target/i386: Remove generic SMT thread check
Stanislav Lanci (2):
target/i386: Populate AMD Processor Cache Information
target/i386: Add support for CPUID_8000_001E for AMD
linux-headers/asm-x86/kvm.h | 2 +-
target/i386/cpu.c | 104 ++++++++++++++++++++++++++++++++++++++++----
target/i386/kvm.c | 31 +++++++++++--
3 files changed, 124 insertions(+), 13 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
2018-02-24 2:30 ` [Qemu-devel] " Babu Moger
@ 2018-02-24 2:30 ` Babu Moger
-1 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: pixo, kvm, mtosatti, Gary.Hook, qemu-devel, babu.moger
Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to KVM_CPUID_FLAG_SIGNIFICANT_INDEX
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
| 2 +-
target/i386/kvm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index f3a9604..6aec661 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -220,7 +220,7 @@ struct kvm_cpuid_entry2 {
__u32 padding[3];
};
-#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX (1 << 0)
+#define KVM_CPUID_FLAG_SIGNIFICANT_INDEX (1 << 0)
#define KVM_CPUID_FLAG_STATEFUL_FUNC (1 << 1)
#define KVM_CPUID_FLAG_STATE_READ_NEXT (1 << 2)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ad4b159..85856b6 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -844,7 +844,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
break;
}
c->function = i;
- c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ c->flags = KVM_CPUID_FLAG_SIGNIFICANT_INDEX;
c->index = j;
cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
@ 2018-02-24 2:30 ` Babu Moger
0 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: mtosatti, qemu-devel, kvm, pixo, Gary.Hook, babu.moger
Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to KVM_CPUID_FLAG_SIGNIFICANT_INDEX
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
| 2 +-
target/i386/kvm.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
index f3a9604..6aec661 100644
--- a/linux-headers/asm-x86/kvm.h
+++ b/linux-headers/asm-x86/kvm.h
@@ -220,7 +220,7 @@ struct kvm_cpuid_entry2 {
__u32 padding[3];
};
-#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX (1 << 0)
+#define KVM_CPUID_FLAG_SIGNIFICANT_INDEX (1 << 0)
#define KVM_CPUID_FLAG_STATEFUL_FUNC (1 << 1)
#define KVM_CPUID_FLAG_STATE_READ_NEXT (1 << 2)
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index ad4b159..85856b6 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -844,7 +844,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
break;
}
c->function = i;
- c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
+ c->flags = KVM_CPUID_FLAG_SIGNIFICANT_INDEX;
c->index = j;
cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
2018-02-24 2:30 ` [Qemu-devel] " Babu Moger
@ 2018-02-28 17:38 ` Radim Krčmář
-1 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-02-28 17:38 UTC (permalink / raw)
To: Babu Moger
Cc: ehabkost, kvm, mtosatti, Gary.Hook, qemu-devel, pbonzini, pixo, rth
2018-02-23 21:30-0500, Babu Moger:
> Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to KVM_CPUID_FLAG_SIGNIFICANT_INDEX
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> linux-headers/asm-x86/kvm.h | 2 +-
> target/i386/kvm.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
> index f3a9604..6aec661 100644
> --- a/linux-headers/asm-x86/kvm.h
> +++ b/linux-headers/asm-x86/kvm.h
> @@ -220,7 +220,7 @@ struct kvm_cpuid_entry2 {
> __u32 padding[3];
> };
>
> -#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX (1 << 0)
The typo is in the original kernel header and we periodically copy it
over with scripts/update-linux-headers.sh, so this change would only add
overhead in the long run.
I'd keep the typo.
(The alternative is to add KVM_CPUID_FLAG_SIGNIFICANT_INDEX to the linux
header, so there would be both variants and use the correct one here.)
Thanks.
> +#define KVM_CPUID_FLAG_SIGNIFICANT_INDEX (1 << 0)
> #define KVM_CPUID_FLAG_STATEFUL_FUNC (1 << 1)
> #define KVM_CPUID_FLAG_STATE_READ_NEXT (1 << 2)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index ad4b159..85856b6 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -844,7 +844,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> break;
> }
> c->function = i;
> - c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + c->flags = KVM_CPUID_FLAG_SIGNIFICANT_INDEX;
> c->index = j;
> cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
@ 2018-02-28 17:38 ` Radim Krčmář
0 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-02-28 17:38 UTC (permalink / raw)
To: Babu Moger
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Gary.Hook
2018-02-23 21:30-0500, Babu Moger:
> Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to KVM_CPUID_FLAG_SIGNIFICANT_INDEX
>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> linux-headers/asm-x86/kvm.h | 2 +-
> target/i386/kvm.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
> index f3a9604..6aec661 100644
> --- a/linux-headers/asm-x86/kvm.h
> +++ b/linux-headers/asm-x86/kvm.h
> @@ -220,7 +220,7 @@ struct kvm_cpuid_entry2 {
> __u32 padding[3];
> };
>
> -#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX (1 << 0)
The typo is in the original kernel header and we periodically copy it
over with scripts/update-linux-headers.sh, so this change would only add
overhead in the long run.
I'd keep the typo.
(The alternative is to add KVM_CPUID_FLAG_SIGNIFICANT_INDEX to the linux
header, so there would be both variants and use the correct one here.)
Thanks.
> +#define KVM_CPUID_FLAG_SIGNIFICANT_INDEX (1 << 0)
> #define KVM_CPUID_FLAG_STATEFUL_FUNC (1 << 1)
> #define KVM_CPUID_FLAG_STATE_READ_NEXT (1 << 2)
>
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index ad4b159..85856b6 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -844,7 +844,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> break;
> }
> c->function = i;
> - c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> + c->flags = KVM_CPUID_FLAG_SIGNIFICANT_INDEX;
> c->index = j;
> cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
>
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
2018-02-28 17:38 ` [Qemu-devel] " Radim Krčmář
@ 2018-02-28 18:49 ` Eric Blake
-1 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2018-02-28 18:49 UTC (permalink / raw)
To: Radim Krčmář, Babu Moger
Cc: ehabkost, kvm, mtosatti, Gary.Hook, qemu-devel, pbonzini, pixo, rth
On 02/28/2018 11:38 AM, Radim Krčmář wrote:
> 2018-02-23 21:30-0500, Babu Moger:
In the subject line: s/reviwing/reviewing/ [It's never a good sign when
a fix claiming to fix a typo introduces a typo ;) ]
Or go for a shorter subject:
target/i386: Fix a minor typo
>> Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to KVM_CPUID_FLAG_SIGNIFICANT_INDEX
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
@ 2018-02-28 18:49 ` Eric Blake
0 siblings, 0 replies; 36+ messages in thread
From: Eric Blake @ 2018-02-28 18:49 UTC (permalink / raw)
To: Radim Krčmář, Babu Moger
Cc: ehabkost, kvm, mtosatti, Gary.Hook, qemu-devel, pbonzini, pixo, rth
On 02/28/2018 11:38 AM, Radim Krčmář wrote:
> 2018-02-23 21:30-0500, Babu Moger:
In the subject line: s/reviwing/reviewing/ [It's never a good sign when
a fix claiming to fix a typo introduces a typo ;) ]
Or go for a shorter subject:
target/i386: Fix a minor typo
>> Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to KVM_CPUID_FLAG_SIGNIFICANT_INDEX
>>
>> Signed-off-by: Babu Moger <babu.moger@amd.com>
>> ---
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
2018-02-28 18:49 ` [Qemu-devel] " Eric Blake
@ 2018-02-28 21:20 ` Moger, Babu
-1 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-02-28 21:20 UTC (permalink / raw)
To: Eric Blake, Radim Krčmář
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
> -----Original Message-----
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Wednesday, February 28, 2018 12:49 PM
> To: Radim Krčmář <rkrcmar@redhat.com>; Moger, Babu
> <Babu.Moger@amd.com>
> Cc: ehabkost@redhat.com; kvm@vger.kernel.org; mtosatti@redhat.com;
> Hook, Gary <Gary.Hook@amd.com>; qemu-devel@nongnu.org;
> pbonzini@redhat.com; pixo@polepetko.eu; rth@twiddle.net
> Subject: Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Fix a minor typo
> found while reviwing
>
> On 02/28/2018 11:38 AM, Radim Krčmář wrote:
> > 2018-02-23 21:30-0500, Babu Moger:
>
> In the subject line: s/reviwing/reviewing/ [It's never a good sign when
> a fix claiming to fix a typo introduces a typo ;) ]
Good catch Eric. Sorry about that. We don’t need this patch anymore. Thanks.
>
> Or go for a shorter subject:
> target/i386: Fix a minor typo
>
> >> Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to
> KVM_CPUID_FLAG_SIGNIFICANT_INDEX
> >>
> >> Signed-off-by: Babu Moger <babu.moger@amd.com>
> >> ---
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
@ 2018-02-28 21:20 ` Moger, Babu
0 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-02-28 21:20 UTC (permalink / raw)
To: Eric Blake, Radim Krčmář
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
> -----Original Message-----
> From: Eric Blake [mailto:eblake@redhat.com]
> Sent: Wednesday, February 28, 2018 12:49 PM
> To: Radim Krčmář <rkrcmar@redhat.com>; Moger, Babu
> <Babu.Moger@amd.com>
> Cc: ehabkost@redhat.com; kvm@vger.kernel.org; mtosatti@redhat.com;
> Hook, Gary <Gary.Hook@amd.com>; qemu-devel@nongnu.org;
> pbonzini@redhat.com; pixo@polepetko.eu; rth@twiddle.net
> Subject: Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Fix a minor typo
> found while reviwing
>
> On 02/28/2018 11:38 AM, Radim Krčmář wrote:
> > 2018-02-23 21:30-0500, Babu Moger:
>
> In the subject line: s/reviwing/reviewing/ [It's never a good sign when
> a fix claiming to fix a typo introduces a typo ;) ]
Good catch Eric. Sorry about that. We don’t need this patch anymore. Thanks.
>
> Or go for a shorter subject:
> target/i386: Fix a minor typo
>
> >> Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to
> KVM_CPUID_FLAG_SIGNIFICANT_INDEX
> >>
> >> Signed-off-by: Babu Moger <babu.moger@amd.com>
> >> ---
> --
> Eric Blake, Principal Software Engineer
> Red Hat, Inc. +1-919-301-3266
> Virtualization: qemu.org | libvirt.org
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
2018-02-28 17:38 ` [Qemu-devel] " Radim Krčmář
@ 2018-02-28 21:12 ` Moger, Babu
-1 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-02-28 21:12 UTC (permalink / raw)
To: Radim Krčmář
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Wednesday, February 28, 2018 11:38 AM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 1/5] target/i386: Fix a minor typo found while
> reviwing
>
> 2018-02-23 21:30-0500, Babu Moger:
> > Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to
> KVM_CPUID_FLAG_SIGNIFICANT_INDEX
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> > linux-headers/asm-x86/kvm.h | 2 +-
> > target/i386/kvm.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
> > index f3a9604..6aec661 100644
> > --- a/linux-headers/asm-x86/kvm.h
> > +++ b/linux-headers/asm-x86/kvm.h
> > @@ -220,7 +220,7 @@ struct kvm_cpuid_entry2 {
> > __u32 padding[3];
> > };
> >
> > -#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX (1 << 0)
>
> The typo is in the original kernel header and we periodically copy it
> over with scripts/update-linux-headers.sh, so this change would only add
> overhead in the long run.
Yes. I see that now.
>
> I'd keep the typo.
Sure. I will drop this patch. Let me not complicate things.
>
> (The alternative is to add KVM_CPUID_FLAG_SIGNIFICANT_INDEX to the
> linux
> header, so there would be both variants and use the correct one here.)
>
> Thanks.
>
> > +#define KVM_CPUID_FLAG_SIGNIFICANT_INDEX (1 << 0)
> > #define KVM_CPUID_FLAG_STATEFUL_FUNC (1 << 1)
> > #define KVM_CPUID_FLAG_STATE_READ_NEXT (1 << 2)
> >
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index ad4b159..85856b6 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -844,7 +844,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > break;
> > }
> > c->function = i;
> > - c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> > + c->flags = KVM_CPUID_FLAG_SIGNIFICANT_INDEX;
> > c->index = j;
> > cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
> >
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v2 1/5] target/i386: Fix a minor typo found while reviwing
@ 2018-02-28 21:12 ` Moger, Babu
0 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-02-28 21:12 UTC (permalink / raw)
To: Radim Krčmář
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Hook, Gary
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Wednesday, February 28, 2018 11:38 AM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 1/5] target/i386: Fix a minor typo found while
> reviwing
>
> 2018-02-23 21:30-0500, Babu Moger:
> > Changed KVM_CPUID_FLAG_SIGNIFCANT_INDEX to
> KVM_CPUID_FLAG_SIGNIFICANT_INDEX
> >
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> > linux-headers/asm-x86/kvm.h | 2 +-
> > target/i386/kvm.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/linux-headers/asm-x86/kvm.h b/linux-headers/asm-x86/kvm.h
> > index f3a9604..6aec661 100644
> > --- a/linux-headers/asm-x86/kvm.h
> > +++ b/linux-headers/asm-x86/kvm.h
> > @@ -220,7 +220,7 @@ struct kvm_cpuid_entry2 {
> > __u32 padding[3];
> > };
> >
> > -#define KVM_CPUID_FLAG_SIGNIFCANT_INDEX (1 << 0)
>
> The typo is in the original kernel header and we periodically copy it
> over with scripts/update-linux-headers.sh, so this change would only add
> overhead in the long run.
Yes. I see that now.
>
> I'd keep the typo.
Sure. I will drop this patch. Let me not complicate things.
>
> (The alternative is to add KVM_CPUID_FLAG_SIGNIFICANT_INDEX to the
> linux
> header, so there would be both variants and use the correct one here.)
>
> Thanks.
>
> > +#define KVM_CPUID_FLAG_SIGNIFICANT_INDEX (1 << 0)
> > #define KVM_CPUID_FLAG_STATEFUL_FUNC (1 << 1)
> > #define KVM_CPUID_FLAG_STATE_READ_NEXT (1 << 2)
> >
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index ad4b159..85856b6 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -844,7 +844,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
> > break;
> > }
> > c->function = i;
> > - c->flags = KVM_CPUID_FLAG_SIGNIFCANT_INDEX;
> > + c->flags = KVM_CPUID_FLAG_SIGNIFICANT_INDEX;
> > c->index = j;
> > cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
> >
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
2018-02-24 2:30 ` [Qemu-devel] " Babu Moger
@ 2018-02-24 2:30 ` Babu Moger
-1 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: pixo, kvm, mtosatti, Gary.Hook, qemu-devel, babu.moger
From: Stanislav Lanci <pixo@polepetko.eu>
Adds information about cache size and topology from cpuid 0x8000001D leaf
for different cache types on AMD processors.
Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
target/i386/kvm.c | 29 ++++++++++++++++++---
2 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b5e431e..a5a480e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -118,6 +118,7 @@
#define L1I_LINE_SIZE 64
#define L1I_ASSOCIATIVITY 8
#define L1I_SETS 64
+#define L1I_SETS_AMD 256
#define L1I_PARTITIONS 1
/* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 32KiB */
#define L1I_DESCRIPTOR CPUID_2_L1I_32KB_8WAY_64B
@@ -129,7 +130,9 @@
/* Level 2 unified cache: */
#define L2_LINE_SIZE 64
#define L2_ASSOCIATIVITY 16
+#define L2_ASSOCIATIVITY_AMD 8
#define L2_SETS 4096
+#define L2_SETS_AMD 1024
#define L2_PARTITIONS 1
/* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 4MiB */
/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
@@ -146,6 +149,7 @@
#define L3_N_LINE_SIZE 64
#define L3_N_ASSOCIATIVITY 16
#define L3_N_SETS 16384
+#define L3_N_SETS_AMD 8192
#define L3_N_PARTITIONS 1
#define L3_N_DESCRIPTOR CPUID_2_L3_16MB_16WAY_64B
#define L3_N_LINES_PER_TAG 1
@@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*edx = 0;
}
break;
+ case 0x8000001D: /* AMD TOPOEXT cache info */
+ if (cpu->cache_info_passthrough) {
+ host_cpuid(index, count, eax, ebx, ecx, edx);
+ break;
+ } else if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
+ *eax = 0;
+ switch (count) {
+ case 0: /* L1 dcache info */
+ *eax |= CPUID_4_TYPE_DCACHE | \
+ CPUID_4_LEVEL(1) | \
+ CPUID_4_SELF_INIT_LEVEL | \
+ ((cs->nr_threads - 1) << 14);
+ *ebx = (L1D_LINE_SIZE - 1) | \
+ ((L1D_PARTITIONS - 1) << 12) | \
+ ((L1D_ASSOCIATIVITY - 1) << 22);
+ *ecx = L1D_SETS - 1;
+ *edx = 0;
+ break;
+ case 1: /* L1 icache info */
+ *eax |= CPUID_4_TYPE_ICACHE | \
+ CPUID_4_LEVEL(1) | \
+ CPUID_4_SELF_INIT_LEVEL | \
+ ((cs->nr_threads - 1) << 14);
+ *ebx = (L1I_LINE_SIZE - 1) | \
+ ((L1I_PARTITIONS - 1) << 12) | \
+ ((L1I_ASSOCIATIVITY_AMD - 1) << 22);
+ *ecx = L1I_SETS_AMD - 1;
+ *edx = 0;
+ break;
+ case 2: /* L2 cache info */
+ *eax |= CPUID_4_TYPE_UNIFIED | \
+ CPUID_4_LEVEL(2) | \
+ CPUID_4_SELF_INIT_LEVEL | \
+ ((cs->nr_threads - 1) << 14);
+ *ebx = (L2_LINE_SIZE - 1) | \
+ ((L2_PARTITIONS - 1) << 12) | \
+ ((L2_ASSOCIATIVITY_AMD - 1) << 22);
+ *ecx = L2_SETS_AMD - 1;
+ *edx = CPUID_4_INCLUSIVE;
+ break;
+ case 3: /* L3 cache info */
+ if (!cpu->enable_l3_cache) {
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ break;
+ }
+ *eax |= CPUID_4_TYPE_UNIFIED | \
+ CPUID_4_LEVEL(3) | \
+ CPUID_4_SELF_INIT_LEVEL | \
+ ((cs->nr_cores * cs->nr_threads - 1) << 14);
+ *ebx = (L3_N_LINE_SIZE - 1) | \
+ ((L3_N_PARTITIONS - 1) << 12) | \
+ ((L3_N_ASSOCIATIVITY - 1) << 22);
+ *ecx = L3_N_SETS_AMD - 1;
+ *edx = CPUID_4_NO_INVD_SHARING;
+ break;
+ default: /* end of info */
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ break;
+ }
+ } else {
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ }
+ break;
case 0xC0000000:
*eax = env->cpuid_xlevel2;
*ebx = 0;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 85856b6..8adf7d1 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -909,9 +909,32 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
c = &cpuid_data.entries[cpuid_i++];
- c->function = i;
- c->flags = 0;
- cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ switch (i) {
+ case 0x8000001d:
+ /* Query for all AMD cache information leaves */
+ for (j = 0; ; j++) {
+ c->function = i;
+ c->flags = KVM_CPUID_FLAG_SIGNIFICANT_INDEX;
+ c->index = j;
+ cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
+
+ if (c->eax == 0) {
+ break;
+ }
+ if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
+ fprintf(stderr, "cpuid_data is full, no space for "
+ "cpuid(eax:0x%x,ecx:0x%x)\n", i, j);
+ abort();
+ }
+ c = &cpuid_data.entries[cpuid_i++];
+ }
+ break;
+ default:
+ c->function = i;
+ c->flags = 0;
+ cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ break;
+ }
}
/* Call Centaur's CPUID instructions they are supported. */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
@ 2018-02-24 2:30 ` Babu Moger
0 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: mtosatti, qemu-devel, kvm, pixo, Gary.Hook, babu.moger
From: Stanislav Lanci <pixo@polepetko.eu>
Adds information about cache size and topology from cpuid 0x8000001D leaf
for different cache types on AMD processors.
Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
target/i386/kvm.c | 29 ++++++++++++++++++---
2 files changed, 102 insertions(+), 3 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index b5e431e..a5a480e 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -118,6 +118,7 @@
#define L1I_LINE_SIZE 64
#define L1I_ASSOCIATIVITY 8
#define L1I_SETS 64
+#define L1I_SETS_AMD 256
#define L1I_PARTITIONS 1
/* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 32KiB */
#define L1I_DESCRIPTOR CPUID_2_L1I_32KB_8WAY_64B
@@ -129,7 +130,9 @@
/* Level 2 unified cache: */
#define L2_LINE_SIZE 64
#define L2_ASSOCIATIVITY 16
+#define L2_ASSOCIATIVITY_AMD 8
#define L2_SETS 4096
+#define L2_SETS_AMD 1024
#define L2_PARTITIONS 1
/* Size = LINE_SIZE*ASSOCIATIVITY*SETS*PARTITIONS = 4MiB */
/*FIXME: CPUID leaf 2 descriptor is inconsistent with CPUID leaf 4 */
@@ -146,6 +149,7 @@
#define L3_N_LINE_SIZE 64
#define L3_N_ASSOCIATIVITY 16
#define L3_N_SETS 16384
+#define L3_N_SETS_AMD 8192
#define L3_N_PARTITIONS 1
#define L3_N_DESCRIPTOR CPUID_2_L3_16MB_16WAY_64B
#define L3_N_LINES_PER_TAG 1
@@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*edx = 0;
}
break;
+ case 0x8000001D: /* AMD TOPOEXT cache info */
+ if (cpu->cache_info_passthrough) {
+ host_cpuid(index, count, eax, ebx, ecx, edx);
+ break;
+ } else if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
+ *eax = 0;
+ switch (count) {
+ case 0: /* L1 dcache info */
+ *eax |= CPUID_4_TYPE_DCACHE | \
+ CPUID_4_LEVEL(1) | \
+ CPUID_4_SELF_INIT_LEVEL | \
+ ((cs->nr_threads - 1) << 14);
+ *ebx = (L1D_LINE_SIZE - 1) | \
+ ((L1D_PARTITIONS - 1) << 12) | \
+ ((L1D_ASSOCIATIVITY - 1) << 22);
+ *ecx = L1D_SETS - 1;
+ *edx = 0;
+ break;
+ case 1: /* L1 icache info */
+ *eax |= CPUID_4_TYPE_ICACHE | \
+ CPUID_4_LEVEL(1) | \
+ CPUID_4_SELF_INIT_LEVEL | \
+ ((cs->nr_threads - 1) << 14);
+ *ebx = (L1I_LINE_SIZE - 1) | \
+ ((L1I_PARTITIONS - 1) << 12) | \
+ ((L1I_ASSOCIATIVITY_AMD - 1) << 22);
+ *ecx = L1I_SETS_AMD - 1;
+ *edx = 0;
+ break;
+ case 2: /* L2 cache info */
+ *eax |= CPUID_4_TYPE_UNIFIED | \
+ CPUID_4_LEVEL(2) | \
+ CPUID_4_SELF_INIT_LEVEL | \
+ ((cs->nr_threads - 1) << 14);
+ *ebx = (L2_LINE_SIZE - 1) | \
+ ((L2_PARTITIONS - 1) << 12) | \
+ ((L2_ASSOCIATIVITY_AMD - 1) << 22);
+ *ecx = L2_SETS_AMD - 1;
+ *edx = CPUID_4_INCLUSIVE;
+ break;
+ case 3: /* L3 cache info */
+ if (!cpu->enable_l3_cache) {
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ break;
+ }
+ *eax |= CPUID_4_TYPE_UNIFIED | \
+ CPUID_4_LEVEL(3) | \
+ CPUID_4_SELF_INIT_LEVEL | \
+ ((cs->nr_cores * cs->nr_threads - 1) << 14);
+ *ebx = (L3_N_LINE_SIZE - 1) | \
+ ((L3_N_PARTITIONS - 1) << 12) | \
+ ((L3_N_ASSOCIATIVITY - 1) << 22);
+ *ecx = L3_N_SETS_AMD - 1;
+ *edx = CPUID_4_NO_INVD_SHARING;
+ break;
+ default: /* end of info */
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ break;
+ }
+ } else {
+ *eax = 0;
+ *ebx = 0;
+ *ecx = 0;
+ *edx = 0;
+ }
+ break;
case 0xC0000000:
*eax = env->cpuid_xlevel2;
*ebx = 0;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 85856b6..8adf7d1 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -909,9 +909,32 @@ int kvm_arch_init_vcpu(CPUState *cs)
}
c = &cpuid_data.entries[cpuid_i++];
- c->function = i;
- c->flags = 0;
- cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ switch (i) {
+ case 0x8000001d:
+ /* Query for all AMD cache information leaves */
+ for (j = 0; ; j++) {
+ c->function = i;
+ c->flags = KVM_CPUID_FLAG_SIGNIFICANT_INDEX;
+ c->index = j;
+ cpu_x86_cpuid(env, i, j, &c->eax, &c->ebx, &c->ecx, &c->edx);
+
+ if (c->eax == 0) {
+ break;
+ }
+ if (cpuid_i == KVM_MAX_CPUID_ENTRIES) {
+ fprintf(stderr, "cpuid_data is full, no space for "
+ "cpuid(eax:0x%x,ecx:0x%x)\n", i, j);
+ abort();
+ }
+ c = &cpuid_data.entries[cpuid_i++];
+ }
+ break;
+ default:
+ c->function = i;
+ c->flags = 0;
+ cpu_x86_cpuid(env, i, 0, &c->eax, &c->ebx, &c->ecx, &c->edx);
+ break;
+ }
}
/* Call Centaur's CPUID instructions they are supported. */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
2018-02-24 2:30 ` [Qemu-devel] " Babu Moger
@ 2018-02-28 18:08 ` Radim Krčmář
-1 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-02-28 18:08 UTC (permalink / raw)
To: Babu Moger
Cc: ehabkost, kvm, mtosatti, Gary.Hook, qemu-devel, pbonzini, pixo, rth
2018-02-23 21:30-0500, Babu Moger:
> From: Stanislav Lanci <pixo@polepetko.eu>
>
> Adds information about cache size and topology from cpuid 0x8000001D leaf
> for different cache types on AMD processors.
>
> Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> @@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> *edx = 0;
> }
> break;
> + case 0x8000001D: /* AMD TOPOEXT cache info */
> + if (cpu->cache_info_passthrough) {
> + host_cpuid(index, count, eax, ebx, ecx, edx);
> + break;
> + } else if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
> + *eax = 0;
> + switch (count) {
> + case 0: /* L1 dcache info */
> + *eax |= CPUID_4_TYPE_DCACHE | \
> + CPUID_4_LEVEL(1) | \
> + CPUID_4_SELF_INIT_LEVEL | \
> + ((cs->nr_threads - 1) << 14);
CPUID_4 uses the same format even for bits 25-14, so this would look
better with a macro.
> + *ebx = (L1D_LINE_SIZE - 1) | \
> + ((L1D_PARTITIONS - 1) << 12) | \
> + ((L1D_ASSOCIATIVITY - 1) << 22);
> + *ecx = L1D_SETS - 1;
These numbers seem to have the same meaning as CPUID 4, but have
conflicting values.
I think we should not expose CPUID 4 with AMD CPUs or at least when they
have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).
> + *edx = 0;
> + break;
> + case 1: /* L1 icache info */
> + *eax |= CPUID_4_TYPE_ICACHE | \
> + CPUID_4_LEVEL(1) | \
> + CPUID_4_SELF_INIT_LEVEL | \
> + ((cs->nr_threads - 1) << 14);
> + *ebx = (L1I_LINE_SIZE - 1) | \
> + ((L1I_PARTITIONS - 1) << 12) | \
> + ((L1I_ASSOCIATIVITY_AMD - 1) << 22);
> + *ecx = L1I_SETS_AMD - 1;
> + *edx = 0;
> + break;
> + case 2: /* L2 cache info */
> + *eax |= CPUID_4_TYPE_UNIFIED | \
> + CPUID_4_LEVEL(2) | \
> + CPUID_4_SELF_INIT_LEVEL | \
> + ((cs->nr_threads - 1) << 14);
> + *ebx = (L2_LINE_SIZE - 1) | \
> + ((L2_PARTITIONS - 1) << 12) | \
> + ((L2_ASSOCIATIVITY_AMD - 1) << 22);
> + *ecx = L2_SETS_AMD - 1;
> + *edx = CPUID_4_INCLUSIVE;
> + break;
> + case 3: /* L3 cache info */
> + if (!cpu->enable_l3_cache) {
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + break;
> + }
> + *eax |= CPUID_4_TYPE_UNIFIED | \
> + CPUID_4_LEVEL(3) | \
> + CPUID_4_SELF_INIT_LEVEL | \
> + ((cs->nr_cores * cs->nr_threads - 1) << 14);
This number seems to be the only difference that isn't just a difference
constant. It tempts me to merge the cases for 4 and 0x8000001D as it
seems that vendors try to be compatible.
> + *ebx = (L3_N_LINE_SIZE - 1) | \
> + ((L3_N_PARTITIONS - 1) << 12) | \
> + ((L3_N_ASSOCIATIVITY - 1) << 22);
> + *ecx = L3_N_SETS_AMD - 1;
> + *edx = CPUID_4_NO_INVD_SHARING;
> + break;
> + default: /* end of info */
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + break;
> + }
> + } else {
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + }
> + break;
> case 0xC0000000:
> *eax = env->cpuid_xlevel2;
> *ebx = 0;
The numbers looks like real hardware,
thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
@ 2018-02-28 18:08 ` Radim Krčmář
0 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-02-28 18:08 UTC (permalink / raw)
To: Babu Moger
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Gary.Hook
2018-02-23 21:30-0500, Babu Moger:
> From: Stanislav Lanci <pixo@polepetko.eu>
>
> Adds information about cache size and topology from cpuid 0x8000001D leaf
> for different cache types on AMD processors.
>
> Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> @@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> *edx = 0;
> }
> break;
> + case 0x8000001D: /* AMD TOPOEXT cache info */
> + if (cpu->cache_info_passthrough) {
> + host_cpuid(index, count, eax, ebx, ecx, edx);
> + break;
> + } else if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
> + *eax = 0;
> + switch (count) {
> + case 0: /* L1 dcache info */
> + *eax |= CPUID_4_TYPE_DCACHE | \
> + CPUID_4_LEVEL(1) | \
> + CPUID_4_SELF_INIT_LEVEL | \
> + ((cs->nr_threads - 1) << 14);
CPUID_4 uses the same format even for bits 25-14, so this would look
better with a macro.
> + *ebx = (L1D_LINE_SIZE - 1) | \
> + ((L1D_PARTITIONS - 1) << 12) | \
> + ((L1D_ASSOCIATIVITY - 1) << 22);
> + *ecx = L1D_SETS - 1;
These numbers seem to have the same meaning as CPUID 4, but have
conflicting values.
I think we should not expose CPUID 4 with AMD CPUs or at least when they
have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).
> + *edx = 0;
> + break;
> + case 1: /* L1 icache info */
> + *eax |= CPUID_4_TYPE_ICACHE | \
> + CPUID_4_LEVEL(1) | \
> + CPUID_4_SELF_INIT_LEVEL | \
> + ((cs->nr_threads - 1) << 14);
> + *ebx = (L1I_LINE_SIZE - 1) | \
> + ((L1I_PARTITIONS - 1) << 12) | \
> + ((L1I_ASSOCIATIVITY_AMD - 1) << 22);
> + *ecx = L1I_SETS_AMD - 1;
> + *edx = 0;
> + break;
> + case 2: /* L2 cache info */
> + *eax |= CPUID_4_TYPE_UNIFIED | \
> + CPUID_4_LEVEL(2) | \
> + CPUID_4_SELF_INIT_LEVEL | \
> + ((cs->nr_threads - 1) << 14);
> + *ebx = (L2_LINE_SIZE - 1) | \
> + ((L2_PARTITIONS - 1) << 12) | \
> + ((L2_ASSOCIATIVITY_AMD - 1) << 22);
> + *ecx = L2_SETS_AMD - 1;
> + *edx = CPUID_4_INCLUSIVE;
> + break;
> + case 3: /* L3 cache info */
> + if (!cpu->enable_l3_cache) {
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + break;
> + }
> + *eax |= CPUID_4_TYPE_UNIFIED | \
> + CPUID_4_LEVEL(3) | \
> + CPUID_4_SELF_INIT_LEVEL | \
> + ((cs->nr_cores * cs->nr_threads - 1) << 14);
This number seems to be the only difference that isn't just a difference
constant. It tempts me to merge the cases for 4 and 0x8000001D as it
seems that vendors try to be compatible.
> + *ebx = (L3_N_LINE_SIZE - 1) | \
> + ((L3_N_PARTITIONS - 1) << 12) | \
> + ((L3_N_ASSOCIATIVITY - 1) << 22);
> + *ecx = L3_N_SETS_AMD - 1;
> + *edx = CPUID_4_NO_INVD_SHARING;
> + break;
> + default: /* end of info */
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + break;
> + }
> + } else {
> + *eax = 0;
> + *ebx = 0;
> + *ecx = 0;
> + *edx = 0;
> + }
> + break;
> case 0xC0000000:
> *eax = env->cpuid_xlevel2;
> *ebx = 0;
The numbers looks like real hardware,
thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
2018-02-28 18:08 ` [Qemu-devel] " Radim Krčmář
@ 2018-03-01 15:55 ` Moger, Babu
-1 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-03-01 15:55 UTC (permalink / raw)
To: Radim Krčmář
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
Radim, Thanks for your comments. I am working on the changes.
But, I need few clarifications on your comments. Please see inline.
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Wednesday, February 28, 2018 12:09 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> Information
>
> 2018-02-23 21:30-0500, Babu Moger:
> > From: Stanislav Lanci <pixo@polepetko.eu>
> >
> > Adds information about cache size and topology from cpuid 0x8000001D
> leaf
> > for different cache types on AMD processors.
> >
> > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > @@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env,
> uint32_t index, uint32_t count,
> > *edx = 0;
> > }
> > break;
> > + case 0x8000001D: /* AMD TOPOEXT cache info */
> > + if (cpu->cache_info_passthrough) {
> > + host_cpuid(index, count, eax, ebx, ecx, edx);
> > + break;
> > + } else if (env->features[FEAT_8000_0001_ECX] &
> CPUID_EXT3_TOPOEXT) {
> > + *eax = 0;
> > + switch (count) {
> > + case 0: /* L1 dcache info */
> > + *eax |= CPUID_4_TYPE_DCACHE | \
> > + CPUID_4_LEVEL(1) | \
> > + CPUID_4_SELF_INIT_LEVEL | \
> > + ((cs->nr_threads - 1) << 14);
>
> CPUID_4 uses the same format even for bits 25-14, so this would look
> better with a macro.
Yes. We can do that.
>
> > + *ebx = (L1D_LINE_SIZE - 1) | \
> > + ((L1D_PARTITIONS - 1) << 12) | \
> > + ((L1D_ASSOCIATIVITY - 1) << 22);
> > + *ecx = L1D_SETS - 1;
>
> These numbers seem to have the same meaning as CPUID 4, but have
> conflicting values.
I am not sure about conflicting values. Looking at the specs(page 78 CPUID_Fn8000001D_EBX_x00)
https://support.amd.com/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf
It looks correct to me.
>
> I think we should not expose CPUID 4 with AMD CPUs or at least when they
> have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).
Can you please elaborate on these comments?
Did you mean we should remove the check CPUID_EXT3_TOPOEXT and remove all CPUID 4 references?
>
> > + *edx = 0;
> > + break;
> > + case 1: /* L1 icache info */
> > + *eax |= CPUID_4_TYPE_ICACHE | \
> > + CPUID_4_LEVEL(1) | \
> > + CPUID_4_SELF_INIT_LEVEL | \
> > + ((cs->nr_threads - 1) << 14);
> > + *ebx = (L1I_LINE_SIZE - 1) | \
> > + ((L1I_PARTITIONS - 1) << 12) | \
> > + ((L1I_ASSOCIATIVITY_AMD - 1) << 22);
> > + *ecx = L1I_SETS_AMD - 1;
> > + *edx = 0;
> > + break;
> > + case 2: /* L2 cache info */
> > + *eax |= CPUID_4_TYPE_UNIFIED | \
> > + CPUID_4_LEVEL(2) | \
> > + CPUID_4_SELF_INIT_LEVEL | \
> > + ((cs->nr_threads - 1) << 14);
> > + *ebx = (L2_LINE_SIZE - 1) | \
> > + ((L2_PARTITIONS - 1) << 12) | \
> > + ((L2_ASSOCIATIVITY_AMD - 1) << 22);
> > + *ecx = L2_SETS_AMD - 1;
> > + *edx = CPUID_4_INCLUSIVE;
> > + break;
> > + case 3: /* L3 cache info */
> > + if (!cpu->enable_l3_cache) {
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + break;
> > + }
> > + *eax |= CPUID_4_TYPE_UNIFIED | \
> > + CPUID_4_LEVEL(3) | \
> > + CPUID_4_SELF_INIT_LEVEL | \
> > + ((cs->nr_cores * cs->nr_threads - 1) << 14);
>
> This number seems to be the only difference that isn't just a difference
> constant. It tempts me to merge the cases for 4 and 0x8000001D as it
> seems that vendors try to be compatible.
Yes. We could merge cases for 4 and 0x8000001D.
>
> > + *ebx = (L3_N_LINE_SIZE - 1) | \
> > + ((L3_N_PARTITIONS - 1) << 12) | \
> > + ((L3_N_ASSOCIATIVITY - 1) << 22);
> > + *ecx = L3_N_SETS_AMD - 1;
> > + *edx = CPUID_4_NO_INVD_SHARING;
> > + break;
> > + default: /* end of info */
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + break;
> > + }
> > + } else {
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + }
> > + break;
> > case 0xC0000000:
> > *eax = env->cpuid_xlevel2;
> > *ebx = 0;
>
> The numbers looks like real hardware,
Do you want me to change anything here?
>
> thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
@ 2018-03-01 15:55 ` Moger, Babu
0 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-03-01 15:55 UTC (permalink / raw)
To: Radim Krčmář
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Hook, Gary
Radim, Thanks for your comments. I am working on the changes.
But, I need few clarifications on your comments. Please see inline.
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Wednesday, February 28, 2018 12:09 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> Information
>
> 2018-02-23 21:30-0500, Babu Moger:
> > From: Stanislav Lanci <pixo@polepetko.eu>
> >
> > Adds information about cache size and topology from cpuid 0x8000001D
> leaf
> > for different cache types on AMD processors.
> >
> > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > @@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env,
> uint32_t index, uint32_t count,
> > *edx = 0;
> > }
> > break;
> > + case 0x8000001D: /* AMD TOPOEXT cache info */
> > + if (cpu->cache_info_passthrough) {
> > + host_cpuid(index, count, eax, ebx, ecx, edx);
> > + break;
> > + } else if (env->features[FEAT_8000_0001_ECX] &
> CPUID_EXT3_TOPOEXT) {
> > + *eax = 0;
> > + switch (count) {
> > + case 0: /* L1 dcache info */
> > + *eax |= CPUID_4_TYPE_DCACHE | \
> > + CPUID_4_LEVEL(1) | \
> > + CPUID_4_SELF_INIT_LEVEL | \
> > + ((cs->nr_threads - 1) << 14);
>
> CPUID_4 uses the same format even for bits 25-14, so this would look
> better with a macro.
Yes. We can do that.
>
> > + *ebx = (L1D_LINE_SIZE - 1) | \
> > + ((L1D_PARTITIONS - 1) << 12) | \
> > + ((L1D_ASSOCIATIVITY - 1) << 22);
> > + *ecx = L1D_SETS - 1;
>
> These numbers seem to have the same meaning as CPUID 4, but have
> conflicting values.
I am not sure about conflicting values. Looking at the specs(page 78 CPUID_Fn8000001D_EBX_x00)
https://support.amd.com/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf
It looks correct to me.
>
> I think we should not expose CPUID 4 with AMD CPUs or at least when they
> have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).
Can you please elaborate on these comments?
Did you mean we should remove the check CPUID_EXT3_TOPOEXT and remove all CPUID 4 references?
>
> > + *edx = 0;
> > + break;
> > + case 1: /* L1 icache info */
> > + *eax |= CPUID_4_TYPE_ICACHE | \
> > + CPUID_4_LEVEL(1) | \
> > + CPUID_4_SELF_INIT_LEVEL | \
> > + ((cs->nr_threads - 1) << 14);
> > + *ebx = (L1I_LINE_SIZE - 1) | \
> > + ((L1I_PARTITIONS - 1) << 12) | \
> > + ((L1I_ASSOCIATIVITY_AMD - 1) << 22);
> > + *ecx = L1I_SETS_AMD - 1;
> > + *edx = 0;
> > + break;
> > + case 2: /* L2 cache info */
> > + *eax |= CPUID_4_TYPE_UNIFIED | \
> > + CPUID_4_LEVEL(2) | \
> > + CPUID_4_SELF_INIT_LEVEL | \
> > + ((cs->nr_threads - 1) << 14);
> > + *ebx = (L2_LINE_SIZE - 1) | \
> > + ((L2_PARTITIONS - 1) << 12) | \
> > + ((L2_ASSOCIATIVITY_AMD - 1) << 22);
> > + *ecx = L2_SETS_AMD - 1;
> > + *edx = CPUID_4_INCLUSIVE;
> > + break;
> > + case 3: /* L3 cache info */
> > + if (!cpu->enable_l3_cache) {
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + break;
> > + }
> > + *eax |= CPUID_4_TYPE_UNIFIED | \
> > + CPUID_4_LEVEL(3) | \
> > + CPUID_4_SELF_INIT_LEVEL | \
> > + ((cs->nr_cores * cs->nr_threads - 1) << 14);
>
> This number seems to be the only difference that isn't just a difference
> constant. It tempts me to merge the cases for 4 and 0x8000001D as it
> seems that vendors try to be compatible.
Yes. We could merge cases for 4 and 0x8000001D.
>
> > + *ebx = (L3_N_LINE_SIZE - 1) | \
> > + ((L3_N_PARTITIONS - 1) << 12) | \
> > + ((L3_N_ASSOCIATIVITY - 1) << 22);
> > + *ecx = L3_N_SETS_AMD - 1;
> > + *edx = CPUID_4_NO_INVD_SHARING;
> > + break;
> > + default: /* end of info */
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + break;
> > + }
> > + } else {
> > + *eax = 0;
> > + *ebx = 0;
> > + *ecx = 0;
> > + *edx = 0;
> > + }
> > + break;
> > case 0xC0000000:
> > *eax = env->cpuid_xlevel2;
> > *ebx = 0;
>
> The numbers looks like real hardware,
Do you want me to change anything here?
>
> thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
2018-03-01 15:55 ` [Qemu-devel] " Moger, Babu
@ 2018-03-01 19:56 ` Radim Krčmář
-1 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-03-01 19:56 UTC (permalink / raw)
To: Moger, Babu
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
2018-03-01 15:55+0000, Moger, Babu:
> Radim, Thanks for your comments. I am working on the changes.
> But, I need few clarifications on your comments. Please see inline.
>
> > -----Original Message-----
> > From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> > Sent: Wednesday, February 28, 2018 12:09 PM
> > To: Moger, Babu <Babu.Moger@amd.com>
> > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> > mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> > pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> > Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> > Information
> >
> > 2018-02-23 21:30-0500, Babu Moger:
> > > From: Stanislav Lanci <pixo@polepetko.eu>
> > >
> > > Adds information about cache size and topology from cpuid 0x8000001D
> > leaf
> > > for different cache types on AMD processors.
> > >
> > > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > ---
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > @@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env,
> > > + *ebx = (L1D_LINE_SIZE - 1) | \
> > > + ((L1D_PARTITIONS - 1) << 12) | \
> > > + ((L1D_ASSOCIATIVITY - 1) << 22);
> > > + *ecx = L1D_SETS - 1;
> >
> > These numbers seem to have the same meaning as CPUID 4, but have
> > conflicting values.
>
> I am not sure about conflicting values. Looking at the specs(page 78 CPUID_Fn8000001D_EBX_x00)
> https://support.amd.com/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf
> It looks correct to me.
I agree. My comment is misplaced -- it should have been under the place
that uses *_AMD macros.
I wanted to point out that CPUID in QEMU is very Intel-focused and
always contains CPUID leaf 4, which has fields of the very same meaning,
but with different values.
> > I think we should not expose CPUID 4 with AMD CPUs or at least when they
> > have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).
>
> Can you please elaborate on these comments?
> Did you mean we should remove the check CPUID_EXT3_TOPOEXT and remove all CPUID 4 references?
CPUID 4 should have never been present when emulating AMD CPUs, but it's
worse now that the numbers are conflicting.
I meant to hide CPUID 4 for all AMD CPUs on future machine types, or at
least when CPUID_EXT3_TOPOEXT is enabled.
Keeping the current logic not a big problem as CPUID 4 should never be
used by operating systems on AMD nor trusted inside a VM. Bringing the
emulation closer to real state would be nice, but this can definitely be
done later (aka never).
> > The numbers looks like real hardware,
>
> Do you want me to change anything here?
No, I was just commending,
thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
@ 2018-03-01 19:56 ` Radim Krčmář
0 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-03-01 19:56 UTC (permalink / raw)
To: Moger, Babu
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Hook, Gary
2018-03-01 15:55+0000, Moger, Babu:
> Radim, Thanks for your comments. I am working on the changes.
> But, I need few clarifications on your comments. Please see inline.
>
> > -----Original Message-----
> > From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> > Sent: Wednesday, February 28, 2018 12:09 PM
> > To: Moger, Babu <Babu.Moger@amd.com>
> > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> > mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> > pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> > Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> > Information
> >
> > 2018-02-23 21:30-0500, Babu Moger:
> > > From: Stanislav Lanci <pixo@polepetko.eu>
> > >
> > > Adds information about cache size and topology from cpuid 0x8000001D
> > leaf
> > > for different cache types on AMD processors.
> > >
> > > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > ---
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > @@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env,
> > > + *ebx = (L1D_LINE_SIZE - 1) | \
> > > + ((L1D_PARTITIONS - 1) << 12) | \
> > > + ((L1D_ASSOCIATIVITY - 1) << 22);
> > > + *ecx = L1D_SETS - 1;
> >
> > These numbers seem to have the same meaning as CPUID 4, but have
> > conflicting values.
>
> I am not sure about conflicting values. Looking at the specs(page 78 CPUID_Fn8000001D_EBX_x00)
> https://support.amd.com/TechDocs/54945_PPR_Family_17h_Models_00h-0Fh.pdf
> It looks correct to me.
I agree. My comment is misplaced -- it should have been under the place
that uses *_AMD macros.
I wanted to point out that CPUID in QEMU is very Intel-focused and
always contains CPUID leaf 4, which has fields of the very same meaning,
but with different values.
> > I think we should not expose CPUID 4 with AMD CPUs or at least when they
> > have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).
>
> Can you please elaborate on these comments?
> Did you mean we should remove the check CPUID_EXT3_TOPOEXT and remove all CPUID 4 references?
CPUID 4 should have never been present when emulating AMD CPUs, but it's
worse now that the numbers are conflicting.
I meant to hide CPUID 4 for all AMD CPUs on future machine types, or at
least when CPUID_EXT3_TOPOEXT is enabled.
Keeping the current logic not a big problem as CPUID 4 should never be
used by operating systems on AMD nor trusted inside a VM. Bringing the
emulation closer to real state would be nice, but this can definitely be
done later (aka never).
> > The numbers looks like real hardware,
>
> Do you want me to change anything here?
No, I was just commending,
thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
2018-03-01 19:56 ` [Qemu-devel] " Radim Krčmář
@ 2018-03-02 16:50 ` Moger, Babu
-1 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-03-02 16:50 UTC (permalink / raw)
To: Radim Krčmář
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Thursday, March 1, 2018 1:56 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> Information
>
> 2018-03-01 15:55+0000, Moger, Babu:
> > Radim, Thanks for your comments. I am working on the changes.
> > But, I need few clarifications on your comments. Please see inline.
> >
> > > -----Original Message-----
> > > From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> > > Sent: Wednesday, February 28, 2018 12:09 PM
> > > To: Moger, Babu <Babu.Moger@amd.com>
> > > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> > > mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> > > pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> > > Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> > > Information
> > >
> > > 2018-02-23 21:30-0500, Babu Moger:
> > > > From: Stanislav Lanci <pixo@polepetko.eu>
> > > >
> > > > Adds information about cache size and topology from cpuid 0x8000001D
> > > leaf
> > > > for different cache types on AMD processors.
> > > >
> > > > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > > ---
> > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > @@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env,
> > > > + *ebx = (L1D_LINE_SIZE - 1) | \
> > > > + ((L1D_PARTITIONS - 1) << 12) | \
> > > > + ((L1D_ASSOCIATIVITY - 1) << 22);
> > > > + *ecx = L1D_SETS - 1;
> > >
> > > These numbers seem to have the same meaning as CPUID 4, but have
> > > conflicting values.
> >
> > I am not sure about conflicting values. Looking at the specs(page 78
> CPUID_Fn8000001D_EBX_x00)
> >
> https://support.amd.com/TechDocs/54945_PPR_Family_17h_Models_00h-
> 0Fh.pdf
> > It looks correct to me.
>
> I agree. My comment is misplaced -- it should have been under the place
> that uses *_AMD macros.
>
> I wanted to point out that CPUID in QEMU is very Intel-focused and
> always contains CPUID leaf 4, which has fields of the very same meaning,
> but with different values.
>
> > > I think we should not expose CPUID 4 with AMD CPUs or at least when
> they
> > > have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).
> >
> > Can you please elaborate on these comments?
> > Did you mean we should remove the check CPUID_EXT3_TOPOEXT and
> remove all CPUID 4 references?
>
> CPUID 4 should have never been present when emulating AMD CPUs, but it's
> worse now that the numbers are conflicting.
>
> I meant to hide CPUID 4 for all AMD CPUs on future machine types, or at
> least when CPUID_EXT3_TOPOEXT is enabled.
Sorry, I think I created some confusion here by using CPUID 4 definitions which are
mostly for intel. Let me rework on this. I will repost the patches. Thanks for the
feedback.
>
> Keeping the current logic not a big problem as CPUID 4 should never be
> used by operating systems on AMD nor trusted inside a VM. Bringing the
> emulation closer to real state would be nice, but this can definitely be
> done later (aka never).
>
> > > The numbers looks like real hardware,
> >
> > Do you want me to change anything here?
>
> No, I was just commending,
>
> thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/5] target/i386: Populate AMD Processor Cache Information
@ 2018-03-02 16:50 ` Moger, Babu
0 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-03-02 16:50 UTC (permalink / raw)
To: Radim Krčmář
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Hook, Gary
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Thursday, March 1, 2018 1:56 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> Information
>
> 2018-03-01 15:55+0000, Moger, Babu:
> > Radim, Thanks for your comments. I am working on the changes.
> > But, I need few clarifications on your comments. Please see inline.
> >
> > > -----Original Message-----
> > > From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> > > Sent: Wednesday, February 28, 2018 12:09 PM
> > > To: Moger, Babu <Babu.Moger@amd.com>
> > > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> > > mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> > > pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> > > Subject: Re: [PATCH v2 2/5] target/i386: Populate AMD Processor Cache
> > > Information
> > >
> > > 2018-02-23 21:30-0500, Babu Moger:
> > > > From: Stanislav Lanci <pixo@polepetko.eu>
> > > >
> > > > Adds information about cache size and topology from cpuid 0x8000001D
> > > leaf
> > > > for different cache types on AMD processors.
> > > >
> > > > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > > ---
> > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > @@ -3590,6 +3594,78 @@ void cpu_x86_cpuid(CPUX86State *env,
> > > > + *ebx = (L1D_LINE_SIZE - 1) | \
> > > > + ((L1D_PARTITIONS - 1) << 12) | \
> > > > + ((L1D_ASSOCIATIVITY - 1) << 22);
> > > > + *ecx = L1D_SETS - 1;
> > >
> > > These numbers seem to have the same meaning as CPUID 4, but have
> > > conflicting values.
> >
> > I am not sure about conflicting values. Looking at the specs(page 78
> CPUID_Fn8000001D_EBX_x00)
> >
> https://support.amd.com/TechDocs/54945_PPR_Family_17h_Models_00h-
> 0Fh.pdf
> > It looks correct to me.
>
> I agree. My comment is misplaced -- it should have been under the place
> that uses *_AMD macros.
>
> I wanted to point out that CPUID in QEMU is very Intel-focused and
> always contains CPUID leaf 4, which has fields of the very same meaning,
> but with different values.
>
> > > I think we should not expose CPUID 4 with AMD CPUs or at least when
> they
> > > have CPUID_EXT3_TOPOEXT (the latter is easier wrt. compatibility).
> >
> > Can you please elaborate on these comments?
> > Did you mean we should remove the check CPUID_EXT3_TOPOEXT and
> remove all CPUID 4 references?
>
> CPUID 4 should have never been present when emulating AMD CPUs, but it's
> worse now that the numbers are conflicting.
>
> I meant to hide CPUID 4 for all AMD CPUs on future machine types, or at
> least when CPUID_EXT3_TOPOEXT is enabled.
Sorry, I think I created some confusion here by using CPUID 4 definitions which are
mostly for intel. Let me rework on this. I will repost the patches. Thanks for the
feedback.
>
> Keeping the current logic not a big problem as CPUID 4 should never be
> used by operating systems on AMD nor trusted inside a VM. Bringing the
> emulation closer to real state would be nice, but this can definitely be
> done later (aka never).
>
> > > The numbers looks like real hardware,
> >
> > Do you want me to change anything here?
>
> No, I was just commending,
>
> thanks.
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
2018-02-24 2:30 ` [Qemu-devel] " Babu Moger
@ 2018-02-24 2:30 ` Babu Moger
-1 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: pixo, kvm, mtosatti, Gary.Hook, qemu-devel, babu.moger
From: Stanislav Lanci <pixo@polepetko.eu>
Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
feature is supported. This is required to support hyperthreading
feature on AMD CPUS. These are supported via CPUID_8000_001E extended
functions.
Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a5a480e..191e850 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*edx = 0;
}
break;
+ case 0x8000001E:
+ if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
+ *eax = cpu->apic_id;
+ *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
+ *ecx = cpu->socket_id;
+ *edx = 0;
+ }
+ break;
case 0xC0000000:
*eax = env->cpuid_xlevel2;
*ebx = 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
@ 2018-02-24 2:30 ` Babu Moger
0 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: mtosatti, qemu-devel, kvm, pixo, Gary.Hook, babu.moger
From: Stanislav Lanci <pixo@polepetko.eu>
Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
feature is supported. This is required to support hyperthreading
feature on AMD CPUS. These are supported via CPUID_8000_001E extended
functions.
Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index a5a480e..191e850 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
*edx = 0;
}
break;
+ case 0x8000001E:
+ if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
+ *eax = cpu->apic_id;
+ *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
+ *ecx = cpu->socket_id;
+ *edx = 0;
+ }
+ break;
case 0xC0000000:
*eax = env->cpuid_xlevel2;
*ebx = 0;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
2018-02-24 2:30 ` [Qemu-devel] " Babu Moger
@ 2018-02-28 18:24 ` Radim Krčmář
-1 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-02-28 18:24 UTC (permalink / raw)
To: Babu Moger
Cc: ehabkost, kvm, mtosatti, Gary.Hook, qemu-devel, pbonzini, pixo, rth
2018-02-23 21:30-0500, Babu Moger:
> From: Stanislav Lanci <pixo@polepetko.eu>
>
> Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
> feature is supported. This is required to support hyperthreading
> feature on AMD CPUS. These are supported via CPUID_8000_001E extended
> functions.
>
> Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> target/i386/cpu.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a5a480e..191e850 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> *edx = 0;
> }
> break;
> + case 0x8000001E:
> + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
> + *eax = cpu->apic_id;
> + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
Do we somewhere assert that AMD cannot have cpu->core_id > 255?
(qemu does allow weird configurations.)
Thanks.
> + *ecx = cpu->socket_id;
> + *edx = 0;
> + }
> + break;
> case 0xC0000000:
> *eax = env->cpuid_xlevel2;
> *ebx = 0;
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
@ 2018-02-28 18:24 ` Radim Krčmář
0 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-02-28 18:24 UTC (permalink / raw)
To: Babu Moger
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Gary.Hook
2018-02-23 21:30-0500, Babu Moger:
> From: Stanislav Lanci <pixo@polepetko.eu>
>
> Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
> feature is supported. This is required to support hyperthreading
> feature on AMD CPUS. These are supported via CPUID_8000_001E extended
> functions.
>
> Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> Signed-off-by: Babu Moger <babu.moger@amd.com>
> ---
> target/i386/cpu.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index a5a480e..191e850 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
> *edx = 0;
> }
> break;
> + case 0x8000001E:
> + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
> + *eax = cpu->apic_id;
> + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
Do we somewhere assert that AMD cannot have cpu->core_id > 255?
(qemu does allow weird configurations.)
Thanks.
> + *ecx = cpu->socket_id;
> + *edx = 0;
> + }
> + break;
> case 0xC0000000:
> *eax = env->cpuid_xlevel2;
> *ebx = 0;
> --
> 1.8.3.1
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
2018-02-28 18:24 ` [Qemu-devel] " Radim Krčmář
@ 2018-02-28 22:18 ` Moger, Babu
-1 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-02-28 22:18 UTC (permalink / raw)
To: Radim Krčmář
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Wednesday, February 28, 2018 12:24 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E
> for AMD
>
> 2018-02-23 21:30-0500, Babu Moger:
> > From: Stanislav Lanci <pixo@polepetko.eu>
> >
> > Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
> > feature is supported. This is required to support hyperthreading
> > feature on AMD CPUS. These are supported via CPUID_8000_001E
> extended
> > functions.
> >
> > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> > target/i386/cpu.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index a5a480e..191e850 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env,
> uint32_t index, uint32_t count,
> > *edx = 0;
> > }
> > break;
> > + case 0x8000001E:
> > + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
> > + *eax = cpu->apic_id;
> > + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
>
> Do we somewhere assert that AMD cannot have cpu->core_id > 255?
> (qemu does allow weird configurations.)
I don't see specific assert on core_id. But, I see that qemu does not allow nr_cores more than 255.
Also I see that core_id is iterated based on nr_cores. If you strongly believe we need to add assert here, I will add it.
Let me know.
> Thanks.
>
> > + *ecx = cpu->socket_id;
> > + *edx = 0;
> > + }
> > + break;
> > case 0xC0000000:
> > *eax = env->cpuid_xlevel2;
> > *ebx = 0;
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
@ 2018-02-28 22:18 ` Moger, Babu
0 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-02-28 22:18 UTC (permalink / raw)
To: Radim Krčmář
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Hook, Gary
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Wednesday, February 28, 2018 12:24 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E
> for AMD
>
> 2018-02-23 21:30-0500, Babu Moger:
> > From: Stanislav Lanci <pixo@polepetko.eu>
> >
> > Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
> > feature is supported. This is required to support hyperthreading
> > feature on AMD CPUS. These are supported via CPUID_8000_001E
> extended
> > functions.
> >
> > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > ---
> > target/i386/cpu.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > index a5a480e..191e850 100644
> > --- a/target/i386/cpu.c
> > +++ b/target/i386/cpu.c
> > @@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env,
> uint32_t index, uint32_t count,
> > *edx = 0;
> > }
> > break;
> > + case 0x8000001E:
> > + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
> > + *eax = cpu->apic_id;
> > + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
>
> Do we somewhere assert that AMD cannot have cpu->core_id > 255?
> (qemu does allow weird configurations.)
I don't see specific assert on core_id. But, I see that qemu does not allow nr_cores more than 255.
Also I see that core_id is iterated based on nr_cores. If you strongly believe we need to add assert here, I will add it.
Let me know.
> Thanks.
>
> > + *ecx = cpu->socket_id;
> > + *edx = 0;
> > + }
> > + break;
> > case 0xC0000000:
> > *eax = env->cpuid_xlevel2;
> > *ebx = 0;
> > --
> > 1.8.3.1
> >
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
2018-02-28 22:18 ` [Qemu-devel] " Moger, Babu
@ 2018-03-01 19:57 ` Radim Krčmář
-1 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-03-01 19:57 UTC (permalink / raw)
To: Moger, Babu
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
2018-02-28 22:18+0000, Moger, Babu:
> > -----Original Message-----
> > From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> > Sent: Wednesday, February 28, 2018 12:24 PM
> > To: Moger, Babu <Babu.Moger@amd.com>
> > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> > mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> > pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> > Subject: Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E
> > for AMD
> >
> > 2018-02-23 21:30-0500, Babu Moger:
> > > From: Stanislav Lanci <pixo@polepetko.eu>
> > >
> > > Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
> > > feature is supported. This is required to support hyperthreading
> > > feature on AMD CPUS. These are supported via CPUID_8000_001E
> > extended
> > > functions.
> > >
> > > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > ---
> > > target/i386/cpu.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > index a5a480e..191e850 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env,
> > uint32_t index, uint32_t count,
> > > *edx = 0;
> > > }
> > > break;
> > > + case 0x8000001E:
> > > + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
> > > + *eax = cpu->apic_id;
> > > + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
> >
> > Do we somewhere assert that AMD cannot have cpu->core_id > 255?
> > (qemu does allow weird configurations.)
>
> I don't see specific assert on core_id. But, I see that qemu does not allow nr_cores more than 255.
> Also I see that core_id is iterated based on nr_cores. If you strongly believe we need to add assert here, I will add it.
> Let me know.
A user can emulate AMD with x2apic and intel-iommu to allow more than
255 vcpus and all of them can be a separate core (or its own socket),
which would overflow this (or the next counter).
Forbidding that crazy configuration may be preferable, but an assert
won't hurt now,
thanks.
> > Thanks.
> >
> > > + *ecx = cpu->socket_id;
> > > + *edx = 0;
> > > + }
> > > + break;
> > > case 0xC0000000:
> > > *eax = env->cpuid_xlevel2;
> > > *ebx = 0;
> > > --
> > > 1.8.3.1
> > >
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
@ 2018-03-01 19:57 ` Radim Krčmář
0 siblings, 0 replies; 36+ messages in thread
From: Radim Krčmář @ 2018-03-01 19:57 UTC (permalink / raw)
To: Moger, Babu
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Hook, Gary
2018-02-28 22:18+0000, Moger, Babu:
> > -----Original Message-----
> > From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> > Sent: Wednesday, February 28, 2018 12:24 PM
> > To: Moger, Babu <Babu.Moger@amd.com>
> > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> > mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> > pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> > Subject: Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E
> > for AMD
> >
> > 2018-02-23 21:30-0500, Babu Moger:
> > > From: Stanislav Lanci <pixo@polepetko.eu>
> > >
> > > Populate threads/core_id/apic_ids/socket_id when CPUID_EXT3_TOPOEXT
> > > feature is supported. This is required to support hyperthreading
> > > feature on AMD CPUS. These are supported via CPUID_8000_001E
> > extended
> > > functions.
> > >
> > > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > ---
> > > target/i386/cpu.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > index a5a480e..191e850 100644
> > > --- a/target/i386/cpu.c
> > > +++ b/target/i386/cpu.c
> > > @@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env,
> > uint32_t index, uint32_t count,
> > > *edx = 0;
> > > }
> > > break;
> > > + case 0x8000001E:
> > > + if (env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) {
> > > + *eax = cpu->apic_id;
> > > + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
> >
> > Do we somewhere assert that AMD cannot have cpu->core_id > 255?
> > (qemu does allow weird configurations.)
>
> I don't see specific assert on core_id. But, I see that qemu does not allow nr_cores more than 255.
> Also I see that core_id is iterated based on nr_cores. If you strongly believe we need to add assert here, I will add it.
> Let me know.
A user can emulate AMD with x2apic and intel-iommu to allow more than
255 vcpus and all of them can be a separate core (or its own socket),
which would overflow this (or the next counter).
Forbidding that crazy configuration may be preferable, but an assert
won't hurt now,
thanks.
> > Thanks.
> >
> > > + *ecx = cpu->socket_id;
> > > + *edx = 0;
> > > + }
> > > + break;
> > > case 0xC0000000:
> > > *eax = env->cpuid_xlevel2;
> > > *ebx = 0;
> > > --
> > > 1.8.3.1
> > >
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
2018-03-01 19:57 ` [Qemu-devel] " Radim Krčmář
@ 2018-03-02 16:50 ` Moger, Babu
-1 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-03-02 16:50 UTC (permalink / raw)
To: Radim Krčmář
Cc: ehabkost, kvm, mtosatti, Hook, Gary, qemu-devel, pbonzini, pixo, rth
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Thursday, March 1, 2018 1:57 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E
> for AMD
>
> 2018-02-28 22:18+0000, Moger, Babu:
> > > -----Original Message-----
> > > From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> > > Sent: Wednesday, February 28, 2018 12:24 PM
> > > To: Moger, Babu <Babu.Moger@amd.com>
> > > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> > > mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> > > pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> > > Subject: Re: [PATCH v2 3/5] target/i386: Add support for
> CPUID_8000_001E
> > > for AMD
> > >
> > > 2018-02-23 21:30-0500, Babu Moger:
> > > > From: Stanislav Lanci <pixo@polepetko.eu>
> > > >
> > > > Populate threads/core_id/apic_ids/socket_id when
> CPUID_EXT3_TOPOEXT
> > > > feature is supported. This is required to support hyperthreading
> > > > feature on AMD CPUS. These are supported via CPUID_8000_001E
> > > extended
> > > > functions.
> > > >
> > > > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > > ---
> > > > target/i386/cpu.c | 8 ++++++++
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > index a5a480e..191e850 100644
> > > > --- a/target/i386/cpu.c
> > > > +++ b/target/i386/cpu.c
> > > > @@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env,
> > > uint32_t index, uint32_t count,
> > > > *edx = 0;
> > > > }
> > > > break;
> > > > + case 0x8000001E:
> > > > + if (env->features[FEAT_8000_0001_ECX] &
> CPUID_EXT3_TOPOEXT) {
> > > > + *eax = cpu->apic_id;
> > > > + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
> > >
> > > Do we somewhere assert that AMD cannot have cpu->core_id > 255?
> > > (qemu does allow weird configurations.)
> >
> > I don't see specific assert on core_id. But, I see that qemu does not allow
> nr_cores more than 255.
> > Also I see that core_id is iterated based on nr_cores. If you strongly
> believe we need to add assert here, I will add it.
> > Let me know.
>
> A user can emulate AMD with x2apic and intel-iommu to allow more than
> 255 vcpus and all of them can be a separate core (or its own socket),
> which would overflow this (or the next counter).
>
> Forbidding that crazy configuration may be preferable, but an assert
> won't hurt now,
Ok. Sure. Will add assert.
>
> thanks.
>
> > > Thanks.
> > >
> > > > + *ecx = cpu->socket_id;
> > > > + *edx = 0;
> > > > + }
> > > > + break;
> > > > case 0xC0000000:
> > > > *eax = env->cpuid_xlevel2;
> > > > *ebx = 0;
> > > > --
> > > > 1.8.3.1
> > > >
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [Qemu-devel] [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E for AMD
@ 2018-03-02 16:50 ` Moger, Babu
0 siblings, 0 replies; 36+ messages in thread
From: Moger, Babu @ 2018-03-02 16:50 UTC (permalink / raw)
To: Radim Krčmář
Cc: pbonzini, rth, ehabkost, mtosatti, qemu-devel, kvm, pixo, Hook, Gary
> -----Original Message-----
> From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> Sent: Thursday, March 1, 2018 1:57 PM
> To: Moger, Babu <Babu.Moger@amd.com>
> Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> Subject: Re: [PATCH v2 3/5] target/i386: Add support for CPUID_8000_001E
> for AMD
>
> 2018-02-28 22:18+0000, Moger, Babu:
> > > -----Original Message-----
> > > From: Radim Krčmář [mailto:rkrcmar@redhat.com]
> > > Sent: Wednesday, February 28, 2018 12:24 PM
> > > To: Moger, Babu <Babu.Moger@amd.com>
> > > Cc: pbonzini@redhat.com; rth@twiddle.net; ehabkost@redhat.com;
> > > mtosatti@redhat.com; qemu-devel@nongnu.org; kvm@vger.kernel.org;
> > > pixo@polepetko.eu; Hook, Gary <Gary.Hook@amd.com>
> > > Subject: Re: [PATCH v2 3/5] target/i386: Add support for
> CPUID_8000_001E
> > > for AMD
> > >
> > > 2018-02-23 21:30-0500, Babu Moger:
> > > > From: Stanislav Lanci <pixo@polepetko.eu>
> > > >
> > > > Populate threads/core_id/apic_ids/socket_id when
> CPUID_EXT3_TOPOEXT
> > > > feature is supported. This is required to support hyperthreading
> > > > feature on AMD CPUS. These are supported via CPUID_8000_001E
> > > extended
> > > > functions.
> > > >
> > > > Signed-off-by: Stanislav Lanci <pixo@polepetko.eu>
> > > > Signed-off-by: Babu Moger <babu.moger@amd.com>
> > > > ---
> > > > target/i386/cpu.c | 8 ++++++++
> > > > 1 file changed, 8 insertions(+)
> > > >
> > > > diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> > > > index a5a480e..191e850 100644
> > > > --- a/target/i386/cpu.c
> > > > +++ b/target/i386/cpu.c
> > > > @@ -3666,6 +3666,14 @@ void cpu_x86_cpuid(CPUX86State *env,
> > > uint32_t index, uint32_t count,
> > > > *edx = 0;
> > > > }
> > > > break;
> > > > + case 0x8000001E:
> > > > + if (env->features[FEAT_8000_0001_ECX] &
> CPUID_EXT3_TOPOEXT) {
> > > > + *eax = cpu->apic_id;
> > > > + *ebx = (cs->nr_threads - 1) << 8 | cpu->core_id;
> > >
> > > Do we somewhere assert that AMD cannot have cpu->core_id > 255?
> > > (qemu does allow weird configurations.)
> >
> > I don't see specific assert on core_id. But, I see that qemu does not allow
> nr_cores more than 255.
> > Also I see that core_id is iterated based on nr_cores. If you strongly
> believe we need to add assert here, I will add it.
> > Let me know.
>
> A user can emulate AMD with x2apic and intel-iommu to allow more than
> 255 vcpus and all of them can be a separate core (or its own socket),
> which would overflow this (or the next counter).
>
> Forbidding that crazy configuration may be preferable, but an assert
> won't hurt now,
Ok. Sure. Will add assert.
>
> thanks.
>
> > > Thanks.
> > >
> > > > + *ecx = cpu->socket_id;
> > > > + *edx = 0;
> > > > + }
> > > > + break;
> > > > case 0xC0000000:
> > > > *eax = env->cpuid_xlevel2;
> > > > *ebx = 0;
> > > > --
> > > > 1.8.3.1
> > > >
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v2 4/5] target/i386: Enable TOPOEXT feature on AMD EPYC CPU
2018-02-24 2:30 ` [Qemu-devel] " Babu Moger
@ 2018-02-24 2:30 ` Babu Moger
-1 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: pixo, kvm, mtosatti, Gary.Hook, qemu-devel, babu.moger
Enable TOPOEXT feature on EPYC CPU. This is required to support
hyperthreading on VM guests. Also extend xlevel to 0x8000001E.
These are supported via CPUID_8000_001E extended functions.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 191e850..6d06637 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1955,7 +1955,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
.features[FEAT_8000_0001_ECX] =
CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
- CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+ CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
+ CPUID_EXT3_TOPOEXT,
.features[FEAT_7_0_EBX] =
CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 |
CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED |
@@ -1970,7 +1971,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
CPUID_XSAVE_XGETBV1,
.features[FEAT_6_EAX] =
CPUID_6_EAX_ARAT,
- .xlevel = 0x8000000A,
+ .xlevel = 0x8000001E,
.model_id = "AMD EPYC Processor",
},
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v2 4/5] target/i386: Enable TOPOEXT feature on AMD EPYC CPU
@ 2018-02-24 2:30 ` Babu Moger
0 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: mtosatti, qemu-devel, kvm, pixo, Gary.Hook, babu.moger
Enable TOPOEXT feature on EPYC CPU. This is required to support
hyperthreading on VM guests. Also extend xlevel to 0x8000001E.
These are supported via CPUID_8000_001E extended functions.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 191e850..6d06637 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -1955,7 +1955,8 @@ static X86CPUDefinition builtin_x86_defs[] = {
.features[FEAT_8000_0001_ECX] =
CPUID_EXT3_OSVW | CPUID_EXT3_3DNOWPREFETCH |
CPUID_EXT3_MISALIGNSSE | CPUID_EXT3_SSE4A | CPUID_EXT3_ABM |
- CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM,
+ CPUID_EXT3_CR8LEG | CPUID_EXT3_SVM | CPUID_EXT3_LAHF_LM |
+ CPUID_EXT3_TOPOEXT,
.features[FEAT_7_0_EBX] =
CPUID_7_0_EBX_FSGSBASE | CPUID_7_0_EBX_BMI1 | CPUID_7_0_EBX_AVX2 |
CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_BMI2 | CPUID_7_0_EBX_RDSEED |
@@ -1970,7 +1971,7 @@ static X86CPUDefinition builtin_x86_defs[] = {
CPUID_XSAVE_XGETBV1,
.features[FEAT_6_EAX] =
CPUID_6_EAX_ARAT,
- .xlevel = 0x8000000A,
+ .xlevel = 0x8000001E,
.model_id = "AMD EPYC Processor",
},
{
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread
* [PATCH v2 5/5] target/i386: Remove generic SMT thread check
2018-02-24 2:30 ` [Qemu-devel] " Babu Moger
@ 2018-02-24 2:30 ` Babu Moger
-1 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: pixo, kvm, mtosatti, Gary.Hook, qemu-devel, babu.moger
Remove generic non-intel check while validating hyperthreading support.
Certain AMD CPUs can support hyperthreading now.
CPU family with TOPOEXT feature can support hyperthreading now.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6d06637..295c409 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4336,17 +4336,20 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
qemu_init_vcpu(cs);
- /* Only Intel CPUs support hyperthreading. Even though QEMU fixes this
- * issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX
- * based on inputs (sockets,cores,threads), it is still better to gives
+ /* Most Intel and certain AMD CPUs support hyperthreading. Even though QEMU
+ * fixes this issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX
+ * based on inputs (sockets,cores,threads), it is still better to give
* users a warning.
*
* NOTE: the following code has to follow qemu_init_vcpu(). Otherwise
* cs->nr_threads hasn't be populated yet and the checking is incorrect.
*/
- if (!IS_INTEL_CPU(env) && cs->nr_threads > 1 && !ht_warned) {
- error_report("AMD CPU doesn't support hyperthreading. Please configure"
- " -smp options properly.");
+ if (IS_AMD_CPU(env) &&
+ !(env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
+ cs->nr_threads > 1 && !ht_warned) {
+ error_report("This family of AMD CPU doesn't support "
+ "hyperthreading. Please configure -smp "
+ "options properly.");
ht_warned = true;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread* [Qemu-devel] [PATCH v2 5/5] target/i386: Remove generic SMT thread check
@ 2018-02-24 2:30 ` Babu Moger
0 siblings, 0 replies; 36+ messages in thread
From: Babu Moger @ 2018-02-24 2:30 UTC (permalink / raw)
To: pbonzini, rth, ehabkost
Cc: mtosatti, qemu-devel, kvm, pixo, Gary.Hook, babu.moger
Remove generic non-intel check while validating hyperthreading support.
Certain AMD CPUs can support hyperthreading now.
CPU family with TOPOEXT feature can support hyperthreading now.
Signed-off-by: Babu Moger <babu.moger@amd.com>
---
target/i386/cpu.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 6d06637..295c409 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -4336,17 +4336,20 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp)
qemu_init_vcpu(cs);
- /* Only Intel CPUs support hyperthreading. Even though QEMU fixes this
- * issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX
- * based on inputs (sockets,cores,threads), it is still better to gives
+ /* Most Intel and certain AMD CPUs support hyperthreading. Even though QEMU
+ * fixes this issue by adjusting CPUID_0000_0001_EBX and CPUID_8000_0008_ECX
+ * based on inputs (sockets,cores,threads), it is still better to give
* users a warning.
*
* NOTE: the following code has to follow qemu_init_vcpu(). Otherwise
* cs->nr_threads hasn't be populated yet and the checking is incorrect.
*/
- if (!IS_INTEL_CPU(env) && cs->nr_threads > 1 && !ht_warned) {
- error_report("AMD CPU doesn't support hyperthreading. Please configure"
- " -smp options properly.");
+ if (IS_AMD_CPU(env) &&
+ !(env->features[FEAT_8000_0001_ECX] & CPUID_EXT3_TOPOEXT) &&
+ cs->nr_threads > 1 && !ht_warned) {
+ error_report("This family of AMD CPU doesn't support "
+ "hyperthreading. Please configure -smp "
+ "options properly.");
ht_warned = true;
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 36+ messages in thread