RFC: EFER in crash notes

RFC: EFER in crash notes

[Andrew Cooper]

[-- Attachment #1: Type: text/plain, Size: 1032 bytes --]

While this patch is very simple, and I hope without any objection, it is
RFC for the reason that our crash ABI is private.

The comments for XEN_ELFNOTE_CRASH_REGS does state that it is
architecture specific, and makes no indication about the size or
contents of the crash note.  However, any code trying to use one of
these types of notes has to make an assumption that it if the note desc
length is 4*8 bytes long, it is representing CR{0,2-4}.

I guess my question boils down to whether it is acceptable to change a
private ABI which is not really so private, or whether we should make a
formal public ABI for all of the inards of the crash notes and use that.

I have some upcoming plans to put quite a lot more information into this
(or an equivalent) structure for extra analysis of the environment at
the point of a crash, so perhaps putting in the proper work to make a
public ABI would be the best course of action.

-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com


[-- Attachment #2: kexec-efer-in-crash-notes.patch --]
[-- Type: text/x-patch, Size: 853 bytes --]

# HG changeset patch
# Parent d364becfb0835f69e85d273fe2b29035c2d975df

diff -r d364becfb083 xen/include/asm-x86/elf.h
--- a/xen/include/asm-x86/elf.h
+++ b/xen/include/asm-x86/elf.h
@@ -3,6 +3,7 @@
 
 typedef struct {
     unsigned long cr0, cr2, cr3, cr4;
+    unsigned long efer;
 } crash_xen_core_t;
 
 #include <asm/x86_64/elf.h>
diff -r d364becfb083 xen/include/asm-x86/x86_64/elf.h
--- a/xen/include/asm-x86/x86_64/elf.h
+++ b/xen/include/asm-x86/x86_64/elf.h
@@ -1,6 +1,8 @@
 #ifndef __X86_64_ELF_H__
 #define __X86_64_ELF_H__
 
+#include <asm/msr.h>
+
 typedef struct {
     unsigned long r15;
     unsigned long r14;
@@ -75,6 +77,8 @@ static inline void elf_core_save_regs(EL
 
     asm volatile("mov %%cr4, %0" : "=r" (tmp) : );
     xen_core_regs->cr4 = tmp;
+
+    rdmsrl(MSR_EFER, xen_core_regs->efer);
 }
 
 #endif /* __X86_64_ELF_H__ */

[-- Attachment #3: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: RFC: EFER in crash notes

[Ian Campbell]

On Tue, 2012-09-25 at 15:18 +0100, Andrew Cooper wrote:
> While this patch is very simple, and I hope without any objection, it is
> RFC for the reason that our crash ABI is private.
> 
> The comments for XEN_ELFNOTE_CRASH_REGS does state that it is
> architecture specific, and makes no indication about the size or
> contents of the crash note.  However, any code trying to use one of
> these types of notes has to make an assumption that it if the note desc
> length is 4*8 bytes long, it is representing CR{0,2-4}.
> 
> I guess my question boils down to whether it is acceptable to change a
> private ABI which is not really so private, or whether we should make a
> formal public ABI for all of the inards of the crash notes and use that.

Is it really private? (or even "not so private"), don't external tools
like crash support it?

I suppose the size field in the notes is a sort of rudimentary version
field. Remember you can always add a new note type though.

Ian.

Re: RFC: EFER in crash notes

[Andrew Cooper]

On 25/09/12 15:33, Ian Campbell wrote:
> On Tue, 2012-09-25 at 15:18 +0100, Andrew Cooper wrote:
>> While this patch is very simple, and I hope without any objection, it is
>> RFC for the reason that our crash ABI is private.
>>
>> The comments for XEN_ELFNOTE_CRASH_REGS does state that it is
>> architecture specific, and makes no indication about the size or
>> contents of the crash note.  However, any code trying to use one of
>> these types of notes has to make an assumption that it if the note desc
>> length is 4*8 bytes long, it is representing CR{0,2-4}.
>>
>> I guess my question boils down to whether it is acceptable to change a
>> private ABI which is not really so private, or whether we should make a
>> formal public ABI for all of the inards of the crash notes and use that.
> Is it really private? (or even "not so private"), don't external tools
> like crash support it?

This is my point.  It is not in xen/public but is used by crash/kdump
and similar utilities, effectively making it a public ABI.

include/public/elfnote.h even explicitly refers to
include/xen/elfcore.h, which is not public by our definition.

>
> I suppose the size field in the notes is a sort of rudimentary version
> field. Remember you can always add a new note type though.

Yes, although looking through my code, I do raise an error if
sizeof(note->desc) != sizeof(my structure representing this note), which
was put in with the best of intentions, but will break with the this RFC
change.

On the other hand, adding a new crash note for every new register will
not scale well, as it is per PCU.

I guess the only sensible way to continue is to present a formal public ABI.

>
> Ian.
>
>
-- 
Andrew Cooper - Dom0 Kernel Engineer, Citrix XenServer
T: +44 (0)1223 225 900, http://www.citrix.com

Re: RFC: EFER in crash notes

[Keir Fraser]

On 25/09/2012 15:53, "Andrew Cooper" <andrew.cooper3@citrix.com> wrote:

>> I suppose the size field in the notes is a sort of rudimentary version
>> field. Remember you can always add a new note type though.
> 
> Yes, although looking through my code, I do raise an error if
> sizeof(note->desc) != sizeof(my structure representing this note), which
> was put in with the best of intentions, but will break with the this RFC
> change.
> 
> On the other hand, adding a new crash note for every new register will
> not scale well, as it is per PCU.
> 
> I guess the only sensible way to continue is to present a formal public ABI.

That seems a good idea.

 -- Keir

Re: RFC: EFER in crash notes

[Ian Campbell]

On Tue, 2012-09-25 at 15:53 +0100, Andrew Cooper wrote:
> On 25/09/12 15:33, Ian Campbell wrote:
> > On Tue, 2012-09-25 at 15:18 +0100, Andrew Cooper wrote:
> >> While this patch is very simple, and I hope without any objection, it is
> >> RFC for the reason that our crash ABI is private.
> >>
> >> The comments for XEN_ELFNOTE_CRASH_REGS does state that it is
> >> architecture specific, and makes no indication about the size or
> >> contents of the crash note.  However, any code trying to use one of
> >> these types of notes has to make an assumption that it if the note desc
> >> length is 4*8 bytes long, it is representing CR{0,2-4}.
> >>
> >> I guess my question boils down to whether it is acceptable to change a
> >> private ABI which is not really so private, or whether we should make a
> >> formal public ABI for all of the inards of the crash notes and use that.
> > Is it really private? (or even "not so private"), don't external tools
> > like crash support it?
> 
> This is my point.  It is not in xen/public but is used by crash/kdump
> and similar utilities, effectively making it a public ABI.
> 
> include/public/elfnote.h even explicitly refers to
> include/xen/elfcore.h, which is not public by our definition.

Hrm, I think it is public despite its location.

> 
> >
> > I suppose the size field in the notes is a sort of rudimentary version
> > field. Remember you can always add a new note type though.
> 
> Yes, although looking through my code, I do raise an error if
> sizeof(note->desc) != sizeof(my structure representing this note), which
> was put in with the best of intentions, but will break with the this RFC
> change.
> 
> On the other hand, adding a new crash note for every new register will
> not scale well, as it is per PCU.

A new note for every batch of registers is what I was thinking. Or a new
note with a more extensible structure.

> 
> I guess the only sensible way to continue is to present a formal public ABI.
> 
> >
> > Ian.
> >
> >

Re: RFC: EFER in crash notes

[Jan Beulich]

>>> On 25.09.12 at 16:53, Andrew Cooper <andrew.cooper3@citrix.com> wrote:

> On 25/09/12 15:33, Ian Campbell wrote:
>> On Tue, 2012-09-25 at 15:18 +0100, Andrew Cooper wrote:
>>> While this patch is very simple, and I hope without any objection, it is
>>> RFC for the reason that our crash ABI is private.
>>>
>>> The comments for XEN_ELFNOTE_CRASH_REGS does state that it is
>>> architecture specific, and makes no indication about the size or
>>> contents of the crash note.  However, any code trying to use one of
>>> these types of notes has to make an assumption that it if the note desc
>>> length is 4*8 bytes long, it is representing CR{0,2-4}.
>>>
>>> I guess my question boils down to whether it is acceptable to change a
>>> private ABI which is not really so private, or whether we should make a
>>> formal public ABI for all of the inards of the crash notes and use that.
>> Is it really private? (or even "not so private"), don't external tools
>> like crash support it?
> 
> This is my point.  It is not in xen/public but is used by crash/kdump
> and similar utilities, effectively making it a public ABI.
> 
> include/public/elfnote.h even explicitly refers to
> include/xen/elfcore.h, which is not public by our definition.
> 
>>
>> I suppose the size field in the notes is a sort of rudimentary version
>> field. Remember you can always add a new note type though.
> 
> Yes, although looking through my code, I do raise an error if
> sizeof(note->desc) != sizeof(my structure representing this note), which
> was put in with the best of intentions, but will break with the this RFC
> change.
> 
> On the other hand, adding a new crash note for every new register will
> not scale well, as it is per PCU.

You don't need a not per addition - if you add a new note now
clearly identifying it as "might grow", then subsequently adding
to it shouldn't be a problem.

Adding to an existing one that wasn't considered extensible, as
you see with your own example above, is not an option unless
you have control over _all_ consumers.

Jan