Re: [Xen-devel] [PATCH for-next v2 09/10] x86/domain: move PV specific code to pv/domain.c

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Apr 25, 2017 at 03:52:06PM +0100, Andrew Cooper wrote:
> On 25/04/17 14:52, Wei Liu wrote:
> > - fail:
> > -    pv_domain_destroy(d);
> > -
> > -    return rc;
> > -}
> > -
> > +void paravirt_ctxt_switch_from(struct vcpu *v);
> > +void paravirt_ctxt_switch_to(struct vcpu *v);
> >  int arch_domain_create(struct domain *d, unsigned int domcr_flags,
> >                         struct xen_arch_domainconfig *config)
> >  {
> > @@ -1919,7 +1717,8 @@ static void save_segments(struct vcpu *v)
> >  
> >  #define switch_kernel_stack(v) ((void)0)
> >  
> > -static void paravirt_ctxt_switch_from(struct vcpu *v)
> > +/* Needed by PV guests */
> > +void paravirt_ctxt_switch_from(struct vcpu *v)
> >  {
> 
> Could these be moved up to avoid the forward declarations above?
> 

Yes and frankly this is going to require more brain power to review, but
I'm not the one who reviews this so I don't care. ;p

> > diff --git a/xen/arch/x86/pv/Makefile b/xen/arch/x86/pv/Makefile
> > index ea94599438..2737824e81 100644
> > --- a/xen/arch/x86/pv/Makefile
> > +++ b/xen/arch/x86/pv/Makefile
> > @@ -1,2 +1,3 @@
> >  obj-y += hypercall.o
> >  obj-bin-y += dom0_build.init.o
> > +obj-y += domain.o
> > diff --git a/xen/arch/x86/pv/domain.c b/xen/arch/x86/pv/domain.c
> > new file mode 100644
> > index 0000000000..562c3d03f5
> > --- /dev/null
> > +++ b/xen/arch/x86/pv/domain.c
> > @@ -0,0 +1,232 @@
[...]
> > +
> > +static int pv_create_gdt_ldt_l1tab(struct domain *d, struct vcpu *v)
> > +{
> > +    return create_perdomain_mapping(d, GDT_VIRT_START(v),
> > +                                    1 << GDT_LDT_VCPU_SHIFT,
> 
> This should be 1u, when introduced in patch 1.
> 

Will fix.

As for other issues you point out, it is rather easier to review and
test if I write separate patches for all of them. Expect more patches.


> > +
> 
> #ifdef CONFIG_PV
> 
> > +void pv_vcpu_destroy(struct vcpu *v);
> > +int pv_vcpu_initialise(struct vcpu *v);
> > +void pv_domain_destroy(struct domain *d);
> > +int pv_domain_initialise(struct domain *d, unsigned int domcr_flags,
> > +                         struct xen_arch_domainconfig *config);
> 
> #else
> 
> static inline void pv_vcpu_destroy(struct vcpu *v) {};
> static inline int pv_vcpu_initialise(struct vcpu *v) { return
> -EOPNOTSUPP; };
> static inline void pv_domain_destroy(struct domain *d) {};
> static inline int pv_domain_initialise(struct domain *d, unsigned int
> domcr_flags,
>                                       struct xen_arch_domainconfig
> *config) { return -EOPNOTSUPP; }
> 
> #endif
> 
> Please can we try to make new code compatible with eventually turning
> off CONFIG_PV and CONFIG_HVM.
> 

My original plan was to do that in next stage. But I'm also ok with
doing it now.

Wei.

> ~Andrew
> 
> > +
> > +#endif     /* __X86_PV_DOMAIN_H__ */
> 

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