Patchwork [1/1] cpu: msr: disabled test for non-Intel/AMD architectures

login
register
mail settings
Submitter Colin King
Date Dec. 5, 2011, 6:20 p.m.
Message ID <1323109229-15402-2-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/129391/
State Accepted
Headers show

Comments

Colin King - Dec. 5, 2011, 6:20 p.m.
From: Colin Ian King <colin.king@canonical.com>

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/cpu/msr/msr.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
Keng-Yu Lin - Dec. 7, 2011, 9:30 a.m.
Not sure why the mail is rejected. see if forward can work.


---------- Forwarded message ----------
From: Keng-Yu Lin <kengyu@canonical.com>
Date: 2011/12/7
Subject: Re: [PATCH 1/1] cpu: msr: disabled test for non-Intel/AMD architectures
To: fwts-devel@lists.ubuntu.com


Ack from me. and also tested building without an error.
Since the patch is from Colin, it may make sense to count as two Acks.
So the patch is applied in git://kernel.ubuntu.com/lexical/fwts.git

於 2011年12月06日 02:20, Colin King 提到:
> From: Colin Ian King<colin.king at canonical.com>
>
> Signed-off-by: Colin Ian King<colin.king at canonical.com>
> ---
>   src/cpu/msr/msr.c |    8 ++++++++
>   1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
> index d2c16a8..06a756f 100644
> --- a/src/cpu/msr/msr.c
> +++ b/src/cpu/msr/msr.c
> @@ -19,6 +19,8 @@
>
>   #include "fwts.h"
>
> +#ifdef FWTS_ARCH_INTEL
> +
>   typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>
>   static int ncpus;
> @@ -32,6 +34,10 @@ static int msr_init(fwts_framework *fw)
>               fwts_log_error(fw, "Cannot get CPU info");
>               return FWTS_ERROR;
>       }
> +     if (cpuinfo->vendor_id == NULL) {
> +             fwts_log_error(fw, "Cannot get CPU vendor_id");
> +             return FWTS_ERROR;
> +     }
>       intel_cpu = strstr(cpuinfo->vendor_id, "Intel") != NULL;
>       amd_cpu = strstr(cpuinfo->vendor_id, "AuthenticAMD") != NULL;
>
> @@ -555,3 +561,5 @@ static fwts_framework_ops msr_ops = {
>   };
>
>   FWTS_REGISTER(msr,&msr_ops, FWTS_TEST_ANYTIME, FWTS_BATCH | FWTS_ROOT_PRIV);
> +
> +#endif
Colin King - Dec. 7, 2011, 9:34 a.m.
On 07/12/11 09:25, Keng-Yu Lin wrote:
> Ack from me. and also tested building without an error.
> Since the patch is from Colin, it may make sense to count as two Acks.
Nah, I need 2 Acks like anyone else. :-)
> So the patch is applied in git://kernel.ubuntu.com/lexical/fwts.git
>
> 於 2011年12月06日 02:20, Colin King 提到:
>> From: Colin Ian King<colin.king at canonical.com>
>>
>> Signed-off-by: Colin Ian King<colin.king at canonical.com>
>> ---
>>   src/cpu/msr/msr.c |    8 ++++++++
>>   1 files changed, 8 insertions(+), 0 deletions(-)
>>
>> diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
>> index d2c16a8..06a756f 100644
>> --- a/src/cpu/msr/msr.c
>> +++ b/src/cpu/msr/msr.c
>> @@ -19,6 +19,8 @@
>>
>>   #include "fwts.h"
>>
>> +#ifdef FWTS_ARCH_INTEL
>> +
>>   typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>>
>>   static int ncpus;
>> @@ -32,6 +34,10 @@ static int msr_init(fwts_framework *fw)
>>   		fwts_log_error(fw, "Cannot get CPU info");
>>   		return FWTS_ERROR;
>>   	}
>> +	if (cpuinfo->vendor_id == NULL) {
>> +		fwts_log_error(fw, "Cannot get CPU vendor_id");
>> +		return FWTS_ERROR;
>> +	}
>>   	intel_cpu = strstr(cpuinfo->vendor_id, "Intel") != NULL;
>>   	amd_cpu = strstr(cpuinfo->vendor_id, "AuthenticAMD") != NULL;
>>
>> @@ -555,3 +561,5 @@ static fwts_framework_ops msr_ops = {
>>   };
>>
>>   FWTS_REGISTER(msr,&msr_ops, FWTS_TEST_ANYTIME, FWTS_BATCH | FWTS_ROOT_PRIV);
>> +
>> +#endif
Keng-Yu Lin - Dec. 7, 2011, 9:37 a.m.
2011/12/7 Colin Ian King <colin.king@canonical.com>:
> On 07/12/11 09:25, Keng-Yu Lin wrote:
>> Ack from me. and also tested building without an error.
>> Since the patch is from Colin, it may make sense to count as two Acks.
> Nah, I need 2 Acks like anyone else. :-)

As your wish :-)  the patch is temporarily reverted in my tree:
git://kernel.ubuntu.com/lexical/fwts.git
Colin King - Dec. 7, 2011, 9:42 a.m.
On 07/12/11 09:37, Keng-Yü Lin wrote:
> 2011/12/7 Colin Ian King<colin.king@canonical.com>:
>> On 07/12/11 09:25, Keng-Yu Lin wrote:
>>> Ack from me. and also tested building without an error.
>>> Since the patch is from Colin, it may make sense to count as two Acks.
>> Nah, I need 2 Acks like anyone else. :-)
> As your wish :-)  the patch is temporarily reverted in my tree:
> git://kernel.ubuntu.com/lexical/fwts.git
I think we have quite a few patches in the queue, I've Ack'd some, who 
else can Ack the patches?

