[Draft C] Boot ABI for HVM guests without a device-model

[Draft C] Boot ABI for HVM guests without a device-model

[Roger Pau Monné]

Hello,

The discussion in [1] lead to an agreement of the missing pieces in PVH
(or HVM without a device-model) in order to progress with it's
implementation.

One of the missing pieces is a new boot ABI, that replaces the PV boot
ABI. The aim of this new boot ABI is to remove the limitations of the
PV boot ABI, that are no longer present when using auto-translated
guests. The new boot protocol should allow to use the same entry point
for both 32bit and 64bit guests, and let the guest choose it's bitness
at run time without the domain builder knowing in advance.

Roger.

[1] http://lists.xen.org/archives/html/xen-devel/2015-06/msg00258.html

---
HVM direct boot ABI
===================

Since the Xen entry point into the kernel can be different from the
native entry point, ELFNOTES are used in order to tell the domain
builder how to load and jump into the kernel entry point. At least the
following ELFNOTES are required in order to use this boot ABI (the
example below shows the values used by FreeBSD):

ELFNOTE(Xen, XEN_ELFNOTE_GUEST_OS,              .asciz, "FreeBSD")
ELFNOTE(Xen, XEN_ELFNOTE_GUEST_VERSION,         .asciz, __XSTRING(__FreeBSD_version))
ELFNOTE(Xen, XEN_ELFNOTE_XEN_VERSION,           .asciz, "xen-3.0")
ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,          .long,  xen_start32)
ELFNOTE(Xen, XEN_ELFNOTE_FEATURES,              .asciz, "writable_descriptor_tables|auto_translated_physmap")
ELFNOTE(Xen, XEN_ELFNOTE_SUPPORTED_FEATURES,    .long ((1 << XENFEAT_writable_page_tables) | \
                                                       (1 << XENFEAT_auto_translated_physmap) | \
                                                       (1 << XENFEAT_hvm_callback_vector))
ELFNOTE(Xen, XEN_ELFNOTE_LOADER,                .asciz, "generic")

The first three notes contain information about the guest kernel and
the Xen hypercall ABI version. The following notes are of special
interest:

 * XEN_ELFNOTE_PHYS32_ENTRY: the 32bit physical entry point into the kernel.
 * XEN_ELFNOTE_FEATURES: features required by the guest kernel in order
   to run.

The presence of the XEN_ELFNOTE_PHYS32_ENTRY note indicates that the
kernel supports the boot ABI described in this document.

The domain builder will load the kernel into the guest memory space and
jump into the entry point defined at XEN_ELFNOTE_PHYS32_ENTRY with the
following machine state:

 * ebx: contains the physical memory address where the loader has placed
   the boot start info structure.

 * cr0: bit 0 (PE) will be set. All the other writeable bits are cleared.

 * cr4: all bits are cleared.

 * cs: must be a 32-bit read/execute code segment with an offset of ‘0’
   and a limit of ‘0xFFFFFFFF’. The selector value is unspecified.

 * ds, es: must be a 32-bit read/write data segment with an offset of
   ‘0’ and a limit of ‘0xFFFFFFFF’. The selector values are all unspecified.

 * tr: must be a 32-bit TSS (active) with a base of '0' and a limit of '0xFF'.

 * eflags: bit 17 (VM) must be cleared. Bit 9 (IF) must be cleared.
   Other bits are all unspecified.

All other processor registers and flag bits are unspecified. The OS is in
charge of setting up it's own stack, GDT and IDT.

The format of the structure passed in the %ebx register is the following:

struct hvm_start_info {
#define HVM_START_MAGIC_VALUE 0x336ec578
    uint32_t magic;             /* Contains the magic value 0x336ec578       */
                                /* ("xEn3" with the 0x80 bit of the "E" set).*/
    uint32_t flags;             /* SIF_xxx flags.                            */
    uint32_t cmdline_paddr;     /* Physical address of the command line.     */
    uint32_t nr_modules;        /* Number of modules passed to the kernel.   */
    uint32_t modlist_paddr;     /* Physical address of an array of           */
                                /* hvm_modlist_entry.                        */
};

struct hvm_modlist_entry {
    uint32_t paddr;             /* Physical address of the module.           */
    uint32_t size;              /* Size of the module in bytes.              */
};

Note that the boot protocol resembles the multiboot1 specification,
this is done so OSes with multiboot1 entry points can reuse those if
desired.

Other relevant information needed in order to boot a guest kernel
(console page address, xenstore event channel...) can be obtained
using HVMPARAMS, just like it's done on HVM guests.

The setup of the hypercall page is also performed in the same way
as HVM guests, using the hypervisor cpuid leaves and msr ranges.

AP startup
==========

AP startup is performed using hypercalls. The following VCPU operations
are used in order to bring up secondary vCPUs:

 * VCPUOP_initialise is used to set the initial state of the vCPU. The
   argument passed to the hypercall must be of the type vcpu_hvm_context.
   See public/hvm/hvm_vcpu.h for the layout of the structure. Note that
   this hypercall allows starting the vCPU in several modes (16/32/64bits),
   regardless of the mode the BSP is currently running on.

 * VCPUOP_up is used to launch the vCPU once the initial state has been
   set using VCPUOP_initialise.

 * VCPUOP_down is used to bring down a vCPU.

 * VCPUOP_is_up is used to scan the number of available vCPUs.

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

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Jan Beulich]

