Re: [Xen-devel] [PATCH v2 02/11] xen/hvmlite: Bootstrap HVMlite guest

[David Vrabel]

On 24/04/16 21:23, Borislav Petkov wrote:
> On Mon, Feb 01, 2016 at 10:38:48AM -0500, Boris Ostrovsky wrote:
>> Start HVMlite guest at XEN_ELFNOTE_PHYS32_ENTRY address. Setup hypercall
>> page, initialize boot_params, enable early page tables.
>>
>> Since this stub is executed before kernel entry point we cannot use
>> variables in .bss which is cleared by kernel. We explicitly place
>> variables that are initialized here into .data.
>>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>> ---
>>  arch/x86/xen/Makefile      |    1 +
>>  arch/x86/xen/enlighten.c   |   86 +++++++++++++++++++++-
>>  arch/x86/xen/xen-hvmlite.S |  175 ++++++++++++++++++++++++++++++++++++++++++++
>>  include/xen/xen.h          |    6 ++
>>  4 files changed, 267 insertions(+), 1 deletions(-)
>>  create mode 100644 arch/x86/xen/xen-hvmlite.S
>>
>> diff --git a/arch/x86/xen/Makefile b/arch/x86/xen/Makefile
>> index e47e527..1d913d7 100644
>> --- a/arch/x86/xen/Makefile
>> +++ b/arch/x86/xen/Makefile
>> @@ -23,3 +23,4 @@ obj-$(CONFIG_XEN_DEBUG_FS)	+= debugfs.o
>>  obj-$(CONFIG_XEN_DOM0)		+= vga.o
>>  obj-$(CONFIG_SWIOTLB_XEN)	+= pci-swiotlb-xen.o
>>  obj-$(CONFIG_XEN_EFI)		+= efi.o
>> +obj-$(CONFIG_XEN_PVHVM) 	+= xen-hvmlite.o
>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
>> index 5774800..5f05fa2 100644
>> --- a/arch/x86/xen/enlighten.c
>> +++ b/arch/x86/xen/enlighten.c
>> @@ -118,7 +118,8 @@ DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
>>   */
>>  DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
>>  
>> -enum xen_domain_type xen_domain_type = XEN_NATIVE;
>> +enum xen_domain_type xen_domain_type
>> +	__attribute__((section(".data"))) = XEN_NATIVE;
>>  EXPORT_SYMBOL_GPL(xen_domain_type);
>>  
>>  unsigned long *machine_to_phys_mapping = (void *)MACH2PHYS_VIRT_START;
>> @@ -171,6 +172,17 @@ struct tls_descs {
>>   */
>>  static DEFINE_PER_CPU(struct tls_descs, shadow_tls_desc);
>>  
>> +#ifdef CONFIG_XEN_PVHVM
>> +/*
>> + * HVMlite variables. These need to live in data segment since they are
>> + * initialized before startup_{32|64}, which clear .bss, are invoked.
> 
> So this jumps into startup_32/64 and I don't think we have talked about
> it yet, have we? I'm not aware of any threads about it. Are we fine with
> it, are we not?
> 
> I think we need to agree on API where xen guests should jump into
> arch/x86/ and adhere to it. Otherwise, we will break xen again if we change
> stuff in x86 and we do like to change stuff in x86 all the time.
> 
> Adding tip guys and leaving in the rest for reference.

It would be good if we could start in the decompresser, but we would
need to be able to:

a) identify that the image is PVH-capable.
b) call a PVH specific entry point that builds the expected struct
boot_params.

I don't see any scope in the existing boot protocol to allow this. Hence
we get Xen to decompress the image and look at ELF notes etc.

We want PVH to be a drop-in replacement for PV as much as possible so
this excludes using a bootloader or post-processing the bzImage into a
PVH-compatible ELF image.

I'm open to other suggestions but what's proposed here seems the least
intrusive and with minimal risk for future breakage.  I don't think the
decompressor to kernel ABI changes often does it?

David