Colin
Keng-Yu Lin - Dec. 7, 2011, 9:58 a.m.
> I think we have quite a few patches in the queue, I've Ack'd some, who else
> can Ack the patches?

I will be looking at these patches.
Jeremy Kerr - Dec. 9, 2011, 4:22 a.m.
Hi Colin,

> From: Colin Ian King <colin.king@canonical.com>
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

For single-patch series, it's probably better just to include the
changelog in this patch, rather than as the series header. Makes it
easier for the committer to just git-am the patch.

> --- a/src/cpu/msr/msr.c
> +++ b/src/cpu/msr/msr.c
> @@ -19,6 +19,8 @@
>
>  #include "fwts.h"
>
> +#ifdef FWTS_ARCH_INTEL
> +
>  typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>
>  static int ncpus;

This is going to result in an #ifdef mess; maybe we need
msr-${FWTS_ARCH}.c instead?

Cheers,


Jeremy
Colin King - Dec. 9, 2011, 8:20 a.m.
On 09/12/11 04:22, Jeremy Kerr wrote:
> Hi Colin,
>
>> From: Colin Ian King<colin.king@canonical.com>
>>
>> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> For single-patch series, it's probably better just to include the
> changelog in this patch, rather than as the series header. Makes it
> easier for the committer to just git-am the patch.
OK, I was just following the kernel-team protocol, and they seemed happy 
with that methodology. Will do next time.
>
>> --- a/src/cpu/msr/msr.c
>> +++ b/src/cpu/msr/msr.c
>> @@ -19,6 +19,8 @@
>>
>>   #include "fwts.h"
>>
>> +#ifdef FWTS_ARCH_INTEL
>> +
>>   typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>>
>>   static int ncpus;
> This is going to result in an #ifdef mess; maybe we need
> msr-${FWTS_ARCH}.c instead?
Ultimately, I agree, but this should be looked at on a different spin.  
I really can't care about how this is done, non-Intel architectures are 
something that I wasn't considering when I first wrote fwts and most of 
the tests are x86 specific, so just getting the critter to build on 
non-ARM was a hack, which I agree needs fixing it a more elegant way, 
but not by me, I can't devote the cycles at the moment to this.
> Cheers,
>
>
> Jeremy
Alex Hung - Jan. 3, 2012, 8:27 a.m.
Hi,

The patch looks good. Ack from me.

On 12/06/2011 02:20 AM, Colin King wrote:
> From: Colin Ian King<colin.king@canonical.com>
>
> Signed-off-by: Colin Ian King<colin.king@canonical.com>
> ---
>   src/cpu/msr/msr.c |    8 ++++++++
>   1 files changed, 8 insertions(+), 0 deletions(-)
>
> diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
> index d2c16a8..06a756f 100644
> --- a/src/cpu/msr/msr.c
> +++ b/src/cpu/msr/msr.c
> @@ -19,6 +19,8 @@
>
>   #include "fwts.h"
>
> +#ifdef FWTS_ARCH_INTEL
> +
>   typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
>
>   static int ncpus;
> @@ -32,6 +34,10 @@ static int msr_init(fwts_framework *fw)
>   		fwts_log_error(fw, "Cannot get CPU info");
>   		return FWTS_ERROR;
>   	}
> +	if (cpuinfo->vendor_id == NULL) {
> +		fwts_log_error(fw, "Cannot get CPU vendor_id");
> +		return FWTS_ERROR;
> +	}
>   	intel_cpu = strstr(cpuinfo->vendor_id, "Intel") != NULL;
>   	amd_cpu = strstr(cpuinfo->vendor_id, "AuthenticAMD") != NULL;
>
> @@ -555,3 +561,5 @@ static fwts_framework_ops msr_ops = {
>   };
>
>   FWTS_REGISTER(msr,&msr_ops, FWTS_TEST_ANYTIME, FWTS_BATCH | FWTS_ROOT_PRIV);
> +
> +#endif

Patch

diff --git a/src/cpu/msr/msr.c b/src/cpu/msr/msr.c
index d2c16a8..06a756f 100644
--- a/src/cpu/msr/msr.c
+++ b/src/cpu/msr/msr.c
@@ -19,6 +19,8 @@ 
 
 #include "fwts.h"
 
+#ifdef FWTS_ARCH_INTEL
+
 typedef void (*msr_callback_check)(fwts_framework *fw, uint64_t val);
 
 static int ncpus;
@@ -32,6 +34,10 @@  static int msr_init(fwts_framework *fw)
 		fwts_log_error(fw, "Cannot get CPU info");
 		return FWTS_ERROR;
 	}
+	if (cpuinfo->vendor_id == NULL) {
+		fwts_log_error(fw, "Cannot get CPU vendor_id");
+		return FWTS_ERROR;
+	}
 	intel_cpu = strstr(cpuinfo->vendor_id, "Intel") != NULL;
 	amd_cpu = strstr(cpuinfo->vendor_id, "AuthenticAMD") != NULL;
 
@@ -555,3 +561,5 @@  static fwts_framework_ops msr_ops = {
 };
 
 FWTS_REGISTER(msr, &msr_ops, FWTS_TEST_ANYTIME, FWTS_BATCH | FWTS_ROOT_PRIV);
+
+#endif