Re: [PATCH for-xen-4.5 v3 04/16] x86: Introduce MultiBoot Data (MBD) type

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 10/10/14 12:47, Daniel Kiper wrote:
>
>>> +       : "+D" (s), "+c" (bytes) : "a" (0));
>>> +}
>>> +
>>>  static u32 copy_struct(u32 src, u32 bytes)
>>>  {
>>>      u32 dst, dst_asm;
>>> @@ -77,41 +87,56 @@ static u32 copy_string(u32 src)
>>>      return copy_struct(src, p - (char *)src + 1);
>>>  }
>>>
>>> -multiboot_info_t *reloc(multiboot_info_t *mbi_old)
>>> +static mbd_t *mb_mbd(mbd_t *mbd, multiboot_info_t *mbi)
>>>  {
>>> -    multiboot_info_t *mbi = (multiboot_info_t *)copy_struct((u32)mbi_old, sizeof(*mbi));
>>> -    int i;
>>> +    boot_module_t *mbd_mods;
>>> +    module_t *mbi_mods;
>>> +    u32 i;
>>> +
>>> +    if ( mbi->flags & MBI_LOADERNAME )
>>> +        mbd->boot_loader_name = copy_string(mbi->boot_loader_name);
>>>
>>>      if ( mbi->flags & MBI_CMDLINE )
>>> -        mbi->cmdline = copy_string(mbi->cmdline);
>>> +        mbd->cmdline = copy_string(mbi->cmdline);
>>> +
>>> +    if ( mbi->flags & MBI_MEMLIMITS )
>>> +    {
>>> +        mbd->mem_lower = mbi->mem_lower;
>>> +        mbd->mem_upper = mbi->mem_upper;
>>> +    }
>> Now I am completely confused.  here you set mbd from mbi, but lower in
>> init_mbi, you set mbi from mbd.  What is the intended dataflow?
> We need mbi together with boot_info to make smooth transition from mbi
> to boot_info possible. Patch #10 completely removes mbi from Xen main code.
> I mentioned about that in commit message.

That is perhaps all well and fine, but my initial point still stands.

In this patch alone, you set mbd here from mbi, but later in
__init_mbi(), initialise mbi from mbd.

I can't spot anywhere else which initialises either of the structures
with values, so I can only assume they are both 0's (or even
uninitialised) all the way through the boot process.

~Andrew