kvm/i386: fix a check that ensures we are running on host intel CPU

[PATCH] kvm/i386: fix a check that ensures we are running on host intel CPU

Posted by Ani Sinha 2 months, 3 weeks ago

is_host_cpu_intel() returns TRUE if the host cpu in Intel based. RAPL needs
Intel host cpus. If the host CPU is not Intel baseed, we should report error.
Fix the check accordingly.

Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 target/i386/kvm/kvm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
index 11c7619bfd..503e8d956e 100644
--- a/target/i386/kvm/kvm.c
+++ b/target/i386/kvm/kvm.c
@@ -2898,7 +2898,7 @@ static int kvm_msr_energy_thread_init(KVMState *s, MachineState *ms)
      * 1. Host cpu must be Intel cpu
      * 2. RAPL must be enabled on the Host
      */
-    if (is_host_cpu_intel()) {
+    if (!is_host_cpu_intel()) {
         error_report("The RAPL feature can only be enabled on hosts\
                       with Intel CPU models");
         ret = 1;
-- 
2.42.0

Re: [PATCH] kvm/i386: fix a check that ensures we are running on host intel CPU

Posted by Paolo Bonzini 2 months, 3 weeks ago

On 9/3/24 09:19, Ani Sinha wrote:
> is_host_cpu_intel() returns TRUE if the host cpu in Intel based. RAPL needs
> Intel host cpus. If the host CPU is not Intel baseed, we should report error.
> Fix the check accordingly.
> 
> Signed-off-by: Ani Sinha <anisinha@redhat.com>

It's the function that is returning the incorrect value too; so your 
patch is breaking the feature: this line in is_host_cpu_intel()

return strcmp(vendor, CPUID_VENDOR_INTEL);

needs to be changed to use g_str_equal.

Paolo

> ---
>   target/i386/kvm/kvm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
> index 11c7619bfd..503e8d956e 100644
> --- a/target/i386/kvm/kvm.c
> +++ b/target/i386/kvm/kvm.c
> @@ -2898,7 +2898,7 @@ static int kvm_msr_energy_thread_init(KVMState *s, MachineState *ms)
>        * 1. Host cpu must be Intel cpu
>        * 2. RAPL must be enabled on the Host
>        */
> -    if (is_host_cpu_intel()) {
> +    if (!is_host_cpu_intel()) {
>           error_report("The RAPL feature can only be enabled on hosts\
>                         with Intel CPU models");
>           ret = 1;

Re: [PATCH] kvm/i386: fix a check that ensures we are running on host intel CPU

Posted by Ani Sinha 2 months, 3 weeks ago


> On 3 Sep 2024, at 1:13 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> 
> On 9/3/24 09:19, Ani Sinha wrote:
>> is_host_cpu_intel() returns TRUE if the host cpu in Intel based. RAPL needs
>> Intel host cpus. If the host CPU is not Intel baseed, we should report error.
>> Fix the check accordingly.
>> Signed-off-by: Ani Sinha <anisinha@redhat.com>
> 
> It's the function that is returning the incorrect value too; so your patch is breaking the feature: this line in is_host_cpu_intel()
> 
> return strcmp(vendor, CPUID_VENDOR_INTEL);
> 
> needs to be changed to use g_str_equal.

Ah that is why it got unnoticed as programatically it was not broken. I will send a v2.

> 
> Paolo
> 
>> ---
>>  target/i386/kvm/kvm.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c
>> index 11c7619bfd..503e8d956e 100644
>> --- a/target/i386/kvm/kvm.c
>> +++ b/target/i386/kvm/kvm.c
>> @@ -2898,7 +2898,7 @@ static int kvm_msr_energy_thread_init(KVMState *s, MachineState *ms)
>>       * 1. Host cpu must be Intel cpu
>>       * 2. RAPL must be enabled on the Host
>>       */
>> -    if (is_host_cpu_intel()) {
>> +    if (!is_host_cpu_intel()) {
>>          error_report("The RAPL feature can only be enabled on hosts\
>>                        with Intel CPU models");
>>          ret = 1;
>