* [PATCH 1/2] x86/cpu: Drop unused X86_VENDOR_* values
@ 2017-01-03 12:06 Andrew Cooper
2017-01-03 12:06 ` [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor() Andrew Cooper
2017-01-03 12:25 ` [PATCH 1/2] x86/cpu: Drop unused X86_VENDOR_* values Jan Beulich
0 siblings, 2 replies; 9+ messages in thread
From: Andrew Cooper @ 2017-01-03 12:06 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Xen only has CPU drivers for Intel, Centaur and AMD. All other contributions
to X86_VENDOR_NUM simply make the cpu_devs[] array longer, reducing the
efficiency of get_cpu_vendor()
There is one remaning hidden reference to X86_VENDOR_CYRIX in the MTRR code.
However, as far as I can tell, Cyrix never realeased a 64bit processor. It is
therefore dead code.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/arch/x86/cpu/mtrr/main.c | 2 +-
xen/include/asm-x86/processor.h | 12 +++---------
2 files changed, 4 insertions(+), 10 deletions(-)
diff --git a/xen/arch/x86/cpu/mtrr/main.c b/xen/arch/x86/cpu/mtrr/main.c
index e002975..47d44d7 100644
--- a/xen/arch/x86/cpu/mtrr/main.c
+++ b/xen/arch/x86/cpu/mtrr/main.c
@@ -94,7 +94,7 @@ static void __init set_num_var_ranges(void)
rdmsrl(MSR_MTRRcap, config);
} else if (is_cpu(AMD))
config = 2;
- else if (is_cpu(CYRIX) || is_cpu(CENTAUR))
+ else if (is_cpu(CENTAUR))
config = 8;
num_var_ranges = config & 0xff;
}
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 9870589..be31586 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -20,15 +20,9 @@
* CPU vendor IDs
*/
#define X86_VENDOR_INTEL 0
-#define X86_VENDOR_CYRIX 1
-#define X86_VENDOR_AMD 2
-#define X86_VENDOR_UMC 3
-#define X86_VENDOR_NEXGEN 4
-#define X86_VENDOR_CENTAUR 5
-#define X86_VENDOR_RISE 6
-#define X86_VENDOR_TRANSMETA 7
-#define X86_VENDOR_NSC 8
-#define X86_VENDOR_NUM 9
+#define X86_VENDOR_AMD 1
+#define X86_VENDOR_CENTAUR 2
+#define X86_VENDOR_NUM 3
#define X86_VENDOR_UNKNOWN 0xff
/*
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor()
2017-01-03 12:06 [PATCH 1/2] x86/cpu: Drop unused X86_VENDOR_* values Andrew Cooper
@ 2017-01-03 12:06 ` Andrew Cooper
2017-01-03 12:40 ` Jan Beulich
2017-01-04 13:33 ` Jan Beulich
2017-01-03 12:25 ` [PATCH 1/2] x86/cpu: Drop unused X86_VENDOR_* values Jan Beulich
1 sibling, 2 replies; 9+ messages in thread
From: Andrew Cooper @ 2017-01-03 12:06 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich
Comparing 3 integers is more efficient than using strcmp(), and is more useful
to the gcv_guest case than having to fabricate a suitable string to pass. The
gcv_host cases have both options easily to hand, and experimentally, the
resulting code is more efficient.
While modifying get_cpu_vendor(), fix a bug where this_cpu got updated even in
the gcv_guest case.
Update the cpu_dev structure to be more efficient. c_vendor[] only needs to
be 8 bytes long to cover all the CPU drivers Xen has, which avoids storing an
8-byte pointer to 8 bytes of data. Drop c_ident[1] as we have no CPU drivers
with a second ident string, and turn it into a transparent union to allow
access to the integer values directly.
This avoids all need for the vendor_id union in update_domain_cpuid_info().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
xen/arch/x86/cpu/common.c | 30 +++++++++++++++---------------
xen/arch/x86/cpu/cpu.h | 10 +++++++---
xen/arch/x86/domctl.c | 15 ++-------------
xen/include/asm-x86/processor.h | 2 +-
4 files changed, 25 insertions(+), 32 deletions(-)
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index a89cf07..d17a2ee 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -156,17 +156,17 @@ void display_cacheinfo(struct cpuinfo_x86 *c)
l2size, ecx & 0xFF);
}
-int get_cpu_vendor(const char v[], enum get_cpu_vendor mode)
+int get_cpu_vendor(uint32_t b, uint32_t c, uint32_t d, enum get_cpu_vendor mode)
{
int i;
static int printed;
for (i = 0; i < X86_VENDOR_NUM; i++) {
if (cpu_devs[i]) {
- if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||
- (cpu_devs[i]->c_ident[1] &&
- !strcmp(v,cpu_devs[i]->c_ident[1]))) {
- this_cpu = cpu_devs[i];
+ if (cpu_devs[i]->b == b && cpu_devs[i]->c == c &&
+ cpu_devs[i]->d == d) {
+ if (mode == gcv_host)
+ this_cpu = cpu_devs[i];
return i;
}
}
@@ -233,12 +233,12 @@ static void __init early_cpu_detect(void)
c->x86_cache_alignment = 32;
/* Get vendor name */
- cpuid(0x00000000, &c->cpuid_level,
- (int *)&c->x86_vendor_id[0],
- (int *)&c->x86_vendor_id[8],
- (int *)&c->x86_vendor_id[4]);
+ cpuid(0x00000000, &c->cpuid_level, &ebx, &ecx, &edx);
+ *(u32 *)&c->x86_vendor_id[0] = ebx;
+ *(u32 *)&c->x86_vendor_id[8] = ecx;
+ *(u32 *)&c->x86_vendor_id[4] = edx;
- c->x86_vendor = get_cpu_vendor(c->x86_vendor_id, gcv_host);
+ c->x86_vendor = get_cpu_vendor(ebx, ecx, edx, gcv_host);
cpuid(0x00000001, &eax, &ebx, &ecx, &edx);
c->x86 = get_cpu_family(eax, &c->x86_model, &c->x86_mask);
@@ -276,12 +276,12 @@ static void generic_identify(struct cpuinfo_x86 *c)
u32 eax, ebx, ecx, edx, tmp;
/* Get vendor name */
- cpuid(0x00000000, &c->cpuid_level,
- (int *)&c->x86_vendor_id[0],
- (int *)&c->x86_vendor_id[8],
- (int *)&c->x86_vendor_id[4]);
+ cpuid(0x00000000, &c->cpuid_level, &ebx, &ecx, &edx);
+ *(u32 *)&c->x86_vendor_id[0] = ebx;
+ *(u32 *)&c->x86_vendor_id[8] = ecx;
+ *(u32 *)&c->x86_vendor_id[4] = edx;
- c->x86_vendor = get_cpu_vendor(c->x86_vendor_id, gcv_host);
+ c->x86_vendor = get_cpu_vendor(ebx, ecx, edx, gcv_host);
/* Initialize the standard set of capabilities */
/* Note that the vendor-specific code below might override */
diff --git a/xen/arch/x86/cpu/cpu.h b/xen/arch/x86/cpu/cpu.h
index 1877e7d..5a7905c 100644
--- a/xen/arch/x86/cpu/cpu.h
+++ b/xen/arch/x86/cpu/cpu.h
@@ -1,9 +1,13 @@
/* attempt to consolidate cpu attributes */
struct cpu_dev {
- char * c_vendor;
+ char c_vendor[8];
- /* some have two possibilities for cpuid string */
- char * c_ident[2];
+ union {
+ char c_ident[13];
+ struct {
+ uint32_t b, d, c;
+ };
+ };
void (*c_early_init)(struct cpuinfo_x86 *c);
void (*c_init)(struct cpuinfo_x86 * c);
diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index ab9ad39..eb71c9e 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -54,21 +54,10 @@ static void update_domain_cpuid_info(struct domain *d,
switch ( ctl->input[0] )
{
case 0: {
- union {
- typeof(boot_cpu_data.x86_vendor_id) str;
- struct {
- uint32_t ebx, edx, ecx;
- } reg;
- } vendor_id = {
- .reg = {
- .ebx = ctl->ebx,
- .edx = ctl->edx,
- .ecx = ctl->ecx
- }
- };
int old_vendor = d->arch.x86_vendor;
- d->arch.x86_vendor = get_cpu_vendor(vendor_id.str, gcv_guest);
+ d->arch.x86_vendor = get_cpu_vendor(
+ ctl->ebx, ctl->ecx, ctl->edx, gcv_guest);
if ( is_hvm_domain(d) && (d->arch.x86_vendor != old_vendor) )
{
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index be31586..aff115b 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -624,7 +624,7 @@ enum get_cpu_vendor {
gcv_guest,
};
-int get_cpu_vendor(const char vendor_id[], enum get_cpu_vendor);
+int get_cpu_vendor(uint32_t b, uint32_t c, uint32_t d, enum get_cpu_vendor mode);
uint8_t get_cpu_family(uint32_t raw, uint8_t *model, uint8_t *stepping);
void pv_cpuid(struct cpu_user_regs *regs);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] x86/cpu: Drop unused X86_VENDOR_* values
2017-01-03 12:06 [PATCH 1/2] x86/cpu: Drop unused X86_VENDOR_* values Andrew Cooper
2017-01-03 12:06 ` [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor() Andrew Cooper
@ 2017-01-03 12:25 ` Jan Beulich
1 sibling, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2017-01-03 12:25 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 03.01.17 at 13:06, <andrew.cooper3@citrix.com> wrote:
> Xen only has CPU drivers for Intel, Centaur and AMD. All other contributions
> to X86_VENDOR_NUM simply make the cpu_devs[] array longer, reducing the
> efficiency of get_cpu_vendor()
>
> There is one remaning hidden reference to X86_VENDOR_CYRIX in the MTRR code.
> However, as far as I can tell, Cyrix never realeased a 64bit processor. It is
> therefore dead code.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor()
2017-01-03 12:06 ` [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor() Andrew Cooper
@ 2017-01-03 12:40 ` Jan Beulich
2017-01-03 12:41 ` Andrew Cooper
2017-01-04 13:33 ` Jan Beulich
1 sibling, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2017-01-03 12:40 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 03.01.17 at 13:06, <andrew.cooper3@citrix.com> wrote:
> Comparing 3 integers is more efficient than using strcmp(), and is more useful
> to the gcv_guest case than having to fabricate a suitable string to pass. The
> gcv_host cases have both options easily to hand, and experimentally, the
> resulting code is more efficient.
>
> While modifying get_cpu_vendor(), fix a bug where this_cpu got updated even in
> the gcv_guest case.
Isn't this something we'd better fix separately, to ease backporting?
> Update the cpu_dev structure to be more efficient. c_vendor[] only needs to
> be 8 bytes long to cover all the CPU drivers Xen has, which avoids storing an
> 8-byte pointer to 8 bytes of data. Drop c_ident[1] as we have no CPU drivers
> with a second ident string, and turn it into a transparent union to allow
> access to the integer values directly.
I think "transparent" is misleading here, as you don't add the respective
gcc attribute. I think you mean "unnamed".
> This avoids all need for the vendor_id union in update_domain_cpuid_info().
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
For the patch itself
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor()
2017-01-03 12:40 ` Jan Beulich
@ 2017-01-03 12:41 ` Andrew Cooper
2017-01-03 12:50 ` Jan Beulich
0 siblings, 1 reply; 9+ messages in thread
From: Andrew Cooper @ 2017-01-03 12:41 UTC (permalink / raw)
To: Jan Beulich; +Cc: Xen-devel
On 03/01/17 12:40, Jan Beulich wrote:
>>>> On 03.01.17 at 13:06, <andrew.cooper3@citrix.com> wrote:
>> Comparing 3 integers is more efficient than using strcmp(), and is more useful
>> to the gcv_guest case than having to fabricate a suitable string to pass. The
>> gcv_host cases have both options easily to hand, and experimentally, the
>> resulting code is more efficient.
>>
>> While modifying get_cpu_vendor(), fix a bug where this_cpu got updated even in
>> the gcv_guest case.
> Isn't this something we'd better fix separately, to ease backporting?
I can do.
>
>> Update the cpu_dev structure to be more efficient. c_vendor[] only needs to
>> be 8 bytes long to cover all the CPU drivers Xen has, which avoids storing an
>> 8-byte pointer to 8 bytes of data. Drop c_ident[1] as we have no CPU drivers
>> with a second ident string, and turn it into a transparent union to allow
>> access to the integer values directly.
> I think "transparent" is misleading here, as you don't add the respective
> gcc attribute. I think you mean "unnamed".
Yes sorry. My mistake.
>
>> This avoids all need for the vendor_id union in update_domain_cpuid_info().
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> For the patch itself
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
Does this still stand if I split the patch into two, for easier backport?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor()
2017-01-03 12:41 ` Andrew Cooper
@ 2017-01-03 12:50 ` Jan Beulich
0 siblings, 0 replies; 9+ messages in thread
From: Jan Beulich @ 2017-01-03 12:50 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 03.01.17 at 13:41, <andrew.cooper3@citrix.com> wrote:
> On 03/01/17 12:40, Jan Beulich wrote:
>> For the patch itself
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>
> Does this still stand if I split the patch into two, for easier backport?
Yes.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor()
2017-01-03 12:06 ` [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor() Andrew Cooper
2017-01-03 12:40 ` Jan Beulich
@ 2017-01-04 13:33 ` Jan Beulich
2017-01-05 16:53 ` Boris Ostrovsky
1 sibling, 1 reply; 9+ messages in thread
From: Jan Beulich @ 2017-01-04 13:33 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Xen-devel
>>> On 03.01.17 at 13:06, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/cpu/cpu.h
> +++ b/xen/arch/x86/cpu/cpu.h
> @@ -1,9 +1,13 @@
> /* attempt to consolidate cpu attributes */
> struct cpu_dev {
> - char * c_vendor;
> + char c_vendor[8];
>
> - /* some have two possibilities for cpuid string */
> - char * c_ident[2];
> + union {
> + char c_ident[13];
> + struct {
> + uint32_t b, d, c;
> + };
> + };
This broke the build with at least gcc 4.3.x, which doesn't allow
initializers for unnamed struct/union.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor()
2017-01-04 13:33 ` Jan Beulich
@ 2017-01-05 16:53 ` Boris Ostrovsky
2017-01-05 17:06 ` Andrew Cooper
0 siblings, 1 reply; 9+ messages in thread
From: Boris Ostrovsky @ 2017-01-05 16:53 UTC (permalink / raw)
To: Jan Beulich, Andrew Cooper; +Cc: Xen-devel
On 01/04/2017 08:33 AM, Jan Beulich wrote:
>>>> On 03.01.17 at 13:06, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/cpu/cpu.h
>> +++ b/xen/arch/x86/cpu/cpu.h
>> @@ -1,9 +1,13 @@
>> /* attempt to consolidate cpu attributes */
>> struct cpu_dev {
>> - char * c_vendor;
>> + char c_vendor[8];
>>
>> - /* some have two possibilities for cpuid string */
>> - char * c_ident[2];
>> + union {
>> + char c_ident[13];
>> + struct {
>> + uint32_t b, d, c;
>> + };
>> + };
> This broke the build with at least gcc 4.3.x, which doesn't allow
> initializers for unnamed struct/union.
4.4 is also broken. I believe anonymous initializers were added in gcc 4.6.
-boris
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor()
2017-01-05 16:53 ` Boris Ostrovsky
@ 2017-01-05 17:06 ` Andrew Cooper
0 siblings, 0 replies; 9+ messages in thread
From: Andrew Cooper @ 2017-01-05 17:06 UTC (permalink / raw)
To: Boris Ostrovsky, Jan Beulich; +Cc: Xen-devel
On 05/01/17 16:53, Boris Ostrovsky wrote:
> On 01/04/2017 08:33 AM, Jan Beulich wrote:
>>>>> On 03.01.17 at 13:06, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/arch/x86/cpu/cpu.h
>>> +++ b/xen/arch/x86/cpu/cpu.h
>>> @@ -1,9 +1,13 @@
>>> /* attempt to consolidate cpu attributes */
>>> struct cpu_dev {
>>> - char * c_vendor;
>>> + char c_vendor[8];
>>>
>>> - /* some have two possibilities for cpuid string */
>>> - char * c_ident[2];
>>> + union {
>>> + char c_ident[13];
>>> + struct {
>>> + uint32_t b, d, c;
>>> + };
>>> + };
>> This broke the build with at least gcc 4.3.x, which doesn't allow
>> initializers for unnamed struct/union.
> 4.4 is also broken. I believe anonymous initializers were added in gcc 4.6.
When I am out of meetings, I will post a fix. I have a plan.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-01-05 17:06 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-03 12:06 [PATCH 1/2] x86/cpu: Drop unused X86_VENDOR_* values Andrew Cooper
2017-01-03 12:06 ` [PATCH 2/2] x86/cpu: Improvements to get_cpu_vendor() Andrew Cooper
2017-01-03 12:40 ` Jan Beulich
2017-01-03 12:41 ` Andrew Cooper
2017-01-03 12:50 ` Jan Beulich
2017-01-04 13:33 ` Jan Beulich
2017-01-05 16:53 ` Boris Ostrovsky
2017-01-05 17:06 ` Andrew Cooper
2017-01-03 12:25 ` [PATCH 1/2] x86/cpu: Drop unused X86_VENDOR_* values Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.