>>> On 04.09.15 at 14:11, <roger.pau@citrix.com> wrote:
>  * cs: must be a 32-bit read/execute code segment with an offset of ‘0’
>    and a limit of ‘0xFFFFFFFF’. The selector value is unspecified.
> 
>  * ds, es: must be a 32-bit read/write data segment with an offset of
>    ‘0’ and a limit of ‘0xFFFFFFFF’. The selector values are all unspecified.

In both cases s/offset/base/.

>  * tr: must be a 32-bit TSS (active) with a base of '0' and a limit of '0xFF'.

Already on the previous version I asked about this strange 0xFF,
and I don't recall any answer.

>  * eflags: bit 17 (VM) must be cleared. Bit 9 (IF) must be cleared.
>    Other bits are all unspecified.

At the very least I'd also require TF to be clear.

> The format of the structure passed in the %ebx register is the following:

Even if it may sound like splitting hair: Please use precise wording. It's
not the structure that's contained in %ebx, but %ebx hold the address
of that structure.

> struct hvm_start_info {
> #define HVM_START_MAGIC_VALUE 0x336ec578
>     uint32_t magic;             /* Contains the magic value 0x336ec578       */
>                                 /* ("xEn3" with the 0x80 bit of the "E" set).*/
>     uint32_t flags;             /* SIF_xxx flags.                            */

Do really mean to re-use the SIF_* flags here?

> AP startup
> ==========
> 
> AP startup is performed using hypercalls. The following VCPU operations
> are used in order to bring up secondary vCPUs:
> 
>  * VCPUOP_initialise is used to set the initial state of the vCPU. The
>    argument passed to the hypercall must be of the type vcpu_hvm_context.

VCPUOP_initialise takes a struct vcpu_guest_context; I don't think
we can or should change that.

Jan

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

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Roger Pau Monné]

El 04/09/15 a les 16.08, Jan Beulich ha escrit:
>>>> On 04.09.15 at 14:11, <roger.pau@citrix.com> wrote:
>>  * cs: must be a 32-bit read/execute code segment with an offset of ‘0’
>>    and a limit of ‘0xFFFFFFFF’. The selector value is unspecified.
>>
>>  * ds, es: must be a 32-bit read/write data segment with an offset of
>>    ‘0’ and a limit of ‘0xFFFFFFFF’. The selector values are all unspecified.
> 
> In both cases s/offset/base/.

Right, sorry.

> 
>>  * tr: must be a 32-bit TSS (active) with a base of '0' and a limit of '0xFF'.
> 
> Already on the previous version I asked about this strange 0xFF,
> and I don't recall any answer.

My bad, I have actually changed this in the code (see v6), but forgot to
update the design document. 0x67 is perfectly fine, and is what should
be used.

Just for the record, I've initially set it to 0xff because that's how
it's done in construct_vmcs.

>>  * eflags: bit 17 (VM) must be cleared. Bit 9 (IF) must be cleared.
>>    Other bits are all unspecified.
> 
> At the very least I'd also require TF to be clear.

Yes, that makes sense. I'm quite surprised that multiboot1 doesn't
mandate TF to also be cleared.

>> The format of the structure passed in the %ebx register is the following:
> 
> Even if it may sound like splitting hair: Please use precise wording. It's
> not the structure that's contained in %ebx, but %ebx hold the address
> of that structure.

Would you be fine with replacing this sentence with:

The format of the boot start info structure is the following:

