Hey guys,
Here are my thoughts about current Xen boot
infrastructure and some changes proposal.
It is linked with EFI development but not only.
Since the beginning Xen image and other needed stuff
could be loaded into memory according to multiboot
protocol. (e.g. implemented by GRUB). It means that
current implementation of Xen takes info about current
system config from multiboot_info_t structure (it is
copied from original place in assembly files and then
passed as an argument to __start_xen()) and some other
BIOS sources if needed (e.g. VGA config, EDD data).
Later when EFI come into the scene there was no significant
change in that. multiboot_info_t structure and others
are initialized artificially by Xen EFI boot stuff.
Additionally, due to that there is no place for extra boot
info in multiboot_info_t e.g. ACPI data is passed via
supplementary global variables. Now there is a requirement
for boot Xen on EFI platform via GRUB. Due to that new
boot protocol called multiboot2 should be supported.
This means that in current situation another conversion
to legacy multiboot_info_t structure and others should be
implemented. However, due to limitations of multiboot_info_t
structure not all arguments (e.g. ACPI data) could be passed
via it. That leads to further code complication. It means
that at this stage it is worth to create completely new
boot structure which is not linked so tightly with any boot
protocol. It should contain all needed stuff, be architecture
independent as much as possible and easily extensible.
In cases when architecture depended things are required
there should be special substructure which would contain
all required stuff. More or less it should look like in x86 case:
/* Xen Boot Info (XBI) module structure. */
typedef struct {
u64 start;
u64 end;
char *cmdline;
} xbi_mod_t;
/* Xen Boot Info Arch (XBIA) memory map structure. */
typedef struct {
/*
* Amount of lower memory accordingly to The Multiboot
* Specification version 0.6.96.
*/
u32 lower;
/*
* Amount of upper memory accordingly to The Multiboot
* Specification version 0.6.96.
*/
u32 upper;
u32 map_size;
struct e820entry *e820map;
} xbia_mem_t;
/* Xen Boot Info Arch (XBIA). */
typedef struct {
EFI_SYSTEM_TABLE *efi_system_table;
u64 mps; /* Pointer to MPS. */
u64 acpi; /* Pointer to ACPI RSDP. */
u64 smbios; /* Pointer to SMBIOS. */
xbia_mem_t mem;
struct xen_vga_console_info vga_console_info;
struct edd_info *edd_info;
} xbia_t;
/* Main Xen Boot Info (XBI) structure. */
typedef struct {
char *boot_loader_name;
char *cmdline;
u32 mods_count;
xbi_mod_t *mods;
xbia_t arch;
} xbi_t;
All data should be placed in above structures as early
as possible. Usually it will be done in some assembly
files before __start_xen() call or in efi_start() on EFI
platform and in __start_xen() itself. Additionally, it
should be mentioned that not all members are valid on
every platform and sometimes some of them would not be
initialized.
Daniel
On Tue, 21 May 2013, Daniel Kiper wrote:> Hey guys, > > Here are my thoughts about current Xen boot > infrastructure and some changes proposal. > It is linked with EFI development but not only. > > Since the beginning Xen image and other needed stuff > could be loaded into memory according to multiboot > protocol. (e.g. implemented by GRUB). It means that > current implementation of Xen takes info about current > system config from multiboot_info_t structure (it is > copied from original place in assembly files and then > passed as an argument to __start_xen()) and some other > BIOS sources if needed (e.g. VGA config, EDD data). > Later when EFI come into the scene there was no significant > change in that. multiboot_info_t structure and others > are initialized artificially by Xen EFI boot stuff. > Additionally, due to that there is no place for extra boot > info in multiboot_info_t e.g. ACPI data is passed via > supplementary global variables. Now there is a requirement > for boot Xen on EFI platform via GRUB. Due to that new > boot protocol called multiboot2 should be supported. > This means that in current situation another conversion > to legacy multiboot_info_t structure and others should be > implemented. However, due to limitations of multiboot_info_t > structure not all arguments (e.g. ACPI data) could be passed > via it. That leads to further code complication. It means > that at this stage it is worth to create completely new > boot structure which is not linked so tightly with any boot > protocol. It should contain all needed stuff, be architecture > independent as much as possible and easily extensible. > In cases when architecture depended things are required > there should be special substructure which would contain > all required stuff. More or less it should look like in x86 case: > > /* Xen Boot Info (XBI) module structure. */ > typedef struct { > u64 start; > u64 end; > char *cmdline; > } xbi_mod_t; > > /* Xen Boot Info Arch (XBIA) memory map structure. */ > typedef struct { > /* > * Amount of lower memory accordingly to The Multiboot > * Specification version 0.6.96. > */ > u32 lower; > /* > * Amount of upper memory accordingly to The Multiboot > * Specification version 0.6.96. > */ > u32 upper; > u32 map_size; > struct e820entry *e820map;e820map needs to be moved to an x86 specific struct. Can we use the arch-independent memory map structs as defined in section 3.4.8 of the multiboot2 spec instead?> } xbia_mem_t; > > /* Xen Boot Info Arch (XBIA). */ > typedef struct { > EFI_SYSTEM_TABLE *efi_system_table; > u64 mps; /* Pointer to MPS. */ > u64 acpi; /* Pointer to ACPI RSDP. */ > u64 smbios; /* Pointer to SMBIOS. */ > xbia_mem_t mem; > struct xen_vga_console_info vga_console_info; > struct edd_info *edd_info; > } xbia_t;We need to add a pointer to device_tree in there.> /* Main Xen Boot Info (XBI) structure. */ > typedef struct { > char *boot_loader_name; > char *cmdline; > u32 mods_count; > xbi_mod_t *mods; > xbia_t arch; > } xbi_t; > > All data should be placed in above structures as early > as possible. Usually it will be done in some assembly > files before __start_xen() call or in efi_start() on EFI > platform and in __start_xen() itself. Additionally, it > should be mentioned that not all members are valid on > every platform and sometimes some of them would not be > initialized.Right. We need to define an unitialized value for these fields so that we can recognized what actually is availble. For example on ARM both device_tree and ACPI or only one of them could be available.
>>> On 21.05.13 at 12:36, Daniel Kiper <daniel.kiper@oracle.com> wrote: > /* Xen Boot Info Arch (XBIA) memory map structure. */ > typedef struct { > /* > * Amount of lower memory accordingly to The Multiboot > * Specification version 0.6.96. > */ > u32 lower; > /* > * Amount of upper memory accordingly to The Multiboot > * Specification version 0.6.96. > */ > u32 upper; > u32 map_size; > struct e820entry *e820map; > } xbia_mem_t;The concepts of lower, upper, and E820 memory are all very much tied to x86.> /* Xen Boot Info Arch (XBIA). */ > typedef struct { > EFI_SYSTEM_TABLE *efi_system_table; > u64 mps; /* Pointer to MPS. */ > u64 acpi; /* Pointer to ACPI RSDP. */ > u64 smbios; /* Pointer to SMBIOS. */ > xbia_mem_t mem; > struct xen_vga_console_info vga_console_info; > struct edd_info *edd_info; > } xbia_t;As are - I think - MPS, EDD, perhaps SMBIOS, and maybe VGA. If you want to design anything here (and other than you try to suggest I don''t think booting is really a process that can be made almost arch neutral/generic), you''d need to completely separate out any _potentially_ arch specific things, not just those that today we know are specific to one arch or common between the only two and a half we support. That may mean that _each_ of the items above should become a separate one, in which case an enumeration concept would likely be the better one. Jan
On 21/05/13 11:36, Daniel Kiper wrote:> Hey guys, > > Here are my thoughts about current Xen boot > infrastructure and some changes proposal. > It is linked with EFI development but not only. > > [...] It means > that at this stage it is worth to create completely new > boot structure which is not linked so tightly with any boot > protocol. It should contain all needed stuff, be architecture > independent as much as possible and easily extensible.It''s not clear how a new set of structures like this fits the requirement to be easily extensible. Aren''t you going to have the same problems if new fields need to be added? I would suggest considering a linear block of (type, length, data) tuples for each field. That way only the needed/applicable fields can be included, and Xen can easily skip over field it does not recognize or care about (so new fields can be added without breaking compatibility).> In cases when architecture depended things are required > there should be special substructure which would contain > all required stuff. More or less it should look like in x86 case: > > /* Xen Boot Info (XBI) module structure. */ > typedef struct { > u64 start; > u64 end; > char *cmdline; > } xbi_mod_t; > > /* Xen Boot Info Arch (XBIA) memory map structure. */ > typedef struct { > /* > * Amount of lower memory accordingly to The Multiboot > * Specification version 0.6.96. > */ > u32 lower; > /* > * Amount of upper memory accordingly to The Multiboot > * Specification version 0.6.96. > */ > u32 upper; > u32 map_size; > struct e820entry *e820map; > } xbia_mem_t; > > /* Xen Boot Info Arch (XBIA). */ > typedef struct { > EFI_SYSTEM_TABLE *efi_system_table; > u64 mps; /* Pointer to MPS. */ > u64 acpi; /* Pointer to ACPI RSDP. */ > u64 smbios; /* Pointer to SMBIOS. */ > xbia_mem_t mem; > struct xen_vga_console_info vga_console_info; > struct edd_info *edd_info; > } xbia_t; > > /* Main Xen Boot Info (XBI) structure. */ > typedef struct { > char *boot_loader_name; > char *cmdline; > u32 mods_count; > xbi_mod_t *mods; > xbia_t arch; > } xbi_t;David
On Tue, 2013-05-21 at 03:36 -0700, Daniel Kiper wrote:> It means > that at this stage it is worth to create completely new > boot structure which is not linked so tightly with any boot > protocol. It should contain all needed stuff, be architecture > independent as much as possible and easily extensible.Is this proposal intended to form a protocol between the bootloader and Xen or between the early (e.g. prepaging, x86 real mode etc) Xen and later Xen? Ian.
On Tue, May 21, 2013 at 12:39:44PM +0100, Stefano Stabellini wrote:> On Tue, 21 May 2013, Daniel Kiper wrote:[...]> > /* Xen Boot Info Arch (XBIA) memory map structure. */ > > typedef struct { > > /* > > * Amount of lower memory accordingly to The Multiboot > > * Specification version 0.6.96. > > */ > > u32 lower; > > /* > > * Amount of upper memory accordingly to The Multiboot > > * Specification version 0.6.96. > > */ > > u32 upper; > > u32 map_size; > > struct e820entry *e820map; > > e820map needs to be moved to an x86 specific struct.It is Xen Boot Info Arch (XBIA) memory map structure which is architecture dependent and it is part of Xen Boot Info Arch (XBIA) (please look below).> Can we use the arch-independent memory map structs as defined in section > 3.4.8 of the multiboot2 spec instead?I thought about that once but I am not sure that it is good idea. It is quiet tightly linked to architecture (e.g. address space which maybe 64-bit, 32-bit, ... or memory types representation). However, I do not insist on staying with that. If we decide to made it arch independed on the base of multiboot2 spec then we should only use base_addr (u64), length (u64) and type (u32).> > } xbia_mem_t; > > > > /* Xen Boot Info Arch (XBIA). */ > > typedef struct { > > EFI_SYSTEM_TABLE *efi_system_table; > > u64 mps; /* Pointer to MPS. */ > > u64 acpi; /* Pointer to ACPI RSDP. */ > > u64 smbios; /* Pointer to SMBIOS. */ > > xbia_mem_t mem; > > struct xen_vga_console_info vga_console_info; > > struct edd_info *edd_info; > > } xbia_t; > > We need to add a pointer to device_tree in there.As I mentioned earlier it is x86 stuff. ARM arch should have relevant separate xbia_t.> > /* Main Xen Boot Info (XBI) structure. */ > > typedef struct { > > char *boot_loader_name; > > char *cmdline; > > u32 mods_count; > > xbi_mod_t *mods; > > xbia_t arch; > > } xbi_t;Daniel
On 21/05/2013 11:36, "Daniel Kiper" <daniel.kiper@oracle.com> wrote:> Hey guys, > > Here are my thoughts about current Xen boot > infrastructure and some changes proposal. > It is linked with EFI development but not only. > > Since the beginning Xen image and other needed stuff > could be loaded into memory according to multiboot > protocol. (e.g. implemented by GRUB). It means that > current implementation of Xen takes info about current > system config from multiboot_info_t structure (it is > copied from original place in assembly files and then > passed as an argument to __start_xen()) and some other > BIOS sources if needed (e.g. VGA config, EDD data). > Later when EFI come into the scene there was no significant > change in that. multiboot_info_t structure and others > are initialized artificially by Xen EFI boot stuff. > Additionally, due to that there is no place for extra boot > info in multiboot_info_t e.g. ACPI data is passed via > supplementary global variables. Now there is a requirement > for boot Xen on EFI platform via GRUB. Due to that new > boot protocol called multiboot2 should be supported. > This means that in current situation another conversion > to legacy multiboot_info_t structure and others should be > implemented. However, due to limitations of multiboot_info_t > structure not all arguments (e.g. ACPI data) could be passed > via it. That leads to further code complication. It means > that at this stage it is worth to create completely new > boot structure which is not linked so tightly with any boot > protocol.> It should contain all needed stuff, be architecture > independent as much as possible and easily extensible.Why and why? Well I mean we shouldn''t make things deliberately architecture *dependent*, but where tables and flags need parsing/translating I''d rather do that once per architecture, rather than once per bootloader format (which I am thinking are mostly arch-dependent, and hence at least as numerous as architectures). As for easily extensible, we are talking about communication between Xen ''pre-loaders'' (we might call them) and Xen proper (eg arch/x86/setup.c). Who cares about the hassle of extensible self-describing formats here? Just make it a struct and extend the struct, it all gets built together as matched sets of pre-loaders and Xen anyway. I''d be looking for a simple extension to what we have to pass stuff like ACPI through, if it''s needed at all. Maybe clean things up a bit and work out a nice way to mate that with the existing multiboot-centric world. I wouldn''t waste my time hitting it with the architecture sledgehammer. -- Keir> In cases when architecture depended things are required > there should be special substructure which would contain > all required stuff. More or less it should look like in x86 case: > > /* Xen Boot Info (XBI) module structure. */ > typedef struct { > u64 start; > u64 end; > char *cmdline; > } xbi_mod_t; > > /* Xen Boot Info Arch (XBIA) memory map structure. */ > typedef struct { > /* > * Amount of lower memory accordingly to The Multiboot > * Specification version 0.6.96. > */ > u32 lower; > /* > * Amount of upper memory accordingly to The Multiboot > * Specification version 0.6.96. > */ > u32 upper; > u32 map_size; > struct e820entry *e820map; > } xbia_mem_t; > > /* Xen Boot Info Arch (XBIA). */ > typedef struct { > EFI_SYSTEM_TABLE *efi_system_table; > u64 mps; /* Pointer to MPS. */ > u64 acpi; /* Pointer to ACPI RSDP. */ > u64 smbios; /* Pointer to SMBIOS. */ > xbia_mem_t mem; > struct xen_vga_console_info vga_console_info; > struct edd_info *edd_info; > } xbia_t; > > /* Main Xen Boot Info (XBI) structure. */ > typedef struct { > char *boot_loader_name; > char *cmdline; > u32 mods_count; > xbi_mod_t *mods; > xbia_t arch; > } xbi_t; > > All data should be placed in above structures as early > as possible. Usually it will be done in some assembly > files before __start_xen() call or in efi_start() on EFI > platform and in __start_xen() itself. Additionally, it > should be mentioned that not all members are valid on > every platform and sometimes some of them would not be > initialized. > > Daniel
On Tue, May 21, 2013 at 01:03:48PM +0100, Jan Beulich wrote:> >>> On 21.05.13 at 12:36, Daniel Kiper <daniel.kiper@oracle.com> wrote: > > /* Xen Boot Info Arch (XBIA) memory map structure. */ > > typedef struct { > > /* > > * Amount of lower memory accordingly to The Multiboot > > * Specification version 0.6.96. > > */ > > u32 lower; > > /* > > * Amount of upper memory accordingly to The Multiboot > > * Specification version 0.6.96. > > */ > > u32 upper; > > u32 map_size; > > struct e820entry *e820map; > > } xbia_mem_t; > > The concepts of lower, upper, and E820 memory are all very much > tied to x86.That is why this is a part of Xen Boot Info Arch (XBIA) not Xen Boot Info (XBI) which is main struct.> > /* Xen Boot Info Arch (XBIA). */ > > typedef struct { > > EFI_SYSTEM_TABLE *efi_system_table; > > u64 mps; /* Pointer to MPS. */ > > u64 acpi; /* Pointer to ACPI RSDP. */ > > u64 smbios; /* Pointer to SMBIOS. */ > > xbia_mem_t mem; > > struct xen_vga_console_info vga_console_info; > > struct edd_info *edd_info; > > } xbia_t; > > As are - I think - MPS, EDD, perhaps SMBIOS, and maybe VGA.As above.> If you want to design anything here (and other than you try to > suggest I don''t think booting is really a process that can be made > almost arch neutral/generic), you''d need to completely separateI am aware of that and I do not insist on doing it in that way. I am just looking for best solution.> out any _potentially_ arch specific things, not just those that > today we know are specific to one arch or common between the > only two and a half we support. That may mean that _each_ ofRight.> the items above should become a separate one, in which case an > enumeration concept would likely be the better one.I do not fully understand what do you mean by "enumeration concept". I think that passing info about system via many not "linked" variables is not the best idea. It works in that way today because multiboot structure is not extensible. That is why I think we should find new solution. Our new custom build structure which contains only stuff required by Xen looks good. All members are easliy accessible from C. It could be easliy extended (if we need it just add new member) because it would not be linked with specific boot protocol. However, I agree that distinction between arch dependent and independent stuff is disputable. Maybe we should drop that idea, assume that Xen Boot Info (XBI) is always arch dependent and put all needed members in one struct. IMO this is better solution than current one too. Daniel
On Tue, May 21, 2013 at 01:43:08PM +0100, David Vrabel wrote:> On 21/05/13 11:36, Daniel Kiper wrote: > > Hey guys, > > > > Here are my thoughts about current Xen boot > > infrastructure and some changes proposal. > > It is linked with EFI development but not only. > > > > [...] It means > > that at this stage it is worth to create completely new > > boot structure which is not linked so tightly with any boot > > protocol. It should contain all needed stuff, be architecture > > independent as much as possible and easily extensible. > > It''s not clear how a new set of structures like this fits the > requirement to be easily extensible. Aren''t you going to have the same > problems if new fields need to be added?No, because it would not be linked with any boot protocol. It will depend only on our idea because it will be Xen internal structure.> I would suggest considering a linear block of (type, length, data) > tuples for each field. That way only the needed/applicable fields can > be included, and Xen can easily skip over field it does not recognize or > care about (so new fields can be added without breaking compatibility).Right, but then you must prepare function(s) which will be looking for required data. I think that, in that case, all needed stuff should be easily accessible just by referencing relevant member in struct. Daniel
On Tue, May 21, 2013 at 01:52:04PM +0100, Ian Campbell wrote:> On Tue, 2013-05-21 at 03:36 -0700, Daniel Kiper wrote: > > It means > > that at this stage it is worth to create completely new > > boot structure which is not linked so tightly with any boot > > protocol. It should contain all needed stuff, be architecture > > independent as much as possible and easily extensible. > > Is this proposal intended to form a protocol between the bootloader andMore or less. The goal is to have one place to store info about current system config. It could be taken from different sources via various stuff (multiboot protocol, BIOS, EFI, ...) and always saved in one place. Currently all stuff is spread over various places which is not very convenient and nice.> Xen or between the early (e.g. prepaging, x86 real mode etc) Xen and > later Xen?Daniel
>>> On 22.05.13 at 16:09, Daniel Kiper <daniel.kiper@oracle.com> wrote: > On Tue, May 21, 2013 at 01:03:48PM +0100, Jan Beulich wrote: >> >>> On 21.05.13 at 12:36, Daniel Kiper <daniel.kiper@oracle.com> wrote: >> > /* Xen Boot Info Arch (XBIA) memory map structure. */ >> > typedef struct { >> > /* >> > * Amount of lower memory accordingly to The Multiboot >> > * Specification version 0.6.96. >> > */ >> > u32 lower; >> > /* >> > * Amount of upper memory accordingly to The Multiboot >> > * Specification version 0.6.96. >> > */ >> > u32 upper; >> > u32 map_size; >> > struct e820entry *e820map; >> > } xbia_mem_t; >> >> The concepts of lower, upper, and E820 memory are all very much >> tied to x86. > > That is why this is a part of Xen Boot Info Arch (XBIA) > not Xen Boot Info (XBI) which is main struct. > >> > /* Xen Boot Info Arch (XBIA). */ >> > typedef struct { >> > EFI_SYSTEM_TABLE *efi_system_table; >> > u64 mps; /* Pointer to MPS. */ >> > u64 acpi; /* Pointer to ACPI RSDP. */ >> > u64 smbios; /* Pointer to SMBIOS. */ >> > xbia_mem_t mem; >> > struct xen_vga_console_info vga_console_info; >> > struct edd_info *edd_info; >> > } xbia_t; >> >> As are - I think - MPS, EDD, perhaps SMBIOS, and maybe VGA. > > As above.Okay, emphasizing this would have helped, the more that the split out xbia_mem_t suggested re-usability considerations.>> the items above should become a separate one, in which case an >> enumeration concept would likely be the better one. > > I do not fully understand what do you mean by "enumeration concept".That point is irrelevant with the above clarification.> I think that passing info about system via many not "linked" variables > is not the best idea. It works in that way today because multiboot > structure is not extensible. That is why I think we should find new > solution. Our new custom build structure which contains only stuff > required by Xen looks good. All members are easliy accessible from C. > It could be easliy extended (if we need it just add new member) because > it would not be linked with specific boot protocol.Why does it matter whether the MBI structure is extensible? Rather than copying everything around a number of times, we can as well use is where it lives naturally. The only requirement is that all information be accessible - whether in one strcture, two, or a dozen doesn''t matter. Jan
>>> On 22.05.13 at 16:27, Daniel Kiper <daniel.kiper@oracle.com> wrote: > On Tue, May 21, 2013 at 01:52:04PM +0100, Ian Campbell wrote: >> On Tue, 2013-05-21 at 03:36 -0700, Daniel Kiper wrote: >> > It means >> > that at this stage it is worth to create completely new >> > boot structure which is not linked so tightly with any boot >> > protocol. It should contain all needed stuff, be architecture >> > independent as much as possible and easily extensible. >> >> Is this proposal intended to form a protocol between the bootloader and > > More or less. The goal is to have one place to store info about > current system config. It could be taken from different sources > via various stuff (multiboot protocol, BIOS, EFI, ...) and always > saved in one place. Currently all stuff is spread over various > places which is not very convenient and nice.As just said in another reply - copying stuff needlessly isn''t nice either, or at least it''s a matter of taste which of the two one may like better. Jan
On Tue, May 21, 2013 at 02:24:37PM +0100, Keir Fraser wrote:> On 21/05/2013 11:36, "Daniel Kiper" <daniel.kiper@oracle.com> wrote:[...]> > It should contain all needed stuff, be architecture > > independent as much as possible and easily extensible. > > Why and why? Well I mean we shouldn''t make things deliberately architecture > *dependent*, but where tables and flags need parsing/translating I''d rather > do that once per architecture, rather than once per bootloader format (which > I am thinking are mostly arch-dependent, and hence at least as numerous as > architectures). As for easily extensible, we are talking about communication > between Xen ''pre-loaders'' (we might call them) and Xen proper (eg > arch/x86/setup.c). Who cares about the hassle of extensible self-describing > formats here? Just make it a struct and extend the struct, it all gets built > together as matched sets of pre-loaders and Xen anyway. > > I''d be looking for a simple extension to what we have to pass stuff like > ACPI through, if it''s needed at all. Maybe clean things up a bit and work > out a nice way to mate that with the existing multiboot-centric world. I > wouldn''t waste my time hitting it with the architecture sledgehammer.I do not insist on using so complicated structure because it has some weakness which I am aware of. However, I think we should build our own structure to pass stuff from preloader (mainly assembly files) to proper Xen. It will be nice if it has place for other boot stuff collected in __start_xen() too. If we stick to old multiboot structure it will make diffculties in multiboot2 protocol implementation. For example it will be very difficult to pass (in sensible way), from preloader to Xen, info about ACPI and EFI stuff. That is why I think that this new structure should not be so tightly linked with any boot protocol. Daniel
On Wed, May 22, 2013 at 03:33:29PM +0100, Jan Beulich wrote:> >>> On 22.05.13 at 16:09, Daniel Kiper <daniel.kiper@oracle.com> wrote:[...]> > I think that passing info about system via many not "linked" variables > > is not the best idea. It works in that way today because multiboot > > structure is not extensible. That is why I think we should find new > > solution. Our new custom build structure which contains only stuff > > required by Xen looks good. All members are easliy accessible from C. > > It could be easliy extended (if we need it just add new member) because > > it would not be linked with specific boot protocol. > > Why does it matter whether the MBI structure is extensible?If we stick to current MBI I am not able to pass (in sensible way), from preloader to __start_xen(), e.g. ACPI and EFI stuff from multiboot2 protocol. That is why I think we should build our own struct which is not linked with any boot protocol. That way we could avoid similar problems in the future.> Rather than copying everything around a number of times, we > can as well use is where it lives naturally. The only requirementThis is the best approach if we would have one type of boot protocol. It is not true if we would like to support more then one.> is that all information be accessible - whether in one strcture, > two, or a dozen doesn''t matter.I agree that this is not a must but I prefer to have "all in one" :-)))... We could do that cleanups (by the way) if we decide what to do with above issues. Daniel
On Wed, 2013-05-22 at 16:27 +0200, Daniel Kiper wrote:> On Tue, May 21, 2013 at 01:52:04PM +0100, Ian Campbell wrote: > > On Tue, 2013-05-21 at 03:36 -0700, Daniel Kiper wrote: > > > It means > > > that at this stage it is worth to create completely new > > > boot structure which is not linked so tightly with any boot > > > protocol. It should contain all needed stuff, be architecture > > > independent as much as possible and easily extensible. > > > > Is this proposal intended to form a protocol between the bootloader and > > More or less.So we are expecting bootloaders to all implement this Xen specific, multiboot2-alike protocol? That doesn''t seem terribly likely to happen. IMHO we''d be far better off working with the multiboot folks to define extensions to multiboot2 and/or multiboot3 which cover this additional information -- especially given that nothing much there seems to be Xen specific. Ian.
>>> On 22.05.13 at 16:43, Daniel Kiper <daniel.kiper@oracle.com> wrote: > For example it will be very difficult to > pass (in sensible way), from preloader to Xen, info about ACPI and EFI > stuff.Why do you think this is going to be "very difficult"? It''s a list of elements (very much like the enumerated concept I had thought of [without having looked at grub2 yet at that point; now I have] and that you had asked back about. All we''d need to do is iterate over the array of blocks and stash away the information we care about (into existing variables where possible, and into newly created ones otherwise). Jan
>>> On 22.05.13 at 17:01, Daniel Kiper <daniel.kiper@oracle.com> wrote: > If we stick to current MBI I am not able to pass (in sensible way), > from preloader to __start_xen(), e.g. ACPI and EFI stuff from multiboot2 > protocol.Why? You get handed a list (almost like an array) of items, and you''d pass the base address instead of the base address of the multiboot structure that we pass right now, together with an indicator which of the two it is. Then __start_xen() has to adopt its behavior to this. Not a big deal afaict. Jan
On Wed, 2013-05-22 at 16:09 +0100, Ian Campbell wrote:> On Wed, 2013-05-22 at 16:27 +0200, Daniel Kiper wrote: > > On Tue, May 21, 2013 at 01:52:04PM +0100, Ian Campbell wrote: > > > On Tue, 2013-05-21 at 03:36 -0700, Daniel Kiper wrote: > > > > It means > > > > that at this stage it is worth to create completely new > > > > boot structure which is not linked so tightly with any boot > > > > protocol. It should contain all needed stuff, be architecture > > > > independent as much as possible and easily extensible. > > > > > > Is this proposal intended to form a protocol between the bootloader and > > > > More or less. > > So we are expecting bootloaders to all implement this Xen specific,.... actually, I''m confused, the other sub-threads here seem to suggest this protocol is between the "preloader" bits of Xen and the main body of Xen, which is the other alternative I gave. Ian.
On Wed, May 22, 2013 at 04:25:19PM +0100, Ian Campbell wrote:> On Wed, 2013-05-22 at 16:09 +0100, Ian Campbell wrote: > > On Wed, 2013-05-22 at 16:27 +0200, Daniel Kiper wrote: > > > On Tue, May 21, 2013 at 01:52:04PM +0100, Ian Campbell wrote: > > > > On Tue, 2013-05-21 at 03:36 -0700, Daniel Kiper wrote: > > > > > It means > > > > > that at this stage it is worth to create completely new > > > > > boot structure which is not linked so tightly with any boot > > > > > protocol. It should contain all needed stuff, be architecture > > > > > independent as much as possible and easily extensible. > > > > > > > > Is this proposal intended to form a protocol between the bootloader and > > > > > > More or less. > > > > So we are expecting bootloaders to all implement this Xen specific, > .... > > actually, I''m confused, the other sub-threads here seem to suggest this > protocol is between the "preloader" bits of Xen and the main body of > Xen, which is the other alternative I gave.Sorry for confusion. I put my reply in incorrect line. I think about protocol "between the "preloader" bits of Xen and the main body of Xen". Daniel
On Wed, 2013-05-22 at 17:34 +0200, Daniel Kiper wrote:> On Wed, May 22, 2013 at 04:25:19PM +0100, Ian Campbell wrote: > > On Wed, 2013-05-22 at 16:09 +0100, Ian Campbell wrote: > > > On Wed, 2013-05-22 at 16:27 +0200, Daniel Kiper wrote: > > > > On Tue, May 21, 2013 at 01:52:04PM +0100, Ian Campbell wrote: > > > > > On Tue, 2013-05-21 at 03:36 -0700, Daniel Kiper wrote: > > > > > > It means > > > > > > that at this stage it is worth to create completely new > > > > > > boot structure which is not linked so tightly with any boot > > > > > > protocol. It should contain all needed stuff, be architecture > > > > > > independent as much as possible and easily extensible. > > > > > > > > > > Is this proposal intended to form a protocol between the bootloader and > > > > > > > > More or less. > > > > > > So we are expecting bootloaders to all implement this Xen specific, > > .... > > > > actually, I''m confused, the other sub-threads here seem to suggest this > > protocol is between the "preloader" bits of Xen and the main body of > > Xen, which is the other alternative I gave. > > Sorry for confusion. I put my reply in incorrect line. I think about > protocol "between the "preloader" bits of Xen and the main body of Xen".OK, thanks for clarifying ;-) In that case my question (which I think others have raised) is why does this need to be a complex extensible protocol at all? The two halves here must surely be upgraded in sync and so both sides can be changed whenever the data needs expanding. i.e. this is just struct boot_info, which perhaps contains "struct arch_boot_info arch;" Or perhaps I''ve just read more into what you are proposing than you were actually proposing. Ian.
On Wed, May 22, 2013 at 04:10:17PM +0100, Jan Beulich wrote:> >>> On 22.05.13 at 16:43, Daniel Kiper <daniel.kiper@oracle.com> wrote: > > For example it will be very difficult to > > pass (in sensible way), from preloader to Xen, info about ACPI and EFI > > stuff. > > Why do you think this is going to be "very difficult"? It''s a list of > elements (very much like the enumerated concept I had thought > of [without having looked at grub2 yet at that point; now I have] > and that you had asked back about. All we''d need to do is iterate > over the array of blocks and stash away the information we care > about (into existing variables where possible, and into newly > created ones otherwise).I though more about taste (that is why I added remark: in sensible way) than about implementation which is of course quiet simple. We could solve the problem of passing info for which place does not exists in MBI at least in three ways: - create next global variable which is awful for me (or use if it exists but is awful too), - pass this multiboot2 stuff almost directly (in real it must be copied to safe place; in multiboot protocol case required stuff is copied to trampoline); you mentioned about that in other email; better but not nice for me, - preloader should extract all needed stuff from structures passed by multiboot or multiboot2 protocol and put it in boot protocol independent struct which is then passed to __start_xen(); best for me; I described why in other emails. Daniel
On Wed, May 22, 2013 at 04:41:31PM +0100, Ian Campbell wrote: [...]> In that case my question (which I think others have raised) is why does > this need to be a complex extensible protocol at all? The two halves > here must surely be upgraded in sync and so both sides can be changed > whenever the data needs expanding. i.e. this is just struct boot_info, > which perhaps contains "struct arch_boot_info arch;" > > Or perhaps I''ve just read more into what you are proposing than you were > actually proposing.MBI structure which is passed to __start_xen() is strictly defined and could not be changed. It worked very nice once but now it is not true. It does not have enough place to pass eny extra information from other bootloaders (in this case multiboot2 protocol aware) and there are some members which are not used. In that case we must have e.g. global variables or other solution to pass that extra info. That is why I think we should build new struture which will replace MBI. It should have only data which is used by Xen. This way we could add in the future any extra members if we require it and we would not be so strictly bound to any boot protocol. However, I do not insist on the split between arch dependent and independent stuff. I know this is disputable. However, I posted that idea because I was not sure that soultion will fit. I just wanted to dicuss pros and cons of this split too. Daniel
On 22/05/2013 16:59, "Daniel Kiper" <daniel.kiper@oracle.com> wrote:> On Wed, May 22, 2013 at 04:10:17PM +0100, Jan Beulich wrote: >>>>> On 22.05.13 at 16:43, Daniel Kiper <daniel.kiper@oracle.com> wrote: >>> For example it will be very difficult to >>> pass (in sensible way), from preloader to Xen, info about ACPI and EFI >>> stuff. >> >> Why do you think this is going to be "very difficult"? It''s a list of >> elements (very much like the enumerated concept I had thought >> of [without having looked at grub2 yet at that point; now I have] >> and that you had asked back about. All we''d need to do is iterate >> over the array of blocks and stash away the information we care >> about (into existing variables where possible, and into newly >> created ones otherwise). > > I though more about taste (that is why I added remark: in sensible way) > than about implementation which is of course quiet simple. We could > solve the problem of passing info for which place does not exists > in MBI at least in three ways: > - create next global variable which is awful for me (or use > if it exists but is awful too), > - pass this multiboot2 stuff almost directly (in real it must > be copied to safe place; in multiboot protocol case required > stuff is copied to trampoline); you mentioned about that in > other email; better but not nice for me, > - preloader should extract all needed stuff from structures > passed by multiboot or multiboot2 protocol and put it in > boot protocol independent struct which is then passed to > __start_xen(); best for me; I described why in other emails.This third option is fine by me, but it is just a big struct to be passed to Xen. The extensible self-describing list format, and architecture independence, don''t really add value that I can see. If I were implementing this, I''d probably start by making a new struct that is a copy of multiboot_info, extend and modify fields as necessary, job done. Quite simple, won''t be much to argue against when the patches are posted for review, everyone happy. ;) -- Keir> Daniel
On Wed, May 22, 2013 at 04:16:30PM +0100, Jan Beulich wrote:> >>> On 22.05.13 at 17:01, Daniel Kiper <daniel.kiper@oracle.com> wrote: > > If we stick to current MBI I am not able to pass (in sensible way), > > from preloader to __start_xen(), e.g. ACPI and EFI stuff from multiboot2 > > protocol. > > Why? You get handed a list (almost like an array) of items, and you''d > pass the base address instead of the base address of the multiboot > structure that we pass right now, together with an indicator which > of the two it is. Then __start_xen() has to adopt its behavior to this. > Not a big deal afaict.Won''t you have to do a bunch of ''if (multibootv1) { use_this_offset } else if (multibootv2) { use this other offset }'' in the code to support both formats? If we just have a mesh of both of them we only have to do this sort of copying only once and can just use the struct that encompasses v2, v1, and whatever else we need (say pointer to RSDT).
On 22/05/2013 17:47, "Konrad Rzeszutek Wilk" <konrad.wilk@oracle.com> wrote:> On Wed, May 22, 2013 at 04:16:30PM +0100, Jan Beulich wrote: >>>>> On 22.05.13 at 17:01, Daniel Kiper <daniel.kiper@oracle.com> wrote: >>> If we stick to current MBI I am not able to pass (in sensible way), >>> from preloader to __start_xen(), e.g. ACPI and EFI stuff from multiboot2 >>> protocol. >> >> Why? You get handed a list (almost like an array) of items, and you''d >> pass the base address instead of the base address of the multiboot >> structure that we pass right now, together with an indicator which >> of the two it is. Then __start_xen() has to adopt its behavior to this. >> Not a big deal afaict. > > Won''t you have to do a bunch of ''if (multibootv1) { use_this_offset } else > if (multibootv2) { use this other offset }'' in the code to support > both formats? > > If we just have a mesh of both of them we only have to do this sort > of copying only once and can just use the struct that encompasses > v2, v1, and whatever else we need (say pointer to RSDT).Yes, having all the x86 (say) preloaders marshal into one single x86-specific format makes sense. That''s the sort of patch I would support. -- Keir> _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel
>>> On 22.05.13 at 18:47, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > On Wed, May 22, 2013 at 04:16:30PM +0100, Jan Beulich wrote: >> >>> On 22.05.13 at 17:01, Daniel Kiper <daniel.kiper@oracle.com> wrote: >> > If we stick to current MBI I am not able to pass (in sensible way), >> > from preloader to __start_xen(), e.g. ACPI and EFI stuff from multiboot2 >> > protocol. >> >> Why? You get handed a list (almost like an array) of items, and you''d >> pass the base address instead of the base address of the multiboot >> structure that we pass right now, together with an indicator which >> of the two it is. Then __start_xen() has to adopt its behavior to this. >> Not a big deal afaict. > > Won''t you have to do a bunch of ''if (multibootv1) { use_this_offset } else > if (multibootv2) { use this other offset }'' in the code to support > both formats?Yes, if this became unwieldy, I would favor the single copy approach. Or the alternative of having the initial parts of __start_xen() split off into two clones or prefixed by additional C code (i.e. basically the approach the current EFI boot code is using). Jan
On Wed, 2013-05-22 at 18:19 +0200, Daniel Kiper wrote:> On Wed, May 22, 2013 at 04:41:31PM +0100, Ian Campbell wrote: > > [...] > > > In that case my question (which I think others have raised) is why does > > this need to be a complex extensible protocol at all? The two halves > > here must surely be upgraded in sync and so both sides can be changed > > whenever the data needs expanding. i.e. this is just struct boot_info, > > which perhaps contains "struct arch_boot_info arch;" > > > > Or perhaps I''ve just read more into what you are proposing than you were > > actually proposing. > > MBI structure which is passed to __start_xen() is strictly defined and > could not be changed. It worked very nice once but now it is not true. > It does not have enough place to pass eny extra information from other > bootloaders (in this case multiboot2 protocol aware) and there are some > members which are not used. In that case we must have e.g. global variables > or other solution to pass that extra info. That is why I think we should > build new struture which will replace MBI."... replace Xen''s internal use of MBI" would be a way of putting this which is less prone to misunderstandings, I think ;-)> It should have only data which > is used by Xen. This way we could add in the future any extra members if we > require it and we would not be so strictly bound to any boot protocol.Yes, this sounds sane.> However, I do not insist on the split between arch dependent and > independent stuff. I know this is disputable. However, I posted > that idea because I was not sure that soultion will fit. > I just wanted to dicuss pros and cons of this split too.I think you''ve set requirements (extensibility etc) which aren''t actually required and this gave (at least me) the impression that this was something much more far reaching than it actually is. I suggest making it as simple as possible, basically a single struct, perhaps containing arch specific struct, although the single struct may actually be arch specific anyway. I''m not actually sure how useful this will be on ARM where we don''t have the problem of passing stuff from real mode to protected mode (because you can only call the BIOS/bootloader routines from real mode). On ARM we already have struct dt_early_info which despite the name isn''t at all specific to DT and is more than sufficient for our current needs. We will obviously just extend that as need be. I''d be happy to move that into an ARM specific place and then x86 could do its own thing too. Ian.
Reasonably Related Threads
- BIOS disk geometry and Linux 2.6
- grub commands problem with Ubuntu 10.04
- [PATCH] libxenctrl: Fix xc_interface_close() crash if it gets NULL as an argument
- [PATCH v3 00/11] xen: Initial kexec/kdump implementation
- [PATCH v3 00/11] xen: Initial kexec/kdump implementation