Re: [Xen-devel] [PATCH v4] x86: relocate pvh_info

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 22/01/18 18:17, Wei Liu wrote:
> On Mon, Jan 22, 2018 at 06:09:14PM +0000, Andrew Cooper wrote:
>> On 22/01/18 16:13, Wei Liu wrote:
>>> Modify early boot code to relocate pvh info as well, so that we can be
>>> sure __va in __start_xen works.
>>>
>>> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>
>>> ---
>>> Cc: Jan Beulich <jbeulich@xxxxxxxx>
>>> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> Cc: Doug Goldstein <cardoe@xxxxxxxxxx>
>>>
>>> v4: inlcude autoconf.h directly. The code itself is unchanged.
>>> ---
>>>  xen/arch/x86/boot/Makefile |  4 +++
>>>  xen/arch/x86/boot/defs.h   |  3 +++
>>>  xen/arch/x86/boot/head.S   | 25 ++++++++++--------
>>>  xen/arch/x86/boot/reloc.c  | 64 
>>> +++++++++++++++++++++++++++++++++++++++++-----
>>>  4 files changed, 78 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/boot/Makefile b/xen/arch/x86/boot/Makefile
>>> index c6246c85d2..1b3f121a2f 100644
>>> --- a/xen/arch/x86/boot/Makefile
>>> +++ b/xen/arch/x86/boot/Makefile
>>> @@ -7,6 +7,10 @@ CMDLINE_DEPS = $(DEFS_H_DEPS) video.h
>>>  RELOC_DEPS = $(DEFS_H_DEPS) $(BASEDIR)/include/xen/multiboot.h \
>>>          $(BASEDIR)/include/xen/multiboot2.h
>> + autoconf.h
>>
>> However, it would much better to take xen/kconfig.h ...
>>
> This is fine by me.
>
>>>  
>>> +ifeq ($(CONFIG_PVH_GUEST),y)
>>> +RELOC_DEPS += $(BASEDIR)/include/public/arch-x86/hvm/start_info.h
>>> +endif
> [...]
>>> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
>>> index b992678b5e..1fe19294ad 100644
>>> --- a/xen/arch/x86/boot/reloc.c
>>> +++ b/xen/arch/x86/boot/reloc.c
>>> @@ -14,8 +14,8 @@
>>>  
>>>  /*
>>>   * This entry point is entered from xen/arch/x86/boot/head.S with:
>>> - *   - 0x4(%esp) = MULTIBOOT_MAGIC,
>>> - *   - 0x8(%esp) = MULTIBOOT_INFORMATION_ADDRESS,
>>> + *   - 0x4(%esp) = MAGIC,
>>> + *   - 0x8(%esp) = INFORMATION_ADDRESS,
>>>   *   - 0xc(%esp) = TOPMOST_LOW_MEMORY_STACK_ADDRESS.
>>>   */
>>>  asm (
>>> @@ -29,6 +29,8 @@ asm (
>>>  #include "../../../include/xen/multiboot.h"
>>>  #include "../../../include/xen/multiboot2.h"
>>>  
>>> +#include "../../../include/generated/autoconf.h"
>>> +
>>>  #define get_mb2_data(tag, type, member)   (((multiboot2_tag_##type##_t 
>>> *)(tag))->member)
>>>  #define get_mb2_string(tag, type, member) ((u32)get_mb2_data(tag, type, 
>>> member))
>>>  
>>> @@ -71,6 +73,41 @@ static u32 copy_string(u32 src)
>>>      return copy_mem(src, p - src + 1);
>>>  }
>>>  
>>> +#ifdef CONFIG_PVH_GUEST
>> ... drop this ifdef and ...
>>
> So you want reloc.o to contain pvh_info_reloc unconditionally?
>
> Fundamentally I don't think I care enough about all the bikeshedding so
> if Jan and you agree on this I will just make the change.

It wont.  The function will be dropped due to DCE, but we'll spot build
breakages far more easily.  (The important bit is that the function call
is guarded by the IS_ENABLED())

~Andrew

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