| Message ID | 1454675320-29429-1-git-send-email-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
>>> On 05.02.16 at 13:28, <roger.pau@citrix.com> wrote: > This will prevent alignments from getting in the way. It's not safe to > define this memory structures using C anyway, since the ABI depends on the > bitness, while our protocol does not. > > Also add a command line parameter to each module, and a reserved field in > order to have the layout aligned. Note that the current implementation in > libxc doesn't make use of the module command line at all. Which would seem wrong then - what use is the field if it doesn't get filled? Or is that because it has nowhere to come from? But even then - wouldn't what I've read on the other thread mean at least the filename should be put there (as kind of the first command line element)? Jan
El 5/2/16 a les 14:13, Jan Beulich ha escrit: >>>> On 05.02.16 at 13:28, <roger.pau@citrix.com> wrote: >> This will prevent alignments from getting in the way. It's not safe to >> define this memory structures using C anyway, since the ABI depends on the >> bitness, while our protocol does not. >> >> Also add a command line parameter to each module, and a reserved field in >> order to have the layout aligned. Note that the current implementation in >> libxc doesn't make use of the module command line at all. > > Which would seem wrong then - what use is the field if it doesn't > get filled? Or is that because it has nowhere to come from? Right now it has nowhere to come from as you say. Once we enable this ABI for Dom0 boot we are just going to copy what's in the "string" field defined in the multiboot spec for each module structure that's passed to Dom0. That's the primary use I can see for this ATM. It was requested by Samuel, and I think it's fine to add it to the spec, even if we are not going to use it right now. Samuel requires something like this in order to properly pass parameters to boot modules in gnumach. > But > even then - wouldn't what I've read on the other thread mean > at least the filename should be put there (as kind of the first > command line element)? Really? I didn't get that impression at all, what's the filename useful for anyway? IMHO this would need plumbing through libxl/xl in order to be able to specify command line arguments for modules, which is something that we don't support at the moment. Roger.
>>> On 05.02.16 at 16:45, <roger.pau@citrix.com> wrote: > El 5/2/16 a les 14:13, Jan Beulich ha escrit: >> But >> even then - wouldn't what I've read on the other thread mean >> at least the filename should be put there (as kind of the first >> command line element)? > > Really? I didn't get that impression at all, what's the filename useful > for anyway? This largely depends on whether you mean to mimic multiboot1 or multiboot2. Remember the placeholder people have to add to certain grub2 invocation lines? I think that's a result of their attempt to had through a file name. But indeed - I've never understood what it's good for until this request came about: How else would the consumer know which module is which if there's no signature or alike inside the file? Iirc someone suggested (in the context of the discussion here) that Linux might look for "initrd" in the filename to identify that one. Seems fragile to me, but may be the only alternative if not demanding multiple modules to be ordered in a certain way. Jan
Jan Beulich, on Tue 09 Feb 2016 01:14:13 -0700, wrote: > >>> On 05.02.16 at 16:45, <roger.pau@citrix.com> wrote: > > El 5/2/16 a les 14:13, Jan Beulich ha escrit: > >> But > >> even then - wouldn't what I've read on the other thread mean > >> at least the filename should be put there (as kind of the first > >> command line element)? > > > > Really? I didn't get that impression at all, what's the filename useful > > for anyway? > > This largely depends on whether you mean to mimic multiboot1 or > multiboot2. Remember the placeholder people have to add to > certain grub2 invocation lines? I think that's a result of their > attempt to had through a file name. Yes. For gnumach we've been adding the file name by hand in the command line. Otherwise the resulting tasks would have at best their first parameter as task name, since gnumach has no idea what these modules... Samuel
El 9/2/16 a les 9:14, Jan Beulich ha escrit: >>>> On 05.02.16 at 16:45, <roger.pau@citrix.com> wrote: >> El 5/2/16 a les 14:13, Jan Beulich ha escrit: >>> But >>> even then - wouldn't what I've read on the other thread mean >>> at least the filename should be put there (as kind of the first >>> command line element)? >> >> Really? I didn't get that impression at all, what's the filename useful >> for anyway? > > This largely depends on whether you mean to mimic multiboot1 or > multiboot2. Remember the placeholder people have to add to > certain grub2 invocation lines? I think that's a result of their > attempt to had through a file name. But indeed - I've never > understood what it's good for until this request came about: How > else would the consumer know which module is which if there's no > signature or alike inside the file? Iirc someone suggested (in the > context of the discussion here) that Linux might look for "initrd" > in the filename to identify that one. Seems fragile to me, but may > be the only alternative if not demanding multiple modules to be > ordered in a certain way. OK, right now xl/libxl only allows passing one module that's considered the initram/ramdisk from Linux PoV. Other OSes that use the pv loader don't pass any module at all AFAIK. The problem is that there's no way to specify a command line parameter for modules, and I think in order to make use of this we should add such option, instead of hard coding "initrd" as the command line of the first module. However, I don't think any of this should prevent this patch from being accepted. If someone has interest in implementing a way to pass module command line arguments please feel free to do it, this patch just sets the right ABI for doing so, although it's unused at the moment. Roger.
Roger Pau Monné, on Tue 09 Feb 2016 11:38:43 +0100, wrote:
> Other OSes that use the pv loader don't pass any module at all AFAIK.
GNU Mach does need several modules.
Samuel
El 9/2/16 a les 11:41, Samuel Thibault ha escrit: > Roger Pau Monné, on Tue 09 Feb 2016 11:38:43 +0100, wrote: >> Other OSes that use the pv loader don't pass any module at all AFAIK. > > GNU Mach does need several modules. How do you usually pass multiple modules from an xl configuration file at the moment? Roger.
Roger Pau Monné, on Tue 09 Feb 2016 11:45:01 +0100, wrote: > El 9/2/16 a les 11:41, Samuel Thibault ha escrit: > > Roger Pau Monné, on Tue 09 Feb 2016 11:38:43 +0100, wrote: > >> Other OSes that use the pv loader don't pass any module at all AFAIK. > > > > GNU Mach does need several modules. > > How do you usually pass multiple modules from an xl configuration file > at the moment? We pack the modules and command lines in one big blob, which we really don't like. Or we just use pv-grub / pv-grub2. Samuel
El 9/2/16 a les 11:49, Samuel Thibault ha escrit: > Roger Pau Monné, on Tue 09 Feb 2016 11:45:01 +0100, wrote: >> El 9/2/16 a les 11:41, Samuel Thibault ha escrit: >>> Roger Pau Monné, on Tue 09 Feb 2016 11:38:43 +0100, wrote: >>>> Other OSes that use the pv loader don't pass any module at all AFAIK. >>> >>> GNU Mach does need several modules. >> >> How do you usually pass multiple modules from an xl configuration file >> at the moment? > > We pack the modules and command lines in one big blob, which we really > don't like. Right, this should allow you to pass multiple modules, but someone needs to implement the xl/libxl/libxc side in order to do it. As said, I don't think this should block it from being accepted. Roger.
On 09/02/16 10:56, Roger Pau Monné wrote: > El 9/2/16 a les 11:49, Samuel Thibault ha escrit: >> Roger Pau Monné, on Tue 09 Feb 2016 11:45:01 +0100, wrote: >>> El 9/2/16 a les 11:41, Samuel Thibault ha escrit: >>>> Roger Pau Monné, on Tue 09 Feb 2016 11:38:43 +0100, wrote: >>>>> Other OSes that use the pv loader don't pass any module at all AFAIK. >>>> GNU Mach does need several modules. >>> How do you usually pass multiple modules from an xl configuration file >>> at the moment? >> We pack the modules and command lines in one big blob, which we really >> don't like. > Right, this should allow you to pass multiple modules, but someone needs > to implement the xl/libxl/libxc side in order to do it. As said, I don't > think this should block it from being accepted. +1 on both counts. ~Andrew
diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h index cac4698..e5ab56c 100644 --- a/tools/libxc/include/xc_dom.h +++ b/tools/libxc/include/xc_dom.h @@ -216,6 +216,34 @@ struct xc_dom_image { struct xc_hvm_firmware_module smbios_module; }; +#if defined(__i386__) || defined(__x86_64__) +/* C representation of the x86/HVM start info layout. + * + * The canonical definition of this layout resides in public/xen.h, this + * is just a way to represent the layout described there using C types. + * + * NB: the packed attribute is not really needed, but it helps us enforce + * the fact this this is just a representation, and it might indeed + * be required in the future if there are alignment changes. + */ +struct hvm_start_info { + uint32_t magic; /* Contains the magic value 0x336ec578 */ + /* ("xEn3" with the 0x80 bit of the "E" set).*/ + uint32_t flags; /* SIF_xxx flags. */ + uint32_t cmdline_paddr; /* Physical address of the command line. */ + uint32_t nr_modules; /* Number of modules passed to the kernel. */ + uint32_t modlist_paddr; /* Physical address of an array of */ + /* hvm_modlist_entry. */ +} __attribute__((packed)); + +struct hvm_modlist_entry { + uint32_t paddr; /* Physical address of the module. */ + uint32_t size; /* Size of the module in bytes. */ + uint32_t cmdline_paddr; /* Physical address of the command line. */ + uint32_t reserved; +} __attribute__((packed)); +#endif /* x86 */ + /* --- pluggable kernel loader ------------------------------------- */ struct xc_dom_loader { diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h index 7b629b1..e1350d0 100644 --- a/xen/include/public/xen.h +++ b/xen/include/public/xen.h @@ -790,22 +790,36 @@ typedef struct start_info start_info_t; * NOTE: nothing will be loaded at physical address 0, so * a 0 value in any of the address fields should be treated * as not present. + * + * 0 +----------------+ + * | magic | Contains the magic value HVM_START_MAGIC_VALUE + * | | ("xEn3" with the 0x80 bit of the "E" set). + * 4 +----------------+ + * | flags | SIF_xxx flags. + * 8 +----------------+ + * | cmdline_paddr | Physical address of the command line, + * | | a zero-terminated ASCII string. + * 12 +----------------+ + * | nr_modules | Number of modules passed to the kernel. + * 16 +----------------+ + * | modlist_paddr | Physical address of an array of modules + * | | (layout of the structure below). + * 20 +----------------+ + * + * The layout of each entry in the module structure is the following: + * + * 0 +----------------+ + * | paddr | Physical address of the module. + * 4 +----------------+ + * | size | Size of the module in bytes. + * 8 +----------------+ + * | cmdline_paddr | Physical address of the command line, + * | | a zero-terminated ASCII string. + * 12 +----------------+ + * | reserved | + * 16 +----------------+ */ -struct hvm_start_info { #define HVM_START_MAGIC_VALUE 0x336ec578 - uint32_t magic; /* Contains the magic value 0x336ec578 */ - /* ("xEn3" with the 0x80 bit of the "E" set).*/ - uint32_t flags; /* SIF_xxx flags. */ - uint32_t cmdline_paddr; /* Physical address of the command line. */ - uint32_t nr_modules; /* Number of modules passed to the kernel. */ - uint32_t modlist_paddr; /* Physical address of an array of */ - /* hvm_modlist_entry. */ -}; - -struct hvm_modlist_entry { - uint32_t paddr; /* Physical address of the module. */ - uint32_t size; /* Size of the module in bytes. */ -}; /* New console union for dom0 introduced in 0x00030203. */ #if __XEN_INTERFACE_VERSION__ < 0x00030203
This will prevent alignments from getting in the way. It's not safe to define this memory structures using C anyway, since the ABI depends on the bitness, while our protocol does not. Also add a command line parameter to each module, and a reserved field in order to have the layout aligned. Note that the current implementation in libxc doesn't make use of the module command line at all. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Cc: Samuel Thibault <samuel.thibault@ens-lyon.org> Cc: Ian Jackson <ian.jackson@eu.citrix.com> Cc: Ian Campbell <ian.campbell@citrix.com> Cc: Wei Liu <wei.liu2@citrix.com> Cc: Jan Beulich <jbeulich@suse.com> Cc: Andrew Cooper <andrew.cooper3@citrix.com> --- tools/libxc/include/xc_dom.h | 28 ++++++++++++++++++++++++++++ xen/include/public/xen.h | 42 ++++++++++++++++++++++++++++-------------- 2 files changed, 56 insertions(+), 14 deletions(-)