Re: [Xen-devel] [RFC PATCH 8/16]: PVH xen: domain creation code changes

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 12.01.13 at 02:57, Mukesh Rathor <mukesh.rathor@xxxxxxxxxx> wrote:
> +#if 0
> +    /* should we allow PV dom0 to create PVH domU's ???? */
> +    if ( is_pvh_vcpu(v) && !is_pvh_vcpu(current) )
> +        return -EINVAL;
> +#endif

Any Dom0 ought to be able to construct any kind of guest imo.

> --- a/xen/include/asm-x86/hvm/hvm.h   Fri Jan 11 16:29:49 2013 -0800
> +++ b/xen/include/asm-x86/hvm/hvm.h   Fri Jan 11 16:31:33 2013 -0800
> @@ -183,6 +183,12 @@ struct hvm_function_table {
>      /* Virtual interrupt delivery */
>      void (*update_eoi_exit_bitmap)(struct vcpu *v, u8 vector, u8 trig);
>      int (*virtual_intr_delivery_enabled)(void);
> +
> +    /* PVH functions */
> +    int (*pvh_set_vcpu_info)(struct vcpu *v, struct vcpu_guest_context 
> *ctxtp);
> +    int (*pvh_read_descriptor)(unsigned int sel, const struct vcpu *v,
> +                         const struct cpu_user_regs *regs, unsigned long 
> *base,
> +                         unsigned long *limit, unsigned int *ar);
>  };
>  
>  extern struct hvm_function_table hvm_funcs;
> @@ -316,6 +322,19 @@ static inline unsigned long hvm_get_shad
>      return hvm_funcs.get_shadow_gs_base(v);
>  }
>  
> +static inline int hvm_pvh_set_vcpu_info(struct vcpu *v, 
> +                                        struct vcpu_guest_context *ctxtp)
> +{
> +    return hvm_funcs.pvh_set_vcpu_info(v, ctxtp);
> +}
> +
> +static inline int hvm_pvh_read_descriptor(unsigned int sel, 
> +               const struct vcpu *v, const struct cpu_user_regs *regs, 
> +               unsigned long *base, unsigned long *limit, unsigned int *ar)
> +{
> +    return hvm_funcs.pvh_read_descriptor(sel, v, regs, base, limit, ar);
> +}
> +

So you add these hooks, call them unconditionally, yet neither VMX
nor SVM implement them? What's the purpose? Series of patches
are expected to be consistent at each patch boundary.

> --- a/xen/include/asm-x86/hvm/vcpu.h  Fri Jan 11 16:29:49 2013 -0800
> +++ b/xen/include/asm-x86/hvm/vcpu.h  Fri Jan 11 16:31:33 2013 -0800
> @@ -104,6 +104,13 @@ struct nestedvcpu {
>  
>  #define vcpu_nestedhvm(v) ((v)->arch.hvm_vcpu.nvcpu)
>  
> +/* add any PVH specific fields here */
> +struct pvh_hvm_vcpu_ext
> +{
> +    /* Guest-specified relocation of vcpu_info. */
> +    unsigned long pvh_vcpu_info_mfn;

Isn't that a field equivalent to v->arch.pv_vcpu.vcpu_info_mfn?
Preferably they would be shared then, or if not, having "pvh" in
the containing structure's field name and the field name here is
clearly one too much.

Jan


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