[PATCH v10 0/9] Introduce HVM without dm and new boot ABI

[PATCH v10 0/9] Introduce HVM without dm and new boot ABI

[Roger Pau Monne]

This are the remaining patches of the HVMlite series. They have been 
successfully tested on the following hardware:

 - Intel Core i3-5010U.
 - AMD Opteron 4184.

With both hap=0 and hap=1 in the configuration file. I've been able to boot
a SMP guest in this mode with a virtual hard drive and a virtual network
card, all working fine AFAICT. Migration/save/restore has also been tested 
with a SMP guest using the FreeBSD kernel provided below.

The series has been compile tested on arm32.

The series can also be found in the following git repo:

git://xenbits.xen.org/people/royger/xen.git branch hvm_without_dm_v10

And for the FreeBSD part:

git://xenbits.xen.org/people/royger/freebsd.git branch new_entry_point_v5

In case someone wants to give it a try, I've uploaded a FreeBSD kernel that
should work when booted into this mode:

https://people.freebsd.org/~royger/kernel_no_dm

This FreeBSD kernel starts the APs in long mode. There are examples for 
starting the APs in other modes in the sys/x86/xen/pv.c file.

The config file that I've used is:

<config>
kernel="/path/to/kernel_no_dm"

builder="hvm"
device_model_version="none"

memory=128
vcpus=2
name = "freebsd"
</config>

Of course if you have a FreeBSD disk already setup it can also be added to
the configuration file, and the following line can be used to point FreeBSD
to the disk:

extra="vfs.root.mountfrom=ufs:/dev/ufsid/<disk_id>"

As usual, each patch has it's own changelog.

  J   xen/x86: set the vPMU interface based on the presence
A     xen/x86: allow disabling all emulated devices inside
AW    libxc: allow creating domains without emulated
    N x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits
   M  xen/x86: allow HVM guests to use hypercalls to bring
AWJ   libxc/xen: introduce a start info structure for
AW    libxc: switch xc_dom_elfloader to be used with HVMlite
 W    libxl: allow the creation of HVM domains without a
AW M  libxl: add support for migrating HVM guests without a

A = Acked/Reviewed by Andrew Cooper.
W = Acked/Reviewed by Wei Liu.
J = Acked/Reviewed by Jan Beulich.
M = Modified in this version.
N = New in this version.

Thanks, Roger.

[PATCH v10 1/9] xen/x86: set the vPMU interface based on the presence of a lapic

[Roger Pau Monne]

Instead of choosing the interface to expose to guests based on the guest
type, do it based on whether the guest has an emulated local apic or not.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v8:
 - Don't add the xenpmu hypercalls to the HVM hypercall table (yet).
 - Add Jan Beulich Acked-by.

Changes since v7:
 - Merge vpmu work from Boris.

Changes since v6:
 - Major rework of the approach.
 - Drop Andrew Cooper Acked-by.

Changes since v4:
 - Add Andrew Cooper Acked-by.
---
 xen/arch/x86/cpu/vpmu.c       | 11 ++++++-----
 xen/arch/x86/cpu/vpmu_amd.c   |  6 +++---
 xen/arch/x86/cpu/vpmu_intel.c |  6 +++---
 xen/arch/x86/hvm/svm/svm.c    |  2 +-
 xen/arch/x86/hvm/vmx/vmx.c    |  2 +-
 5 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index d870dcc..4856e98 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -94,7 +94,7 @@ void vpmu_lvtpc_update(uint32_t val)
     vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | (val & APIC_LVT_MASKED);
 
     /* Postpone APIC updates for PV(H) guests if PMU interrupt is pending */
-    if ( is_hvm_vcpu(curr) || !vpmu->xenpmu_data ||
+    if ( has_vlapic(curr->domain) || !vpmu->xenpmu_data ||
          !vpmu_is_set(vpmu, VPMU_CACHED) )
         apic_write(APIC_LVTPC, vpmu->hw_lapic_lvtpc);
 }
@@ -129,7 +129,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
      * and since do_wr/rdmsr may load VPMU context we should save
      * (and unload) it again.
      */
