Re: [Xen-devel] [PATCH v1 2/8]: PVH mmu changes

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 26 Sep 2012, Mukesh Rathor wrote:
> On Mon, 24 Sep 2012 15:04:22 +0100
> Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> 
> > On Fri, 21 Sep 2012, Mukesh Rathor wrote:
> > > +struct pvh_remap_data {
> > >  struct remap_data {
> > > @@ -2329,7 +2468,9 @@ static int remap_area_mfn_pte_fn(pte_t *ptep,
> > > pgtable_t token, int xen_remap_domain_mfn_range(struct
> > > vm_area_struct *vma, unsigned long addr,
> > >                          unsigned long mfn, int nr,
> > > -                        pgprot_t prot, unsigned domid)
> > > +                        pgprot_t prot, unsigned domid,
> > > +                        struct xen_pvh_pfn_info *pvhp)
> > > +
> > 
> > xen_remap_domain_mfn_range is a cross-architecture call (it is
> > available on ARM as well). We cannot leak architecture specific
> > informations like xen_pvh_pfn_info in the parameter list.
> > It seems to be that xen_pvh_pfn_info contains arch-agnostic
> > information: in that case you just need to rename the struct to
> > something more generic. Otherwise if it really contains x86 specific
> > info, you can change it into an opaque pointer.
> 
> Ok, how about I just change it to:
> 
> -       struct xen_pvh_pfn_info *pvhp)
> +       void *arch_spec_info)

Sorry for misleading you, but on second thought, an opaque pointer is
not a good idea: xen_remap_domain_mfn_range is called from privcmd.c,
that is arch-agnostic code, so I think that all the parameters should be
well specified and arch-agnostic.

I think that you should:

- rename xen_pvh_pfn_info to something non-x86 specific, for example
  xen_remap_pfn_info;
- move the definition of struct xen_remap_pfn_info to
  include/xen/xen-ops.h;
- add a good comment on top of the struct to explain what each field
  represents, because other people (me and Ian) might have to reimplement
  xen_remap_domain_mfn_range in the near future for another popular
  architecture ;-)

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