x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
diff mbox series

Message ID 20190913160404.495-1-andrew.cooper3@citrix.com
State New
Headers show
Series
  • x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
Related show

Commit Message

Andrew Cooper Sept. 13, 2019, 4:04 p.m. UTC
Message such as:

  (XEN) d3v0 VIRIDIAN CRASH: 51 1 ffff9700e146b000 1000 204

have confused many people into thinking the the problem is a bug in the
viridian code.  The prefix was intended to signify the use of the viridian
crash-reporting interface.

Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
non-xen-developers trying to interpret the message.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Paul Durrant <paul.durrant@citrix.com>

This wants backporting to every stable tree which has viridian crash interface
support.
---
 xen/arch/x86/hvm/viridian/viridian.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Paul Durrant Sept. 15, 2019, 11:51 a.m. UTC | #1
> -----Original Message-----
> From: Andrew Cooper <andrew.cooper3@citrix.com>
> Sent: 13 September 2019 17:04
> To: Xen-devel <xen-devel@lists.xenproject.org>
> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Wei Liu <wl@xen.org>;
> Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>
> Subject: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
> 
> Message such as:
> 
>   (XEN) d3v0 VIRIDIAN CRASH: 51 1 ffff9700e146b000 1000 204
> 
> have confused many people into thinking the the problem is a bug in the
> viridian code.  The prefix was intended to signify the use of the viridian
> crash-reporting interface.
> 
> Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
> non-xen-developers trying to interpret the message.

This is a message that is peculiar to Windows VMs, so how about "Windows VM CRASH"?

 Paul

> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wl@xen.org>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Paul Durrant <paul.durrant@citrix.com>
> 
> This wants backporting to every stable tree which has viridian crash interface
> support.
> ---
>  xen/arch/x86/hvm/viridian/viridian.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
> index 4b06b78a27..f98c8e7753 100644
> --- a/xen/arch/x86/hvm/viridian/viridian.c
> +++ b/xen/arch/x86/hvm/viridian/viridian.c
> @@ -357,7 +357,7 @@ int guest_wrmsr_viridian(struct vcpu *v, uint32_t idx, uint64_t val)
>          d->shutdown_code = SHUTDOWN_crash;
>          spin_unlock(&d->shutdown_lock);
> 
> -        gprintk(XENLOG_WARNING, "VIRIDIAN CRASH: %lx %lx %lx %lx %lx\n",
> +        gprintk(XENLOG_WARNING, "reported CRASH: %lx %lx %lx %lx %lx\n",
>                  vv->crash_param[0], vv->crash_param[1], vv->crash_param[2],
>                  vv->crash_param[3], vv->crash_param[4]);
>          break;
> --
> 2.11.0
Andrew Cooper Sept. 16, 2019, 12:47 p.m. UTC | #2
On 15/09/2019 12:51, Paul Durrant wrote:
>> -----Original Message-----
>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>> Sent: 13 September 2019 17:04
>> To: Xen-devel <xen-devel@lists.xenproject.org>
>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Wei Liu <wl@xen.org>;
>> Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>
>> Subject: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
>>
>> Message such as:
>>
>>   (XEN) d3v0 VIRIDIAN CRASH: 51 1 ffff9700e146b000 1000 204
>>
>> have confused many people into thinking the the problem is a bug in the
>> viridian code.  The prefix was intended to signify the use of the viridian
>> crash-reporting interface.
>>
>> Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
>> non-xen-developers trying to interpret the message.
> This is a message that is peculiar to Windows VMs, so how about "Windows VM CRASH"?

I presume you mean particular, but no - it isn't windows which is the
exclusive user of this interface.  Linux has a driver to use it when
running under HyperV.

~Andrew
Paul Durrant Sept. 16, 2019, 12:56 p.m. UTC | #3
> -----Original Message-----
> From: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Sent: 16 September 2019 13:48
> To: Paul Durrant <Paul.Durrant@citrix.com>; Xen-devel <xen-devel@lists.xenproject.org>
> Cc: Jan Beulich <JBeulich@suse.com>; Wei Liu <wl@xen.org>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: Re: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
> 
> On 15/09/2019 12:51, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Andrew Cooper <andrew.cooper3@citrix.com>
> >> Sent: 13 September 2019 17:04
> >> To: Xen-devel <xen-devel@lists.xenproject.org>
> >> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Wei Liu
> <wl@xen.org>;
> >> Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>
> >> Subject: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
> >>
> >> Message such as:
> >>
> >>   (XEN) d3v0 VIRIDIAN CRASH: 51 1 ffff9700e146b000 1000 204
> >>
> >> have confused many people into thinking the the problem is a bug in the
> >> viridian code.  The prefix was intended to signify the use of the viridian
> >> crash-reporting interface.
> >>
> >> Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
> >> non-xen-developers trying to interpret the message.
> > This is a message that is peculiar to Windows VMs, so how about "Windows VM CRASH"?
> 
> I presume you mean particular, but no - it isn't windows which is the
> exclusive user of this interface.  Linux has a driver to use it when
> running under HyperV.

Hmm, that's a bit odd. I thought the crash codes are Windows specific. Perhaps they can be distinguished in some way. All the same, the log line needs to lead people to some way of decoding the magic numbers I think. How about:

"VIRIDIAN REPORTED CRASH"

?

  Paul

> 
> ~Andrew
Andrew Cooper Sept. 16, 2019, 1:13 p.m. UTC | #4
On 16/09/2019 13:56, Paul Durrant wrote:
>> -----Original Message-----
>> From: Andrew Cooper <Andrew.Cooper3@citrix.com>
>> Sent: 16 September 2019 13:48
>> To: Paul Durrant <Paul.Durrant@citrix.com>; Xen-devel <xen-devel@lists.xenproject.org>
>> Cc: Jan Beulich <JBeulich@suse.com>; Wei Liu <wl@xen.org>; Roger Pau Monne <roger.pau@citrix.com>
>> Subject: Re: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
>>
>> On 15/09/2019 12:51, Paul Durrant wrote:
>>>> -----Original Message-----
>>>> From: Andrew Cooper <andrew.cooper3@citrix.com>
>>>> Sent: 13 September 2019 17:04
>>>> To: Xen-devel <xen-devel@lists.xenproject.org>
>>>> Cc: Andrew Cooper <Andrew.Cooper3@citrix.com>; Jan Beulich <JBeulich@suse.com>; Wei Liu
>> <wl@xen.org>;
>>>> Roger Pau Monne <roger.pau@citrix.com>; Paul Durrant <Paul.Durrant@citrix.com>
>>>> Subject: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
>>>>
>>>> Message such as:
>>>>
>>>>   (XEN) d3v0 VIRIDIAN CRASH: 51 1 ffff9700e146b000 1000 204
>>>>
>>>> have confused many people into thinking the the problem is a bug in the
>>>> viridian code.  The prefix was intended to signify the use of the viridian
>>>> crash-reporting interface.
>>>>
>>>> Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
>>>> non-xen-developers trying to interpret the message.
>>> This is a message that is peculiar to Windows VMs, so how about "Windows VM CRASH"?
>> I presume you mean particular, but no - it isn't windows which is the
>> exclusive user of this interface.  Linux has a driver to use it when
>> running under HyperV.
> Hmm, that's a bit odd. I thought the crash codes are Windows specific. Perhaps they can be distinguished in some way.

Linux sets its own guest OS identifier, which is some function of
HV_LINUX_VENDOR_ID.

>  All the same, the log line needs to lead people to some way of decoding the magic numbers I think. How about:
>
> "VIRIDIAN REPORTED CRASH"
>
> ?

That is still just as confusing to read.

There is no way to decode the numbers without knowing what OS is
running, and simply saying "Viridian" doesn't help with that.

~Andrew
Wei Liu Sept. 16, 2019, 1:28 p.m. UTC | #5
On Mon, 16 Sep 2019 at 14:13, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
[...]
> >>>> Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
> >>>> non-xen-developers trying to interpret the message.
> >>> This is a message that is peculiar to Windows VMs, so how about "Windows VM CRASH"?
> >> I presume you mean particular, but no - it isn't windows which is the
> >> exclusive user of this interface.  Linux has a driver to use it when
> >> running under HyperV.
> > Hmm, that's a bit odd. I thought the crash codes are Windows specific. Perhaps they can be distinguished in some way.
>
> Linux sets its own guest OS identifier, which is some function of
> HV_LINUX_VENDOR_ID.
>
> >  All the same, the log line needs to lead people to some way of decoding the magic numbers I think. How about:
> >
> > "VIRIDIAN REPORTED CRASH"
> >
> > ?
>
> That is still just as confusing to read.
>
> There is no way to decode the numbers without knowing what OS is
> running, and simply saying "Viridian" doesn't help with that.
>

Would it make sense to call dump_guest_os_id here as well? Seeing that
it is only printed when it was first set.

Wei.

> ~Andrew
Paul Durrant Sept. 16, 2019, 1:56 p.m. UTC | #6
> -----Original Message-----
> From: Wei Liu <wl@xen.org>
> Sent: 16 September 2019 14:29
> To: Andrew Cooper <Andrew.Cooper3@citrix.com>
> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Xen-devel <xen-devel@lists.xenproject.org>; Jan Beulich
> <JBeulich@suse.com>; Wei Liu <wl@xen.org>; Roger Pau Monne <roger.pau@citrix.com>
> Subject: Re: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
> 
> On Mon, 16 Sep 2019 at 14:13, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> [...]
> > >>>> Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
> > >>>> non-xen-developers trying to interpret the message.
> > >>> This is a message that is peculiar to Windows VMs, so how about "Windows VM CRASH"?
> > >> I presume you mean particular, but no - it isn't windows which is the
> > >> exclusive user of this interface.  Linux has a driver to use it when
> > >> running under HyperV.
> > > Hmm, that's a bit odd. I thought the crash codes are Windows specific. Perhaps they can be
> distinguished in some way.
> >
> > Linux sets its own guest OS identifier, which is some function of
> > HV_LINUX_VENDOR_ID.
> >
> > >  All the same, the log line needs to lead people to some way of decoding the magic numbers I
> think. How about:
> > >
> > > "VIRIDIAN REPORTED CRASH"
> > >
> > > ?
> >
> > That is still just as confusing to read.
> >
> > There is no way to decode the numbers without knowing what OS is
> > running, and simply saying "Viridian" doesn't help with that.
> >
> 
> Would it make sense to call dump_guest_os_id here as well? Seeing that
> it is only printed when it was first set.

Yes, that's not a bad idea.

  Paul

> 
> Wei.
> 
> > ~Andrew
Andrew Cooper Sept. 17, 2019, 4:31 p.m. UTC | #7
On 16/09/2019 14:56, Paul Durrant wrote:
>> -----Original Message-----
>> From: Wei Liu <wl@xen.org>
>> Sent: 16 September 2019 14:29
>> To: Andrew Cooper <Andrew.Cooper3@citrix.com>
>> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Xen-devel <xen-devel@lists.xenproject.org>; Jan Beulich
>> <JBeulich@suse.com>; Wei Liu <wl@xen.org>; Roger Pau Monne <roger.pau@citrix.com>
>> Subject: Re: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
>>
>> On Mon, 16 Sep 2019 at 14:13, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>> [...]
>>>>>>> Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
>>>>>>> non-xen-developers trying to interpret the message.
>>>>>> This is a message that is peculiar to Windows VMs, so how about "Windows VM CRASH"?
>>>>> I presume you mean particular, but no - it isn't windows which is the
>>>>> exclusive user of this interface.  Linux has a driver to use it when
>>>>> running under HyperV.
>>>> Hmm, that's a bit odd. I thought the crash codes are Windows specific. Perhaps they can be
>> distinguished in some way.
>>> Linux sets its own guest OS identifier, which is some function of
>>> HV_LINUX_VENDOR_ID.
>>>
>>>>  All the same, the log line needs to lead people to some way of decoding the magic numbers I
>> think. How about:
>>>> "VIRIDIAN REPORTED CRASH"
>>>>
>>>> ?
>>> That is still just as confusing to read.
>>>
>>> There is no way to decode the numbers without knowing what OS is
>>> running, and simply saying "Viridian" doesn't help with that.
>>>
>> Would it make sense to call dump_guest_os_id here as well? Seeing that
>> it is only printed when it was first set.
> Yes, that's not a bad idea.

This is as maybe, but still doesn't help with ambiguity because you
can't expect people to recognise guest-id's by their hex value.  It also
doesn't help with the confusion of having the word viridian in the
printed string.

~Andrew
Wei Liu Sept. 18, 2019, 10:21 a.m. UTC | #8
On Tue, 17 Sep 2019 at 17:31, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 16/09/2019 14:56, Paul Durrant wrote:
> >> -----Original Message-----
> >> From: Wei Liu <wl@xen.org>
> >> Sent: 16 September 2019 14:29
> >> To: Andrew Cooper <Andrew.Cooper3@citrix.com>
> >> Cc: Paul Durrant <Paul.Durrant@citrix.com>; Xen-devel <xen-devel@lists.xenproject.org>; Jan Beulich
> >> <JBeulich@suse.com>; Wei Liu <wl@xen.org>; Roger Pau Monne <roger.pau@citrix.com>
> >> Subject: Re: [PATCH] x86/viridian: Reword HV_X64_MSR_CRASH_CTL print message
> >>
> >> On Mon, 16 Sep 2019 at 14:13, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
> >> [...]
> >>>>>>> Replace the VIRIDIAN prefix with 'reported' to reduce the confusion to
> >>>>>>> non-xen-developers trying to interpret the message.
> >>>>>> This is a message that is peculiar to Windows VMs, so how about "Windows VM CRASH"?
> >>>>> I presume you mean particular, but no - it isn't windows which is the
> >>>>> exclusive user of this interface.  Linux has a driver to use it when
> >>>>> running under HyperV.
> >>>> Hmm, that's a bit odd. I thought the crash codes are Windows specific. Perhaps they can be
> >> distinguished in some way.
> >>> Linux sets its own guest OS identifier, which is some function of
> >>> HV_LINUX_VENDOR_ID.
> >>>
> >>>>  All the same, the log line needs to lead people to some way of decoding the magic numbers I
> >> think. How about:
> >>>> "VIRIDIAN REPORTED CRASH"
> >>>>
> >>>> ?
> >>> That is still just as confusing to read.
> >>>
> >>> There is no way to decode the numbers without knowing what OS is
> >>> running, and simply saying "Viridian" doesn't help with that.
> >>>
> >> Would it make sense to call dump_guest_os_id here as well? Seeing that
> >> it is only printed when it was first set.
> > Yes, that's not a bad idea.
>
> This is as maybe, but still doesn't help with ambiguity because you
> can't expect people to recognise guest-id's by their hex value.  It also
> doesn't help with the confusion of having the word viridian in the
> printed string.
>

I look closer today. They can be interpreted at least for Linux. It is
useful information to have. If we want to print crash control values
at all, OS information shouldn't be omitted.

I don't really have an opinion on whether the viridian prefix is useful or not.

Wei.

> ~Andrew

Patch
diff mbox series

diff --git a/xen/arch/x86/hvm/viridian/viridian.c b/xen/arch/x86/hvm/viridian/viridian.c
index 4b06b78a27..f98c8e7753 100644
--- a/xen/arch/x86/hvm/viridian/viridian.c
+++ b/xen/arch/x86/hvm/viridian/viridian.c
@@ -357,7 +357,7 @@  int guest_wrmsr_viridian(struct vcpu *v, uint32_t idx, uint64_t val)
         d->shutdown_code = SHUTDOWN_crash;
         spin_unlock(&d->shutdown_lock);
 
-        gprintk(XENLOG_WARNING, "VIRIDIAN CRASH: %lx %lx %lx %lx %lx\n",
+        gprintk(XENLOG_WARNING, "reported CRASH: %lx %lx %lx %lx %lx\n",
                 vv->crash_param[0], vv->crash_param[1], vv->crash_param[2],
                 vv->crash_param[3], vv->crash_param[4]);
         break;