Re: [Xen-devel] [PATCH v12 02/10] x86: add multiboot2 protocol support
On 1/19/17 8:34 PM, Daniel Kiper wrote: > Add multiboot2 protocol support. Alter min memory limit handling as we > now may not find it from either multiboot (v1) or multiboot2. > > This way we are laying the foundation for EFI + GRUB2 + Xen development. > > Signed-off-by: Daniel Kiper > Reviewed-by: Jan Beulich > Reviewed-by: Doug Goldstein > --- > v12 - suggestions/fixes: > - replace TABs with spaces in xen/include/xen/multiboot2.h > (suggested by Doug Goldstein). > > v9 - suggestions/fixes: >- use .L label instead of numeric one in multiboot2 data scanning loop; > I hope that this change does not invalidate Jan's Reviewed-by > (suggested by Jan Beulich). > > v8 - suggestions/fixes: >- use sizeof(/) instead of sizeof() > if it is possible > (suggested by Jan Beulich). > > v7 - suggestions/fixes: >- rename mbi_mbi/mbi2_mbi to mbi_reloc/mbi2_reloc respectively > (suggested by Jan Beulich), >- initialize mbi_out->flags using "|=" instead of "=" > (suggested by Jan Beulich), >- use sizeof(*mmap_dst) instead of sizeof(memory_map_t) > if it makes sense > (suggested by Jan Beulich). > > v6 - suggestions/fixes: >- properly index multiboot2_tag_mmap_t.entries[] > (suggested by Jan Beulich), >- do not index mbi_out_mods[] beyond its end > (suggested by Andrew Cooper), >- reduce number of casts > (suggested by Andrew Cooper and Jan Beulich), >- add braces to increase code readability > (suggested by Andrew Cooper). > > v5 - suggestions/fixes: >- check multiboot2_tag_mmap_t.entry_size before > multiboot2_tag_mmap_t.entries[] use > (suggested by Jan Beulich), >- properly index multiboot2_tag_mmap_t.entries[] > (suggested by Jan Beulich), >- use "type name[]" instad of "type name[0]" > in xen/include/xen/multiboot2.h > (suggested by Jan Beulich), >- remove unneeded comment > (suggested by Jan Beulich). > > v4 - suggestions/fixes: >- avoid assembly usage in xen/arch/x86/boot/reloc.c, >- fix boundary check issue and optimize > for() loops in mbi2_mbi(), >- move to stdcall calling convention, >- remove unneeded typeof() from ALIGN_UP() macro > (suggested by Jan Beulich), >- add and use NULL definition in xen/arch/x86/boot/reloc.c > (suggested by Jan Beulich), >- do not read data beyond the end of multiboot2 > information in xen/arch/x86/boot/head.S > (suggested by Jan Beulich), >- add :req to some .macro arguments > (suggested by Jan Beulich), >- use cmovcc if possible, >- add .L to multiboot2_header_end label > (suggested by Jan Beulich), >- add .L to multiboot2_proto label > (suggested by Jan Beulich), >- improve label names > (suggested by Jan Beulich). > > v3 - suggestions/fixes: >- reorder reloc() arguments > (suggested by Jan Beulich), >- remove .L from multiboot2 header labels > (suggested by Andrew Cooper, Jan Beulich and Konrad Rzeszutek Wilk), >- take into account alignment when skipping multiboot2 fixed part > (suggested by Konrad Rzeszutek Wilk), >- create modules data if modules count != 0 > (suggested by Jan Beulich), >- improve macros > (suggested by Jan Beulich), >- reduce number of casts > (suggested by Jan Beulich), >- use const if possible > (suggested by Jan Beulich), >- drop static and __used__ attribute from reloc() > (suggested by Jan Beulich), >- remove isolated/stray __packed attribute from > multiboot2_memory_map_t type definition > (suggested by Jan Beulich), >- reformat xen/include/xen/multiboot2.h > (suggested by Konrad Rzeszutek Wilk), >- improve comments > (suggested by Konrad Rzeszutek Wilk), >- remove hard tabs > (suggested by Jan Beulich and Konrad Rzeszutek Wilk). > > v2 - suggestions/fixes: >- generate multiboot2 header using macros > (suggested by Jan Beulich), >- improve comments > (suggested by Jan Beulich), >- simplify assembly in xen/arch/x86/boot/head.S > (suggested by Jan Beulich), >- do not include include/xen/compiler.h > in xen/arch/x86/boot/reloc.c > (suggested by Jan Beulich), >- do not read data beyond the end of multiboot2 information > (suggested by Jan Beulich). > > v2 - not fixed yet: >- dynamic dependency generation for xen/arch/x86/boot/reloc.S; > this requires more work; I am not sure that it pays because > potential patch requires more changes than addition of just > multiboot2.h to Makefile > (suggested by Jan Beulich), >- isolated/stray __packed attribute usage for multiboot2_memory_map_t > (suggested by Jan Beulich). > --- > xen/arch/x86/boot/Makefile|3 +- > xen/arch/x86/boot/head.S | 107 ++- > xen/arch/x86/boot/reloc.c | 144 +-- > xen/arc
Re: [Xen-devel] [PATCH v12 02/10] x86: add multiboot2 protocol support
On 20/01/17 17:24, Daniel Kiper wrote: > On Fri, Jan 20, 2017 at 04:52:30PM +, Andrew Cooper wrote: >> >> here please? > Will do. > >> With this, Reviewed-by: Andrew Cooper > Thanks! > >> With this patch present, should legacy booting with MB2 work properly? >> If so, I have a good mind to take the patch now so it can get better >> testing (at which point I can fix up this one issue). I have confirmed that MB2 seems to work for me, with a few extra diagnostics in reloc.c (XEN) Bootloader: GRUB 2.02~beta2, MB protocol 2 > Yep, however, you have to remember that there is no support for > relocatable images here. It is added by subsequent patches. Using MB2 as the boot protocol is orthogonal to supporting relocation of the images. The former a useful piece of functionality to have even in the absence of the latter. ~Andrew ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v12 02/10] x86: add multiboot2 protocol support
On Fri, Jan 20, 2017 at 04:52:30PM +, Andrew Cooper wrote:
> On 20/01/17 01:34, Daniel Kiper wrote:
> > @@ -101,3 +112,126 @@ multiboot_info_t __stdcall *reloc(u32 mbi_in, u32
> > trampoline)
> >
> > return mbi_out;
> > }
> > +
> > +static multiboot_info_t *mbi2_reloc(u32 mbi_in)
> > +{
> > +const multiboot2_fixed_t *mbi_fix = _p(mbi_in);
> > +const multiboot2_memory_map_t *mmap_src;
> > +const multiboot2_tag_t *tag;
> > +module_t *mbi_out_mods = NULL;
> > +memory_map_t *mmap_dst;
> > +multiboot_info_t *mbi_out;
> > +u32 ptr;
> > +unsigned int i, mod_idx = 0;
> > +
> > +ptr = alloc_mem(sizeof(*mbi_out));
> > +mbi_out = _p(ptr);
> > +zero_mem(ptr, sizeof(*mbi_out));
> > +
> > +/* Skip Multiboot2 information fixed part. */
> > +ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
> > +
> > +/* Get the number of modules. */
> > +for ( tag = _p(ptr); (u32)tag - mbi_in < mbi_fix->total_size;
> > + tag = _p(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
> > +{
> > +if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
> > +++mbi_out->mods_count;
> > +else if ( tag->type == MULTIBOOT2_TAG_TYPE_END )
> > +break;
> > +}
> > +
> > +if ( mbi_out->mods_count )
> > +{
> > +mbi_out->flags |= MBI_MODULES;
> > +mbi_out->mods_addr = alloc_mem(mbi_out->mods_count *
> > sizeof(*mbi_out_mods));
> > +mbi_out_mods = _p(mbi_out->mods_addr);
> > +}
> > +
> > +/* Skip Multiboot2 information fixed part. */
> > +ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
> > +
> > +/* Put all needed data into mbi_out. */
> > +for ( tag = _p(ptr); (u32)tag - mbi_in < mbi_fix->total_size;
> > + tag = _p(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
> > +switch ( tag->type )
> > +{
> > +case MULTIBOOT2_TAG_TYPE_BOOT_LOADER_NAME:
> > +mbi_out->flags |= MBI_LOADERNAME;
> > +ptr = get_mb2_string(tag, string, string);
> > +mbi_out->boot_loader_name = copy_string(ptr);
> > +break;
> > +
> > +case MULTIBOOT2_TAG_TYPE_CMDLINE:
> > +mbi_out->flags |= MBI_CMDLINE;
> > +ptr = get_mb2_string(tag, string, string);
> > +mbi_out->cmdline = copy_string(ptr);
> > +break;
> > +
> > +case MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO:
> > +mbi_out->flags |= MBI_MEMLIMITS;
> > +mbi_out->mem_lower = get_mb2_data(tag, basic_meminfo,
> > mem_lower);
> > +mbi_out->mem_upper = get_mb2_data(tag, basic_meminfo,
> > mem_upper);
> > +break;
> > +
> > +case MULTIBOOT2_TAG_TYPE_MMAP:
> > +if ( get_mb2_data(tag, mmap, entry_size) < sizeof(*mmap_src) )
> > +break;
> > +
> > +mbi_out->flags |= MBI_MEMMAP;
> > +mbi_out->mmap_length = get_mb2_data(tag, mmap, size);
> > +mbi_out->mmap_length -= sizeof(multiboot2_tag_mmap_t);
> > +mbi_out->mmap_length /= get_mb2_data(tag, mmap, entry_size);
> > +mbi_out->mmap_length *= sizeof(*mmap_dst);
> > +
> > +mbi_out->mmap_addr = alloc_mem(mbi_out->mmap_length);
> > +
> > +mmap_src = get_mb2_data(tag, mmap, entries);
> > +mmap_dst = _p(mbi_out->mmap_addr);
> > +
> > +for ( i = 0; i < mbi_out->mmap_length / sizeof(*mmap_dst); i++
> > )
> > +{
> > +/* Init size member properly. */
> > +mmap_dst[i].size = sizeof(*mmap_dst);
> > +mmap_dst[i].size -= sizeof(mmap_dst[i].size);
> > +/* Now copy a given region data. */
> > +mmap_dst[i].base_addr_low = (u32)mmap_src->addr;
> > +mmap_dst[i].base_addr_high = (u32)(mmap_src->addr >> 32);
> > +mmap_dst[i].length_low = (u32)mmap_src->len;
> > +mmap_dst[i].length_high = (u32)(mmap_src->len >> 32);
> > +mmap_dst[i].type = mmap_src->type;
> > +mmap_src = _p(mmap_src) + get_mb2_data(tag, mmap,
> > entry_size);
> > +}
> > +break;
> > +
> > +case MULTIBOOT2_TAG_TYPE_MODULE:
> > +if ( mod_idx >= mbi_out->mods_count )
> > +break;
> > +
> > +mbi_out_mods[mod_idx].mod_start = get_mb2_data(tag, module,
> > mod_start);
> > +mbi_out_mods[mod_idx].mod_end = get_mb2_data(tag, module,
> > mod_end);
> > +ptr = get_mb2_string(tag, module, cmdline);
> > +mbi_out_mods[mod_idx].string = copy_string(ptr);
> > +mbi_out_mods[mod_idx].reserved = 0;
> > +++mod_idx;
> > +break;
> > +
> > +case MULTIBOOT2_TAG_TYPE_END:
> > +return mbi_out;
> > +
> > +default:
> > +break;
> > +}
> > +
> > +return mbi_out;
> > +}
> > +
> > +multiboot_
Re: [Xen-devel] [PATCH v12 02/10] x86: add multiboot2 protocol support
On 20/01/17 01:34, Daniel Kiper wrote:
> @@ -101,3 +112,126 @@ multiboot_info_t __stdcall *reloc(u32 mbi_in, u32
> trampoline)
>
> return mbi_out;
> }
> +
> +static multiboot_info_t *mbi2_reloc(u32 mbi_in)
> +{
> +const multiboot2_fixed_t *mbi_fix = _p(mbi_in);
> +const multiboot2_memory_map_t *mmap_src;
> +const multiboot2_tag_t *tag;
> +module_t *mbi_out_mods = NULL;
> +memory_map_t *mmap_dst;
> +multiboot_info_t *mbi_out;
> +u32 ptr;
> +unsigned int i, mod_idx = 0;
> +
> +ptr = alloc_mem(sizeof(*mbi_out));
> +mbi_out = _p(ptr);
> +zero_mem(ptr, sizeof(*mbi_out));
> +
> +/* Skip Multiboot2 information fixed part. */
> +ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
> +
> +/* Get the number of modules. */
> +for ( tag = _p(ptr); (u32)tag - mbi_in < mbi_fix->total_size;
> + tag = _p(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
> +{
> +if ( tag->type == MULTIBOOT2_TAG_TYPE_MODULE )
> +++mbi_out->mods_count;
> +else if ( tag->type == MULTIBOOT2_TAG_TYPE_END )
> +break;
> +}
> +
> +if ( mbi_out->mods_count )
> +{
> +mbi_out->flags |= MBI_MODULES;
> +mbi_out->mods_addr = alloc_mem(mbi_out->mods_count *
> sizeof(*mbi_out_mods));
> +mbi_out_mods = _p(mbi_out->mods_addr);
> +}
> +
> +/* Skip Multiboot2 information fixed part. */
> +ptr = ALIGN_UP(mbi_in + sizeof(*mbi_fix), MULTIBOOT2_TAG_ALIGN);
> +
> +/* Put all needed data into mbi_out. */
> +for ( tag = _p(ptr); (u32)tag - mbi_in < mbi_fix->total_size;
> + tag = _p(ALIGN_UP((u32)tag + tag->size, MULTIBOOT2_TAG_ALIGN)) )
> +switch ( tag->type )
> +{
> +case MULTIBOOT2_TAG_TYPE_BOOT_LOADER_NAME:
> +mbi_out->flags |= MBI_LOADERNAME;
> +ptr = get_mb2_string(tag, string, string);
> +mbi_out->boot_loader_name = copy_string(ptr);
> +break;
> +
> +case MULTIBOOT2_TAG_TYPE_CMDLINE:
> +mbi_out->flags |= MBI_CMDLINE;
> +ptr = get_mb2_string(tag, string, string);
> +mbi_out->cmdline = copy_string(ptr);
> +break;
> +
> +case MULTIBOOT2_TAG_TYPE_BASIC_MEMINFO:
> +mbi_out->flags |= MBI_MEMLIMITS;
> +mbi_out->mem_lower = get_mb2_data(tag, basic_meminfo, mem_lower);
> +mbi_out->mem_upper = get_mb2_data(tag, basic_meminfo, mem_upper);
> +break;
> +
> +case MULTIBOOT2_TAG_TYPE_MMAP:
> +if ( get_mb2_data(tag, mmap, entry_size) < sizeof(*mmap_src) )
> +break;
> +
> +mbi_out->flags |= MBI_MEMMAP;
> +mbi_out->mmap_length = get_mb2_data(tag, mmap, size);
> +mbi_out->mmap_length -= sizeof(multiboot2_tag_mmap_t);
> +mbi_out->mmap_length /= get_mb2_data(tag, mmap, entry_size);
> +mbi_out->mmap_length *= sizeof(*mmap_dst);
> +
> +mbi_out->mmap_addr = alloc_mem(mbi_out->mmap_length);
> +
> +mmap_src = get_mb2_data(tag, mmap, entries);
> +mmap_dst = _p(mbi_out->mmap_addr);
> +
> +for ( i = 0; i < mbi_out->mmap_length / sizeof(*mmap_dst); i++ )
> +{
> +/* Init size member properly. */
> +mmap_dst[i].size = sizeof(*mmap_dst);
> +mmap_dst[i].size -= sizeof(mmap_dst[i].size);
> +/* Now copy a given region data. */
> +mmap_dst[i].base_addr_low = (u32)mmap_src->addr;
> +mmap_dst[i].base_addr_high = (u32)(mmap_src->addr >> 32);
> +mmap_dst[i].length_low = (u32)mmap_src->len;
> +mmap_dst[i].length_high = (u32)(mmap_src->len >> 32);
> +mmap_dst[i].type = mmap_src->type;
> +mmap_src = _p(mmap_src) + get_mb2_data(tag, mmap,
> entry_size);
> +}
> +break;
> +
> +case MULTIBOOT2_TAG_TYPE_MODULE:
> +if ( mod_idx >= mbi_out->mods_count )
> +break;
> +
> +mbi_out_mods[mod_idx].mod_start = get_mb2_data(tag, module,
> mod_start);
> +mbi_out_mods[mod_idx].mod_end = get_mb2_data(tag, module,
> mod_end);
> +ptr = get_mb2_string(tag, module, cmdline);
> +mbi_out_mods[mod_idx].string = copy_string(ptr);
> +mbi_out_mods[mod_idx].reserved = 0;
> +++mod_idx;
> +break;
> +
> +case MULTIBOOT2_TAG_TYPE_END:
> +return mbi_out;
> +
> +default:
> +break;
> +}
> +
> +return mbi_out;
> +}
> +
> +multiboot_info_t __stdcall *reloc(u32 mb_magic, u32 mbi_in, u32 trampoline)
> +{
> +alloc = trampoline;
> +
> +if ( mb_magic == MULTIBOOT2_BOOTLOADER_MAGIC )
> +return mbi2_reloc(mbi_in);
> +else
> +return mbi_reloc(mbi_in);
> +}
Can we get a
/*
[Xen-devel] [PATCH v12 02/10] x86: add multiboot2 protocol support
Add multiboot2 protocol support. Alter min memory limit handling as we now may not find it from either multiboot (v1) or multiboot2. This way we are laying the foundation for EFI + GRUB2 + Xen development. Signed-off-by: Daniel Kiper Reviewed-by: Jan Beulich Reviewed-by: Doug Goldstein --- v12 - suggestions/fixes: - replace TABs with spaces in xen/include/xen/multiboot2.h (suggested by Doug Goldstein). v9 - suggestions/fixes: - use .L label instead of numeric one in multiboot2 data scanning loop; I hope that this change does not invalidate Jan's Reviewed-by (suggested by Jan Beulich). v8 - suggestions/fixes: - use sizeof(/) instead of sizeof() if it is possible (suggested by Jan Beulich). v7 - suggestions/fixes: - rename mbi_mbi/mbi2_mbi to mbi_reloc/mbi2_reloc respectively (suggested by Jan Beulich), - initialize mbi_out->flags using "|=" instead of "=" (suggested by Jan Beulich), - use sizeof(*mmap_dst) instead of sizeof(memory_map_t) if it makes sense (suggested by Jan Beulich). v6 - suggestions/fixes: - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - do not index mbi_out_mods[] beyond its end (suggested by Andrew Cooper), - reduce number of casts (suggested by Andrew Cooper and Jan Beulich), - add braces to increase code readability (suggested by Andrew Cooper). v5 - suggestions/fixes: - check multiboot2_tag_mmap_t.entry_size before multiboot2_tag_mmap_t.entries[] use (suggested by Jan Beulich), - properly index multiboot2_tag_mmap_t.entries[] (suggested by Jan Beulich), - use "type name[]" instad of "type name[0]" in xen/include/xen/multiboot2.h (suggested by Jan Beulich), - remove unneeded comment (suggested by Jan Beulich). v4 - suggestions/fixes: - avoid assembly usage in xen/arch/x86/boot/reloc.c, - fix boundary check issue and optimize for() loops in mbi2_mbi(), - move to stdcall calling convention, - remove unneeded typeof() from ALIGN_UP() macro (suggested by Jan Beulich), - add and use NULL definition in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - add :req to some .macro arguments (suggested by Jan Beulich), - use cmovcc if possible, - add .L to multiboot2_header_end label (suggested by Jan Beulich), - add .L to multiboot2_proto label (suggested by Jan Beulich), - improve label names (suggested by Jan Beulich). v3 - suggestions/fixes: - reorder reloc() arguments (suggested by Jan Beulich), - remove .L from multiboot2 header labels (suggested by Andrew Cooper, Jan Beulich and Konrad Rzeszutek Wilk), - take into account alignment when skipping multiboot2 fixed part (suggested by Konrad Rzeszutek Wilk), - create modules data if modules count != 0 (suggested by Jan Beulich), - improve macros (suggested by Jan Beulich), - reduce number of casts (suggested by Jan Beulich), - use const if possible (suggested by Jan Beulich), - drop static and __used__ attribute from reloc() (suggested by Jan Beulich), - remove isolated/stray __packed attribute from multiboot2_memory_map_t type definition (suggested by Jan Beulich), - reformat xen/include/xen/multiboot2.h (suggested by Konrad Rzeszutek Wilk), - improve comments (suggested by Konrad Rzeszutek Wilk), - remove hard tabs (suggested by Jan Beulich and Konrad Rzeszutek Wilk). v2 - suggestions/fixes: - generate multiboot2 header using macros (suggested by Jan Beulich), - improve comments (suggested by Jan Beulich), - simplify assembly in xen/arch/x86/boot/head.S (suggested by Jan Beulich), - do not include include/xen/compiler.h in xen/arch/x86/boot/reloc.c (suggested by Jan Beulich), - do not read data beyond the end of multiboot2 information (suggested by Jan Beulich). v2 - not fixed yet: - dynamic dependency generation for xen/arch/x86/boot/reloc.S; this requires more work; I am not sure that it pays because potential patch requires more changes than addition of just multiboot2.h to Makefile (suggested by Jan Beulich), - isolated/stray __packed attribute usage for multiboot2_memory_map_t (suggested by Jan Beulich). --- xen/arch/x86/boot/Makefile|3 +- xen/arch/x86/boot/head.S | 107 ++- xen/arch/x86/boot/reloc.c | 144 +-- xen/arch/x86/x86_64/asm-offsets.c |9 ++ xen/include/xen/multiboot2.h | 169 + 5 files changed, 422 insertions(+), 10 deletions(-) create mode 100644 xen/include/xen/multiboot2.h diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile index 6d2
