qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH for-2.10 v2] kvm: Print MSR information if KVM_{GET, SET}_MSRS failed
@ 2017-03-09 19:46 Eduardo Habkost
  2017-03-09 20:08 ` Dr. David Alan Gilbert
  0 siblings, 1 reply; 4+ messages in thread
From: Eduardo Habkost @ 2017-03-09 19:46 UTC (permalink / raw)
  To: qemu-devel, Paolo Bonzini; +Cc: Dr. David Alan Gilbert

When a KVM_{GET,SET}_MSRS ioctl() fails, it is difficult to find
out which MSR caused the problem. Print an error message for
debugging, before we trigger the (ret == cpu->kvm_msr_buf->nmsrs)
assert.

Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Print error message on KVM_GET_MSRS too
  * Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
---
 target/i386/kvm.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 887a81268f..705e971a63 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1807,6 +1807,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
         return ret;
     }
 
+    if (ret < cpu->kvm_msr_buf->nmsrs) {
+        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
+        error_report("error: failed to set MSR 0x%" PRIx32 " to 0x%" PRIx64,
+                     (uint32_t)e->index, (uint64_t)e->data);
+    }
+
     assert(ret == cpu->kvm_msr_buf->nmsrs);
     return 0;
 }
@@ -2172,6 +2178,12 @@ static int kvm_get_msrs(X86CPU *cpu)
         return ret;
     }
 
+    if (ret < cpu->kvm_msr_buf->nmsrs) {
+        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
+        error_report("error: failed to get MSR 0x%" PRIx32,
+                     (uint32_t)e->index);
+    }
+
     assert(ret == cpu->kvm_msr_buf->nmsrs);
     /*
      * MTRR masks: Each mask consists of 5 parts
-- 
2.11.0.259.g40922b1

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.10 v2] kvm: Print MSR information if KVM_{GET, SET}_MSRS failed
  2017-03-09 19:46 [Qemu-devel] [PATCH for-2.10 v2] kvm: Print MSR information if KVM_{GET, SET}_MSRS failed Eduardo Habkost
@ 2017-03-09 20:08 ` Dr. David Alan Gilbert
  2017-03-09 20:26   ` [Qemu-devel] [PATCH for-2.9? " Eduardo Habkost
  0 siblings, 1 reply; 4+ messages in thread
From: Dr. David Alan Gilbert @ 2017-03-09 20:08 UTC (permalink / raw)
  To: Eduardo Habkost; +Cc: qemu-devel, Paolo Bonzini

* Eduardo Habkost (ehabkost@redhat.com) wrote:
> When a KVM_{GET,SET}_MSRS ioctl() fails, it is difficult to find
> out which MSR caused the problem. Print an error message for
> debugging, before we trigger the (ret == cpu->kvm_msr_buf->nmsrs)
> assert.
> 
> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>

Thanks!

Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

I'd actually suggest it for 2.9; it's only added debug output.

Dave

> ---
> Changes v1 -> v2:
> * Print error message on KVM_GET_MSRS too
>   * Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> ---
>  target/i386/kvm.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> index 887a81268f..705e971a63 100644
> --- a/target/i386/kvm.c
> +++ b/target/i386/kvm.c
> @@ -1807,6 +1807,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>          return ret;
>      }
>  
> +    if (ret < cpu->kvm_msr_buf->nmsrs) {
> +        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
> +        error_report("error: failed to set MSR 0x%" PRIx32 " to 0x%" PRIx64,
> +                     (uint32_t)e->index, (uint64_t)e->data);
> +    }
> +
>      assert(ret == cpu->kvm_msr_buf->nmsrs);
>      return 0;
>  }
> @@ -2172,6 +2178,12 @@ static int kvm_get_msrs(X86CPU *cpu)
>          return ret;
>      }
>  
> +    if (ret < cpu->kvm_msr_buf->nmsrs) {
> +        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
> +        error_report("error: failed to get MSR 0x%" PRIx32,
> +                     (uint32_t)e->index);
> +    }
> +
>      assert(ret == cpu->kvm_msr_buf->nmsrs);
>      /*
>       * MTRR masks: Each mask consists of 5 parts
> -- 
> 2.11.0.259.g40922b1
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9? v2] kvm: Print MSR information if KVM_{GET, SET}_MSRS failed
  2017-03-09 20:08 ` Dr. David Alan Gilbert
@ 2017-03-09 20:26   ` Eduardo Habkost
  2017-03-10  8:29     ` Paolo Bonzini
  0 siblings, 1 reply; 4+ messages in thread
From: Eduardo Habkost @ 2017-03-09 20:26 UTC (permalink / raw)
  To: Dr. David Alan Gilbert; +Cc: qemu-devel, Paolo Bonzini

On Thu, Mar 09, 2017 at 08:08:00PM +0000, Dr. David Alan Gilbert wrote:
> * Eduardo Habkost (ehabkost@redhat.com) wrote:
> > When a KVM_{GET,SET}_MSRS ioctl() fails, it is difficult to find
> > out which MSR caused the problem. Print an error message for
> > debugging, before we trigger the (ret == cpu->kvm_msr_buf->nmsrs)
> > assert.
> > 
> > Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> 
> Thanks!
> 
> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> I'd actually suggest it for 2.9; it's only added debug output.