>> struct hvm_start_info {
>> #define HVM_START_MAGIC_VALUE 0x336ec578
>>     uint32_t magic;             /* Contains the magic value 0x336ec578       */
>>                                 /* ("xEn3" with the 0x80 bit of the "E" set).*/
>>     uint32_t flags;             /* SIF_xxx flags.                            */
> 
> Do really mean to re-use the SIF_* flags here?

We can introduce a new set of flags, HVM_INIT_*, which ATM is only going
to be:

#define HVM_FLAGS_INITDOMAIN (1<<0)

> 
>> AP startup
>> ==========
>>
>> AP startup is performed using hypercalls. The following VCPU operations
>> are used in order to bring up secondary vCPUs:
>>
>>  * VCPUOP_initialise is used to set the initial state of the vCPU. The
>>    argument passed to the hypercall must be of the type vcpu_hvm_context.
> 
> VCPUOP_initialise takes a struct vcpu_guest_context; I don't think
> we can or should change that.

Didn't we agree that vcpu_guest_context was not suitable for HVM/PVH guests?

Patch 24 of my HVM-without-dm series already introduces this new
structure and the necessary helpers.

Roger.


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

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Jan Beulich]

>>> On 04.09.15 at 16:31, <roger.pau@citrix.com> wrote:
> El 04/09/15 a les 16.08, Jan Beulich ha escrit:
>>>>> On 04.09.15 at 14:11, <roger.pau@citrix.com> wrote:
>>>  * tr: must be a 32-bit TSS (active) with a base of '0' and a limit of '0xFF'.
>> 
>> Already on the previous version I asked about this strange 0xFF,
>> and I don't recall any answer.
> 
> My bad, I have actually changed this in the code (see v6), but forgot to
> update the design document. 0x67 is perfectly fine, and is what should
> be used.
> 
> Just for the record, I've initially set it to 0xff because that's how
> it's done in construct_vmcs.

Which seems bogus too. Jun, Kevin?

Jan

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Jan Beulich]

>>> On 04.09.15 at 16:31, <roger.pau@citrix.com> wrote:
> El 04/09/15 a les 16.08, Jan Beulich ha escrit:
>>>>> On 04.09.15 at 14:11, <roger.pau@citrix.com> wrote:
>>> The format of the structure passed in the %ebx register is the following:
>> 
>> Even if it may sound like splitting hair: Please use precise wording. It's
>> not the structure that's contained in %ebx, but %ebx hold the address
>> of that structure.
> 
> Would you be fine with replacing this sentence with:
> 
> The format of the boot start info structure is the following:

Yes (maybe insert "(pointed to be %ebx)").

>>> struct hvm_start_info {
>>> #define HVM_START_MAGIC_VALUE 0x336ec578
>>>     uint32_t magic;             /* Contains the magic value 0x336ec578       */
>>>                                 /* ("xEn3" with the 0x80 bit of the "E" set).*/
>>>     uint32_t flags;             /* SIF_xxx flags.                            */
>> 
>> Do really mean to re-use the SIF_* flags here?
> 
> We can introduce a new set of flags, HVM_INIT_*, which ATM is only going
> to be:
> 
> #define HVM_FLAGS_INITDOMAIN (1<<0)

>From an abstract pov I'd prefer that. Maybe I'm overlooking
something which would be simplified by using the same values...

>>> AP startup
>>> ==========
>>>
>>> AP startup is performed using hypercalls. The following VCPU operations
>>> are used in order to bring up secondary vCPUs:
>>>
>>>  * VCPUOP_initialise is used to set the initial state of the vCPU. The
>>>    argument passed to the hypercall must be of the type vcpu_hvm_context.
>> 
>> VCPUOP_initialise takes a struct vcpu_guest_context; I don't think
>> we can or should change that.
> 
> Didn't we agree that vcpu_guest_context was not suitable for HVM/PVH guests?

Yes we did.

> Patch 24 of my HVM-without-dm series already introduces this new
> structure and the necessary helpers.

I didn't look at most of the series yet (despite it already being at v6;
I'm sorry, I just didn't get around so far). But I think you agree that
we can't just change an existing hypercall. Iirc along with agreeing
on vcpu_guest_context not being suitable we also agreed that this
will need to be a new sub-op, and I wondered whether calling it
VCPUOP_initialize would be too subtle.

Jan

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Ian Campbell]

