WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-ia64-devel

Re: [Xen-ia64-devel] [PATCH 1/3] [RESEND] [read-only mapping] GNTMAP_rea

To: Isaku Yamahata <yamahata@xxxxxxxxxxxxx>
Subject: Re: [Xen-ia64-devel] [PATCH 1/3] [RESEND] [read-only mapping] GNTMAP_readonly support xen part
From: Alex Williamson <alex.williamson@xxxxxx>
Date: Tue, 23 May 2006 13:08:48 -0600
Cc: xen-ia64-devel@xxxxxxxxxxxxxxxxxxx
Delivery-date: Tue, 23 May 2006 12:09:01 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
In-reply-to: <20060519083104.GA23736%yamahata@xxxxxxxxxxxxx>
List-help: <mailto:xen-ia64-devel-request@lists.xensource.com?subject=help>
List-id: Discussion of the ia64 port of Xen <xen-ia64-devel.lists.xensource.com>
List-post: <mailto:xen-ia64-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-ia64-devel>, <mailto:xen-ia64-devel-request@lists.xensource.com?subject=unsubscribe>
Organization: LOSL
References: <20060519083104.GA23736%yamahata@xxxxxxxxxxxxx>
Sender: xen-ia64-devel-bounces@xxxxxxxxxxxxxxxxxxx
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,

        Alex
 
-- 
Alex Williamson                             HP Linux & Open Source Lab


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