Re: [Xen-ia64-devel] [PATCH 1/3] [RESEND] [read-only mapping] GNTMAP_readonly support xen part

[Isaku Yamahata <yamahata@xxxxxxxxxxxxx>]

On Tue, May 23, 2006 at 01:08:48PM -0600, Alex Williamson wrote:
> On Fri, 2006-05-19 at 17:31 +0900, Isaku Yamahata wrote:
> >      page = mfn_to_page(mfn);
> >      ret = get_page(page, page_get_owner(page));
> >      BUG_ON(ret == 0);
> > -    assign_domain_page_replace(d, gpaddr, mfn, flags);
> > -
> > +
> > +    arflags = _PAGE_AR_RWX;
> > +    if (flags & GNTMAP_readonly) {
> > +        arflags = _PAGE_AR_R;
> > +    }
> > +    assign_domain_page_replace(d, gpaddr, mfn, arflags);
> >      return GNTST_okay;
> >  }
> 
>    I think we're still parsing flags too high up in the stack.  Couldn't
> we pass flags "as is" to assign_domain_page_replace(), then do
> 
> arflags = (flags & GNTMAP_readonly) ? _PAGE_AR_R : _PAGE_AR_RWX;
> 
> in that end function?  It also seems more logical to call
> __assign_domain_page() as:
> 
> __assign_domain_page(d, j, io_ranges[i].type, GNTMAP_readonly /* or 0 for RWX 
> */)
> 
> than to pass in explicit page flags.  Doing these, we could get rid of
> the BUG_ON checks.  For example:
> 
> __assign_domain_page(struct domain *d,
>                      unsigned long mpaddr, unsigned long physaddr,
>                      unsigned long flags)
> {
>     pte_t *pte;
>     unsigned long arflags = (flags & GNTMAP_readonly) ? _PAGE_AR_R :
>                                                         _PAGE_AR_RWX;
> 
> Thoughts?  I think there are only a couple minor places in patches 2 and
> 3 that would need to be updated to reflect this.  Thanks,

Hmm. __assign_domain_page_replace() isn't specific for grant table.
and it is used by __assign_domain_page() and dom0vp add physmap hypercall.
So I don't think GNTMAP_readonly is good flag name for read only.
How about ASSIGN_readonly? Or can you suggest better name?

-- 
yamahata

_______________________________________________
Xen-ia64-devel mailing list
Xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-ia64-devel