Re: [Xen-devel] [PATCH v12 02/10] x86: add multiboot2 protocol support

[Daniel Kiper <daniel.kiper@xxxxxxxxxx>]

On Fri, Jan 20, 2017 at 04:52:30PM +0000, 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_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
>
> /*
>
>  * Local
> variables:
>
>  * mode:
> C
>
>  * c-file-style:
> "BSD"
>
>  * c-basic-offset:
> 4
>
>  * indent-tabs-mode:
> nil
>
>  *
> End:
>
>  */
>
> here please?

Will do.

> With this, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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).

Yep, however, you have to remember that there is no support for
relocatable images here. It is added by subsequent patches.

Daniel

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