I think I agree. The patch is reasonably safe because we would be
already hitting the assert() if (ret < cpu->kvm_msr_buf->nmsrs)
anyway.

Paolo, do you want to queue it for 2.9?

> 
> Dave
> 
> > ---
> > Changes v1 -> v2:
> > * Print error message on KVM_GET_MSRS too
> >   * Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> > ---
> >  target/i386/kvm.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> > 
> > diff --git a/target/i386/kvm.c b/target/i386/kvm.c
> > index 887a81268f..705e971a63 100644
> > --- a/target/i386/kvm.c
> > +++ b/target/i386/kvm.c
> > @@ -1807,6 +1807,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
> >          return ret;
> >      }
> >  
> > +    if (ret < cpu->kvm_msr_buf->nmsrs) {
> > +        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
> > +        error_report("error: failed to set MSR 0x%" PRIx32 " to 0x%" PRIx64,
> > +                     (uint32_t)e->index, (uint64_t)e->data);
> > +    }
> > +
> >      assert(ret == cpu->kvm_msr_buf->nmsrs);
> >      return 0;
> >  }
> > @@ -2172,6 +2178,12 @@ static int kvm_get_msrs(X86CPU *cpu)
> >          return ret;
> >      }
> >  
> > +    if (ret < cpu->kvm_msr_buf->nmsrs) {
> > +        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
> > +        error_report("error: failed to get MSR 0x%" PRIx32,
> > +                     (uint32_t)e->index);
> > +    }
> > +
> >      assert(ret == cpu->kvm_msr_buf->nmsrs);
> >      /*
> >       * MTRR masks: Each mask consists of 5 parts
> > -- 
> > 2.11.0.259.g40922b1
> > 
> --
> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

-- 
Eduardo

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Qemu-devel] [PATCH for-2.9? v2] kvm: Print MSR information if KVM_{GET, SET}_MSRS failed
  2017-03-09 20:26   ` [Qemu-devel] [PATCH for-2.9? " Eduardo Habkost
@ 2017-03-10  8:29     ` Paolo Bonzini
  0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2017-03-10  8:29 UTC (permalink / raw)
  To: Eduardo Habkost, Dr. David Alan Gilbert; +Cc: qemu-devel



On 09/03/2017 21:26, Eduardo Habkost wrote:
> On Thu, Mar 09, 2017 at 08:08:00PM +0000, Dr. David Alan Gilbert wrote:
>> * Eduardo Habkost (ehabkost@redhat.com) wrote:
>>> When a KVM_{GET,SET}_MSRS ioctl() fails, it is difficult to find
>>> out which MSR caused the problem. Print an error message for
>>> debugging, before we trigger the (ret == cpu->kvm_msr_buf->nmsrs)
>>> assert.
>>>
>>> Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
>>
>> Thanks!
>>
>> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>
>> I'd actually suggest it for 2.9; it's only added debug output.
> 
> I think I agree. The patch is reasonably safe because we would be
> already hitting the assert() if (ret < cpu->kvm_msr_buf->nmsrs)
> anyway.
> 
> Paolo, do you want to queue it for 2.9?

Yes, with pleasure.

Paolo

>>
>> Dave
>>
>>> ---
>>> Changes v1 -> v2:
>>> * Print error message on KVM_GET_MSRS too
>>>   * Suggested-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> ---
>>>  target/i386/kvm.c | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/target/i386/kvm.c b/target/i386/kvm.c
>>> index 887a81268f..705e971a63 100644
>>> --- a/target/i386/kvm.c
>>> +++ b/target/i386/kvm.c
>>> @@ -1807,6 +1807,12 @@ static int kvm_put_msrs(X86CPU *cpu, int level)
>>>          return ret;
>>>      }
>>>  
>>> +    if (ret < cpu->kvm_msr_buf->nmsrs) {
>>> +        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
>>> +        error_report("error: failed to set MSR 0x%" PRIx32 " to 0x%" PRIx64,
>>> +                     (uint32_t)e->index, (uint64_t)e->data);
>>> +    }
>>> +
>>>      assert(ret == cpu->kvm_msr_buf->nmsrs);
>>>      return 0;
>>>  }
>>> @@ -2172,6 +2178,12 @@ static int kvm_get_msrs(X86CPU *cpu)
>>>          return ret;
>>>      }
>>>  
>>> +    if (ret < cpu->kvm_msr_buf->nmsrs) {
>>> +        struct kvm_msr_entry *e = &cpu->kvm_msr_buf->entries[ret];
>>> +        error_report("error: failed to get MSR 0x%" PRIx32,
>>> +                     (uint32_t)e->index);
>>> +    }
>>> +
>>>      assert(ret == cpu->kvm_msr_buf->nmsrs);
>>>      /*
>>>       * MTRR masks: Each mask consists of 5 parts
>>> -- 
>>> 2.11.0.259.g40922b1
>>>
>> --
>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-03-10  8:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-03-09 19:46 [Qemu-devel] [PATCH for-2.10 v2] kvm: Print MSR information if KVM_{GET, SET}_MSRS failed Eduardo Habkost
2017-03-09 20:08 ` Dr. David Alan Gilbert
2017-03-09 20:26   ` [Qemu-devel] [PATCH for-2.9? " Eduardo Habkost
2017-03-10  8:29     ` Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).