On Fri, 2015-09-04 at 09:21 -0600, Jan Beulich wrote:
> > 
> > > > On 04.09.15 at 16:31, <roger.pau@citrix.com> wrote:
> > El 04/09/15 a les 16.08, Jan Beulich ha escrit:
> > > > > > On 04.09.15 at 14:11, <roger.pau@citrix.com> wrote:
> > > > The format of the structure passed in the %ebx register is the 
> > > > following:
> > > 
> > > Even if it may sound like splitting hair: Please use precise wording. 
> > > It's
> > > not the structure that's contained in %ebx, but %ebx hold the address
> > > of that structure.
> > 
> > Would you be fine with replacing this sentence with:
> > 
> > The format of the boot start info structure is the following:
> 
> Yes (maybe insert "(pointed to be %ebx)").
> 
> > > > struct hvm_start_info {
> > > > #define HVM_START_MAGIC_VALUE 0x336ec578
> > > >     uint32_t magic;             /* Contains the magic value 
> > > > 0x336ec578       */
> > > >                                 /* ("xEn3" with the 0x80 bit of the 
> > > > "E" set).*/
> > > >     uint32_t flags;             /* SIF_xxx flags.                  
> > > >           */
> > > 
> > > Do really mean to re-use the SIF_* flags here?
> > 
> > We can introduce a new set of flags, HVM_INIT_*, which ATM is only 
> > going
> > to be:
> > 
> > #define HVM_FLAGS_INITDOMAIN (1<<0)
> 
> From an abstract pov I'd prefer that. Maybe I'm overlooking
> something which would be simplified by using the same values...
> 
> > > > AP startup
> > > > ==========
> > > > 
> > > > AP startup is performed using hypercalls. The following VCPU 
> > > > operations
> > > > are used in order to bring up secondary vCPUs:
> > > > 
> > > >  * VCPUOP_initialise is used to set the initial state of the vCPU. 
> > > > The
> > > >    argument passed to the hypercall must be of the type > > > > vcpu_hvm_context.
> > > 
> > > VCPUOP_initialise takes a struct vcpu_guest_context; I don't think
> > > we can or should change that.
> > 
> > Didn't we agree that vcpu_guest_context was not suitable for HVM/PVH 
> > guests?
> 
> Yes we did.
> 
> > Patch 24 of my HVM-without-dm series already introduces this new
> > structure and the necessary helpers.
> 
> I didn't look at most of the series yet (despite it already being at v6;
> I'm sorry, I just didn't get around so far). But I think you agree that
> we can't just change an existing hypercall. Iirc along with agreeing
> on vcpu_guest_context not being suitable we also agreed that this
> will need to be a new sub-op, and I wondered whether calling it
> VCPUOP_initialize would be too subtle.

You mean literally only s/s/z/? In which case, yes, far far to subtle.

Even initialise2 would be better than that alternative...

> 
> Jan
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Roger Pau Monné]

El 04/09/15 a les 17.21, Jan Beulich ha escrit:
>>>> AP startup
>>>> >>> ==========
>>>> >>>
>>>> >>> AP startup is performed using hypercalls. The following VCPU operations
>>>> >>> are used in order to bring up secondary vCPUs:
>>>> >>>
>>>> >>>  * VCPUOP_initialise is used to set the initial state of the vCPU. The
>>>> >>>    argument passed to the hypercall must be of the type vcpu_hvm_context.
>>> >> 
>>> >> VCPUOP_initialise takes a struct vcpu_guest_context; I don't think
>>> >> we can or should change that.
>> > 
>> > Didn't we agree that vcpu_guest_context was not suitable for HVM/PVH guests?
> Yes we did.
> 
>> > Patch 24 of my HVM-without-dm series already introduces this new
>> > structure and the necessary helpers.
> I didn't look at most of the series yet (despite it already being at v6;
> I'm sorry, I just didn't get around so far). But I think you agree that
> we can't just change an existing hypercall. Iirc along with agreeing
> on vcpu_guest_context not being suitable we also agreed that this
> will need to be a new sub-op, and I wondered whether calling it
> VCPUOP_initialize would be too subtle.

VCPUOP_initialize was never available to HVM guests, so I don't think
changing the argument is a problem. However, I understand that for the
sake of clarity overloading an hypercall this way is not the best
practice. What about naming it VCPUOP_hvm_initialise?

Would it make sense to add aliases to have:

#define VCPU_hvm_up VCPU_up
#define VCPU_hvm_down VCPU_down
#define VCPU_hvm_is_up VCPU_is_up

Just for symmetry reasons?

Roger.

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Jan Beulich]

>>> On 04.09.15 at 17:26, <ian.campbell@citrix.com> wrote:
> On Fri, 2015-09-04 at 09:21 -0600, Jan Beulich wrote:
>> > > > On 04.09.15 at 16:31, <roger.pau@citrix.com> wrote:
>> > El 04/09/15 a les 16.08, Jan Beulich ha escrit:
>> > > > > > On 04.09.15 at 14:11, <roger.pau@citrix.com> wrote:
>> > > > AP startup
>> > > > ==========
>> > > > 
>> > > > AP startup is performed using hypercalls. The following VCPU 
>> > > > operations
>> > > > are used in order to bring up secondary vCPUs:
>> > > > 
>> > > >  * VCPUOP_initialise is used to set the initial state of the vCPU. 
>> > > > The
>> > > >    argument passed to the hypercall must be of the type > > > > 
> vcpu_hvm_context.
>> > > 
>> > > VCPUOP_initialise takes a struct vcpu_guest_context; I don't think
>> > > we can or should change that.
>> > 
>> > Didn't we agree that vcpu_guest_context was not suitable for HVM/PVH 
>> > guests?
>> 
>> Yes we did.
>> 
>> > Patch 24 of my HVM-without-dm series already introduces this new
>> > structure and the necessary helpers.
>> 
>> I didn't look at most of the series yet (despite it already being at v6;
>> I'm sorry, I just didn't get around so far). But I think you agree that
>> we can't just change an existing hypercall. Iirc along with agreeing
>> on vcpu_guest_context not being suitable we also agreed that this
>> will need to be a new sub-op, and I wondered whether calling it
>> VCPUOP_initialize would be too subtle.
> 
> You mean literally only s/s/z/?

Indeed ;-)

> In which case, yes, far far to subtle.

I was afraid I would get feedback like this (and I was surprised I
didn't already when I first suggested this).

Jan

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Jan Beulich]

>>> On 04.09.15 at 17:47, <roger.pau@citrix.com> wrote:
> VCPUOP_initialize was never available to HVM guests, so I don't think
> changing the argument is a problem. However, I understand that for the
> sake of clarity overloading an hypercall this way is not the best
> practice. What about naming it VCPUOP_hvm_initialise?

Hmm, don't know, the more that we want it primarily for PVH.

> Would it make sense to add aliases to have:
> 
> #define VCPU_hvm_up VCPU_up
> #define VCPU_hvm_down VCPU_down
> #define VCPU_hvm_is_up VCPU_is_up
> 
> Just for symmetry reasons?

If using the above, maybe. But I'd be afraid of this just becoming
useless clutter.

Jan

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Roger Pau Monné]

El 04/09/15 a les 18.03, Jan Beulich ha escrit:
>>>> On 04.09.15 at 17:47, <roger.pau@citrix.com> wrote:
>> VCPUOP_initialize was never available to HVM guests, so I don't think
>> changing the argument is a problem. However, I understand that for the
>> sake of clarity overloading an hypercall this way is not the best
>> practice. What about naming it VCPUOP_hvm_initialise?
> 
> Hmm, don't know, the more that we want it primarily for PVH.

Well, it will be available to HVM guests in general, both pure HVM and
PVH guests.

Roger.

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Ian Campbell]

On Fri, 2015-09-04 at 10:01 -0600, Jan Beulich wrote:
> > 
> > > > On 04.09.15 at 17:26, <ian.campbell@citrix.com> wrote:
> > On Fri, 2015-09-04 at 09:21 -0600, Jan Beulich wrote:
> > > > > > On 04.09.15 at 16:31, <roger.pau@citrix.com> wrote:
> > > > El 04/09/15 a les 16.08, Jan Beulich ha escrit:
> > > > > > > > On 04.09.15 at 14:11, <roger.pau@citrix.com> wrote:
> > > > > > AP startup
> > > > > > ==========
> > > > > > 
> > > > > > AP startup is performed using hypercalls. The following VCPU 
> > > > > > operations
> > > > > > are used in order to bring up secondary vCPUs:
> > > > > > 
> > > > > >  * VCPUOP_initialise is used to set the initial state of the 
> > > > > > vCPU. 
> > > > > > The
> > > > > >    argument passed to the hypercall must be of the type > > > > 
> > > > > > 
> > vcpu_hvm_context.
> > > > > 
> > > > > VCPUOP_initialise takes a struct vcpu_guest_context; I don't 
> > > > > think
> > > > > we can or should change that.
> > > > 
> > > > Didn't we agree that vcpu_guest_context was not suitable for 
> > > > HVM/PVH 
> > > > guests?
> > > 
> > > Yes we did.
> > > 
> > > > Patch 24 of my HVM-without-dm series already introduces this new
> > > > structure and the necessary helpers.
> > > 
> > > I didn't look at most of the series yet (despite it already being at 
> > > v6;
> > > I'm sorry, I just didn't get around so far). But I think you agree 
> > > that
> > > we can't just change an existing hypercall. Iirc along with agreeing
> > > on vcpu_guest_context not being suitable we also agreed that this
> > > will need to be a new sub-op, and I wondered whether calling it
> > > VCPUOP_initialize would be too subtle.
> > 
> > You mean literally only s/s/z/?
> 
> Indeed ;-)
> 
> > In which case, yes, far far to subtle.
> 
> I was afraid I would get feedback like this (and I was surprised I
> didn't already when I first suggested this).

I think even if I'd have seen it the first time I'd have thought you were
joking...

Ian.

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Ian Campbell]

On Fri, 2015-09-04 at 17:47 +0200, Roger Pau Monné wrote:
> El 04/09/15 a les 17.21, Jan Beulich ha escrit:
> > > > > AP startup
> > > > > > > > ==========
> > > > > > > > 
> > > > > > > > AP startup is performed using hypercalls. The following 
> > > > > > > > VCPU operations
> > > > > > > > are used in order to bring up secondary vCPUs:
> > > > > > > > 
> > > > > > > >  * VCPUOP_initialise is used to set the initial state of 
> > > > > > > > the vCPU. The
> > > > > > > >    argument passed to the hypercall must be of the type 
> > > > > > > > vcpu_hvm_context.
> > > > > > 
> > > > > > VCPUOP_initialise takes a struct vcpu_guest_context; I don't 
> > > > > > think
> > > > > > we can or should change that.
> > > > 
> > > > Didn't we agree that vcpu_guest_context was not suitable for 
> > > > HVM/PVH guests?
> > Yes we did.
> > 
> > > > Patch 24 of my HVM-without-dm series already introduces this new
> > > > structure and the necessary helpers.
> > I didn't look at most of the series yet (despite it already being at 
> > v6;
> > I'm sorry, I just didn't get around so far). But I think you agree that
> > we can't just change an existing hypercall. Iirc along with agreeing
> > on vcpu_guest_context not being suitable we also agreed that this
> > will need to be a new sub-op, and I wondered whether calling it
> > VCPUOP_initialize would be too subtle.
> 
> VCPUOP_initialize was never available to HVM guests, so I don't think
> changing the argument is a problem. However, I understand that for the
> sake of clarity overloading an hypercall this way is not the best
> practice. What about naming it VCPUOP_hvm_initialise?

If the new interface could support both PV (vcpu_guest_context) and the new
thing (i.e. with a type field and a union perhaps), or if the new interface
can work for PV some other way then it's not unheard of to rename the
existing number with _compat and take over the name with a new number.

It just needs some compat __XEN_INTERFACE_VERSION__ stuff in the headers,
like with e.g. __HYPERVISOR_sched_op vs __HYPERVISOR_sched_op_compat.

(I've not looked at this interface and I don't really remember what the old
one looks like, so maybe this is an insane idea in this case)

Ian.


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

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Roger Pau Monné]

Hello,

El 04/09/15 a les 18.12, Ian Campbell ha escrit:
> On Fri, 2015-09-04 at 17:47 +0200, Roger Pau Monné wrote:
>> VCPUOP_initialize was never available to HVM guests, so I don't think
>> changing the argument is a problem. However, I understand that for the
>> sake of clarity overloading an hypercall this way is not the best
>> practice. What about naming it VCPUOP_hvm_initialise?
> 
> If the new interface could support both PV (vcpu_guest_context) and the new
> thing (i.e. with a type field and a union perhaps), or if the new interface
> can work for PV some other way then it's not unheard of to rename the
> existing number with _compat and take over the name with a new number.
> 
> It just needs some compat __XEN_INTERFACE_VERSION__ stuff in the headers,
> like with e.g. __HYPERVISOR_sched_op vs __HYPERVISOR_sched_op_compat.
> 
> (I've not looked at this interface and I don't really remember what the old
> one looks like, so maybe this is an insane idea in this case)

So AFAICS we have 3 options:

1. Overload VCPUOP_initialise like it's done in the current series (v6).
For PV guests the hypercall parameter is of type vcpu_guest_context,
while for HVM guests the parameter is of type vcpu_hvm_context.

2. Create a new hypercall (VCPUOP_hvm_initialise) only available to HVM
guests, that only allows vcpu_hvm_context as a parameter.

3. Deprecate current VCPUOP_initialise, introduce a new
VCPUOP_initialise, that takes the following parameter:

union vcpu_context {
	struct vcpu_guest_context pv_ctx;
	struct vcpu_hvm_context hvm_ctx;
};

TBH, I don't have an opinion between 2 and 3, but I would like to get a
consensus before I start implementing any of those.

Roger.


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

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Jan Beulich]

>>> On 07.09.15 at 11:34, <roger.pau@citrix.com> wrote:
> So AFAICS we have 3 options:
> 
> 1. Overload VCPUOP_initialise like it's done in the current series (v6).
> For PV guests the hypercall parameter is of type vcpu_guest_context,
> while for HVM guests the parameter is of type vcpu_hvm_context.
> 
> 2. Create a new hypercall (VCPUOP_hvm_initialise) only available to HVM
> guests, that only allows vcpu_hvm_context as a parameter.
> 
> 3. Deprecate current VCPUOP_initialise, introduce a new
> VCPUOP_initialise, that takes the following parameter:
> 
> union vcpu_context {
> 	struct vcpu_guest_context pv_ctx;
> 	struct vcpu_hvm_context hvm_ctx;
> };
> 
> TBH, I don't have an opinion between 2 and 3, but I would like to get a
> consensus before I start implementing any of those.

Let me first take a look at how your implementation of 1 looks like.

Jan

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Roger Pau Monné]

El 07/09/15 a les 12.05, Jan Beulich ha escrit:
>>>> On 07.09.15 at 11:34, <roger.pau@citrix.com> wrote:
>> So AFAICS we have 3 options:
>>
>> 1. Overload VCPUOP_initialise like it's done in the current series (v6).
>> For PV guests the hypercall parameter is of type vcpu_guest_context,
>> while for HVM guests the parameter is of type vcpu_hvm_context.
>>
>> 2. Create a new hypercall (VCPUOP_hvm_initialise) only available to HVM
>> guests, that only allows vcpu_hvm_context as a parameter.
>>
>> 3. Deprecate current VCPUOP_initialise, introduce a new
>> VCPUOP_initialise, that takes the following parameter:
>>
>> union vcpu_context {
>> 	struct vcpu_guest_context pv_ctx;
>> 	struct vcpu_hvm_context hvm_ctx;
>> };
>>
>> TBH, I don't have an opinion between 2 and 3, but I would like to get a
>> consensus before I start implementing any of those.
> 
> Let me first take a look at how your implementation of 1 looks like.

Ping?

Re: [Draft C] Boot ABI for HVM guests without a device-model

[Jan Beulich]

>>> On 15.09.15 at 09:08, <roger.pau@citrix.com> wrote:
> El 07/09/15 a les 12.05, Jan Beulich ha escrit:
>>>>> On 07.09.15 at 11:34, <roger.pau@citrix.com> wrote:
>>> So AFAICS we have 3 options:
>>>
>>> 1. Overload VCPUOP_initialise like it's done in the current series (v6).
>>> For PV guests the hypercall parameter is of type vcpu_guest_context,
>>> while for HVM guests the parameter is of type vcpu_hvm_context.
>>>
>>> 2. Create a new hypercall (VCPUOP_hvm_initialise) only available to HVM
>>> guests, that only allows vcpu_hvm_context as a parameter.
>>>
>>> 3. Deprecate current VCPUOP_initialise, introduce a new
>>> VCPUOP_initialise, that takes the following parameter:
>>>
>>> union vcpu_context {
>>> 	struct vcpu_guest_context pv_ctx;
>>> 	struct vcpu_hvm_context hvm_ctx;
>>> };
>>>
>>> TBH, I don't have an opinion between 2 and 3, but I would like to get a
>>> consensus before I start implementing any of those.
>> 
>> Let me first take a look at how your implementation of 1 looks like.
> 
> Ping?

Yeah, sorry, it's on my list, but keeps getting preempted.

Jan