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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Sat, 2012-10-06 at 03:00 +0100, Mukesh Rathor wrote:
> On Thu, 4 Oct 2012 09:27:59 +0100
> Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> 
> > On Wed, 2012-10-03 at 23:29 +0100, Mukesh Rathor wrote:
> > > On Wed, 3 Oct 2012 16:42:43 +0100
> > > Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> > >
> > While implementing the ARM version of this interface it occurred to me
> > that the num_pgs and next_todo fields here are not really needed
> > across the arch interface e.g. you already get pi_num_pgs from the nr
> > argument to xen_remap_domain_mfn_range and pi_next_todo is really
> > state which fits in struct pvh_remap_data. That would mean that
> > remap_foo could take a struct page * directly and I think you would
> > save an allocation.
> 
> Ok, take a look at the attached and inlined diffs. I redid it, passing
> struct page to the api, and getting rid of the pi_* struct completely.

Excellent, thanks.

> @@ -260,14 +266,20 @@ struct mmap_batch_state {
>         xen_pfn_t __user *user_mfn;
>  };
> 
> +/* PVH dom0 fyi: if domU being created is PV, then mfn is mfn(addr on bus). 
> If
> + * it's PVH then mfn is pfn (input to HAP). */

Can we say XENFEAT_auto_translated_physmap (or some abbrev.) instead of
PVH here, since this is generic code.

>  static int mmap_batch_fn(void *data, void *state)
>  {
>         xen_pfn_t *mfnp = data;
>         struct mmap_batch_state *st = state;
> +       struct vm_area_struct *vma = st->vma;
> +       struct page **pages = vma ? vma->vm_private_data : NULL;
> +       struct page *cur_page = pages[st->index++];
>         int ret;
> 
>         ret = xen_remap_domain_mfn_range(st->vma, st->va & PAGE_MASK, *mfnp, 
> 1,
> -                                        st->vma->vm_page_prot, st->domain);
> +                                        st->vma->vm_page_prot, st->domain,
> +                                        &cur_page);
> 
>         /* Store error code for second pass. */
>         *(st->err++) = ret;
[...]
> @@ -370,10 +408,17 @@ static long privcmd_ioctl_mmap_batch(void __user 
> *udata, int version)
>                 up_write(&mm->mmap_sem);
>                 goto out;
>         }
> +       if (xen_feature(XENFEAT_auto_translated_physmap)) {
> +               if ((ret = pvh_privcmd_resv_pfns(vma, m.num)) < 0) {

I renamed this alloc_empty_pages to avoid the PVH terminology in common
code.

There could be an argument for moving this to next to
alloc_xenballooned_pages e.g. as alloc_xenballooned_pagearray. We used
to have alloc_empty_pages_and_pagevec() In the classic kernels which was
effectively that same function.


Ian.


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