-    if ( !is_hvm_vcpu(curr) && vpmu->xenpmu_data &&
+    if ( !has_vlapic(curr->domain) && vpmu->xenpmu_data &&
         vpmu_is_set(vpmu, VPMU_CACHED) )
     {
         vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
@@ -184,7 +184,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
         return;
 
     /* PV(H) guest */
-    if ( !is_hvm_vcpu(sampling) || (vpmu_mode & XENPMU_MODE_ALL) )
+    if ( !has_vlapic(sampling->domain) || (vpmu_mode & XENPMU_MODE_ALL) )
     {
         const struct cpu_user_regs *cur_regs;
         uint64_t *flags = &vpmu->xenpmu_data->pmu.pmu_flags;
@@ -411,7 +411,8 @@ int vpmu_load(struct vcpu *v, bool_t from_guest)
 
     /* Only when PMU is counting, we load PMU context immediately. */
     if ( !vpmu_is_set(vpmu, VPMU_RUNNING) ||
-         (!is_hvm_vcpu(vpmu_vcpu(vpmu)) && vpmu_is_set(vpmu, VPMU_CACHED)) )
+         (!has_vlapic(vpmu_vcpu(vpmu)->domain) &&
+         vpmu_is_set(vpmu, VPMU_CACHED)) )
         return 0;
 
     if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_load )
@@ -637,7 +638,7 @@ long do_xenpmu_op(unsigned int op, XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
     struct xen_pmu_data *xenpmu_data;
     struct vpmu_struct *vpmu;
 
-    if ( !opt_vpmu_enabled )
+    if ( !opt_vpmu_enabled || has_vlapic(current->domain) )
         return -EOPNOTSUPP;
 
     ret = xsm_pmu_op(XSM_OTHER, current->domain, op);
diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index 04da81a..990e6f3 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -238,7 +238,7 @@ static int amd_vpmu_load(struct vcpu *v, bool_t from_guest)
         bool_t is_running = 0;
         struct xen_pmu_amd_ctxt *guest_ctxt = &vpmu->xenpmu_data->pmu.c.amd;
 
-        ASSERT(!is_hvm_vcpu(v));
+        ASSERT(!has_vlapic(v->domain));
 
         ctxt = vpmu->context;
         ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
@@ -314,7 +314,7 @@ static int amd_vpmu_save(struct vcpu *v,  bool_t to_guest)
     {
         struct xen_pmu_amd_ctxt *guest_ctxt, *ctxt;
 
-        ASSERT(!is_hvm_vcpu(v));
+        ASSERT(!has_vlapic(v->domain));
         ctxt = vpmu->context;
         guest_ctxt = &vpmu->xenpmu_data->pmu.c.amd;
         memcpy(&guest_ctxt->regs[0], &ctxt->regs[0], regs_sz);
@@ -525,7 +525,7 @@ int svm_vpmu_initialise(struct vcpu *v)
     vpmu->context = ctxt;
     vpmu->priv_context = NULL;
 
-    if ( !is_hvm_vcpu(v) )
+    if ( !has_vlapic(v->domain) )
     {
         /* Copy register offsets to shared area */
         ASSERT(vpmu->xenpmu_data);
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index d5ea7fe..d5fde80 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -336,7 +336,7 @@ static int core2_vpmu_save(struct vcpu *v, bool_t to_guest)
 
     if ( to_guest )
     {
-        ASSERT(!is_hvm_vcpu(v));
+        ASSERT(!has_vlapic(v->domain));
         memcpy((void *)(&vpmu->xenpmu_data->pmu.c.intel) + regs_off,
                vpmu->context + regs_off, regs_sz);
     }
@@ -441,7 +441,7 @@ static int core2_vpmu_load(struct vcpu *v, bool_t from_guest)
     {
         int ret;
 
-        ASSERT(!is_hvm_vcpu(v));
+        ASSERT(!has_vlapic(v->domain));
 
         memcpy(vpmu->context + regs_off,
                (void *)&v->arch.vpmu.xenpmu_data->pmu.c.intel + regs_off,
@@ -501,7 +501,7 @@ static int core2_vpmu_alloc_resource(struct vcpu *v)
     vpmu->context = core2_vpmu_cxt;
     vpmu->priv_context = p;
 
-    if ( !is_hvm_vcpu(v) )
+    if ( !has_vlapic(v->domain) )
     {
         /* Copy fixed/arch register offsets to shared area */
         ASSERT(vpmu->xenpmu_data);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 9415fb1..389cc16 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1167,7 +1167,7 @@ static int svm_vcpu_initialise(struct vcpu *v)
     }
 
     /* PVH's VPMU is initialized via hypercall */
-    if ( is_hvm_vcpu(v) )
+    if ( is_hvm_vcpu(v) && has_vlapic(v->domain) )
         vpmu_initialise(v);
 
     svm_guest_osvw_init(v);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 2581e97..06c12e1 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -148,7 +148,7 @@ static int vmx_vcpu_initialise(struct vcpu *v)
     }
 
     /* PVH's VPMU is initialized via hypercall */
-    if ( is_hvm_vcpu(v) )
+    if ( is_hvm_vcpu(v) && has_vlapic(v->domain) )
         vpmu_initialise(v);
 
     vmx_install_vlapic_mapping(v);
-- 
1.9.5 (Apple Git-50.3)


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

Re: [PATCH v10 1/9] xen/x86: set the vPMU interface based on the presence of a lapic

[Jan Beulich]

>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
> ---
>  xen/arch/x86/cpu/vpmu.c       | 11 ++++++-----
>  xen/arch/x86/cpu/vpmu_amd.c   |  6 +++---
>  xen/arch/x86/cpu/vpmu_intel.c |  6 +++---
>  xen/arch/x86/hvm/svm/svm.c    |  2 +-
>  xen/arch/x86/hvm/vmx/vmx.c    |  2 +-
>  5 files changed, 14 insertions(+), 13 deletions(-)

You should be Cc-ing the VMX maintainers, or this is likely to never
get the necessary ack (even more so with the recent restoration of
maintainership for xen/arch/x86/cpu/vpmu_intel.c).

Jan

Re: [PATCH v10 1/9] xen/x86: set the vPMU interface based on the presence of a lapic

[Roger Pau Monné]

Ccing the vPMU maintainers.

El 07/12/15 a les 17.48, Roger Pau Monne ha escrit:
> Instead of choosing the interface to expose to guests based on the guest
> type, do it based on whether the guest has an emulated local apic or not.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>


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

Re: [PATCH v10 1/9] xen/x86: set the vPMU interface based on the presence of a lapic

[Tian, Kevin]

> From: Roger Pau Monné [mailto:roger.pau@citrix.com]
> Sent: Wednesday, December 09, 2015 6:18 PM
> 
> Ccing the vPMU maintainers.

Next time please include the whole patch when CCing me, which is easier
to do review. :-)

> 
> El 07/12/15 a les 17.48, Roger Pau Monne ha escrit:
> > Instead of choosing the interface to expose to guests based on the guest
> > type, do it based on whether the guest has an emulated local apic or not.
> >
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > Acked-by: Jan Beulich <jbeulich@suse.com>
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>

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

Re: [PATCH v10 1/9] xen/x86: set the vPMU interface based on the presence of a lapic

[Tian, Kevin]

> From: Roger Pau Monne
> Sent: Tuesday, December 08, 2015 12:48 AM
> 
> Instead of choosing the interface to expose to guests based on the guest
> type, do it based on whether the guest has an emulated local apic or not.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> ---

[...]

> @@ -637,7 +638,7 @@ long do_xenpmu_op(unsigned int op,
> XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
>      struct xen_pmu_data *xenpmu_data;
>      struct vpmu_struct *vpmu;
> 
> -    if ( !opt_vpmu_enabled )
> +    if ( !opt_vpmu_enabled || has_vlapic(current->domain) )
>          return -EOPNOTSUPP;

!has_vlapic(current->domain)?

[...]

> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
> index 2581e97..06c12e1 100644
> --- a/xen/arch/x86/hvm/vmx/vmx.c
> +++ b/xen/arch/x86/hvm/vmx/vmx.c
> @@ -148,7 +148,7 @@ static int vmx_vcpu_initialise(struct vcpu *v)
>      }
> 
>      /* PVH's VPMU is initialized via hypercall */
> -    if ( is_hvm_vcpu(v) )
> +    if ( is_hvm_vcpu(v) && has_vlapic(v->domain) )
>          vpmu_initialise(v);

Is is_hvm_vcpu still required here?

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

Re: [PATCH v10 1/9] xen/x86: set the vPMU interface based on the presence of a lapic

[Jan Beulich]

>>> On 11.12.15 at 08:51, <kevin.tian@intel.com> wrote:
>>  From: Roger Pau Monne
>> Sent: Tuesday, December 08, 2015 12:48 AM
>> 
>> Instead of choosing the interface to expose to guests based on the guest
>> type, do it based on whether the guest has an emulated local apic or not.
>> 
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
> 
> [...]
> 
>> @@ -637,7 +638,7 @@ long do_xenpmu_op(unsigned int op,
>> XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
>>      struct xen_pmu_data *xenpmu_data;
>>      struct vpmu_struct *vpmu;
>> 
>> -    if ( !opt_vpmu_enabled )
>> +    if ( !opt_vpmu_enabled || has_vlapic(current->domain) )
>>          return -EOPNOTSUPP;
> 
> !has_vlapic(current->domain)?

Definitely not - a guest with vLAPIC shouldn't use the hypercall.

Jan

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

Re: [PATCH v10 1/9] xen/x86: set the vPMU interface based on the presence of a lapic

[Roger Pau Monné]

El 11/12/15 a les 8.51, Tian, Kevin ha escrit:
>> From: Roger Pau Monne
>> Sent: Tuesday, December 08, 2015 12:48 AM
>>
>> Instead of choosing the interface to expose to guests based on the guest
>> type, do it based on whether the guest has an emulated local apic or not.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
>> Acked-by: Jan Beulich <jbeulich@suse.com>
>> Cc: Jan Beulich <jbeulich@suse.com>
>> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
> 
> [...]
> 
>> @@ -637,7 +638,7 @@ long do_xenpmu_op(unsigned int op,
>> XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
>>      struct xen_pmu_data *xenpmu_data;
>>      struct vpmu_struct *vpmu;
>>
>> -    if ( !opt_vpmu_enabled )
>> +    if ( !opt_vpmu_enabled || has_vlapic(current->domain) )
>>          return -EOPNOTSUPP;
> 
> !has_vlapic(current->domain)?

No, the vpmu hypercall interface is only available to guests that don't
have a local apic.

> [...]
> 
>> diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
>> index 2581e97..06c12e1 100644
>> --- a/xen/arch/x86/hvm/vmx/vmx.c
>> +++ b/xen/arch/x86/hvm/vmx/vmx.c
>> @@ -148,7 +148,7 @@ static int vmx_vcpu_initialise(struct vcpu *v)
>>      }
>>
>>      /* PVH's VPMU is initialized via hypercall */
>> -    if ( is_hvm_vcpu(v) )
>> +    if ( is_hvm_vcpu(v) && has_vlapic(v->domain) )
>>          vpmu_initialise(v);
> 
> Is is_hvm_vcpu still required here?

No, I will remove it.

Thanks, Roger.


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

[PATCH v11 1/09] xen/x86: set the vPMU interface based on the presence of a lapic

[Roger Pau Monne]

Instead of choosing the interface to expose to guests based on the guest
type, do it based on whether the guest has an emulated local apic or not.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
Cc: Aravind Gopalakrishnan <Aravind.Gopalakrishnan@amd.com>
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
---
Changes since v10:
 - Remove the is_hvm_cpu from the VMX/SVM vPMU initialization code, just
   checking for has_vlapic is enough.

Changes since v8:
 - Don't add the xenpmu hypercalls to the HVM hypercall table (yet).
 - Add Jan Beulich Acked-by.

Changes since v7:
 - Merge vpmu work from Boris.

Changes since v6:
 - Major rework of the approach.
 - Drop Andrew Cooper Acked-by.

Changes since v4:
 - Add Andrew Cooper Acked-by.
---
 xen/arch/x86/cpu/vpmu.c       | 11 ++++++-----
 xen/arch/x86/cpu/vpmu_amd.c   |  6 +++---
 xen/arch/x86/cpu/vpmu_intel.c |  6 +++---
 xen/arch/x86/hvm/svm/svm.c    |  2 +-
 xen/arch/x86/hvm/vmx/vmx.c    |  2 +-
 5 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
index d870dcc..4856e98 100644
--- a/xen/arch/x86/cpu/vpmu.c
+++ b/xen/arch/x86/cpu/vpmu.c
@@ -94,7 +94,7 @@ void vpmu_lvtpc_update(uint32_t val)
     vpmu->hw_lapic_lvtpc = PMU_APIC_VECTOR | (val & APIC_LVT_MASKED);
 
     /* Postpone APIC updates for PV(H) guests if PMU interrupt is pending */
-    if ( is_hvm_vcpu(curr) || !vpmu->xenpmu_data ||
+    if ( has_vlapic(curr->domain) || !vpmu->xenpmu_data ||
          !vpmu_is_set(vpmu, VPMU_CACHED) )
         apic_write(APIC_LVTPC, vpmu->hw_lapic_lvtpc);
 }
@@ -129,7 +129,7 @@ int vpmu_do_msr(unsigned int msr, uint64_t *msr_content,
      * and since do_wr/rdmsr may load VPMU context we should save
      * (and unload) it again.
      */
-    if ( !is_hvm_vcpu(curr) && vpmu->xenpmu_data &&
+    if ( !has_vlapic(curr->domain) && vpmu->xenpmu_data &&
         vpmu_is_set(vpmu, VPMU_CACHED) )
     {
         vpmu_set(vpmu, VPMU_CONTEXT_SAVE);
@@ -184,7 +184,7 @@ void vpmu_do_interrupt(struct cpu_user_regs *regs)
         return;
 
     /* PV(H) guest */
-    if ( !is_hvm_vcpu(sampling) || (vpmu_mode & XENPMU_MODE_ALL) )
+    if ( !has_vlapic(sampling->domain) || (vpmu_mode & XENPMU_MODE_ALL) )
     {
         const struct cpu_user_regs *cur_regs;
         uint64_t *flags = &vpmu->xenpmu_data->pmu.pmu_flags;
@@ -411,7 +411,8 @@ int vpmu_load(struct vcpu *v, bool_t from_guest)
 
     /* Only when PMU is counting, we load PMU context immediately. */
     if ( !vpmu_is_set(vpmu, VPMU_RUNNING) ||
-         (!is_hvm_vcpu(vpmu_vcpu(vpmu)) && vpmu_is_set(vpmu, VPMU_CACHED)) )
+         (!has_vlapic(vpmu_vcpu(vpmu)->domain) &&
+         vpmu_is_set(vpmu, VPMU_CACHED)) )
         return 0;
 
     if ( vpmu->arch_vpmu_ops && vpmu->arch_vpmu_ops->arch_vpmu_load )
@@ -637,7 +638,7 @@ long do_xenpmu_op(unsigned int op, XEN_GUEST_HANDLE_PARAM(xen_pmu_params_t) arg)
     struct xen_pmu_data *xenpmu_data;
     struct vpmu_struct *vpmu;
 
-    if ( !opt_vpmu_enabled )
+    if ( !opt_vpmu_enabled || has_vlapic(current->domain) )
         return -EOPNOTSUPP;
 
     ret = xsm_pmu_op(XSM_OTHER, current->domain, op);
diff --git a/xen/arch/x86/cpu/vpmu_amd.c b/xen/arch/x86/cpu/vpmu_amd.c
index 04da81a..990e6f3 100644
--- a/xen/arch/x86/cpu/vpmu_amd.c
+++ b/xen/arch/x86/cpu/vpmu_amd.c
@@ -238,7 +238,7 @@ static int amd_vpmu_load(struct vcpu *v, bool_t from_guest)
         bool_t is_running = 0;
         struct xen_pmu_amd_ctxt *guest_ctxt = &vpmu->xenpmu_data->pmu.c.amd;
 
-        ASSERT(!is_hvm_vcpu(v));
+        ASSERT(!has_vlapic(v->domain));
 
         ctxt = vpmu->context;
         ctrl_regs = vpmu_reg_pointer(ctxt, ctrls);
@@ -314,7 +314,7 @@ static int amd_vpmu_save(struct vcpu *v,  bool_t to_guest)
     {
         struct xen_pmu_amd_ctxt *guest_ctxt, *ctxt;
 
-        ASSERT(!is_hvm_vcpu(v));
+        ASSERT(!has_vlapic(v->domain));
         ctxt = vpmu->context;
         guest_ctxt = &vpmu->xenpmu_data->pmu.c.amd;
         memcpy(&guest_ctxt->regs[0], &ctxt->regs[0], regs_sz);
@@ -525,7 +525,7 @@ int svm_vpmu_initialise(struct vcpu *v)
     vpmu->context = ctxt;
     vpmu->priv_context = NULL;
 
-    if ( !is_hvm_vcpu(v) )
+    if ( !has_vlapic(v->domain) )
     {
         /* Copy register offsets to shared area */
         ASSERT(vpmu->xenpmu_data);
diff --git a/xen/arch/x86/cpu/vpmu_intel.c b/xen/arch/x86/cpu/vpmu_intel.c
index d5ea7fe..d5fde80 100644
--- a/xen/arch/x86/cpu/vpmu_intel.c
+++ b/xen/arch/x86/cpu/vpmu_intel.c
@@ -336,7 +336,7 @@ static int core2_vpmu_save(struct vcpu *v, bool_t to_guest)
 
     if ( to_guest )
     {
-        ASSERT(!is_hvm_vcpu(v));
+        ASSERT(!has_vlapic(v->domain));
         memcpy((void *)(&vpmu->xenpmu_data->pmu.c.intel) + regs_off,
                vpmu->context + regs_off, regs_sz);
     }
@@ -441,7 +441,7 @@ static int core2_vpmu_load(struct vcpu *v, bool_t from_guest)
     {
         int ret;
 
-        ASSERT(!is_hvm_vcpu(v));
+        ASSERT(!has_vlapic(v->domain));
 
         memcpy(vpmu->context + regs_off,
                (void *)&v->arch.vpmu.xenpmu_data->pmu.c.intel + regs_off,
@@ -501,7 +501,7 @@ static int core2_vpmu_alloc_resource(struct vcpu *v)
     vpmu->context = core2_vpmu_cxt;
     vpmu->priv_context = p;
 
-    if ( !is_hvm_vcpu(v) )
+    if ( !has_vlapic(v->domain) )
     {
         /* Copy fixed/arch register offsets to shared area */
         ASSERT(vpmu->xenpmu_data);
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 9415fb1..067bc4d 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -1167,7 +1167,7 @@ static int svm_vcpu_initialise(struct vcpu *v)
     }
 
     /* PVH's VPMU is initialized via hypercall */
-    if ( is_hvm_vcpu(v) )
+    if ( has_vlapic(v->domain) )
         vpmu_initialise(v);
 
     svm_guest_osvw_init(v);
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 2581e97..2af0780 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -148,7 +148,7 @@ static int vmx_vcpu_initialise(struct vcpu *v)
     }
 
     /* PVH's VPMU is initialized via hypercall */
-    if ( is_hvm_vcpu(v) )
+    if ( has_vlapic(v->domain) )
         vpmu_initialise(v);
 
     vmx_install_vlapic_mapping(v);
-- 
1.9.5 (Apple Git-50.3)


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

Re: [PATCH v11 1/09] xen/x86: set the vPMU interface based on the presence of a lapic

[Tian, Kevin]

> From: Roger Pau Monne [mailto:roger.pau@citrix.com]
> Sent: Friday, December 11, 2015 6:17 PM
> 
> Instead of choosing the interface to expose to guests based on the guest
> type, do it based on whether the guest has an emulated local apic or not.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Acked-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH v10 2/9] xen/x86: allow disabling all emulated devices inside of Xen

[Roger Pau Monne]

Only allow enabling or disabling all the emulated devices inside of Xen,
right now Xen doesn't support enabling specific emulated devices only.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v7:
 - Rework if condition.

Changes since v5:
 - Add Andrew Cooper Reviewed-by.
---
 xen/arch/x86/domain.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 2a8d5c1..df40dc6 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -532,8 +532,8 @@ int arch_domain_create(struct domain *d, unsigned int domcr_flags,
                    d->domain_id, config->emulation_flags);
             return -EINVAL;
         }
-        if ( is_hvm_domain(d) ? (config->emulation_flags != XEN_X86_EMU_ALL)
-                              : (config->emulation_flags != 0) )
+        if ( config->emulation_flags != 0 &&
+             (!is_hvm_domain(d) || config->emulation_flags != XEN_X86_EMU_ALL) )
         {
             printk(XENLOG_G_ERR "d%d: Xen does not allow %s domain creation "
                    "with the current selection of emulators: %#x\n",
-- 
1.9.5 (Apple Git-50.3)


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

[PATCH v10 3/9] libxc: allow creating domains without emulated devices.

[Roger Pau Monne]

Introduce a new flag in xc_dom_image that turns on and off the emulated
devices. This prevents creating the VGA hole, the hvm_info page and the
ioreq server pages. libxl unconditionally sets it to true for all HVM
domains at the moment.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v5:
 - Add Andrew Cooper Reviewed-by.

Changes since v4:
 - Store the size of the VGA hole inside of the xc_dom_image struct and set
   it from libxl.
 - Rename dom->emulation to dom->device_model (no functional change).
 - Add Wei Liu Acked-by.

Changes since v3:
 - Explain the meaning of the "emulation" xc_dom_image field.
---
 tools/libxc/include/xc_dom.h |  4 +++
 tools/libxc/xc_dom_x86.c     | 73 ++++++++++++++++++++++++--------------------
 tools/libxl/libxl_dom.c      |  2 ++
 tools/libxl/libxl_internal.h |  1 +
 4 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index 3c94b57..efa6c6e 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -200,6 +200,10 @@ struct xc_dom_image {
     xen_paddr_t mmio_size;
     xen_paddr_t lowmem_end;
     xen_paddr_t highmem_end;
+    xen_pfn_t vga_hole_size;
+
+    /* If unset disables the setup of the IOREQ pages. */
+    bool device_model;
 
     /* Extra ACPI tables passed to HVMLOADER */
     struct xc_hvm_firmware_module acpi_module;
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 71b042e..1d4ad3e 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -50,8 +50,6 @@
 #define X86_CR0_PE 0x01
 #define X86_CR0_ET 0x10
 
-#define VGA_HOLE_SIZE (0x20)
-
 #define SPECIALPAGE_PAGING   0
 #define SPECIALPAGE_ACCESS   1
 #define SPECIALPAGE_SHARING  2
@@ -595,12 +593,15 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
     xen_pfn_t ioreq_server_array[NR_IOREQ_SERVER_PAGES];
     xc_interface *xch = dom->xch;
 
-    if ( (hvm_info_page = xc_map_foreign_range(
-              xch, domid, PAGE_SIZE, PROT_READ | PROT_WRITE,
-              HVM_INFO_PFN)) == NULL )
-        goto error_out;
-    build_hvm_info(hvm_info_page, dom);
-    munmap(hvm_info_page, PAGE_SIZE);
+    if ( dom->device_model )
+    {
+        if ( (hvm_info_page = xc_map_foreign_range(
+                  xch, domid, PAGE_SIZE, PROT_READ | PROT_WRITE,
+                  HVM_INFO_PFN)) == NULL )
+            goto error_out;
+        build_hvm_info(hvm_info_page, dom);
+        munmap(hvm_info_page, PAGE_SIZE);
+    }
 
     /* Allocate and clear special pages. */
     for ( i = 0; i < NR_SPECIAL_PAGES; i++ )
@@ -632,30 +633,33 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
     xc_hvm_param_set(xch, domid, HVM_PARAM_SHARING_RING_PFN,
                      special_pfn(SPECIALPAGE_SHARING));
 
-    /*
-     * Allocate and clear additional ioreq server pages. The default
-     * server will use the IOREQ and BUFIOREQ special pages above.
-     */
-    for ( i = 0; i < NR_IOREQ_SERVER_PAGES; i++ )
-        ioreq_server_array[i] = ioreq_server_pfn(i);
-
-    rc = xc_domain_populate_physmap_exact(xch, domid, NR_IOREQ_SERVER_PAGES, 0,
-                                          0, ioreq_server_array);
-    if ( rc != 0 )
+    if ( dom->device_model )
     {
-        DOMPRINTF("Could not allocate ioreq server pages.");
-        goto error_out;
-    }
+        /*
+         * Allocate and clear additional ioreq server pages. The default
+         * server will use the IOREQ and BUFIOREQ special pages above.
+         */
+        for ( i = 0; i < NR_IOREQ_SERVER_PAGES; i++ )
+            ioreq_server_array[i] = ioreq_server_pfn(i);
 
-    if ( xc_clear_domain_pages(xch, domid, ioreq_server_pfn(0),
-                               NR_IOREQ_SERVER_PAGES) )
+        rc = xc_domain_populate_physmap_exact(xch, domid, NR_IOREQ_SERVER_PAGES, 0,
+                                              0, ioreq_server_array);
+        if ( rc != 0 )
+        {
+            DOMPRINTF("Could not allocate ioreq server pages.");
             goto error_out;
+        }
 
-    /* Tell the domain where the pages are and how many there are */
-    xc_hvm_param_set(xch, domid, HVM_PARAM_IOREQ_SERVER_PFN,
-                     ioreq_server_pfn(0));
-    xc_hvm_param_set(xch, domid, HVM_PARAM_NR_IOREQ_SERVER_PAGES,
-                     NR_IOREQ_SERVER_PAGES);
+        if ( xc_clear_domain_pages(xch, domid, ioreq_server_pfn(0),
+                                   NR_IOREQ_SERVER_PAGES) )
+                goto error_out;
+
+        /* Tell the domain where the pages are and how many there are */
+        xc_hvm_param_set(xch, domid, HVM_PARAM_IOREQ_SERVER_PFN,
+                         ioreq_server_pfn(0));
+        xc_hvm_param_set(xch, domid, HVM_PARAM_NR_IOREQ_SERVER_PAGES,
+                         NR_IOREQ_SERVER_PAGES);
+    }
 
     /*
      * Identity-map page table is required for running with CR0.PG=0 when
@@ -1393,7 +1397,8 @@ static int meminit_hvm(struct xc_dom_image *dom)
      * allocated is pointless.
      */
     if ( claim_enabled ) {
-        rc = xc_domain_claim_pages(xch, domid, target_pages - VGA_HOLE_SIZE);
+        rc = xc_domain_claim_pages(xch, domid,
+                                   target_pages - dom->vga_hole_size);
         if ( rc != 0 )
         {
             DOMPRINTF("Could not allocate memory for HVM guest as we cannot claim memory!");
@@ -1409,7 +1414,8 @@ static int meminit_hvm(struct xc_dom_image *dom)
          * tot_pages will be target_pages - VGA_HOLE_SIZE after
          * this call.
          */
-        rc = xc_domain_set_pod_target(xch, domid, target_pages - VGA_HOLE_SIZE,
+        rc = xc_domain_set_pod_target(xch, domid,
+                                      target_pages - dom->vga_hole_size,
                                       NULL, NULL, NULL);
         if ( rc != 0 )
         {
@@ -1428,8 +1434,9 @@ static int meminit_hvm(struct xc_dom_image *dom)
      * Under 2MB mode, we allocate pages in batches of no more than 8MB to 
      * ensure that we can be preempted and hence dom0 remains responsive.
      */
-    rc = xc_domain_populate_physmap_exact(
-        xch, domid, 0xa0, 0, memflags, &dom->p2m_host[0x00]);
+    if ( dom->device_model )
+        rc = xc_domain_populate_physmap_exact(
+            xch, domid, 0xa0, 0, memflags, &dom->p2m_host[0x00]);
 
     stat_normal_pages = 0;
     for ( vmemid = 0; vmemid < nr_vmemranges; vmemid++ )
@@ -1448,7 +1455,7 @@ static int meminit_hvm(struct xc_dom_image *dom)
          * 0xA0000-0xC0000. Note that 0x00000-0xA0000 is populated just
          * before this loop.
          */
-        if ( vmemranges[vmemid].start == 0 )
+        if ( vmemranges[vmemid].start == 0 && dom->device_model )
         {
             cur_pages = 0xc0;
             stat_normal_pages += 0xc0;
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 009ca9c..49d7b90 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -997,6 +997,8 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
     dom->lowmem_end = lowmem_end;
     dom->highmem_end = highmem_end;
     dom->mmio_start = mmio_start;
+    dom->vga_hole_size = LIBXL_VGA_HOLE_SIZE;
+    dom->device_model = true;
 
     rc = libxl__domain_device_construct_rdm(gc, d_config,
                                             info->u.hvm.rdm_mem_boundary_memkb*1024,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index d2bda0a..570a85a 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -102,6 +102,7 @@
 #define LIBXL_HVM_EXTRA_MEMORY 2048
 #define LIBXL_MIN_DOM0_MEM (128*1024)
 #define LIBXL_INVALID_GFN (~(uint64_t)0)
+#define LIBXL_VGA_HOLE_SIZE 0x20
 /* use 0 as the domid of the toolstack domain for now */
 #define LIBXL_TOOLSTACK_DOMID 0
 #define QEMU_SIGNATURE "DeviceModelRecord0002"
-- 
1.9.5 (Apple Git-50.3)


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

Re: [PATCH v10 3/9] libxc: allow creating domains without emulated devices.

[Olaf Hering]

On Mon, Dec 07, Roger Pau Monne wrote:

> Introduce a new flag in xc_dom_image that turns on and off the emulated
> devices. This prevents creating the VGA hole, the hvm_info page and the
> ioreq server pages. libxl unconditionally sets it to true for all HVM
> domains at the moment.

> @@ -1428,8 +1434,9 @@ static int meminit_hvm(struct xc_dom_image *dom)
>       * Under 2MB mode, we allocate pages in batches of no more than 8MB to 
>       * ensure that we can be preempted and hence dom0 remains responsive.
>       */
> -    rc = xc_domain_populate_physmap_exact(
> -        xch, domid, 0xa0, 0, memflags, &dom->p2m_host[0x00]);
> +    if ( dom->device_model )
> +        rc = xc_domain_populate_physmap_exact(
> +            xch, domid, 0xa0, 0, memflags, &dom->p2m_host[0x00]);

I think this causes a build failure when building tools/ with -O1: 

[  149s] xc_dom_x86.c: In function 'meminit_hvm':
[  149s] xc_dom_x86.c:1262: error: 'rc' may be used uninitialized in this function
[  149s] make[4]: *** [xc_dom_x86.o] Error 1


Olaf

[PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Roger Pau Monne]

From: Roger Pau Monne <royger@FreeBSD.org>

Due to the HVMlite changes there's a chance that the value in rc is checked
without being initialised. Fix this by initialising it to 0.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reported-by: Olaf Hering <olaf@aepfle.de>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/xc_dom_x86.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index ef474a8..08337b2 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -1259,7 +1259,7 @@ static int meminit_hvm(struct xc_dom_image *dom)
     unsigned long p2m_size;
     unsigned long target_pages = dom->target_pages;
     unsigned long cur_pages, cur_pfn;
-    int rc;
+    int rc = 0;
     xen_capabilities_info_t caps;
     unsigned long stat_normal_pages = 0, stat_2mb_pages = 0, 
         stat_1gb_pages = 0;
-- 
2.5.4 (Apple Git-61)


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

Re: [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Wei Liu]

On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
> From: Roger Pau Monne <royger@FreeBSD.org>
> 
> Due to the HVMlite changes there's a chance that the value in rc is checked
> without being initialised. Fix this by initialising it to 0.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Reported-by: Olaf Hering <olaf@aepfle.de>

Acked-by: Wei Liu <wei.liu2@citrix.com>

> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  tools/libxc/xc_dom_x86.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index ef474a8..08337b2 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -1259,7 +1259,7 @@ static int meminit_hvm(struct xc_dom_image *dom)
>      unsigned long p2m_size;
>      unsigned long target_pages = dom->target_pages;
>      unsigned long cur_pages, cur_pfn;
> -    int rc;
> +    int rc = 0;
>      xen_capabilities_info_t caps;
>      unsigned long stat_normal_pages = 0, stat_2mb_pages = 0, 
>          stat_1gb_pages = 0;
> -- 
> 2.5.4 (Apple Git-61)
> 

Re: [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Ian Campbell]

On Tue, 2016-02-02 at 12:37 +0000, Wei Liu wrote:
> On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
> > From: Roger Pau Monne <royger@FreeBSD.org>
> > 
> > Due to the HVMlite changes there's a chance that the value in rc is
> > checked
> > without being initialised. Fix this by initialising it to 0.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > Reported-by: Olaf Hering <olaf@aepfle.de>
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

This is CID 1351229, I think?

** CID 1351229:  Uninitialized variables  (UNINIT)
> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> 
> 
> ________________________________________________________________________________________________________
> *** CID 1351229:  Uninitialized variables  (UNINIT)
> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> 1437                 cur_pages = 0xc0;
> 1438                 stat_normal_pages += 0xc0;
> 1439             }
> 1440             else
> 1441                 cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
> 1442     
> >>>     CID 1351229:  Uninitialized variables  (UNINIT)
> >>>     Using uninitialized value "rc".
> 1443             while ( (rc == 0) && (end_pages > cur_pages) )
> 1444             {
> 1445                 /* Clip count to maximum 1GB extent. */
> 1446                 unsigned long count = end_pages - cur_pages;
> 1447                 unsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
> 1448    

Note that this while loop ends with:
        if ( rc != 0 )
            break;
and there are no continue statements.

Therefore I wonder if we would be better off removing the rc == 0 part of
the loop condition?

The issue with this patch is the usual one that it will hide other
unintentional uses of rc before it is set to a good value.

This issue was exposed by a prior "rc = xc_domain_populate_physmap_exact"
becoming conditional on device_model. What is also concerning is the lack
of error checking on that call -- is it really ok to just barrel on under
these circumstance?

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

Re: [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Wei Liu]

On Wed, Feb 03, 2016 at 10:30:54AM +0000, Ian Campbell wrote:
> On Tue, 2016-02-02 at 12:37 +0000, Wei Liu wrote:
> > On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
> > > From: Roger Pau Monne <royger@FreeBSD.org>
> > > 
> > > Due to the HVMlite changes there's a chance that the value in rc is
> > > checked
> > > without being initialised. Fix this by initialising it to 0.
> > > 
> > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > Reported-by: Olaf Hering <olaf@aepfle.de>
> > 
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> This is CID 1351229, I think?
> 

Yes, I think so.

> ** CID 1351229:  Uninitialized variables  (UNINIT)
> > /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> > 
> > 
> > ________________________________________________________________________________________________________
> > *** CID 1351229:  Uninitialized variables  (UNINIT)
> > /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> > 1437                 cur_pages = 0xc0;
> > 1438                 stat_normal_pages += 0xc0;
> > 1439             }
> > 1440             else
> > 1441                 cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
> > 1442     
> > >>>     CID 1351229:  Uninitialized variables  (UNINIT)
> > >>>     Using uninitialized value "rc".
> > 1443             while ( (rc == 0) && (end_pages > cur_pages) )
> > 1444             {
> > 1445                 /* Clip count to maximum 1GB extent. */
> > 1446                 unsigned long count = end_pages - cur_pages;
> > 1447                 unsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
> > 1448    
> 
> Note that this while loop ends with:
>         if ( rc != 0 )
>             break;
> and there are no continue statements.
> 
> Therefore I wonder if we would be better off removing the rc == 0 part of
> the loop condition?
> 

No, there is no if ( rc != 0 )  inside the said while loop.  That "if"
is for the outer for loop.

We could add a test for the while loop, if that looks clearer to you.

> The issue with this patch is the usual one that it will hide other
> unintentional uses of rc before it is set to a good value.
> 
> This issue was exposed by a prior "rc = xc_domain_populate_physmap_exact"
> becoming conditional on device_model. What is also concerning is the lack
> of error checking on that call -- is it really ok to just barrel on under
> these circumstance?
> 

Yeah, that should ideally be fixed, too.

Wei.

> Ian.

Re: [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Roger Pau Monné]

El 3/2/16 a les 11:30, Ian Campbell ha escrit:
> On Tue, 2016-02-02 at 12:37 +0000, Wei Liu wrote:
>> On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
>>> From: Roger Pau Monne <royger@FreeBSD.org>
>>>
>>> Due to the HVMlite changes there's a chance that the value in rc is
>>> checked
>>> without being initialised. Fix this by initialising it to 0.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> Reported-by: Olaf Hering <olaf@aepfle.de>
>>
>> Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> This is CID 1351229, I think?

Looks like, according the the description below.

> 
> ** CID 1351229:  Uninitialized variables  (UNINIT)
>> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
>>  
>>  
>> ________________________________________________________________________________________________________
>> *** CID 1351229:  Uninitialized variables  (UNINIT)
>> /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
>> 1437                 cur_pages = 0xc0;
>> 1438                 stat_normal_pages += 0xc0;
>> 1439             }
>> 1440             else
>> 1441                 cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
>> 1442     
>>>>>      CID 1351229:  Uninitialized variables  (UNINIT)
>>>>>      Using uninitialized value "rc".
>> 1443             while ( (rc == 0) && (end_pages > cur_pages) )
>> 1444             {
>> 1445                 /* Clip count to maximum 1GB extent. */
>> 1446                 unsigned long count = end_pages - cur_pages;
>> 1447                 unsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
>> 1448    
> 
> Note that this while loop ends with:
>         if ( rc != 0 )
>             break;
> and there are no continue statements.
> 
> Therefore I wonder if we would be better off removing the rc == 0 part of
> the loop condition?

We could, but I think we would still have the same issue with the "if (
rc != 0 )" at the end of the loop, AFAICT rc is never unconditionally
set inside of the loop itself, so gcc and coverity would still complain
about uninitialized usage.

> The issue with this patch is the usual one that it will hide other
> unintentional uses of rc before it is set to a good value.
> 
> This issue was exposed by a prior "rc = xc_domain_populate_physmap_exact"
> becoming conditional on device_model. What is also concerning is the lack
> of error checking on that call -- is it really ok to just barrel on under
> these circumstance?

Hm, I guess we then rely on the rc == 0 at the start of the while loop
in order to bail out. IMHO the logic in this function is overly complicated.

Roger.

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

Re: [PATCH] libxc: fix uninitialised usage of rc in meminit_hvm

[Ian Campbell]

On Wed, 2016-02-03 at 11:44 +0100, Roger Pau Monné wrote:
> El 3/2/16 a les 11:30, Ian Campbell ha escrit:
> > On Tue, 2016-02-02 at 12:37 +0000, Wei Liu wrote:
> > > On Tue, Feb 02, 2016 at 12:33:20PM +0100, Roger Pau Monne wrote:
> > > > From: Roger Pau Monne <royger@FreeBSD.org>
> > > > 
> > > > Due to the HVMlite changes there's a chance that the value in rc is
> > > > checked
> > > > without being initialised. Fix this by initialising it to 0.
> > > > 
> > > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > > > Reported-by: Olaf Hering <olaf@aepfle.de>
> > > 
> > > Acked-by: Wei Liu <wei.liu2@citrix.com>
> > 
> > This is CID 1351229, I think?
> 
> Looks like, according the the description below.
> 
> > 
> > ** CID 1351229:  Uninitialized variables  (UNINIT)
> > > /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> > >  
> > >  
> > > _____________________________________________________________________
> > > ___________________________________
> > > *** CID 1351229:  Uninitialized variables  (UNINIT)
> > > /tools/libxc/xc_dom_x86.c: 1443 in meminit_hvm()
> > > 1437                 cur_pages = 0xc0;
> > > 1438                 stat_normal_pages += 0xc0;
> > > 1439             }
> > > 1440             else
> > > 1441                 cur_pages = vmemranges[vmemid].start >>
> > > PAGE_SHIFT;
> > > 1442     
> > > > > >      CID 1351229:  Uninitialized variables  (UNINIT)
> > > > > >      Using uninitialized value "rc".
> > > 1443             while ( (rc == 0) && (end_pages > cur_pages) )
> > > 1444             {
> > > 1445                 /* Clip count to maximum 1GB extent. */
> > > 1446                 unsigned long count = end_pages - cur_pages;
> > > 1447                 unsigned long max_pages = SUPERPAGE_1GB_NR_PFNS;
> > > 1448    
> > 
> > Note that this while loop ends with:
> >         if ( rc != 0 )
> >             break;
> > and there are no continue statements.
> > 
> > Therefore I wonder if we would be better off removing the rc == 0 part
> > of
> > the loop condition?
> 
> We could, but I think we would still have the same issue with the "if (
> rc != 0 )" at the end of the loop, AFAICT rc is never unconditionally
> set inside of the loop itself, so gcc and coverity would still complain
> about uninitialized usage.

Right, I was looking at the wrong loop as Wei pointed out.

I think "rc = 0" before the while might be a reasonable option.

> > The issue with this patch is the usual one that it will hide other
> > unintentional uses of rc before it is set to a good value.
> > 
> > This issue was exposed by a prior "rc =
> > xc_domain_populate_physmap_exact"
> > becoming conditional on device_model. What is also concerning is the
> > lack
> > of error checking on that call -- is it really ok to just barrel on
> > under
> > these circumstance?
> 
> Hm, I guess we then rely on the rc == 0 at the start of the while loop
> in order to bail out. IMHO the logic in this function is overly
> complicated.

Indeed, although we do some other (I suppose pointless) work first in that
case too.

Moving some of it into separate helpers would be a nice further cleanup.

Ian.

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

[PATCH v2] libxc: fix uninitialised usage of rc in meminit_hvm

[Roger Pau Monne]

From: Roger Pau Monne <royger@FreeBSD.org>

Due to the HVMlite changes there's a chance that the value in rc is checked
without being initialised. Fix this by initialising it to 0 prior to the
while loop. Also add a specific error check to a previous populate_physmap
call, this prevents us from overwriting this error.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reported-by: Olaf Hering <olaf@aepfle.de>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v1:
 - Only set rc = 0 prior to the while loop.
 - Add an error check to the previous populate_physmap call.
---
 tools/libxc/xc_dom_x86.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index ef474a8..2a3f64b 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -1412,8 +1412,15 @@ static int meminit_hvm(struct xc_dom_image *dom)
      * ensure that we can be preempted and hence dom0 remains responsive.
      */
     if ( dom->device_model )
+    {
         rc = xc_domain_populate_physmap_exact(
             xch, domid, 0xa0, 0, memflags, &dom->p2m_host[0x00]);
+        if ( rc != 0 )
+        {
+            DOMPRINTF("Could not populate low memory (< 0xA0).\n");
+            goto error_out;
+        }
+    }
 
     stat_normal_pages = 0;
     for ( vmemid = 0; vmemid < nr_vmemranges; vmemid++ )
@@ -1440,6 +1447,7 @@ static int meminit_hvm(struct xc_dom_image *dom)
         else
             cur_pages = vmemranges[vmemid].start >> PAGE_SHIFT;
 
+        rc = 0;
         while ( (rc == 0) && (end_pages > cur_pages) )
         {
             /* Clip count to maximum 1GB extent. */
-- 
2.5.4 (Apple Git-61)


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

Re: [PATCH v2] libxc: fix uninitialised usage of rc in meminit_hvm

[Ian Campbell]

On Wed, 2016-02-03 at 14:21 +0100, Roger Pau Monne wrote:
> From: Roger Pau Monne <royger@FreeBSD.org>

I dropped this as discussed on IRC.

> Due to the HVMlite changes there's a chance that the value in rc is
> checked
> without being initialised. Fix this by initialising it to 0 prior to the
> while loop. Also add a specific error check to a previous
> populate_physmap
> call, this prevents us from overwriting this error.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Reported-by: Olaf Hering <olaf@aepfle.de>

Acked + Applied.


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

[PATCH v10 4/9] x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits and hvm_efer_valid

[Roger Pau Monne]

Loosen up the condition so we make sure that the current vcpu belongs to the
same domain.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
 xen/arch/x86/hvm/hvm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index af3d4d7..92d57ff 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1842,7 +1842,7 @@ static const char * hvm_efer_valid(const struct vcpu *v, uint64_t value,
     {
         unsigned int level;
 
-        ASSERT(v == current);
+        ASSERT(v->domain == current->domain);
         hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
         if ( level >= 0x80000001 )
         {
@@ -1912,7 +1912,7 @@ static unsigned long hvm_cr4_guest_reserved_bits(const struct vcpu *v,
     {
         unsigned int level;
 
-        ASSERT(v == current);
+        ASSERT(v->domain == current->domain);
         hvm_cpuid(0, &level, NULL, NULL, NULL);
         if ( level >= 1 )
             hvm_cpuid(1, NULL, NULL, &leaf1_ecx, &leaf1_edx);
-- 
1.9.5 (Apple Git-50.3)


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

Re: [PATCH v10 4/9] x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits and hvm_efer_valid

[Andrew Cooper]

On 07/12/15 16:48, Roger Pau Monne wrote:
> Loosen up the condition so we make sure that the current vcpu belongs to the
> same domain.
>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>

The "v == current" is a genuine restriction on hvm_cpuid(), but only
matters for areas not probed by these two uses.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Longterm, I will remove the restriction, but there is a substantial
quantity of work before that can happen.

> ---
>  xen/arch/x86/hvm/hvm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index af3d4d7..92d57ff 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1842,7 +1842,7 @@ static const char * hvm_efer_valid(const struct vcpu *v, uint64_t value,
>      {
>          unsigned int level;
>  
> -        ASSERT(v == current);
> +        ASSERT(v->domain == current->domain);
>          hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
>          if ( level >= 0x80000001 )
>          {
> @@ -1912,7 +1912,7 @@ static unsigned long hvm_cr4_guest_reserved_bits(const struct vcpu *v,
>      {
>          unsigned int level;
>  
> -        ASSERT(v == current);
> +        ASSERT(v->domain == current->domain);
>          hvm_cpuid(0, &level, NULL, NULL, NULL);
>          if ( level >= 1 )
>              hvm_cpuid(1, NULL, NULL, &leaf1_ecx, &leaf1_edx);


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

Re: [PATCH v10 4/9] x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits and hvm_efer_valid

[Jan Beulich]

>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -1842,7 +1842,7 @@ static const char * hvm_efer_valid(const struct vcpu *v, uint64_t value,
>      {
>          unsigned int level;
>  
> -        ASSERT(v == current);
> +        ASSERT(v->domain == current->domain);
>          hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
>          if ( level >= 0x80000001 )
>          {
> @@ -1912,7 +1912,7 @@ static unsigned long hvm_cr4_guest_reserved_bits(const struct vcpu *v,
>      {
>          unsigned int level;
>  
> -        ASSERT(v == current);
> +        ASSERT(v->domain == current->domain);
>          hvm_cpuid(0, &level, NULL, NULL, NULL);
>          if ( level >= 1 )
>              hvm_cpuid(1, NULL, NULL, &leaf1_ecx, &leaf1_edx);

The only reason both functions get v passed are the two ASSERT()s.
Relaxing them means you should now be passing a const struct
domain * instead.

Jan

Re: [PATCH v10 4/9] x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits and hvm_efer_valid

[Andrew Cooper]

On 08/12/15 08:28, Jan Beulich wrote:
>>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -1842,7 +1842,7 @@ static const char * hvm_efer_valid(const struct vcpu *v, uint64_t value,
>>      {
>>          unsigned int level;
>>  
>> -        ASSERT(v == current);
>> +        ASSERT(v->domain == current->domain);
>>          hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
>>          if ( level >= 0x80000001 )
>>          {
>> @@ -1912,7 +1912,7 @@ static unsigned long hvm_cr4_guest_reserved_bits(const struct vcpu *v,
>>      {
>>          unsigned int level;
>>  
>> -        ASSERT(v == current);
>> +        ASSERT(v->domain == current->domain);
>>          hvm_cpuid(0, &level, NULL, NULL, NULL);
>>          if ( level >= 1 )
>>              hvm_cpuid(1, NULL, NULL, &leaf1_ecx, &leaf1_edx);
> The only reason both functions get v passed are the two ASSERT()s.
> Relaxing them means you should now be passing a const struct
> domain * instead.

v is correct here.  cpuid information is genuinely per-vcpu, although
this isn't currently reflected in Xen's understanding of the matter.

Part of my further cpuid improvements will be to fix Xen's
understanding. i.e. domain_cpuid() will soon start taking a vcpu parameter.

~Andrew

Re: [PATCH v10 4/9] x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits and hvm_efer_valid

[Jan Beulich]

>>> On 08.12.15 at 12:37, <andrew.cooper3@citrix.com> wrote:
> On 08/12/15 08:28, Jan Beulich wrote:
>>>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -1842,7 +1842,7 @@ static const char * hvm_efer_valid(const struct vcpu 
> *v, uint64_t value,
>>>      {
>>>          unsigned int level;
>>>  
>>> -        ASSERT(v == current);
>>> +        ASSERT(v->domain == current->domain);
>>>          hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
>>>          if ( level >= 0x80000001 )
>>>          {
>>> @@ -1912,7 +1912,7 @@ static unsigned long hvm_cr4_guest_reserved_bits(const 
> struct vcpu *v,
>>>      {
>>>          unsigned int level;
>>>  
>>> -        ASSERT(v == current);
>>> +        ASSERT(v->domain == current->domain);
>>>          hvm_cpuid(0, &level, NULL, NULL, NULL);
>>>          if ( level >= 1 )
>>>              hvm_cpuid(1, NULL, NULL, &leaf1_ecx, &leaf1_edx);
>> The only reason both functions get v passed are the two ASSERT()s.
>> Relaxing them means you should now be passing a const struct
>> domain * instead.
> 
> v is correct here.  cpuid information is genuinely per-vcpu, although
> this isn't currently reflected in Xen's understanding of the matter.

You can't have both: Either the ASSERT() remains as it was, or
you stand on the position voiced along with your R-b that only
the domain matters here (in which case passing around v just to
obtain v->domain is bogus).

Jan

Re: [PATCH v10 4/9] x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits and hvm_efer_valid

[Andrew Cooper]

On 08/12/15 12:54, Jan Beulich wrote:
>>>> On 08.12.15 at 12:37, <andrew.cooper3@citrix.com> wrote:
>> On 08/12/15 08:28, Jan Beulich wrote:
>>>>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
>>>> --- a/xen/arch/x86/hvm/hvm.c
>>>> +++ b/xen/arch/x86/hvm/hvm.c
>>>> @@ -1842,7 +1842,7 @@ static const char * hvm_efer_valid(const struct vcpu 
>> *v, uint64_t value,
>>>>      {
>>>>          unsigned int level;
>>>>  
>>>> -        ASSERT(v == current);
>>>> +        ASSERT(v->domain == current->domain);
>>>>          hvm_cpuid(0x80000000, &level, NULL, NULL, NULL);
>>>>          if ( level >= 0x80000001 )
>>>>          {
>>>> @@ -1912,7 +1912,7 @@ static unsigned long hvm_cr4_guest_reserved_bits(const 
>> struct vcpu *v,
>>>>      {
>>>>          unsigned int level;
>>>>  
>>>> -        ASSERT(v == current);
>>>> +        ASSERT(v->domain == current->domain);
>>>>          hvm_cpuid(0, &level, NULL, NULL, NULL);
>>>>          if ( level >= 1 )
>>>>              hvm_cpuid(1, NULL, NULL, &leaf1_ecx, &leaf1_edx);
>>> The only reason both functions get v passed are the two ASSERT()s.
>>> Relaxing them means you should now be passing a const struct
>>> domain * instead.
>> v is correct here.  cpuid information is genuinely per-vcpu, although
>> this isn't currently reflected in Xen's understanding of the matter.
> You can't have both: Either the ASSERT() remains as it was, or
> you stand on the position voiced along with your R-b that only
> the domain matters here (in which case passing around v just to
> obtain v->domain is bogus).

hvm_cpuid() uses current.

This behavior is problematic and I will be removing its dependence on
current with future work (passing v instead and tweaking some of the
lower level bits), but at the moment it is hard requirement.

As a result, the ASSERT(v == current) was correct.

Roger is introducing a new codepath where v != current, but v->domain ==
current->domain.  The specific cpuid leafs requested from hvm_cpuid() do
indeed only use current->domain, which is the basis of my R-b

It is very definitely wrong to remove the current check; the ASSERT()
needs to stay.  However, the relaxation of the constraint is acceptable
until the cpuid infrastructure gets fixed up, at which point the
ASSERT() can disappear.

As v is the eventual correct parameter to be passed here, I do not want
to see it changed to a domain, just for me to revert that change shortly.

Therefore, the original patch is correct and I stand by my R-b.

~Andrew

Re: [PATCH v10 4/9] x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits and hvm_efer_valid

[Jan Beulich]

>>> On 08.12.15 at 15:43, <andrew.cooper3@citrix.com> wrote:
> As v is the eventual correct parameter to be passed here, I do not want
> to see it changed to a domain, just for me to revert that change shortly.

I'll take you on the "shortly" here (keeping in mind that it looks like
this was meant to happen "shortly" for quite some time), and will
commit the patch as is on that basis, despite my disagreement.

Jan

[PATCH v10 5/9] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Roger Pau Monne]

Allow the usage of the VCPUOP_initialise, VCPUOP_up, VCPUOP_down,
VCPUOP_is_up, VCPUOP_get_physid and VCPUOP_send_nmi hypercalls from HVM
guests.

This patch introduces a new structure (vcpu_hvm_context) that should be used
in conjuction with the VCPUOP_initialise hypercall in order to initialize
vCPUs for HVM guests.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@citrix.com>
---
Changes since v9:
 - s/arch_initialize_vcpu/arch_initialise_vcpu.
 - s/default_initalize_vcpu/default_initialise_vcpu.
 - Expanded the SEG macro in order to also de the sanity checking.
 - Removed setting the .sel field in the 32bit macro.
 - Removed setting the .sel and .base fields in the 64bit SEG macro.
 - Fixed the calls to hvm_cr4_guest_reserved_bits and hvm_efer_valid in
   order to perform the checking against the guest cpuid features.
 - Moved the prototype of default_initialise_vcpu so it's after
   arch_initialise_vcpu.
 - Remove the HVM wrappers around vcpu_op, all vcpu ops are now available to
   HVM guests.

Changes since v8:
 - Create a generic default_initalize_vcpu function that can be shared with
   x86 PV guests and ARM guests.
 - Pass check_segment register by reference.
 - Check that code/data segments always have the 'S' attribute bit set.
 - Fix some of the checks done in check_segment.
 - Fix indentation of gprintk calls.
 - Remove failure messages in case the padding fields are not zero.
 - Make hvm_efer_valid and hvm_cr4_guest_reserved_bits global and use them
   to check the input values provided by the user.
 - Only check the attribute field in order to figure out if a segment is
   null.
 - Make sure the TR segment is a system segment.

Changes since v7:
 - Improved error messages.
 - Set EFER.LMA if EFER.LME is set by the user.
 - Fix calculation of CS limit.
 - Add more checks to segment registers.
 - Add checks to make sure padding fields are 0.
 - Remove ugly arch ifdefs from common domain.c.
 - Add the implicit padding of vcpu_hvm_x86_32 explicitly in the structure.
 - Simplify the compat vcpu code since it's only used on x86.

Changes since v6:
 - Add comments to clarify some initializations.
 - Introduce a generic default_initialize_vcpu that's used to initialize a
   ARM vCPU or a x86 PV vCPU.
 - Move the undef of the SEG macro.
 - Fix the size of the eflags register, it should be 32bits.
 - Add a comment regarding the value of the 12-15 bits of the _ar fields.
 - Remove the 16bit strucutre, the 32bit one can be used to start the cpu in
   real mode.
 - Add some sanity checks to the values passed in.
 - Add paddings to vcpu_hvm_context so the layout on 32/64bits is the same.
 - Add support for the compat version of VCPUOP_initialise.

Changes since v5:
 - Fix a coding style issue.
 - Merge the code from wip-dmlite-v5-refactor by Andrew in order to reduce
   bloat.
 - Print the offending %cr3 in case of error when using shadow.
 - Reduce the scope of local variables in arch_initialize_vcpu.
 - s/current->domain/v->domain/g in arch_initialize_vcpu.
 - Expand the comment in public/vcpu.h to document the usage of
   vcpu_hvm_context for HVM guests.
 - Add myself as the copyright holder for the public hvm_vcpu.h header.

Changes since v4:
 - Don't assume mode is 64B, add an explicit check.
 - Don't set TF_kernel_mode, it is only needed for PV guests.
 - Don't set CR0_ET unconditionally.
---
 xen/arch/arm/domain.c             |   5 +
 xen/arch/x86/domain.c             | 308 ++++++++++++++++++++++++++++++++++++++
 xen/arch/x86/hvm/hvm.c            |  61 +-------
 xen/common/compat/domain.c        |  50 +++++--
 xen/common/domain.c               |  41 +++--
 xen/include/Makefile              |   1 +
 xen/include/asm-x86/domain.h      |   3 +
 xen/include/asm-x86/hvm/hvm.h     |   5 +
 xen/include/public/hvm/hvm_vcpu.h | 144 ++++++++++++++++++
 xen/include/public/vcpu.h         |   6 +-
 xen/include/xen/domain.h          |   3 +
 xen/include/xlat.lst              |   3 +
 12 files changed, 541 insertions(+), 89 deletions(-)
 create mode 100644 xen/include/public/hvm/hvm_vcpu.h

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 860ac7d..3d274ae 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -756,6 +756,11 @@ int arch_set_info_guest(
     return 0;
 }
 
+int arch_initialise_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    return default_initialise_vcpu(v, arg);
+}
+
 int arch_vcpu_reset(struct vcpu *v)
 {
     vcpu_end_shutdown_deferral(v);
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index df40dc6..ca5247d 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -37,6 +37,7 @@
 #include <xen/wait.h>
 #include <xen/guest_access.h>
 #include <public/sysctl.h>
+#include <public/hvm/hvm_vcpu.h>
 #include <asm/regs.h>
 #include <asm/mc146818rtc.h>
 #include <asm/system.h>
@@ -1188,6 +1189,313 @@ int arch_set_info_guest(
 #undef c
 }
 
+static inline int check_segment(struct segment_register *reg,
+                                enum x86_segment seg)
+{
+
+    if ( reg->attr.fields.pad != 0 )
+    {
+        gprintk(XENLOG_ERR,
+                "Attribute bits 12-15 of the segment are not zero\n");
+        return -EINVAL;
+    }
+
+    if ( reg->attr.bytes == 0 )
+    {
+        if ( seg != x86_seg_ds && seg != x86_seg_es )
+        {
+            gprintk(XENLOG_ERR, "Null selector provided for CS, SS or TR\n");
+            return -EINVAL;
+        }
+        return 0;
+    }
+
+    if ( seg != x86_seg_tr && !reg->attr.fields.s )
+    {
+        gprintk(XENLOG_ERR,
+                "System segment provided for a code or data segment\n");
+        return -EINVAL;
+    }
+
+    if ( seg == x86_seg_tr && reg->attr.fields.s )
+    {
+        gprintk(XENLOG_ERR,
+                "Code or data segment provided for TR\n");
+        return -EINVAL;
+    }
+
+    if ( !reg->attr.fields.p )
+    {
+        gprintk(XENLOG_ERR, "Non-present segment provided\n");
+        return -EINVAL;
+    }
+
+    if ( seg == x86_seg_cs && !(reg->attr.fields.type & 0x8) )
+    {
+        gprintk(XENLOG_ERR, "CS is missing code type\n");
+        return -EINVAL;
+    }
+
+    if ( seg == x86_seg_ss &&
+         ((reg->attr.fields.type & 0x8) || !(reg->attr.fields.type & 0x2)) )
+    {
+        gprintk(XENLOG_ERR,
+                "Unable load a code or non writeable segment into SS\n");
+        return -EINVAL;
+    }
+
+    if ( reg->attr.fields.s && seg != x86_seg_ss && seg != x86_seg_cs &&
+         (reg->attr.fields.type & 0x8) && !(reg->attr.fields.type & 0x2) )
+    {
+        gprintk(XENLOG_ERR,
+                "Unable to load a non readable code segment into DS or ES\n");
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
+/* Called by VCPUOP_initialise for HVM guests. */
+int arch_set_info_hvm_guest(struct vcpu *v, const vcpu_hvm_context_t *ctx)
+{
+    struct cpu_user_regs *uregs = &v->arch.user_regs;
+    struct segment_register cs, ds, ss, es, tr;
+    const char *errstr;
+    int rc;
+
+    if ( ctx->pad != 0 )
+        return -EINVAL;
+
+    switch ( ctx->mode )
+    {
+    default:
+        return -EINVAL;
+
+    case VCPU_HVM_MODE_32B:
+    {
+        const struct vcpu_hvm_x86_32 *regs = &ctx->cpu_regs.x86_32;
+        uint32_t limit;
+
+        if ( ctx->cpu_regs.x86_32.pad1 != 0 ||
+             ctx->cpu_regs.x86_32.pad2[0] != 0 ||
+             ctx->cpu_regs.x86_32.pad2[1] != 0 ||
+             ctx->cpu_regs.x86_32.pad2[2] != 0 )
+            return -EINVAL;
+
+#define SEG(s, r) ({                                                        \
+    s = (struct segment_register){ .base = (r)->s ## _base,                 \
+                                   .limit = (r)->s ## _limit,               \
+                                   .attr.bytes = (r)->s ## _ar };           \
+    check_segment(&s, x86_seg_ ## s); })
+
+        rc = SEG(cs, regs);
+        rc |= SEG(ds, regs);
+        rc |= SEG(ss, regs);
+        rc |= SEG(es, regs);
+        rc |= SEG(tr, regs);
+#undef SEG
+
+        if ( rc != 0 )
+            return rc;
+
+        /* Basic sanity checks. */
+        limit = cs.limit;
+        if ( cs.attr.fields.g )
+            limit = (limit << 12) | 0xfff;
+        if ( regs->eip > limit )
+        {
+            gprintk(XENLOG_ERR, "EIP (%#08x) outside CS limit (%#08x)\n",
+                    regs->eip, limit);
+            return -EINVAL;
+        }
+
+        if ( ss.attr.fields.dpl != cs.attr.fields.dpl )
+        {
+            gprintk(XENLOG_ERR, "SS.DPL (%u) is different than CS.DPL (%u)\n",
+                    ss.attr.fields.dpl, cs.attr.fields.dpl);
+            return -EINVAL;
+        }
+
+        if ( ds.attr.fields.p && ds.attr.fields.dpl > cs.attr.fields.dpl )
+        {
+            gprintk(XENLOG_ERR, "DS.DPL (%u) is greater than CS.DPL (%u)\n",
+                    ds.attr.fields.dpl, cs.attr.fields.dpl);
+            return -EINVAL;
+        }
+
+        if ( es.attr.fields.p && es.attr.fields.dpl > cs.attr.fields.dpl )
+        {
+            gprintk(XENLOG_ERR, "ES.DPL (%u) is greater than CS.DPL (%u)\n",
+                    es.attr.fields.dpl, cs.attr.fields.dpl);
+            return -EINVAL;
+        }
+
+        if ( (regs->efer & EFER_LMA) && !(regs->efer & EFER_LME) )
+        {
+            gprintk(XENLOG_ERR, "EFER.LMA set without EFER.LME (%#016lx)\n",
+                    regs->efer);
+            return -EINVAL;
+        }
+
+        uregs->rax    = regs->eax;
+        uregs->rcx    = regs->ecx;
+        uregs->rdx    = regs->edx;
+        uregs->rbx    = regs->ebx;
+        uregs->rsp    = regs->esp;
+        uregs->rbp    = regs->ebp;
+        uregs->rsi    = regs->esi;
+        uregs->rdi    = regs->edi;
+        uregs->rip    = regs->eip;
+        uregs->rflags = regs->eflags;
+
+        v->arch.hvm_vcpu.guest_cr[0] = regs->cr0;
+        v->arch.hvm_vcpu.guest_cr[3] = regs->cr3;
+        v->arch.hvm_vcpu.guest_cr[4] = regs->cr4;
+        v->arch.hvm_vcpu.guest_efer  = regs->efer;
+    }
+    break;
+
+    case VCPU_HVM_MODE_64B:
+    {
+        const struct vcpu_hvm_x86_64 *regs = &ctx->cpu_regs.x86_64;
+
+        /* Basic sanity checks. */
+        if ( !is_canonical_address(regs->rip) )
+        {
+            gprintk(XENLOG_ERR, "RIP contains a non-canonical address (%#lx)\n",
+                    regs->rip);
+            return -EINVAL;
+        }
+
+        if ( !(regs->cr0 & X86_CR0_PG) )
+        {
+            gprintk(XENLOG_ERR, "CR0 doesn't have paging enabled (%#016lx)\n",
+                    regs->cr0);
+            return -EINVAL;
+        }
+
+        if ( !(regs->cr4 & X86_CR4_PAE) )
+        {
+            gprintk(XENLOG_ERR, "CR4 doesn't have PAE enabled (%#016lx)\n",
+                    regs->cr4);
+            return -EINVAL;
+        }
+
+        if ( !(regs->efer & EFER_LME) )
+        {
+            gprintk(XENLOG_ERR, "EFER doesn't have LME enabled (%#016lx)\n",
+                    regs->efer);
+            return -EINVAL;
+        }
+
+        uregs->rax    = regs->rax;
+        uregs->rcx    = regs->rcx;
+        uregs->rdx    = regs->rdx;
+        uregs->rbx    = regs->rbx;
+        uregs->rsp    = regs->rsp;
+        uregs->rbp    = regs->rbp;
+        uregs->rsi    = regs->rsi;
+        uregs->rdi    = regs->rdi;
+        uregs->rip    = regs->rip;
+        uregs->rflags = regs->rflags;
+
+        v->arch.hvm_vcpu.guest_cr[0] = regs->cr0;
+        v->arch.hvm_vcpu.guest_cr[3] = regs->cr3;
+        v->arch.hvm_vcpu.guest_cr[4] = regs->cr4;
+        v->arch.hvm_vcpu.guest_efer  = regs->efer;
+
+#define SEG(l, a) (struct segment_register){ .limit = (l), .attr.bytes = (a) }
+        cs = SEG(~0u, 0xa9b); /* 64bit code segment. */
+        ds = ss = es = SEG(~0u, 0xc93);
+        tr = SEG(0x67, 0x8b); /* 64bit TSS (busy). */
+#undef SEG
+    }
+    break;
+
+    }
+
+    if ( v->arch.hvm_vcpu.guest_efer & EFER_LME )
+        v->arch.hvm_vcpu.guest_efer |= EFER_LMA;
+
+    if ( v->arch.hvm_vcpu.guest_cr[4] & hvm_cr4_guest_reserved_bits(v, 0) )
+    {
+        gprintk(XENLOG_ERR, "Bad CR4 value: %#016lx\n",
+                v->arch.hvm_vcpu.guest_cr[4]);
+        return -EINVAL;
+    }
+
+    errstr = hvm_efer_valid(v, v->arch.hvm_vcpu.guest_efer, -1);
+    if ( errstr )
+    {
+        gprintk(XENLOG_ERR, "Bad EFER value (%#016lx): %s\n",
+               v->arch.hvm_vcpu.guest_efer, errstr);
+        return -EINVAL;
+    }
+
+    hvm_update_guest_cr(v, 0);
+    hvm_update_guest_cr(v, 3);
+    hvm_update_guest_cr(v, 4);
+    hvm_update_guest_efer(v);
+
+    if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
+    {
+        /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
+        struct page_info *page = get_page_from_gfn(v->domain,
+                                 v->arch.hvm_vcpu.guest_cr[3] >> PAGE_SHIFT,
+                                 NULL, P2M_ALLOC);
+        if ( !page )
+        {
+            gprintk(XENLOG_ERR, "Invalid CR3: %#lx\n",
+                    v->arch.hvm_vcpu.guest_cr[3]);
+            domain_crash(v->domain);
+            return -EINVAL;
+        }
+
+        v->arch.guest_table = pagetable_from_page(page);
+    }
+
+    hvm_set_segment_register(v, x86_seg_cs, &cs);
+    hvm_set_segment_register(v, x86_seg_ds, &ds);
+    hvm_set_segment_register(v, x86_seg_ss, &ss);
+    hvm_set_segment_register(v, x86_seg_es, &es);
+    hvm_set_segment_register(v, x86_seg_tr, &tr);
+
+    /* Sync AP's TSC with BSP's. */
+    v->arch.hvm_vcpu.cache_tsc_offset =
+        v->domain->vcpu[0]->arch.hvm_vcpu.cache_tsc_offset;
+    hvm_funcs.set_tsc_offset(v, v->arch.hvm_vcpu.cache_tsc_offset,
+                             v->domain->arch.hvm_domain.sync_tsc);
+
+    paging_update_paging_modes(v);
+
+    v->is_initialised = 1;
+    set_bit(_VPF_down, &v->pause_flags);
+
+    return 0;
+}
+
+int arch_initialise_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    int rc;
+
+    if ( is_hvm_vcpu(v) )
+    {
+        struct domain *d = v->domain;
+        struct vcpu_hvm_context ctxt;
+
+        if ( copy_from_guest(&ctxt, arg, 1) )
+            return -EFAULT;
+
+        domain_lock(d);
+        rc = v->is_initialised ? -EEXIST : arch_set_info_hvm_guest(v, &ctxt);
+        domain_unlock(d);
+    }
+    else
+        rc = default_initialise_vcpu(v, arg);
+
+    return rc;
+}
+
 int arch_vcpu_reset(struct vcpu *v)
 {
     if ( is_pv_vcpu(v) )
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 92d57ff..08cef1f 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -1833,8 +1833,8 @@ static int hvm_save_cpu_ctxt(struct domain *d, hvm_domain_context_t *h)
 }
 
 /* Return a string indicating the error, or NULL for valid. */
-static const char * hvm_efer_valid(const struct vcpu *v, uint64_t value,
-                                   signed int cr0_pg)
+const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
+                           signed int cr0_pg)
 {
     unsigned int ext1_ecx = 0, ext1_edx = 0;
 
@@ -1902,8 +1902,7 @@ static const char * hvm_efer_valid(const struct vcpu *v, uint64_t value,
         X86_CR0_CD | X86_CR0_PG)))
 
 /* These bits in CR4 cannot be set by the guest. */
-static unsigned long hvm_cr4_guest_reserved_bits(const struct vcpu *v,
-                                                 bool_t restore)
+unsigned long hvm_cr4_guest_reserved_bits(const struct vcpu *v,bool_t restore)
 {
     unsigned int leaf1_ecx = 0, leaf1_edx = 0;
     unsigned int leaf7_0_ebx = 0, leaf7_0_ecx = 0;
@@ -5108,31 +5107,6 @@ static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     }
 }
 
-static long hvm_vcpu_op(
-    int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
-{
-    long rc;
-
-    switch ( cmd )
-    {
-    case VCPUOP_register_runstate_memory_area:
-    case VCPUOP_get_runstate_info:
-    case VCPUOP_set_periodic_timer:
-    case VCPUOP_stop_periodic_timer:
-    case VCPUOP_set_singleshot_timer:
-    case VCPUOP_stop_singleshot_timer:
-    case VCPUOP_register_vcpu_info:
-    case VCPUOP_register_vcpu_time_memory_area:
-        rc = do_vcpu_op(cmd, vcpuid, arg);
-        break;
-    default:
-        rc = -ENOSYS;
-        break;
-    }
-
-    return rc;
-}
-
 typedef unsigned long hvm_hypercall_t(
     unsigned long, unsigned long, unsigned long, unsigned long, unsigned long,
     unsigned long);
@@ -5166,31 +5140,6 @@ static long hvm_memory_op_compat32(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
     return compat_memory_op(cmd, arg);
 }
 
-static long hvm_vcpu_op_compat32(
-    int cmd, unsigned vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
-{
-    long rc;
-
-    switch ( cmd )
-    {
-    case VCPUOP_register_runstate_memory_area:
-    case VCPUOP_get_runstate_info:
-    case VCPUOP_set_periodic_timer:
-    case VCPUOP_stop_periodic_timer:
-    case VCPUOP_set_singleshot_timer:
-    case VCPUOP_stop_singleshot_timer:
-    case VCPUOP_register_vcpu_info:
-    case VCPUOP_register_vcpu_time_memory_area:
-        rc = compat_vcpu_op(cmd, vcpuid, arg);
-        break;
-    default:
-        rc = -ENOSYS;
-        break;
-    }
-
-    return rc;
-}
-
 static long hvm_physdev_op_compat32(
     int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
@@ -5212,7 +5161,7 @@ static long hvm_physdev_op_compat32(
 static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = {
     [ __HYPERVISOR_memory_op ] = (hvm_hypercall_t *)hvm_memory_op,
     [ __HYPERVISOR_grant_table_op ] = (hvm_hypercall_t *)hvm_grant_table_op,
-    [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t *)hvm_vcpu_op,
+    HYPERCALL(vcpu_op),
     [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op,
     HYPERCALL(xen_version),
     HYPERCALL(console_io),
@@ -5233,7 +5182,7 @@ static hvm_hypercall_t *const hvm_hypercall64_table[NR_hypercalls] = {
 static hvm_hypercall_t *const hvm_hypercall32_table[NR_hypercalls] = {
     [ __HYPERVISOR_memory_op ] = (hvm_hypercall_t *)hvm_memory_op_compat32,
     [ __HYPERVISOR_grant_table_op ] = (hvm_hypercall_t *)hvm_grant_table_op_compat32,
-    [ __HYPERVISOR_vcpu_op ] = (hvm_hypercall_t *)hvm_vcpu_op_compat32,
+    COMPAT_CALL(vcpu_op),
     [ __HYPERVISOR_physdev_op ] = (hvm_hypercall_t *)hvm_physdev_op_compat32,
     COMPAT_CALL(xen_version),
     HYPERCALL(console_io),
diff --git a/xen/common/compat/domain.c b/xen/common/compat/domain.c
index c60d824..88bfdc8 100644
--- a/xen/common/compat/domain.c
+++ b/xen/common/compat/domain.c
@@ -11,6 +11,7 @@ asm(".file \"" __FILE__ "\"");
 #include <xen/guest_access.h>
 #include <xen/hypercall.h>
 #include <compat/vcpu.h>
+#include <compat/hvm/hvm_vcpu.h>
 
 #define xen_vcpu_set_periodic_timer vcpu_set_periodic_timer
 CHECK_vcpu_set_periodic_timer;
@@ -24,6 +25,14 @@ CHECK_SIZE_(struct, vcpu_info);
 CHECK_vcpu_register_vcpu_info;
 #undef xen_vcpu_register_vcpu_info
 
+#define xen_vcpu_hvm_context vcpu_hvm_context
+#define xen_vcpu_hvm_x86_32 vcpu_hvm_x86_32
+#define xen_vcpu_hvm_x86_64 vcpu_hvm_x86_64
+CHECK_vcpu_hvm_context;
+#undef xen_vcpu_hvm_x86_64
+#undef xen_vcpu_hvm_x86_32
+#undef xen_vcpu_hvm_context
+
 int compat_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct domain *d = current->domain;
@@ -37,33 +46,44 @@ int compat_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) ar
     {
     case VCPUOP_initialise:
     {
-        struct compat_vcpu_guest_context *cmp_ctxt;
-
         if ( v->vcpu_info == &dummy_vcpu_info )
             return -EINVAL;
 
-        if ( (cmp_ctxt = xmalloc(struct compat_vcpu_guest_context)) == NULL )
+        if ( is_hvm_vcpu(v) )
         {
-            rc = -ENOMEM;
-            break;
-        }
+            struct vcpu_hvm_context ctxt;
 
-        if ( copy_from_guest(cmp_ctxt, arg, 1) )
-        {
-            xfree(cmp_ctxt);
-            rc = -EFAULT;
-            break;
+            if ( copy_from_guest(&ctxt, arg, 1) )
+                return -EFAULT;
+
+            domain_lock(d);
+            rc = v->is_initialised ? -EEXIST : arch_set_info_hvm_guest(v, &ctxt);
+            domain_unlock(d);
         }
+        else
+        {
+            struct compat_vcpu_guest_context *ctxt;
+
+            if ( (ctxt = xmalloc(struct compat_vcpu_guest_context)) == NULL )
+                return -ENOMEM;
+
+            if ( copy_from_guest(ctxt, arg, 1) )
+            {
+                xfree(ctxt);
+                return -EFAULT;
+            }
 
-        domain_lock(d);
-        rc = v->is_initialised ? -EEXIST : arch_set_info_guest(v, cmp_ctxt);
-        domain_unlock(d);
+            domain_lock(d);
+            rc = v->is_initialised ? -EEXIST : arch_set_info_guest(v, ctxt);
+            domain_unlock(d);
+
+            xfree(ctxt);
+        }
 
         if ( rc == -ERESTART )
             rc = hypercall_create_continuation(__HYPERVISOR_vcpu_op, "iuh",
                                                cmd, vcpuid, arg);
 
-        xfree(cmp_ctxt);
         break;
     }
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index f56b7ff..c4661d8 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1208,11 +1208,34 @@ void unmap_vcpu_info(struct vcpu *v)
     put_page_and_type(mfn_to_page(mfn));
 }
 
+int default_initialise_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
+{
+    struct vcpu_guest_context *ctxt;
+    struct domain *d = v->domain;
+    int rc;
+
+    if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
+        return -ENOMEM;
+
+    if ( copy_from_guest(ctxt, arg, 1) )
+    {
+        free_vcpu_guest_context(ctxt);
+        return -EFAULT;
+    }
+
+    domain_lock(d);
+    rc = v->is_initialised ? -EEXIST : arch_set_info_guest(v, ctxt);
+    domain_unlock(d);
+
+    free_vcpu_guest_context(ctxt);
+
+    return rc;
+}
+
 long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     struct domain *d = current->domain;
     struct vcpu *v;
-    struct vcpu_guest_context *ctxt;
     long rc = 0;
 
     if ( vcpuid >= d->max_vcpus || (v = d->vcpu[vcpuid]) == NULL )
@@ -1224,21 +1247,7 @@ long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( v->vcpu_info == &dummy_vcpu_info )
             return -EINVAL;
 
-        if ( (ctxt = alloc_vcpu_guest_context()) == NULL )
-            return -ENOMEM;
-
-        if ( copy_from_guest(ctxt, arg, 1) )
-        {
-            free_vcpu_guest_context(ctxt);
-            return -EFAULT;
-        }
-
-        domain_lock(d);
-        rc = v->is_initialised ? -EEXIST : arch_set_info_guest(v, ctxt);
-        domain_unlock(d);
-
-        free_vcpu_guest_context(ctxt);
-
+        rc = arch_initialise_vcpu(v, arg);
         if ( rc == -ERESTART )
             rc = hypercall_create_continuation(__HYPERVISOR_vcpu_op, "iuh",
                                                cmd, vcpuid, arg);
diff --git a/xen/include/Makefile b/xen/include/Makefile
index c674f7f..94ba3d8 100644
--- a/xen/include/Makefile
+++ b/xen/include/Makefile
@@ -26,6 +26,7 @@ headers-$(CONFIG_X86)     += compat/arch-x86/pmu.h
 headers-$(CONFIG_X86)     += compat/arch-x86/xen-mca.h
 headers-$(CONFIG_X86)     += compat/arch-x86/xen.h
 headers-$(CONFIG_X86)     += compat/arch-x86/xen-$(compat-arch-y).h
+headers-$(CONFIG_X86)     += compat/hvm/hvm_vcpu.h
 headers-y                 += compat/arch-$(compat-arch-y).h compat/pmu.h compat/xlat.h
 headers-$(FLASK_ENABLE)   += compat/xsm/flask_op.h
 
diff --git a/xen/include/asm-x86/domain.h b/xen/include/asm-x86/domain.h
index c825975..e8f7037 100644
--- a/xen/include/asm-x86/domain.h
+++ b/xen/include/asm-x86/domain.h
@@ -600,6 +600,9 @@ static inline void free_vcpu_guest_context(struct vcpu_guest_context *vgc)
     vfree(vgc);
 }
 
+struct vcpu_hvm_context;
+int arch_set_info_hvm_guest(struct vcpu *v, const struct vcpu_hvm_context *ctx);
+
 #endif /* __ASM_DOMAIN_H__ */
 
 /*
diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h
index f80e143..b9d893d 100644
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -560,6 +560,11 @@ void altp2m_vcpu_update_vmfunc_ve(struct vcpu *v);
 /* emulates #VE */
 bool_t altp2m_vcpu_emulate_ve(struct vcpu *v);
 
+/* Check CR4/EFER values */
+const char *hvm_efer_valid(const struct vcpu *v, uint64_t value,
+                           signed int cr0_pg);
+unsigned long hvm_cr4_guest_reserved_bits(const struct vcpu *v, bool_t restore);
+
 #endif /* __ASM_X86_HVM_HVM_H__ */
 
 /*
diff --git a/xen/include/public/hvm/hvm_vcpu.h b/xen/include/public/hvm/hvm_vcpu.h
new file mode 100644
index 0000000..d21abf1
--- /dev/null
+++ b/xen/include/public/hvm/hvm_vcpu.h
@@ -0,0 +1,144 @@
+/*
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to
+ * deal in the Software without restriction, including without limitation the
+ * rights to use, copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ * Copyright (c) 2015, Roger Pau Monne <roger.pau@citrix.com>
+ */
+
+#ifndef __XEN_PUBLIC_HVM_HVM_VCPU_H__
+#define __XEN_PUBLIC_HVM_HVM_VCPU_H__
+
+#include "../xen.h"
+
+struct vcpu_hvm_x86_32 {
+    uint32_t eax;
+    uint32_t ecx;
+    uint32_t edx;
+    uint32_t ebx;
+    uint32_t esp;
+    uint32_t ebp;
+    uint32_t esi;
+    uint32_t edi;
+    uint32_t eip;
+    uint32_t eflags;
+
+    uint32_t cr0;
+    uint32_t cr3;
+    uint32_t cr4;
+
+    uint32_t pad1;
+
+    /*
+     * EFER should only be used to set the NXE bit (if required)
+     * when starting a vCPU in 32bit mode with paging enabled or
+     * to set the LME/LMA bits in order to start the vCPU in
+     * compatibility mode.
+     */
+    uint64_t efer;
+
+    uint32_t cs_base;
+    uint32_t ds_base;
+    uint32_t ss_base;
+    uint32_t es_base;
+    uint32_t tr_base;
+    uint32_t cs_limit;
+    uint32_t ds_limit;
+    uint32_t ss_limit;
+    uint32_t es_limit;
+    uint32_t tr_limit;
+    uint16_t cs_ar;
+    uint16_t ds_ar;
+    uint16_t ss_ar;
+    uint16_t es_ar;
+    uint16_t tr_ar;
+
+    uint16_t pad2[3];
+};
+
+/*
+ * The layout of the _ar fields of the segment registers is the
+ * following:
+ *
+ * Bits   [0,3]: type (bits 40-43).
+ * Bit        4: s    (descriptor type, bit 44).
+ * Bit    [5,6]: dpl  (descriptor privilege level, bits 45-46).
+ * Bit        7: p    (segment-present, bit 47).
+ * Bit        8: avl  (available for system software, bit 52).
+ * Bit        9: l    (64-bit code segment, bit 53).
+ * Bit       10: db   (meaning depends on the segment, bit 54).
+ * Bit       11: g    (granularity, bit 55)
+ * Bits [12,15]: unused, must be blank.
+ *
+ * A more complete description of the meaning of this fields can be
+ * obtained from the Intel SDM, Volume 3, section 3.4.5.
+ */
+
+struct vcpu_hvm_x86_64 {
+    uint64_t rax;
+    uint64_t rcx;
+    uint64_t rdx;
+    uint64_t rbx;
+    uint64_t rsp;
+    uint64_t rbp;
+    uint64_t rsi;
+    uint64_t rdi;
+    uint64_t rip;
+    uint64_t rflags;
+
+    uint64_t cr0;
+    uint64_t cr3;
+    uint64_t cr4;
+    uint64_t efer;
+
+    /*
+     * Using VCPU_HVM_MODE_64B implies that the vCPU is launched
+     * directly in long mode, so the cached parts of the segment
+     * registers get set to match that environment.
+     *
+     * If the user wants to launch the vCPU in compatibility mode
+     * the 32-bit structure should be used instead.
+     */
+};
+
+struct vcpu_hvm_context {
+#define VCPU_HVM_MODE_32B 0  /* 32bit fields of the structure will be used. */
+#define VCPU_HVM_MODE_64B 1  /* 64bit fields of the structure will be used. */
+    uint32_t mode;
+
+    uint32_t pad;
+
+    /* CPU registers. */
+    union {
+        struct vcpu_hvm_x86_32 x86_32;
+        struct vcpu_hvm_x86_64 x86_64;
+    } cpu_regs;
+};
+typedef struct vcpu_hvm_context vcpu_hvm_context_t;
+DEFINE_XEN_GUEST_HANDLE(vcpu_hvm_context_t);
+
+#endif /* __XEN_PUBLIC_HVM_HVM_VCPU_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/public/vcpu.h b/xen/include/public/vcpu.h
index 898b89f..692b87a 100644
--- a/xen/include/public/vcpu.h
+++ b/xen/include/public/vcpu.h
@@ -41,8 +41,10 @@
  * Initialise a VCPU. Each VCPU can be initialised only once. A 
  * newly-initialised VCPU will not run until it is brought up by VCPUOP_up.
  * 
- * @extra_arg == pointer to vcpu_guest_context structure containing initial
- *               state for the VCPU.
+ * @extra_arg == For PV or ARM guests this is a pointer to a vcpu_guest_context
+ *               structure containing the initial state for the VCPU. For x86
+ *               HVM based guests this is a pointer to a vcpu_hvm_context
+ *               structure.
  */
 #define VCPUOP_initialise            0
 
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 3dca3f6..a1a6f25 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -64,6 +64,9 @@ int arch_domain_soft_reset(struct domain *d);
 int arch_set_info_guest(struct vcpu *, vcpu_guest_context_u);
 void arch_get_info_guest(struct vcpu *, vcpu_guest_context_u);
 
+int arch_initialise_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
+int default_initialise_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
+
 int domain_relinquish_resources(struct domain *d);
 
 void dump_pageframe_info(struct domain *d);
diff --git a/xen/include/xlat.lst b/xen/include/xlat.lst
index 3795059..fda1137 100644
--- a/xen/include/xlat.lst
+++ b/xen/include/xlat.lst
@@ -56,6 +56,9 @@
 ?       grant_entry_header              grant_table.h
 ?	grant_entry_v2			grant_table.h
 ?	gnttab_swap_grant_ref		grant_table.h
+?	vcpu_hvm_context		hvm/hvm_vcpu.h
+?	vcpu_hvm_x86_32			hvm/hvm_vcpu.h
+?	vcpu_hvm_x86_64			hvm/hvm_vcpu.h
 ?	kexec_exec			kexec.h
 !	kexec_image			kexec.h
 !	kexec_range			kexec.h
-- 
1.9.5 (Apple Git-50.3)


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

Re: [PATCH v10 5/9] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Ian Campbell]

On Mon, 2015-12-07 at 17:48 +0100, Roger Pau Monne wrote:
> Allow the usage of the VCPUOP_initialise, VCPUOP_up, VCPUOP_down,
> VCPUOP_is_up, VCPUOP_get_physid and VCPUOP_send_nmi hypercalls from HVM
> guests.
> 
> This patch introduces a new structure (vcpu_hvm_context) that should be
> used
> in conjuction with the VCPUOP_initialise hypercall in order to initialize

"conjunction"

> vCPUs for HVM guests.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>

For the (trivial) ARM bit:

> Acked-by: Ian Campbell <ian.campbell@citrix.com>
[...]
>  xen/arch/arm/domain.c             |   5 +


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

Re: [PATCH v10 5/9] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Jan Beulich]

>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
> Allow the usage of the VCPUOP_initialise, VCPUOP_up, VCPUOP_down,
> VCPUOP_is_up, VCPUOP_get_physid and VCPUOP_send_nmi hypercalls from HVM
> guests.
> 
> This patch introduces a new structure (vcpu_hvm_context) that should be used
> in conjuction with the VCPUOP_initialise hypercall in order to initialize
> vCPUs for HVM guests.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
albeit I may fiddle with some of the messages in check_segment()
upon committing, and pending clarification on ...

> +    if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
> +    {
> +        /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
> +        struct page_info *page = get_page_from_gfn(v->domain,
> +                                 v->arch.hvm_vcpu.guest_cr[3] >> PAGE_SHIFT,
> +                                 NULL, P2M_ALLOC);
> +        if ( !page )
> +        {
> +            gprintk(XENLOG_ERR, "Invalid CR3: %#lx\n",
> +                    v->arch.hvm_vcpu.guest_cr[3]);
> +            domain_crash(v->domain);
> +            return -EINVAL;
> +        }

... why you crash the domain here when you don't on any on the
earlier error paths.

Jan

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

Re: [PATCH v10 5/9] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Roger Pau Monné]

El 10/12/15 a les 17.53, Jan Beulich ha escrit:
>>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
>> Allow the usage of the VCPUOP_initialise, VCPUOP_up, VCPUOP_down,
>> VCPUOP_is_up, VCPUOP_get_physid and VCPUOP_send_nmi hypercalls from HVM
>> guests.
>>
>> This patch introduces a new structure (vcpu_hvm_context) that should be used
>> in conjuction with the VCPUOP_initialise hypercall in order to initialize
>> vCPUs for HVM guests.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> albeit I may fiddle with some of the messages in check_segment()
> upon committing, and pending clarification on ...
> 
>> +    if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
>> +    {
>> +        /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>> +        struct page_info *page = get_page_from_gfn(v->domain,
>> +                                 v->arch.hvm_vcpu.guest_cr[3] >> PAGE_SHIFT,
>> +                                 NULL, P2M_ALLOC);
>> +        if ( !page )
>> +        {
>> +            gprintk(XENLOG_ERR, "Invalid CR3: %#lx\n",
>> +                    v->arch.hvm_vcpu.guest_cr[3]);
>> +            domain_crash(v->domain);
>> +            return -EINVAL;
>> +        }
> 
> ... why you crash the domain here when you don't on any on the
> earlier error paths.

I don't see any reason why we should crash the domain, I'm not sure
where the domain_crash call it's coming from, it's been here since the
first version of this patch.

If you want I can send a new version without the domain crash, or you
can amend it while committing. AFAICT removing the domain_crash call
doesn't have any side effects.

Roger.


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

Re: [PATCH v10 5/9] xen/x86: allow HVM guests to use hypercalls to bring up vCPUs

[Jan Beulich]

>>> On 10.12.15 at 18:18, <roger.pau@citrix.com> wrote:
> El 10/12/15 a les 17.53, Jan Beulich ha escrit:
>>>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
>>> Allow the usage of the VCPUOP_initialise, VCPUOP_up, VCPUOP_down,
>>> VCPUOP_is_up, VCPUOP_get_physid and VCPUOP_send_nmi hypercalls from HVM
>>> guests.
>>>
>>> This patch introduces a new structure (vcpu_hvm_context) that should be used
>>> in conjuction with the VCPUOP_initialise hypercall in order to initialize
>>> vCPUs for HVM guests.
>>>
>>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> 
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> albeit I may fiddle with some of the messages in check_segment()
>> upon committing, and pending clarification on ...
>> 
>>> +    if ( hvm_paging_enabled(v) && !paging_mode_hap(v->domain) )
>>> +    {
>>> +        /* Shadow-mode CR3 change. Check PDBR and update refcounts. */
>>> +        struct page_info *page = get_page_from_gfn(v->domain,
>>> +                                 v->arch.hvm_vcpu.guest_cr[3] >> PAGE_SHIFT,
>>> +                                 NULL, P2M_ALLOC);
>>> +        if ( !page )
>>> +        {
>>> +            gprintk(XENLOG_ERR, "Invalid CR3: %#lx\n",
>>> +                    v->arch.hvm_vcpu.guest_cr[3]);
>>> +            domain_crash(v->domain);
>>> +            return -EINVAL;
>>> +        }
>> 
>> ... why you crash the domain here when you don't on any on the
>> earlier error paths.
> 
> I don't see any reason why we should crash the domain, I'm not sure
> where the domain_crash call it's coming from, it's been here since the
> first version of this patch.
> 
> If you want I can send a new version without the domain crash, or you
> can amend it while committing. AFAICT removing the domain_crash call
> doesn't have any side effects.

I don't see a need for another version, unless other feedback you
might get would make that necessary.

Jan

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

[PATCH v10 6/9] libxc/xen: introduce a start info structure for HVMlite guests

[Roger Pau Monne]

This structure contains the physical address of the command line, as well as
the physical address of the list of loaded modules. The physical address of
this structure is passed to the guest at boot time in the %ebx register.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
---
Changes since v8:
 - Slightly reword comment.

Changes since v7:
 - Add a comment to clarify that nothing will be loaded at physical address
   0.
 - Add Andrew Cooper Reviewed-by.
 - Add Wei Liu Ack.

Changes since v6:
 - Add a check to make sure the start info data is placed below 4GB.
 - Make sure byte addresses are treated as uintptr_t.
 - Fix single-line comment.

Changes since v5:
 - Change some of the calculations performed to get the total size of the
   start_info region.
 - Replace the mention of HVMlite in a comment with PVH.
 - Don't use 64bit integers in hvm_modlist_entry.
---
 tools/libxc/xc_dom_x86.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++-
 xen/include/public/xen.h | 23 ++++++++++++++++
 2 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index 1d4ad3e..d058949 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -633,7 +633,70 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
     xc_hvm_param_set(xch, domid, HVM_PARAM_SHARING_RING_PFN,
                      special_pfn(SPECIALPAGE_SHARING));
 
-    if ( dom->device_model )
+    if ( !dom->device_model )
+    {
+        struct xc_dom_seg seg;
+        struct hvm_start_info *start_info;
+        char *cmdline;
+        struct hvm_modlist_entry *modlist;
+        void *start_page;
+        size_t cmdline_size = 0;
+        size_t start_info_size = sizeof(*start_info);
+
+        if ( dom->cmdline )
+        {
+            cmdline_size = ROUNDUP(strlen(dom->cmdline) + 1, 8);
+            start_info_size += cmdline_size;
+
+        }
+        if ( dom->ramdisk_blob )
+            start_info_size += sizeof(*modlist); /* Limited to one module. */
+
+        rc = xc_dom_alloc_segment(dom, &seg, "HVMlite start info", 0,
+                                  start_info_size);
+        if ( rc != 0 )
+        {
+            DOMPRINTF("Unable to reserve memory for the start info");
+            goto out;
+        }
+
+        start_page = xc_map_foreign_range(xch, domid, start_info_size,
+                                          PROT_READ | PROT_WRITE,
+                                          seg.pfn);
+        if ( start_page == NULL )
+        {
+            DOMPRINTF("Unable to map HVM start info page");
+            goto error_out;
+        }
+
+        start_info = start_page;
+        cmdline = start_page + sizeof(*start_info);
+        modlist = start_page + sizeof(*start_info) + cmdline_size;
+
+        if ( dom->cmdline )
+        {
+            strncpy(cmdline, dom->cmdline, MAX_GUEST_CMDLINE);
+            cmdline[MAX_GUEST_CMDLINE - 1] = '\0';
+            start_info->cmdline_paddr = (seg.pfn << PAGE_SHIFT) +
+                                ((uintptr_t)cmdline - (uintptr_t)start_info);
+        }
+
+        if ( dom->ramdisk_blob )
+        {
+            modlist[0].paddr = dom->ramdisk_seg.vstart - dom->parms.virt_base;
+            modlist[0].size = dom->ramdisk_seg.vend - dom->ramdisk_seg.vstart;
+            start_info->modlist_paddr = (seg.pfn << PAGE_SHIFT) +
+                                ((uintptr_t)modlist - (uintptr_t)start_info);
+            start_info->nr_modules = 1;
+        }
+
+        start_info->magic = HVM_START_MAGIC_VALUE;
+
+        munmap(start_page, start_info_size);
+
+        dom->start_info_pfn = seg.pfn;
+    }
+    else
     {
         /*
          * Allocate and clear additional ioreq server pages. The default
@@ -1032,6 +1095,9 @@ static int vcpu_hvm(struct xc_dom_image *dom)
     /* Set the IP. */
     bsp_ctx.cpu.rip = dom->parms.phys_entry;
 
+    if ( dom->start_info_pfn )
+        bsp_ctx.cpu.rbx = dom->start_info_pfn << PAGE_SHIFT;
+
     /* Set the end descriptor. */
     bsp_ctx.end_d.typecode = HVM_SAVE_CODE(END);
     bsp_ctx.end_d.instance = 0;
diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index ff5547e..7b629b1 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -784,6 +784,29 @@ struct start_info {
 };
 typedef struct start_info start_info_t;
 
+/*
+ * Start of day structure passed to PVH guests in %ebx.
+ *
+ * NOTE: nothing will be loaded at physical address 0, so
+ * a 0 value in any of the address fields should be treated
+ * as not present.
+ */
+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.              */
+};
+
 /* New console union for dom0 introduced in 0x00030203. */
 #if __XEN_INTERFACE_VERSION__ < 0x00030203
 #define console_mfn    console.domU.mfn
-- 
1.9.5 (Apple Git-50.3)


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

[PATCH v10 7/9] libxc: switch xc_dom_elfloader to be used with HVMlite domains

[Roger Pau Monne]

Allow xc_dom_elfloader to report a guest type as hvm-3.0-x86_32 if it's
running inside of a HVM container and has the PHYS32_ENTRY elfnote set.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Only xc_dom_elfloader has been switched to support HVMlite, other loaders
should also be switched once we have a HVMlite compatible kernel that uses
them.
---
Changes since v5:
 - Add Wei Liu Ack.

Changes since v4:
 - Add Andrew Cooper Reviewed-by.
---
 tools/libxc/xc_dom_elfloader.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/tools/libxc/xc_dom_elfloader.c b/tools/libxc/xc_dom_elfloader.c
index 36a115e..2ae575e 100644
--- a/tools/libxc/xc_dom_elfloader.c
+++ b/tools/libxc/xc_dom_elfloader.c
@@ -56,6 +56,10 @@ static char *xc_dom_guest_type(struct xc_dom_image *dom,
 {
     uint64_t machine = elf_uval(elf, elf->ehdr, e_machine);
 
+    if ( dom->container_type == XC_DOM_HVM_CONTAINER &&
+         dom->parms.phys_entry != UNSET_ADDR )
+        return "hvm-3.0-x86_32";
+
     switch ( machine )
     {
     case EM_386:
-- 
1.9.5 (Apple Git-50.3)


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

[PATCH v10 8/9] libxl: allow the creation of HVM domains without a device model.

[Roger Pau Monne]

Replace the firmware loaded into HVM guests with an OS kernel. Since the HVM
builder now uses the PV xc_dom_* set of functions this kernel will be parsed
and loaded inside the guest like on PV, but the container is a pure HVM
guest.

Also, if device_model_version is set to none or a device model for the
specified domain is not present unconditinally set the nic type to
LIBXL_NIC_TYPE_VIF.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v7:
 - Add LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE.

Changes since v5:
 - Add Wei Liu Acked-by.

Changes since v4:
 - Set dom->mmio_size to match the size of the special pages if there's no
   device model for the guest. This implies moving NR_SPECIAL_PAGES and
   X86_HVM_END_SPECIAL_REGION to a public header so they can be known by
   libxl when creating the memory map.
 - Reword the xl.cfg man page description of the "none" device model option.
 - Use libxl__device_model_version_running instead of creating a new
   function.

Changes since v3:
 - Add explicit /* fall through */ comments.
 - Expand libxl__device_nic_setdefault so that it sets the right nic type
   for HVMlite guests.
 - Remove stray space in hvm_build_set_params.
 - Fix the error paths of libxl__domain_firmware.
---
 docs/man/xl.cfg.pod.5        |  5 +++
 tools/libxc/include/xc_dom.h |  2 ++
 tools/libxc/xc_dom_x86.c     | 15 ++++-----
 tools/libxl/libxl.c          | 44 ++++++++++++++++++--------
 tools/libxl/libxl.h          |  8 +++++
 tools/libxl/libxl_create.c   | 16 +++++++++-
 tools/libxl/libxl_dm.c       |  3 +-
 tools/libxl/libxl_dom.c      | 74 ++++++++++++++++++++++++++++++--------------
 tools/libxl/libxl_internal.h |  9 +++++-
 tools/libxl/libxl_types.idl  |  1 +
 tools/libxl/libxl_x86.c      | 15 ++++++---
 tools/libxl/xl_cmdimpl.c     |  2 ++
 12 files changed, 144 insertions(+), 50 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 3b695bd..8899f75 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1789,6 +1789,11 @@ This device-model is the default for Linux dom0.
 Use the device-model based upon the historical Xen fork of Qemu.
 This device-model is still the default for NetBSD dom0.
 
+=item B<none>
+
+Don't use any device model. This requires a kernel capable of booting
+without emulated devices.
+
 =back
 
 It is recommended to accept the default value for new guests.  If
diff --git a/tools/libxc/include/xc_dom.h b/tools/libxc/include/xc_dom.h
index efa6c6e..2460818 100644
--- a/tools/libxc/include/xc_dom.h
+++ b/tools/libxc/include/xc_dom.h
@@ -20,6 +20,8 @@
 #include <xenguest.h>
 
 #define INVALID_PFN ((xen_pfn_t)-1)
+#define X86_HVM_NR_SPECIAL_PAGES    8
+#define X86_HVM_END_SPECIAL_REGION  0xff000u
 
 /* --- typedefs and structs ---------------------------------------- */
 
diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
index d058949..3960875 100644
--- a/tools/libxc/xc_dom_x86.c
+++ b/tools/libxc/xc_dom_x86.c
@@ -58,8 +58,8 @@
 #define SPECIALPAGE_IOREQ    5
 #define SPECIALPAGE_IDENT_PT 6
 #define SPECIALPAGE_CONSOLE  7
-#define NR_SPECIAL_PAGES     8
-#define special_pfn(x) (0xff000u - NR_SPECIAL_PAGES + (x))
+#define special_pfn(x) \
+    (X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES + (x))
 
 #define NR_IOREQ_SERVER_PAGES 8
 #define ioreq_server_pfn(x) (special_pfn(0) - NR_IOREQ_SERVER_PAGES + (x))
@@ -589,7 +589,7 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
     void *hvm_info_page;
     uint32_t *ident_pt, domid = dom->guest_domid;
     int rc;
-    xen_pfn_t special_array[NR_SPECIAL_PAGES];
+    xen_pfn_t special_array[X86_HVM_NR_SPECIAL_PAGES];
     xen_pfn_t ioreq_server_array[NR_IOREQ_SERVER_PAGES];
     xc_interface *xch = dom->xch;
 
@@ -604,18 +604,19 @@ static int alloc_magic_pages_hvm(struct xc_dom_image *dom)
     }
 
     /* Allocate and clear special pages. */
-    for ( i = 0; i < NR_SPECIAL_PAGES; i++ )
+    for ( i = 0; i < X86_HVM_NR_SPECIAL_PAGES; i++ )
         special_array[i] = special_pfn(i);
 
-    rc = xc_domain_populate_physmap_exact(xch, domid, NR_SPECIAL_PAGES, 0, 0,
-                                          special_array);
+    rc = xc_domain_populate_physmap_exact(xch, domid, X86_HVM_NR_SPECIAL_PAGES,
+                                          0, 0, special_array);
     if ( rc != 0 )
     {
         DOMPRINTF("Could not allocate special pages.");
         goto error_out;
     }
 
-    if ( xc_clear_domain_pages(xch, domid, special_pfn(0), NR_SPECIAL_PAGES) )
+    if ( xc_clear_domain_pages(xch, domid, special_pfn(0),
+                               X86_HVM_NR_SPECIAL_PAGES) )
             goto error_out;
 
     xc_hvm_param_set(xch, domid, HVM_PARAM_STORE_PFN,
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index bd3aac8..db4f4e2 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1065,11 +1065,14 @@ int libxl_domain_unpause(libxl_ctx *ctx, uint32_t domid)
     }
 
     if (type == LIBXL_DOMAIN_TYPE_HVM) {
-        rc = libxl__domain_resume_device_model(gc, domid);
-        if (rc < 0) {
-            LOG(ERROR, "failed to unpause device model for domain %u:%d",
-                domid, rc);
-            goto out;
+        if (libxl__device_model_version_running(gc, domid) !=
+            LIBXL_DEVICE_MODEL_VERSION_NONE) {
+            rc = libxl__domain_resume_device_model(gc, domid);
+            if (rc < 0) {
+                LOG(ERROR, "failed to unpause device model for domain %u:%d",
+                    domid, rc);
+                goto out;
+            }
         }
     }
     ret = xc_domain_unpause(ctx->xch, domid);
@@ -1616,11 +1619,11 @@ void libxl__destroy_domid(libxl__egc *egc, libxl__destroy_domid_state *dis)
 
     switch (libxl__domain_type(gc, domid)) {
     case LIBXL_DOMAIN_TYPE_HVM:
-        if (!libxl_get_stubdom_id(CTX, domid))
-            dm_present = 1;
-        else
+        if (libxl_get_stubdom_id(CTX, domid)) {
             dm_present = 0;
-        break;
+            break;
+        }
+        /* fall through */
     case LIBXL_DOMAIN_TYPE_PV:
         pid = libxl__xs_read(gc, XBT_NULL, GCSPRINTF("/local/domain/%d/image/device-model-pid", domid));
         dm_present = (pid != NULL);
@@ -3240,7 +3243,7 @@ out:
 /******************************************************************************/
 
 int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
-                                 uint32_t domid)
+                                 uint32_t domid, libxl_domain_build_info *info)
 {
     int rc;
 
@@ -3277,8 +3280,23 @@ int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
 
     switch (libxl__domain_type(gc, domid)) {
     case LIBXL_DOMAIN_TYPE_HVM:
-        if (!nic->nictype)
-            nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
+        if (!nic->nictype) {
+            if (info != NULL) {
+                /* Path taken at creation time. */
+                if (info->device_model_version ==
+                    LIBXL_DEVICE_MODEL_VERSION_NONE)
+                    nic->nictype = LIBXL_NIC_TYPE_VIF;
+                else
+                    nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
+            } else {
+                /* Path taken when hot-adding a nic. */
+                if (libxl__device_model_version_running(gc, domid) ==
+                    LIBXL_DEVICE_MODEL_VERSION_NONE)
+                    nic->nictype = LIBXL_NIC_TYPE_VIF;
+                else
+                    nic->nictype = LIBXL_NIC_TYPE_VIF_IOEMU;
+            }
+        }
         break;
     case LIBXL_DOMAIN_TYPE_PV:
         if (nic->nictype == LIBXL_NIC_TYPE_VIF_IOEMU) {
@@ -3327,7 +3345,7 @@ void libxl__device_nic_add(libxl__egc *egc, uint32_t domid,
     libxl_device_nic_init(&nic_saved);
     libxl_device_nic_copy(CTX, &nic_saved, nic);
 
-    rc = libxl__device_nic_setdefault(gc, nic, domid);
+    rc = libxl__device_nic_setdefault(gc, nic, domid, NULL);
     if (rc) goto out;
 
     front = flexarray_make(gc, 16, 1);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6b73848..0dc3be3 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -859,6 +859,14 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst, libxl_mac *src);
 */
 #define LIBXL_HAVE_GFX_PASSTHRU_KIND
 
+/*
+ * LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE
+ *
+ * In the case that LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE is set libxl
+ * allows the creation of HVM guests without a device model.
+ */
+#define LIBXL_HAVE_DEVICE_MODEL_VERSION_NONE 1
+
 typedef char **libxl_string_list;
 void libxl_string_list_dispose(libxl_string_list *sl);
 int libxl_string_list_length(const libxl_string_list *sl);
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index a1ccf23..903f1b4 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -119,6 +119,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
                 b_info->u.hvm.bios = LIBXL_BIOS_TYPE_ROMBIOS; break;
             case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
                 b_info->u.hvm.bios = LIBXL_BIOS_TYPE_SEABIOS; break;
+            case LIBXL_DEVICE_MODEL_VERSION_NONE:
+                break;
             default:return ERROR_INVAL;
             }
 
@@ -132,6 +134,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
             if (b_info->u.hvm.bios == LIBXL_BIOS_TYPE_ROMBIOS)
                 return ERROR_INVAL;
             break;
+        case LIBXL_DEVICE_MODEL_VERSION_NONE:
+            break;
         default:abort();
         }
 
@@ -236,6 +240,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
                 break;
             }
             break;
+        case LIBXL_DEVICE_MODEL_VERSION_NONE:
+            b_info->video_memkb = 0;
+            break;
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
         default:
             switch (b_info->u.hvm.vga.kind) {
@@ -920,7 +927,8 @@ static void initiate_domain_create(libxl__egc *egc,
          * called libxl_device_nic_add when domcreate_launch_dm gets called,
          * but qemu needs the nic information to be complete.
          */
-        ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid);
+        ret = libxl__device_nic_setdefault(gc, &d_config->nics[i], domid,
+                                           &d_config->b_info);
         if (ret) {
             LOG(ERROR, "Unable to set nic defaults for nic %d", i);
             goto error_out;
@@ -1261,6 +1269,12 @@ static void domcreate_launch_dm(libxl__egc *egc, libxl__multidev *multidev,
         libxl__device_console_add(gc, domid, &console, state, &device);
         libxl__device_console_dispose(&console);
 
+        if (d_config->b_info.device_model_version ==
+            LIBXL_DEVICE_MODEL_VERSION_NONE) {
+            domcreate_devmodel_started(egc, &dcs->dmss.dm, 0);
+            return;
+        }
+
         libxl_device_vkb_init(&vkb);
         libxl__device_vkb_add(gc, domid, &vkb);
         libxl_device_vkb_dispose(&vkb);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index a4934df..efcc174 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -1606,7 +1606,8 @@ static void spawn_stub_launch_dm(libxl__egc *egc,
          * called libxl_device_nic_add at this point, but qemu needs
          * the nic information to be complete.
          */
-        ret = libxl__device_nic_setdefault(gc, &dm_config->nics[i], dm_domid);
+        ret = libxl__device_nic_setdefault(gc, &dm_config->nics[i], dm_domid,
+                                           &dm_config->b_info);
         if (ret)
             goto out;
     }
diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 49d7b90..0f166e9 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -790,21 +790,23 @@ static int hvm_build_set_params(xc_interface *handle, uint32_t domid,
     uint64_t str_mfn, cons_mfn;
     int i;
 
-    va_map = xc_map_foreign_range(handle, domid,
-                                  XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
-                                  HVM_INFO_PFN);
-    if (va_map == NULL)
-        return ERROR_FAIL;
+    if (info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        va_map = xc_map_foreign_range(handle, domid,
+                                      XC_PAGE_SIZE, PROT_READ | PROT_WRITE,
+                                      HVM_INFO_PFN);
+        if (va_map == NULL)
+            return ERROR_FAIL;
 
-    va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
-    va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
-    va_hvm->nr_vcpus = info->max_vcpus;
-    memset(va_hvm->vcpu_online, 0, sizeof(va_hvm->vcpu_online));
-    memcpy(va_hvm->vcpu_online, info->avail_vcpus.map, info->avail_vcpus.size);
-    for (i = 0, sum = 0; i < va_hvm->length; i++)
-        sum += ((uint8_t *) va_hvm)[i];
-    va_hvm->checksum -= sum;
-    munmap(va_map, XC_PAGE_SIZE);
+        va_hvm = (struct hvm_info_table *)(va_map + HVM_INFO_OFFSET);
+        va_hvm->apic_mode = libxl_defbool_val(info->u.hvm.apic);
+        va_hvm->nr_vcpus = info->max_vcpus;
+        memset(va_hvm->vcpu_online, 0, sizeof(va_hvm->vcpu_online));
+        memcpy(va_hvm->vcpu_online, info->avail_vcpus.map, info->avail_vcpus.size);
+        for (i = 0, sum = 0; i < va_hvm->length; i++)
+            sum += ((uint8_t *) va_hvm)[i];
+        va_hvm->checksum -= sum;
+        munmap(va_map, XC_PAGE_SIZE);
+    }
 
     xc_hvm_param_get(handle, domid, HVM_PARAM_STORE_PFN, &str_mfn);
     xc_hvm_param_get(handle, domid, HVM_PARAM_CONSOLE_PFN, &cons_mfn);
@@ -870,7 +872,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
 {
     libxl_ctx *ctx = libxl__gc_owner(gc);
     const char *firmware;
-    int e, rc = ERROR_FAIL;
+    int e, rc;
     int datalen = 0;
     void *data;
 
@@ -885,18 +887,34 @@ static int libxl__domain_firmware(libxl__gc *gc,
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
             firmware = "hvmloader";
             break;
+        case LIBXL_DEVICE_MODEL_VERSION_NONE:
+            if (info->kernel == NULL) {
+                LOG(ERROR, "no device model requested without a kernel");
+                rc = ERROR_FAIL;
+                goto out;
+            }
+            break;
         default:
             LOG(ERROR, "invalid device model version %d",
                 info->device_model_version);
-            return ERROR_FAIL;
-            break;
+            rc = ERROR_FAIL;
+            goto out;
         }
     }
 
-    rc = xc_dom_kernel_file(dom, libxl__abs_path(gc, firmware,
+    if (info->kernel != NULL &&
+        info->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        /* Try to load a kernel instead of the firmware. */
+        rc = xc_dom_kernel_file(dom, info->kernel);
+        if (rc == 0 && info->ramdisk != NULL)
+            rc = xc_dom_ramdisk_file(dom, info->ramdisk);
+    } else {
+        rc = xc_dom_kernel_file(dom, libxl__abs_path(gc, firmware,
                                                  libxl__xenfirmwaredir_path()));
+    }
+
     if (rc != 0) {
-        LOGE(ERROR, "xc_dom_kernel_file failed");
+        LOGE(ERROR, "xc_dom_{kernel_file/ramdisk_file} failed");
         goto out;
     }
 
@@ -907,6 +925,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
         if (e) {
             LOGEV(ERROR, e, "failed to read SMBIOS firmware file %s",
                 info->u.hvm.smbios_firmware);
+            rc = ERROR_FAIL;
             goto out;
         }
         libxl__ptr_add(gc, data);
@@ -924,6 +943,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
         if (e) {
             LOGEV(ERROR, e, "failed to read ACPI firmware file %s",
                 info->u.hvm.acpi_firmware);
+            rc = ERROR_FAIL;
             goto out;
         }
         libxl__ptr_add(gc, data);
@@ -936,6 +956,7 @@ static int libxl__domain_firmware(libxl__gc *gc,
 
     return 0;
 out:
+    assert(rc != 0);
     return rc;
 }
 
@@ -948,10 +969,13 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
     uint64_t mmio_start, lowmem_end, highmem_end, mem_size;
     libxl_domain_build_info *const info = &d_config->b_info;
     struct xc_dom_image *dom = NULL;
+    bool device_model =
+        info->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE ?
+        true : false;
 
     xc_dom_loginit(ctx->xch);
 
-    dom = xc_dom_allocate(ctx->xch, NULL, NULL);
+    dom = xc_dom_allocate(ctx->xch, info->cmdline, NULL);
     if (!dom) {
         LOGE(ERROR, "xc_dom_allocate failed");
         rc = ERROR_NOMEM;
@@ -984,8 +1008,12 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
 
     if (dom->target_pages == 0)
         dom->target_pages = mem_size >> XC_PAGE_SHIFT;
-    if (dom->mmio_size == 0)
+    if (dom->mmio_size == 0 && device_model)
         dom->mmio_size = HVM_BELOW_4G_MMIO_LENGTH;
+    else if (dom->mmio_size == 0 && !device_model)
+        dom->mmio_size = GB(4) -
+                    ((X86_HVM_END_SPECIAL_REGION - X86_HVM_NR_SPECIAL_PAGES)
+                    << XC_PAGE_SHIFT);
     lowmem_end = mem_size;
     highmem_end = 0;
     mmio_start = (1ull << 32) - dom->mmio_size;
@@ -997,8 +1025,8 @@ int libxl__build_hvm(libxl__gc *gc, uint32_t domid,
     dom->lowmem_end = lowmem_end;
     dom->highmem_end = highmem_end;
     dom->mmio_start = mmio_start;
-    dom->vga_hole_size = LIBXL_VGA_HOLE_SIZE;
-    dom->device_model = true;
+    dom->vga_hole_size = device_model ? LIBXL_VGA_HOLE_SIZE : 0;
+    dom->device_model = device_model;
 
     rc = libxl__domain_device_construct_rdm(gc, d_config,
                                             info->u.hvm.rdm_mem_boundary_memkb*1024,
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 570a85a..c8402de 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -115,6 +115,12 @@
 #define DOMID_XS_PATH "domid"
 #define INVALID_DOMID ~0
 
+/* Size macros. */
+#define __AC(X,Y)   (X##Y)
+#define _AC(X,Y)    __AC(X,Y)
+#define MB(_mb)     (_AC(_mb, ULL) << 20)
+#define GB(_gb)     (_AC(_gb, ULL) << 30)
+
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
 
 #define ROUNDUP(_val, _order)                                           \
@@ -1188,7 +1194,8 @@ _hidden int libxl__domain_build_info_setdefault(libxl__gc *gc,
 _hidden int libxl__device_disk_setdefault(libxl__gc *gc,
                                           libxl_device_disk *disk);
 _hidden int libxl__device_nic_setdefault(libxl__gc *gc, libxl_device_nic *nic,
-                                         uint32_t domid);
+                                         uint32_t domid,
+                                         libxl_domain_build_info *info);
 _hidden int libxl__device_vtpm_setdefault(libxl__gc *gc, libxl_device_vtpm *vtpm);
 _hidden int libxl__device_vfb_setdefault(libxl__gc *gc, libxl_device_vfb *vfb);
 _hidden int libxl__device_vkb_setdefault(libxl__gc *gc, libxl_device_vkb *vkb);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index cf3730f..9658356 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -98,6 +98,7 @@ libxl_device_model_version = Enumeration("device_model_version", [
     (0, "UNKNOWN"),
     (1, "QEMU_XEN_TRADITIONAL"), # Historical qemu-xen device model (qemu-dm)
     (2, "QEMU_XEN"),             # Upstream based qemu-xen device model
+    (3, "NONE"),                 # No device model
     ])
 
 libxl_console_type = Enumeration("console_type", [
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 183b6c2..46cfafb 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -7,10 +7,16 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
                                       libxl_domain_config *d_config,
                                       xc_domain_configuration_t *xc_config)
 {
-    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM)
+
+    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
+        d_config->b_info.device_model_version !=
+        LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        /* HVM domains with a device model. */
         xc_config->emulation_flags = XEN_X86_EMU_ALL;
-    else
+    } else {
+        /* PV or HVM domains without a device model. */
         xc_config->emulation_flags = 0;
+    }
 
     return 0;
 }
@@ -485,6 +491,7 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
     struct e820entry *e820 = NULL;
     uint64_t highmem_size =
                     dom->highmem_end ? dom->highmem_end - (1ull << 32) : 0;
+    uint32_t lowmem_start = dom->device_model ? GUEST_LOW_MEM_START_DEFAULT : 0;
 
     /* Add all rdm entries. */
     for (i = 0; i < d_config->num_rdms; i++)
@@ -505,8 +512,8 @@ int libxl__arch_domain_construct_memmap(libxl__gc *gc,
     e820 = libxl__malloc(gc, sizeof(struct e820entry) * e820_entries);
 
     /* Low memory */
-    e820[nr].addr = GUEST_LOW_MEM_START_DEFAULT;
-    e820[nr].size = dom->lowmem_end - GUEST_LOW_MEM_START_DEFAULT;
+    e820[nr].addr = lowmem_start;
+    e820[nr].size = dom->lowmem_end - lowmem_start;
     e820[nr].type = E820_RAM;
     nr++;
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2b6371d..f9933cb 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2186,6 +2186,8 @@ skip_vfb:
         } else if (!strcmp(buf, "qemu-xen")) {
             b_info->device_model_version
                 = LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN;
+        } else if (!strcmp(buf, "none")) {
+            b_info->device_model_version = LIBXL_DEVICE_MODEL_VERSION_NONE;
         } else {
             fprintf(stderr,
                     "Unknown device_model_version \"%s\" specified\n", buf);
-- 
1.9.5 (Apple Git-50.3)


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

[PATCH v10 9/9] libxl: add support for migrating HVM guests without a device model

[Roger Pau Monne]

Only some minor libxl changes are needed in order to be able to migrate HVM
guests without a device model, no hypervisor changes are needed.

This change prevents sending the emulator context if the device model
version is set to none.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
Changes since v9:
 - Use the correct type to store the device model version.
 - Initialize device_model_version in libxl__stream_write_init.
 - Add Andrew Reviewed-by and Wei Acked-by.

Changes since v8:
 - Cache device model version inside of the libxl__stream_write_state
   structure.
 - Add a seatbelt by setting the emulator id to UNKNOWN in the HVMlite case.
   This can be used to assert we don't reach certain paths.
 - Remove stray STATE_AO_GC in emulator_xenstore_record_done.

Changes since v7:
 - Prevent sending the emulator context and xenstore record in
   write_emulator_context_record and write_emulator_xenstore_record.
 - Error out if an emulator record is received for a no-dm guest.
---
 tools/libxl/libxl_dom.c          |  3 +++
 tools/libxl/libxl_dom_suspend.c  |  2 ++
 tools/libxl/libxl_internal.h     |  3 +++
 tools/libxl/libxl_stream_read.c  | 16 ++++++++++++++++
 tools/libxl/libxl_stream_write.c | 21 ++++++++++++++++++++-
 5 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
index 0f166e9..e730912 100644
--- a/tools/libxl/libxl_dom.c
+++ b/tools/libxl/libxl_dom.c
@@ -1318,6 +1318,9 @@ void libxl__domain_suspend_common_switch_qemu_logdirty
     case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
         domain_suspend_switch_qemu_xen_logdirty(domid, enable, shs);
         break;
+    case LIBXL_DEVICE_MODEL_VERSION_NONE:
+        libxl__xc_domain_saverestore_async_callback_done(egc, shs, 0);
+        break;
     default:
         LOG(ERROR,"logdirty switch failed"
             ", no valid device model version found, abandoning suspend");
diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index 4cc01ad..3313ad1 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -43,6 +43,8 @@ int libxl__domain_suspend_device_model(libxl__gc *gc,
         if (ret)
             unlink(filename);
         break;
+    case LIBXL_DEVICE_MODEL_VERSION_NONE:
+        break;
     default:
         return ERROR_INVAL;
     }
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index c8402de..7ec2eb3 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3037,6 +3037,9 @@ struct libxl__stream_write_state {
     libxl__datacopier_state dc;
     sws_record_done_cb record_done_callback;
 
+    /* Cache device model version. */
+    libxl_device_model_version device_model_version;
+
     /* Only used when constructing EMULATOR records. */
     libxl__datacopier_state emu_dc;
     libxl__carefd *emu_carefd;
diff --git a/tools/libxl/libxl_stream_read.c b/tools/libxl/libxl_stream_read.c
index 4ec29da..258dec4 100644
--- a/tools/libxl/libxl_stream_read.c
+++ b/tools/libxl/libxl_stream_read.c
@@ -539,6 +539,14 @@ static bool process_record(libxl__egc *egc,
         break;
 
     case REC_TYPE_EMULATOR_XENSTORE_DATA:
+        if (dcs->guest_config->b_info.device_model_version ==
+            LIBXL_DEVICE_MODEL_VERSION_NONE) {
+            rc = ERROR_FAIL;
+            LOG(ERROR,
+                "Received a xenstore emulator record when none was expected");
+            goto err;
+        }
+
         if (rec->hdr.length < sizeof(libxl__sr_emulator_hdr)) {
             rc = ERROR_FAIL;
             LOG(ERROR,
@@ -560,6 +568,14 @@ static bool process_record(libxl__egc *egc,
         break;
 
     case REC_TYPE_EMULATOR_CONTEXT:
+        if (dcs->guest_config->b_info.device_model_version ==
+            LIBXL_DEVICE_MODEL_VERSION_NONE) {
+            rc = ERROR_FAIL;
+            LOG(ERROR,
+                "Received an emulator context record when none was expected");
+            goto err;
+        }
+
         write_emulator_blob(egc, stream, rec);
         break;
 
diff --git a/tools/libxl/libxl_stream_write.c b/tools/libxl/libxl_stream_write.c
index 52a60d7..80d9208 100644
--- a/tools/libxl/libxl_stream_write.c
+++ b/tools/libxl/libxl_stream_write.c
@@ -167,6 +167,8 @@ static void setup_emulator_write(libxl__egc *egc,
                                  void *body,
                                  sws_record_done_cb cb)
 {
+    assert(stream->emu_sub_hdr.id != EMULATOR_UNKNOWN);
+    assert(stream->device_model_version != LIBXL_DEVICE_MODEL_VERSION_NONE);
     setup_generic_write(egc, stream, what, hdr, emu_hdr, body, cb);
 }
 
@@ -207,6 +209,7 @@ void libxl__stream_write_init(libxl__stream_write_state *stream)
     FILLZERO(stream->emu_rec_hdr);
     FILLZERO(stream->emu_sub_hdr);
     stream->emu_body = NULL;
+    stream->device_model_version = LIBXL_DEVICE_MODEL_VERSION_UNKNOWN;
 }
 
 void libxl__stream_write_start(libxl__egc *egc,
@@ -223,7 +226,9 @@ void libxl__stream_write_start(libxl__egc *egc,
     stream->running = true;
 
     if (dss->type == LIBXL_DOMAIN_TYPE_HVM) {
-        switch (libxl__device_model_version_running(gc, dss->domid)) {
+        stream->device_model_version =
+            libxl__device_model_version_running(gc, dss->domid);
+        switch (stream->device_model_version) {
         case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
             stream->emu_sub_hdr.id = EMULATOR_QEMU_TRADITIONAL;
             break;
@@ -232,6 +237,10 @@ void libxl__stream_write_start(libxl__egc *egc,
             stream->emu_sub_hdr.id = EMULATOR_QEMU_UPSTREAM;
             break;
 
+        case LIBXL_DEVICE_MODEL_VERSION_NONE:
+            stream->emu_sub_hdr.id = EMULATOR_UNKNOWN;
+            break;
+
         default:
             rc = ERROR_FAIL;
             LOG(ERROR, "Unknown emulator for HVM domain");
@@ -359,6 +368,11 @@ static void write_emulator_xenstore_record(libxl__egc *egc,
     char *buf = NULL;
     uint32_t len = 0;
 
+    if (stream->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        emulator_xenstore_record_done(egc, stream);
+        return;
+    }
+
     rc = libxl__save_emulator_xenstore_data(dss, &buf, &len);
     if (rc)
         goto err;
@@ -410,6 +424,11 @@ static void write_emulator_context_record(libxl__egc *egc,
 
     assert(dss->type == LIBXL_DOMAIN_TYPE_HVM);
 
+    if (stream->device_model_version == LIBXL_DEVICE_MODEL_VERSION_NONE) {
+        emulator_context_record_done(egc, stream);
+        return;
+    }
+
     /* Convenience aliases */
     const char *const filename = dss->dm_savefile;
 
-- 
1.9.5 (Apple Git-50.3)


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

Re: [PATCH v10 0/9] Introduce HVM without dm and new boot ABI

[Jan Beulich]

>>> On 07.12.15 at 17:48, <roger.pau@citrix.com> wrote:
> This are the remaining patches of the HVMlite series. They have been 
> successfully tested on the following hardware:
> 
>  - Intel Core i3-5010U.
>  - AMD Opteron 4184.
> 
> With both hap=0 and hap=1 in the configuration file. I've been able to boot
> a SMP guest in this mode with a virtual hard drive and a virtual network
> card, all working fine AFAICT. Migration/save/restore has also been tested 
> with a SMP guest using the FreeBSD kernel provided below.
> 
> The series has been compile tested on arm32.
> 
> The series can also be found in the following git repo:
> 
> git://xenbits.xen.org/people/royger/xen.git branch hvm_without_dm_v10
> 
> And for the FreeBSD part:
> 
> git://xenbits.xen.org/people/royger/freebsd.git branch new_entry_point_v5
> 
> In case someone wants to give it a try, I've uploaded a FreeBSD kernel that
> should work when booted into this mode:
> 
> https://people.freebsd.org/~royger/kernel_no_dm 
> 
> This FreeBSD kernel starts the APs in long mode. There are examples for 
> starting the APs in other modes in the sys/x86/xen/pv.c file.
> 
> The config file that I've used is:
> 
> <config>
> kernel="/path/to/kernel_no_dm"
> 
> builder="hvm"
> device_model_version="none"
> 
> memory=128
> vcpus=2
> name = "freebsd"
> </config>
> 
> Of course if you have a FreeBSD disk already setup it can also be added to
> the configuration file, and the following line can be used to point FreeBSD
> to the disk:
> 
> extra="vfs.root.mountfrom=ufs:/dev/ufsid/<disk_id>"
> 
> As usual, each patch has it's own changelog.
> 
>   J   xen/x86: set the vPMU interface based on the presence
> A     xen/x86: allow disabling all emulated devices inside
> AW    libxc: allow creating domains without emulated
>     N x86/hvm: loosen up the ASSERT in hvm_cr4_guest_reserved_bits
>    M  xen/x86: allow HVM guests to use hypercalls to bring
> AWJ   libxc/xen: introduce a start info structure for
> AW    libxc: switch xc_dom_elfloader to be used with HVMlite
>  W    libxl: allow the creation of HVM domains without a
> AW M  libxl: add support for migrating HVM guests without a

I've just applied 1-3, 5, and 6. 4 has been applied earlier. I'd
appreciate if the last three (tools only) patches could be taken
care of by a tools committer.

Thanks, Jan

Re: [PATCH v10 0/9] Introduce HVM without dm and new boot ABI

[Ian Campbell]

On Tue, 2015-12-15 at 06:21 -0700, Jan Beulich wrote:

> I've just applied 1-3, 5, and 6. 4 has been applied earlier. I'd
> appreciate if the last three (tools only) patches could be taken
> care of by a tools committer.

All done, thanks all.

Ian.

Re: [PATCH v10 0/9] Introduce HVM without dm and new boot ABI

[Ian Campbell]

On Tue, 2015-12-15 at 15:08 +0000, Ian Campbell wrote:
> On Tue, 2015-12-15 at 06:21 -0700, Jan Beulich wrote:
> 
> > I've just applied 1-3, 5, and 6. 4 has been applied earlier. I'd
> > appreciate if the last three (tools only) patches could be taken
> > care of by a tools committer.
> 
> All done, thanks all.

Roger,

While cleaning out my queue folder of related things I came across various
versions of "Boot ABI for HVM guests without a device-model". Did all of
that end up in some form in this series? I don't see anything under docs/*
is all, but maybe it all ended up in xen/include/public/*.h?

Cheers,
Ian.

Re: [PATCH v10 0/9] Introduce HVM without dm and new boot ABI

[Roger Pau Monné]

El 15/12/15 a les 16.11, Ian Campbell ha escrit:
> On Tue, 2015-12-15 at 15:08 +0000, Ian Campbell wrote:
>> On Tue, 2015-12-15 at 06:21 -0700, Jan Beulich wrote:
>>
>>> I've just applied 1-3, 5, and 6. 4 has been applied earlier. I'd
>>> appreciate if the last three (tools only) patches could be taken
>>> care of by a tools committer.
>>
>> All done, thanks all.

Thanks :).

> Roger,
> 
> While cleaning out my queue folder of related things I came across various
> versions of "Boot ABI for HVM guests without a device-model". Did all of
> that end up in some form in this series? I don't see anything under docs/*
> is all, but maybe it all ended up in xen/include/public/*.h?

No, it didn't end up anywhere. TBH I'm not sure whether I should commit
it as a separate file or as an update to pvh.markdown, but then I should
probably do it when the original PVH implementation is gone. Any opinions?

Roger.

Re: [PATCH v10 0/9] Introduce HVM without dm and new boot ABI

[Ian Campbell]

On Tue, 2015-12-15 at 16:52 +0100, Roger Pau Monné wrote:
> El 15/12/15 a les 16.11, Ian Campbell ha escrit:
> > On Tue, 2015-12-15 at 15:08 +0000, Ian Campbell wrote:
> > > On Tue, 2015-12-15 at 06:21 -0700, Jan Beulich wrote:
> > > 
> > > > I've just applied 1-3, 5, and 6. 4 has been applied earlier. I'd
> > > > appreciate if the last three (tools only) patches could be taken
> > > > care of by a tools committer.
> > > 
> > > All done, thanks all.
> 
> Thanks :).
> 
> > Roger,
> > 
> > While cleaning out my queue folder of related things I came across
> > various
> > versions of "Boot ABI for HVM guests without a device-model". Did all
> > of
> > that end up in some form in this series? I don't see anything under
> > docs/*
> > is all, but maybe it all ended up in xen/include/public/*.h?
> 
> No, it didn't end up anywhere. TBH I'm not sure whether I should commit
> it as a separate file or as an update to pvh.markdown, but then I should
> probably do it when the original PVH implementation is gone. Any
> opinions?

How close is dm-lite to replacing the existing set of pvh functionality?

If it's close then lets just replace, if it is still a little way off then
lets commit a docs/misc/dm-lite.markdown next to pvh.markdown with a view
to eventually doing:
	git mv docs/misc/{dm-lite,pvh}.markdown

Maybe include a little blurb at the top explaining what's going on?

Ian.

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

Re: [PATCH v10 0/9] Introduce HVM without dm and new boot ABI

[Roger Pau Monné]

El 15/12/15 a les 16.58, Ian Campbell ha escrit:
> On Tue, 2015-12-15 at 16:52 +0100, Roger Pau Monné wrote:
>> El 15/12/15 a les 16.11, Ian Campbell ha escrit:
>>> On Tue, 2015-12-15 at 15:08 +0000, Ian Campbell wrote:
>>>> On Tue, 2015-12-15 at 06:21 -0700, Jan Beulich wrote:
>>>>
>>>>> I've just applied 1-3, 5, and 6. 4 has been applied earlier. I'd
>>>>> appreciate if the last three (tools only) patches could be taken
>>>>> care of by a tools committer.
>>>>
>>>> All done, thanks all.
>>
>> Thanks :).
>>
>>> Roger,
>>>
>>> While cleaning out my queue folder of related things I came across
>>> various
>>> versions of "Boot ABI for HVM guests without a device-model". Did all
>>> of
>>> that end up in some form in this series? I don't see anything under
>>> docs/*
>>> is all, but maybe it all ended up in xen/include/public/*.h?
>>
>> No, it didn't end up anywhere. TBH I'm not sure whether I should commit
>> it as a separate file or as an update to pvh.markdown, but then I should
>> probably do it when the original PVH implementation is gone. Any
>> opinions?
> 
> How close is dm-lite to replacing the existing set of pvh functionality?
> 
> If it's close then lets just replace, if it is still a little way off then
> lets commit a docs/misc/dm-lite.markdown next to pvh.markdown with a view
> to eventually doing:
> 	git mv docs/misc/{dm-lite,pvh}.markdown
> 
> Maybe include a little blurb at the top explaining what's going on?

IMHO, it should be committed as a different file until we have Dom0
support for HVMlite and we can effectively replace the PVH
implementation with HVMlite. I'm going to refresh the document and send
the patch.

Roger.


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