Re: [Xen-devel] [PATCH 5/9] x86: relocate pvh_info

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.01.18 at 11:53, <roger.pau@xxxxxxxxxx> wrote:
> On Thu, Jan 18, 2018 at 06:16:48PM +0000, Wei Liu wrote:
>> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
>> index 0f652cea11..2f94c286d5 100644
>> --- a/xen/arch/x86/boot/head.S
>> +++ b/xen/arch/x86/boot/head.S
>> @@ -414,6 +414,7 @@ __pvh_start:
>>  
>>          /* Set trampoline_phys to use mfn 1 to avoid having a mapping at VA 
> 0 */
>>          movw    $0x1000, sym_esi(trampoline_phys)
>> +        xor     %eax, %eax /* Needed by reloc.c */
> 
> I would rather do:
> 
> movl $XEN_HVM_START_MAGIC_VALUE,%eax
> 
> Or:
> 
> mov (%ebx),%eax
> 
> So that...
> 
>> -multiboot_info_t __stdcall *reloc(u32 mb_magic, u32 mbi_in, u32 trampoline)
>> +void __stdcall *reloc(u32 magic, u32 in, u32 trampoline)
>>  {
>>      alloc = trampoline;
>>  
>> -    if ( mb_magic == MULTIBOOT2_BOOTLOADER_MAGIC )
>> -        return mbi2_reloc(mbi_in);
>> +#ifdef CONFIG_PVH_GUEST
>> +    if ( magic == 0 )
> 
> ... here you can do magic == XEN_HVM_START_MAGIC_VALUE.
> 
>> +        return pvh_info_reloc(in);
>> +    else
>> +#endif
>> +    if ( magic == MULTIBOOT2_BOOTLOADER_MAGIC )
>> +        return mbi2_reloc(in);
>>      else
>> -        return mbi_reloc(mbi_in);
>> +        return mbi_reloc(in);
> 
> A 'switch ( magic )' would be better here IMHO.

Ah, indeed, that's better.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel