Skip to contentSkip to conversation

grant table and bogus mfns

PATCH In review Samuel Thibault 6 participants 14 messages Nov 9, 2007 – Dec 2, 2007

Subsystems (2)
Xen hypervisor interface TPM device driver
lore
Share & tools
lore

Samuel Thibault

author
#0 lore raw

PATCH 1/1 grant table and bogus mfns

Hi,

While developping a frontend, I noticed that if I gave a grant on mfn 0 to a backend (which is bogus), that backend would happily be allowed to map it, and hence Oops...

This is because mfn 0 is considered as iomem, and in that case gnttab_map_grant_ref only checks that the target domain is allowed to access it. The patch below makes it check that the source domain was allowed to access it (and then give the target access to it).

Samuel

Signed-off-by Samuel Thibault <[email protected]>
xen/common/grant_table.c +2 -1
diff -r 4bf62459d45a xen/common/grant_table.c
@@ -335,7 +335,8 @@ __gnttab_map_grant_ref(
         if ( iomem_page_test(frame, mfn_to_page(frame)) )
         {
             is_iomem = 1;
-            if ( iomem_permit_access(ld, frame, frame) )
+            if ( !iomem_access_permitted(rd, frame, frame)
+                  || iomem_permit_access(ld, frame, frame) )
             {
                 gdprintk(XENLOG_WARNING, 
                          "Could not permit access to grant frame "
xen/common/grant_table.c +2 -1
diff -r 4bf62459d45a xen/common/grant_table.c
@@ -335,7 +335,8 @@ __gnttab_map_grant_ref(
         if ( iomem_page_test(frame, mfn_to_page(frame)) )
         {
             is_iomem = 1;
-            if ( iomem_permit_access(ld, frame, frame) )
+            if ( !iomem_access_permitted(rd, frame, frame)
+                  || iomem_permit_access(ld, frame, frame) )
             {
                 gdprintk(XENLOG_WARNING, 
                          "Could not permit access to grant frame "

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw

Thanks, but actually I think the iomem integration is more broken than it first appears. I'm not sure why the local iomem permissions are modified at all. The remote permissions should be interrogated, and that should suffice alone. But you certainly have the gist of a fix here -- AFAICS the code as-is allows unprivileged domains to 'grant' each other access to arbitrary pages of I/O memory, which isn't good. I've cc'ed Kieran who was the originator of that code.

-- Keir

Quoted 32 lines · Samuel Thibault

Hi,

While developping a frontend, I noticed that if I gave a grant on mfn 0 to a backend (which is bogus), that backend would happily be allowed to map it, and hence Oops...

This is because mfn 0 is considered as iomem, and in that case gnttab_map_grant_ref only checks that the target domain is allowed to access it. The patch below makes it check that the source domain was allowed to access it (and then give the target access to it).

Samuel

Signed-off-by Samuel Thibault <[email protected]>

diff -r 4bf62459d45a xen/common/grant_table.c

xen/common/grant_table.c Fri Nov 09 15:01:57 2007 +0000 renamed from xen/common/grant_table.c Wed Nov 07 11:29:38 2007 +0000 +2 -1
@@ -335,7 +335,8 @@ __gnttab_map_grant_ref(
         if ( iomem_page_test(frame, mfn_to_page(frame)) )
         {
             is_iomem = 1;
-            if ( iomem_permit_access(ld, frame, frame) )
+            if ( !iomem_access_permitted(rd, frame, frame)
+                  || iomem_permit_access(ld, frame, frame) )
             {
                 gdprintk(XENLOG_WARNING,
                          "Could not permit access to grant frame "

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#2 lore raw
Quoted 7 lines · Keir Fraser

Thanks, but actually I think the iomem integration is more broken than it first appears. I'm not sure why the local iomem permissions are modified at all. The remote permissions should be interrogated, and that should suffice alone. But you certainly have the gist of a fix here -- AFAICS the code as-is allows unprivileged domains to 'grant' each other access to arbitrary pages of I/O memory, which isn't good. I've cc'ed Kieran who was the originator of that code.

As background: this code was written to provide a means to programatically give I/O memory permissions to unprivileged domains, roughly equivalent to a domctl iomem_permission operation. The suggestion at the time was that we should use the grant tables to achieve this. It's supposed to work as follows:

1) dom0 does a grant op for a page of I/O memory; at this stage no
different to a normal grant.
2) grant reference passed (e.g. through xenstore) to domU
3) domU performs a map operation on that grant
4) hypervisor notices that the grant is for an I/O memory page and
instead of mapping it to a domU virtual address it instead sets up the I/O mem permissions for that domain to access the region (ie. calls iomem_permit_access())
5) domU can then call ioremap() to get a kernel virtual address for the
I/O memory region, and access it as normal.

The bug I think stems from a weakness in iomem_page_test() (recently renamed something like is_iomem_page() I think). I had intended that the call to check the owner of the page was dom_io would prevent unprivileged domains granting access, but it of course needs to also check that the granting domain owns the page. i.e. a quick fix would be to check where iomem_page_test() is called in __gnttab_map_grant_ref() that "rd == dom_io".

I can test and provide a patch for this later today.

If we wanted it to be more general (so that unprivileged domains could legitimately give others access to their own regions of I/O memory) then the suggestion of testing iomem_access_permitted() on the remote domain would then be necessary, but if we restrict it to dom0 providing access (as was initially intended) I'm not sure that is necessary.

Kieran

Quoted 38 lines

-- Keir

Quoted 32 lines · Samuel Thibault

Hi,

While developping a frontend, I noticed that if I gave a grant on mfn 0 to a backend (which is bogus), that backend would happily be allowed to map it, and hence Oops...

This is because mfn 0 is considered as iomem, and in that case gnttab_map_grant_ref only checks that the target domain is allowed to access it. The patch below makes it check that the source domain was allowed to access it (and then give the target access to it).

Samuel

Signed-off-by Samuel Thibault <[email protected]>

diff -r 4bf62459d45a xen/common/grant_table.c

xen/common/grant_table.c Fri Nov 09 15:01:57 2007 +0000 renamed from xen/common/grant_table.c Wed Nov 07 11:29:38 2007 +0000 +2 -1
@@ -335,7 +335,8 @@ __gnttab_map_grant_ref(
         if ( iomem_page_test(frame, mfn_to_page(frame)) )
         {
             is_iomem = 1;
-            if ( iomem_permit_access(ld, frame, frame) )
+            if ( !iomem_access_permitted(rd, frame, frame)
+                  || iomem_permit_access(ld, frame, frame) )
             {
                 gdprintk(XENLOG_WARNING,
                          "Could not permit access to grant frame "

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#3 lore raw
Quoted 1 line · Kieran Mansley

I can test and provide a patch for this later today.

Here it is. I left in the check of the remote domain's iomem permission as although it is unnecessary in the case of dom0 providing the grant it still makes logical sense. Thanks for pointing this problem out.

Signed-off-by Kieran Mansley <[email protected]>

xen/common/grant_table.c +5 -2
diff -r e40591366a0f xen/common/grant_table.c
@@ -332,7 +332,9 @@ __gnttab_map_grant_ref(
     if ( op->flags & GNTMAP_host_map ) 
     {
         /* Could be an iomem page for setting up permission */
-        if ( is_iomem_page(frame) )
+        if ( rd->domain_id == 0 && 
+             is_iomem_page(frame) &&
+             iomem_access_permitted(rd, frame, frame) )
         {
             is_iomem = 1;
             if ( iomem_permit_access(ld, frame, frame) )

@@ -527,7 +529,8 @@ __gnttab_unmap_common(
                                                   op->flags)) < 0 )
                 goto unmap_out;
         }
-        else if ( is_iomem_page(op->frame) &&
+        else if ( rd->domain_id == 0 &&
+                  is_iomem_page(op->frame) &&
                   iomem_access_permitted(ld, op->frame, op->frame) )
         {
             if ( (rc = iomem_deny_access(ld, op->frame, op->frame)) < 0 )
#4 lore raw
In reply to Kieran Mansley
Quoted 10 lines · Kieran Mansley

1) dom0 does a grant op for a page of I/O memory; at this stage no
different to a normal grant.
2) grant reference passed (e.g. through xenstore) to domU
3) domU performs a map operation on that grant
4) hypervisor notices that the grant is for an I/O memory page and
instead of mapping it to a domU virtual address it instead sets up the I/O mem permissions for that domain to access the region (ie. calls iomem_permit_access())
5) domU can then call ioremap() to get a kernel virtual address for the
I/O memory region, and access it as normal.

I didn't realise this was how it worked. I think it's a bad idea -- mapping the grantref should map the I/O page. The mapping domain's io capabilities should not be affected. Apart from being the obvious semantics for map_grant, using the current scheme we cannot be sure when all mappings to the granted page have gone away.

-- Keir

#5 lore raw
Quoted 13 lines · Keir Fraser
Quoted 10 lines · Kieran Mansley

1) dom0 does a grant op for a page of I/O memory; at this stage no
different to a normal grant.
2) grant reference passed (e.g. through xenstore) to domU
3) domU performs a map operation on that grant
4) hypervisor notices that the grant is for an I/O memory page and
instead of mapping it to a domU virtual address it instead sets up the I/O mem permissions for that domain to access the region (ie. calls iomem_permit_access())
5) domU can then call ioremap() to get a kernel virtual address for the
I/O memory region, and access it as normal.

Quoted 4 lines

I didn't realise this was how it worked. I think it's a bad idea -- mapping the grantref should map the I/O page. The mapping domain's io capabilities should not be affected. Apart from being the obvious semantics for map_grant,

I agree that mapping the I/O page when you map the grant ref makes for a much better interface. I'd just obviously got the wrong end of the stick and thought that you wanted it the other way when we first discussed this, and it hadn't occurred to me that doing the map without first getting the right permissions was sane.

Attached is pair of patches against the current xen- unstable.hg/linux-2.6.18-xen.hg (i.e. instead of not on top of the other patch I posted in this thread) that should do the I/O page map when the grant is mapped. The I/O capabilities of the domain are no longer modified. It seems to work for me, is definitely an improvement in the API, and should fix the original bug that was posted.

I added a GNTMAP_nocache flag to match the GNTMAP_readonly etc flags so that you can get the equivalent behaviour of ioremap_nocache() vs ioremap(). The grant is mapped in as normal; e.g.:

  struct gnttab_map_grant_ref op;
  gnttab_set_map_op(&op, (unsigned long)vaddr, GNTMAP_host_map | GNTMAP_nocache, gnt_ref, dev->otherend_id);
  BUG_ON(HYPERVISOR_grant_table_op(GNTTABOP_map_grant_ref, &op, 1));

I'm slightly surprised that without the call to iomem_permit_access() I don't trip over the iomem_access_permitted() check in xen/arch/x86/mm.c:get_page_from_l1e() when the grant is mapped. I hope this doesn't indicate that I've missed something.

Thanks

Kieran

xen/arch/x86/mm.c +5 -1
diff -r e40591366a0f xen/arch/x86/mm.c
@@ -849,12 +849,14 @@ void put_page_from_l1e(l1_pgentry_t l1e,
 void put_page_from_l1e(l1_pgentry_t l1e, struct domain *d)
 {
     unsigned long    pfn  = l1e_get_pfn(l1e);
-    struct page_info *page = mfn_to_page(pfn);
+    struct page_info *page;
     struct domain   *e;
     struct vcpu     *v;
 
     if ( !(l1e_get_flags(l1e) & _PAGE_PRESENT) || !mfn_valid(pfn) )
         return;
+
+    page = mfn_to_page(pfn);
 
     e = page_get_owner(page);
 

@@ -2772,6 +2774,8 @@ int create_grant_host_mapping(
         l1e_add_flags(pte,_PAGE_USER);
     if ( !(flags & GNTMAP_readonly) )
         l1e_add_flags(pte,_PAGE_RW);
+    if ( (flags & GNTMAP_nocache) )
+        l1e_add_flags(pte,_PAGE_PCD);
 
     if ( flags & GNTMAP_contains_pte )
         return create_grant_pte_mapping(addr, pte, current);
xen/common/grant_table.c +56 -30

125 lines not on this page · raw message

xen/include/public/grant_table.h +3 -0
diff -r e40591366a0f xen/include/public/grant_table.h
@@ -380,6 +380,9 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_unmap_and
 #define _GNTMAP_contains_pte    (4)
 #define GNTMAP_contains_pte     (1<<_GNTMAP_contains_pte)
 
+#define _GNTMAP_nocache         (5)
+#define GNTMAP_nocache          (1<<_GNTMAP_nocache)
+
 /*
  * Values for error status returns. All errors are -ve.
  */
include/xen/interface/grant_table.h +3 -0
diff -r 4c96da4993b8 include/xen/interface/grant_table.h
@@ -380,6 +380,9 @@ DEFINE_XEN_GUEST_HANDLE(gnttab_unmap_and
 #define _GNTMAP_contains_pte    (4)
 #define GNTMAP_contains_pte     (1<<_GNTMAP_contains_pte)
 
+#define _GNTMAP_nocache         (5)
+#define GNTMAP_nocache          (1<<_GNTMAP_nocache)
+
 /*
  * Values for error status returns. All errors are -ve.
  */

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#6 lore raw
Quoted 4 lines · Kieran Mansley

I'm slightly surprised that without the call to iomem_permit_access() I don't trip over the iomem_access_permitted() check in xen/arch/x86/mm.c:get_page_from_l1e() when the grant is mapped. I hope this doesn't indicate that I've missed something.

get_page_from_l1e() is not executed on the map_grant path, which explains this behaviour.

As for GNTMAP_nocache, I think actually we should not trust the mapper to get the cache attributes correct (because sometimes if the attributes are wrong the system can be destabilised). Actually we now in xen-unstable have cache-attribute tracking for RAM pages. The cache attributes for the granted mapping should pick up PAT/PWT/PCD from the page's count_info field. If dom0 has the page mapped with the right attributes this will then do the right thing immediately. If dom0 doesn't have the page mapped then we'll need to do a bit more work...

-- Keir

#7 lore raw
Quoted 3 lines · Keir Fraser

As for GNTMAP_nocache, I think actually we should not trust the mapper to get the cache attributes correct (because sometimes if the attributes are wrong the system can be destabilised).

Is this just (or mainly?) a concern for RAM pages? If not, you'll have the same problem when using the domctl iomem_permission and ioremap_nocache() in the guest.

Quoted 6 lines

Actually we now in xen-unstable have
cache-attribute tracking for RAM pages. The cache attributes for the granted mapping should pick up PAT/PWT/PCD from the page's count_info field. If dom0 has the page mapped with the right attributes this will then do the right thing immediately. If dom0 doesn't have the page mapped then we'll need to do a bit more work...

dom0 does have the page mapped, but I think there is no page_info struct (and so no count_info) for some I/O memory pages. Perhaps a short-term fix would be to remove the GNTMAP_nocache option that I've added and instead just make all I/O memory pages that get mapped through this mechanism forced to _PAGE_PCD. That way the mapper doesn't get to choose (and we loose the ability to do an ioremap() equivalent as opposed to an ioremap_nocache() equivalent, but that's not a big problem right now).

If that sounds agreeable I can spin another patch.

Kieran

#8 lore raw

You make a good point. Let's instead allow the granter to explicitly specify the cache attributes in the grant entry. We have space in the flags field. Let's use bits 5,6,7 of that field. They can have the same format as the three contiguous bits we have added in page->count_info. Conveniently, all zeroes means map-WB-cacheable, which is the correct default.

-- Keir

Quoted 29 lines · Kieran Mansley
Quoted 3 lines · Keir Fraser

As for GNTMAP_nocache, I think actually we should not trust the mapper to get the cache attributes correct (because sometimes if the attributes are wrong the system can be destabilised).

Is this just (or mainly?) a concern for RAM pages? If not, you'll have the same problem when using the domctl iomem_permission and ioremap_nocache() in the guest.

Quoted 6 lines

Actually we now in xen-unstable have
cache-attribute tracking for RAM pages. The cache attributes for the granted mapping should pick up PAT/PWT/PCD from the page's count_info field. If dom0 has the page mapped with the right attributes this will then do the right thing immediately. If dom0 doesn't have the page mapped then we'll need to do a bit more work...

dom0 does have the page mapped, but I think there is no page_info struct (and so no count_info) for some I/O memory pages. Perhaps a short-term fix would be to remove the GNTMAP_nocache option that I've added and instead just make all I/O memory pages that get mapped through this mechanism forced to _PAGE_PCD. That way the mapper doesn't get to choose (and we loose the ability to do an ioremap() equivalent as opposed to an ioremap_nocache() equivalent, but that's not a big problem right now).

If that sounds agreeable I can spin another patch.

Kieran

#9 lore raw
Quoted 5 lines · Keir Fraser

You make a good point. Let's instead allow the granter to explicitly specify the cache attributes in the grant entry. We have space in the flags field. Let's use bits 5,6,7 of that field. They can have the same format as the three contiguous bits we have added in page->count_info. Conveniently, all zeroes means map-WB-cacheable, which is the correct default.

Attached is another spin of the patches to do this, based on a tree with 16067 reverted as requested.

Quoted 1 line

From the guest API point of view, previously gnttab_grant_foreign_access

() (and _ref()) took a "readonly" flag as an argument. This is now a more general flags field to allow the granter to specify the cache attributes. I also took the opportunity to remove the "readonly" flag from gnttab_remove_foreign_access() as it was unused and I couldn't see how to make useful in light of this change.

Within the hypervisor, the granter's specified cache attributes are ignored at the point of mapping unless it is an I/O memory grant; for RAM grants the cache attributes should I think be picked up automatically using the cache-attribute tracking you mentioned, and this seemed safer than allowing the granter to override.

In the case of an I/O memory map the granter-specified flags are passed into create_grant_host_mapping() to add to the page table entry's flags.

I/0 memory maps using the grant table are only allowed in the case where dom0 is doing the granting, the granter has iomem_access_permitted(), GNTMAP_host_map is specified and GNTMAP_device_map is not. Reference counting using put_page/get_page is only done for those I/0 memory pages that have mfn_valid()

I'm pretty happy with this now.

Signed-off-by Kieran Mansley <[email protected]>

Thanks

Kieran

drivers/char/tpm/tpm_xen.c +1 -2
diff -r 4c96da4993b8 drivers/char/tpm/tpm_xen.c
@@ -266,8 +266,7 @@ static void destroy_tpmring(struct tpm_p
 	tpmif_set_connected_state(tp, 0);
 
 	if (tp->ring_ref != GRANT_INVALID_REF) {
-		gnttab_end_foreign_access(tp->ring_ref, 0,
-					  (unsigned long)tp->tx);
+		gnttab_end_foreign_access(tp->ring_ref, (unsigned long)tp->tx);
 		tp->ring_ref = GRANT_INVALID_REF;
 		tp->tx = NULL;
 	}
drivers/xen/blkfront/blkfront.c +6 -6
diff -r 4c96da4993b8 drivers/xen/blkfront/blkfront.c
@@ -622,7 +622,7 @@ static int blkif_queue_request(struct re
 				ref,
 				info->xbdev->otherend_id,
 				buffer_mfn,
-				rq_data_dir(req) );
+				rq_data_dir(req) ? GTF_readonly : 0 );
 
 			info->shadow[id].frame[ring_req->nr_segments] =
 				mfn_to_pfn(buffer_mfn);

@@ -790,7 +790,7 @@ static void blkif_free(struct blkfront_i
 
 	/* Free resources associated with old device channel. */
 	if (info->ring_ref != GRANT_INVALID_REF) {
-		gnttab_end_foreign_access(info->ring_ref, 0,
+		gnttab_end_foreign_access(info->ring_ref, 
 					  (unsigned long)info->ring.sring);
 		info->ring_ref = GRANT_INVALID_REF;
 		info->ring.sring = NULL;

@@ -804,7 +804,7 @@ static void blkif_completion(struct blk_
 {
 	int i;
 	for (i = 0; i < s->req.nr_segments; i++)
-		gnttab_end_foreign_access(s->req.seg[i].gref, 0, 0UL);
+		gnttab_end_foreign_access(s->req.seg[i].gref, 0UL);
 }
 
 static void blkif_recover(struct blkfront_info *info)

@@ -846,9 +846,9 @@ static void blkif_recover(struct blkfron
 				req->seg[j].gref,
 				info->xbdev->otherend_id,
 				pfn_to_mfn(info->shadow[req->id].frame[j]),
-				rq_data_dir(
-					(struct request *)
-					info->shadow[req->id].request));
+				rq_data_dir((struct request *)
+					    info->shadow[req->id].request) ?
+				GTF_readonly : 0);
 		info->shadow[req->id].req = *req;
 
 		info->ring.req_prod_pvt++;
drivers/xen/core/gnttab.c +10 -9
diff -r 4c96da4993b8 drivers/xen/core/gnttab.c
@@ -140,7 +140,7 @@ static void put_free_entry(grant_ref_t r
  */
 
 int gnttab_grant_foreign_access(domid_t domid, unsigned long frame,
-				int readonly)
+				int flags)
 {
 	int ref;
 

@@ -150,19 +150,21 @@ int gnttab_grant_foreign_access(domid_t 
 	shared[ref].frame = frame;
 	shared[ref].domid = domid;
 	wmb();
-	shared[ref].flags = GTF_permit_access | (readonly ? GTF_readonly : 0);
+	BUG_ON(flags & (GTF_accept_transfer | GTF_reading | GTF_writing));
+	shared[ref].flags = GTF_permit_access | flags;
 
 	return ref;
 }
 EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access);
 
 void gnttab_grant_foreign_access_ref(grant_ref_t ref, domid_t domid,
-				     unsigned long frame, int readonly)
+				     unsigned long frame, int flags)
 {
 	shared[ref].frame = frame;
 	shared[ref].domid = domid;
 	wmb();
-	shared[ref].flags = GTF_permit_access | (readonly ? GTF_readonly : 0);
+	BUG_ON(flags & (GTF_accept_transfer | GTF_reading | GTF_writing));
+	shared[ref].flags = GTF_permit_access | flags;
 }
 EXPORT_SYMBOL_GPL(gnttab_grant_foreign_access_ref);
 

@@ -177,7 +179,7 @@ int gnttab_query_foreign_access(grant_re
 }
 EXPORT_SYMBOL_GPL(gnttab_query_foreign_access);
 
-int gnttab_end_foreign_access_ref(grant_ref_t ref, int readonly)
+int gnttab_end_foreign_access_ref(grant_ref_t ref)
 {
 	u16 flags, nflags;
 

@@ -194,10 +196,9 @@ int gnttab_end_foreign_access_ref(grant_
 }
 EXPORT_SYMBOL_GPL(gnttab_end_foreign_access_ref);
 
-void gnttab_end_foreign_access(grant_ref_t ref, int readonly,
-			       unsigned long page)
-{
-	if (gnttab_end_foreign_access_ref(ref, readonly)) {
+void gnttab_end_foreign_access(grant_ref_t ref, unsigned long page)
+{
+	if (gnttab_end_foreign_access_ref(ref)) {
 		put_free_entry(ref);
 		if (page != 0)
 			free_page(page);
drivers/xen/netfront/netfront.c +8 -10
diff -r 4c96da4993b8 drivers/xen/netfront/netfront.c
@@ -663,8 +663,7 @@ static void network_tx_buf_gc(struct net
 				       "domain.\n");
 				BUG();
 			}
-			gnttab_end_foreign_access_ref(
-				np->grant_tx_ref[id], GNTMAP_readonly);
+			gnttab_end_foreign_access_ref(np->grant_tx_ref[id]);
 			gnttab_release_grant_reference(
 				&np->gref_tx_head, np->grant_tx_ref[id]);
 			np->grant_tx_ref[id] = GRANT_INVALID_REF;

@@ -888,7 +887,7 @@ static void xennet_make_frags(struct sk_
 
 		mfn = virt_to_mfn(data);
 		gnttab_grant_foreign_access_ref(ref, np->xbdev->otherend_id,
-						mfn, GNTMAP_readonly);
+						mfn, GTF_readonly);
 
 		tx->gref = np->grant_tx_ref[id] = ref;
 		tx->offset = offset;

@@ -910,7 +909,7 @@ static void xennet_make_frags(struct sk_
 
 		mfn = pfn_to_mfn(page_to_pfn(frag->page));
 		gnttab_grant_foreign_access_ref(ref, np->xbdev->otherend_id,
-						mfn, GNTMAP_readonly);
+						mfn, GTF_readonly);
 
 		tx->gref = np->grant_tx_ref[id] = ref;
 		tx->offset = frag->page_offset;

@@ -972,7 +971,7 @@ static int network_start_xmit(struct sk_
 	BUG_ON((signed short)ref < 0);
 	mfn = virt_to_mfn(data);
 	gnttab_grant_foreign_access_ref(
-		ref, np->xbdev->otherend_id, mfn, GNTMAP_readonly);
+		ref, np->xbdev->otherend_id, mfn, GTF_readonly);
 	tx->gref = np->grant_tx_ref[id] = ref;
 	tx->offset = offset;
 	tx->size = len;

@@ -1198,7 +1197,7 @@ static int xennet_get_responses(struct n
 			}
 			pages_flipped++;
 		} else {
-			ret = gnttab_end_foreign_access_ref(ref, 0);
+			ret = gnttab_end_foreign_access_ref(ref);
 			BUG_ON(!ret);
 		}
 

@@ -1531,8 +1530,7 @@ static void netif_release_tx_bufs(struct
 			continue;
 
 		skb = np->tx_skbs[i];
-		gnttab_end_foreign_access_ref(
-			np->grant_tx_ref[i], GNTMAP_readonly);
+		gnttab_end_foreign_access_ref(np->grant_tx_ref[i]);
 		gnttab_release_grant_reference(
 			&np->gref_tx_head, np->grant_tx_ref[i]);
 		np->grant_tx_ref[i] = GRANT_INVALID_REF;

@@ -1642,7 +1640,7 @@ static void netif_release_rx_bufs_copy(s
 
 		skb = np->rx_skbs[i];
 
-		if (!gnttab_end_foreign_access_ref(ref, 0))
+		if (!gnttab_end_foreign_access_ref(ref))
 		{
 			busy++;
 			continue;

@@ -2151,7 +2149,7 @@ static void end_access(int ref, void *pa
 static void end_access(int ref, void *page)
 {
 	if (ref != GRANT_INVALID_REF)
-		gnttab_end_foreign_access(ref, 0, (unsigned long)page);
+		gnttab_end_foreign_access(ref, (unsigned long)page);
 }
drivers/xen/pcifront/xenbus.c +1 -1
diff -r 4c96da4993b8 drivers/xen/pcifront/xenbus.c
@@ -57,7 +57,7 @@ static void free_pdev(struct pcifront_de
 		xenbus_free_evtchn(pdev->xdev, pdev->evtchn);
 
 	if (pdev->gnt_ref != INVALID_GRANT_REF)
-		gnttab_end_foreign_access(pdev->gnt_ref, 0,
+		gnttab_end_foreign_access(pdev->gnt_ref,
 					  (unsigned long)pdev->sh_info);
 
 	pdev->xdev->dev.driver_data = NULL;
include/xen/gnttab.h +4 -5
diff -r 4c96da4993b8 include/xen/gnttab.h
@@ -51,14 +51,14 @@ struct gnttab_free_callback {
 };
 
 int gnttab_grant_foreign_access(domid_t domid, unsigned long frame,
-				int readonly);
+				int flags);
 
 /*
  * End access through the given grant reference, iff the grant entry is no
  * longer in use.  Return 1 if the grant entry was freed, 0 if it is still in
  * use.
  */
-int gnttab_end_foreign_access_ref(grant_ref_t ref, int readonly);
+int gnttab_end_foreign_access_ref(grant_ref_t ref);
 
 /*
  * Eventually end access through the given grant reference, and once that

@@ -66,8 +66,7 @@ int gnttab_end_foreign_access_ref(grant_
  * immediately iff the grant entry is not in use, otherwise it will happen
  * some time later.  page may be 0, in which case no freeing will occur.
  */
-void gnttab_end_foreign_access(grant_ref_t ref, int readonly,
-			       unsigned long page);
+void gnttab_end_foreign_access(grant_ref_t ref, unsigned long page);
 
 int gnttab_grant_foreign_transfer(domid_t domid, unsigned long pfn);
 

@@ -97,7 +96,7 @@ void gnttab_cancel_free_callback(struct 
 void gnttab_cancel_free_callback(struct gnttab_free_callback *callback);
 
 void gnttab_grant_foreign_access_ref(grant_ref_t ref, domid_t domid,
-				     unsigned long frame, int readonly);
+				     unsigned long frame, int flags);
 
 void gnttab_grant_foreign_transfer_ref(grant_ref_t, domid_t domid,
 				       unsigned long pfn);
include/xen/interface/grant_table.h +7 -0
diff -r 4c96da4993b8 include/xen/interface/grant_table.h
@@ -119,6 +119,7 @@ typedef struct grant_entry grant_entry_t
  *  GTF_readonly: Restrict @domid to read-only mappings and accesses. [GST]
  *  GTF_reading: Grant entry is currently mapped for reading by @domid. [XEN]
  *  GTF_writing: Grant entry is currently mapped for writing by @domid. [XEN]
+ *  GTF_PAT, GTF_PWT, GTF_PCD: Cache attribute flags for the grant [GST]
  */
 #define _GTF_readonly       (2)
 #define GTF_readonly        (1U<<_GTF_readonly)

@@ -126,6 +127,12 @@ typedef struct grant_entry grant_entry_t
 #define GTF_reading         (1U<<_GTF_reading)
 #define _GTF_writing        (4)
 #define GTF_writing         (1U<<_GTF_writing)
+#define _GTF_PAT            (5)
+#define GTF_PAT             (1U<<_GTF_PAT)
+#define _GTF_PWT            (6)
+#define GTF_PWT             (1U<<_GTF_PWT)
+#define _GTF_PCD            (7)
+#define GTF_PCD             (1U<<_GTF_PCD)
 
 /*
  * Subflags for GTF_accept_transfer:
xen/arch/ia64/xen/mm.c +7 -5
diff -r b9071cba1c2d xen/arch/ia64/xen/mm.c
@@ -2144,16 +2144,18 @@ dom0vp_unexpose_foreign_p2m(struct domai
 // mfn: frame: machine page frame
 // flags: GNTMAP_readonly | GNTMAP_application_map | GNTMAP_contains_pte
 int
-create_grant_host_mapping(unsigned long gpaddr,
-              unsigned long mfn, unsigned int flags)
+create_grant_host_mapping(unsigned long gpaddr, unsigned long mfn, 
+                          unsigned int flags, unsigned int cache_flags)
 {
     struct domain* d = current->domain;
     struct page_info* page;
     int ret;
 
-    if (flags & (GNTMAP_device_map |
-                 GNTMAP_application_map | GNTMAP_contains_pte)) {
-        gdprintk(XENLOG_INFO, "%s: flags 0x%x\n", __func__, flags);
+    if ((flags & (GNTMAP_device_map | 
+                  GNTMAP_application_map | GNTMAP_contains_pte)) ||
+        (cache_flags)) {
+        gdprintk(XENLOG_INFO, "%s: flags 0x%x cache_flags 0x%x\n",
+                 __func__, flags, cache_flags);
         return GNTST_general_error;
     }
xen/arch/powerpc/mm.c +6 -1
diff -r b9071cba1c2d xen/arch/powerpc/mm.c
@@ -168,7 +168,7 @@ static int destroy_grant_va_mapping(
 }
 
 int create_grant_host_mapping(
-    unsigned long addr, unsigned long frame, unsigned int flags)
+    unsigned long addr, unsigned long frame, unsigned int flags, unsigned int cache_flags)
 {
     if (flags & GNTMAP_application_map) {
         printk("%s: GNTMAP_application_map not supported\n", __func__);

@@ -177,6 +177,11 @@ int create_grant_host_mapping(
     }
     if (flags & GNTMAP_contains_pte) {
         printk("%s: GNTMAP_contains_pte not supported\n", __func__);
+        BUG();
+        return GNTST_general_error;
+    }
+    if (cache_flags) {
+        printk("%s: cache_flags not supported\n", __func__);
         BUG();
         return GNTST_general_error;
     }
xen/arch/x86/mm.c +12 -3
diff -r b9071cba1c2d xen/arch/x86/mm.c
@@ -849,12 +849,14 @@ void put_page_from_l1e(l1_pgentry_t l1e,
 void put_page_from_l1e(l1_pgentry_t l1e, struct domain *d)
 {
     unsigned long    pfn  = l1e_get_pfn(l1e);
-    struct page_info *page = mfn_to_page(pfn);
+    struct page_info *page;
     struct domain   *e;
     struct vcpu     *v;
 
     if ( !(l1e_get_flags(l1e) & _PAGE_PRESENT) || !mfn_valid(pfn) )
         return;
+
+    page = mfn_to_page(pfn);
 
     e = page_get_owner(page);
 

@@ -2763,8 +2765,8 @@ static int destroy_grant_va_mapping(
     return replace_grant_va_mapping(addr, frame, l1e_empty(), v);
 }
 
-int create_grant_host_mapping(
-    uint64_t addr, unsigned long frame, unsigned int flags)
+int create_grant_host_mapping(uint64_t addr, unsigned long frame, 
+                              unsigned int flags, unsigned int cache_flags)
 {
     l1_pgentry_t pte = l1e_from_pfn(frame, GRANT_PTE_FLAGS);
 

@@ -2772,6 +2774,13 @@ int create_grant_host_mapping(
         l1e_add_flags(pte,_PAGE_USER);
     if ( !(flags & GNTMAP_readonly) )
         l1e_add_flags(pte,_PAGE_RW);
+
+    if ( (cache_flags & GTF_PAT) )
+        l1e_add_flags(pte,_PAGE_PAT);
+    if ( (cache_flags & GTF_PWT) )
+        l1e_add_flags(pte,_PAGE_PWT);
+    if ( (cache_flags & GTF_PCD) )
+        l1e_add_flags(pte,_PAGE_PCD);
 
     if ( flags & GNTMAP_contains_pte )
         return create_grant_pte_mapping(addr, pte, current);
xen/common/grant_table.c +113 -27

179 lines not on this page · raw message

xen/include/asm-ia64/grant_table.h +2 -1
diff -r b9071cba1c2d xen/include/asm-ia64/grant_table.h
@@ -8,7 +8,8 @@
 #define INITIAL_NR_GRANT_FRAMES 1
 
 // for grant map/unmap
-int create_grant_host_mapping(unsigned long gpaddr, unsigned long mfn, unsigned int flags);
+int create_grant_host_mapping(unsigned long gpaddr, unsigned long mfn, 
+			      unsigned int flags, unsigned int cache_flags);
 int replace_grant_host_mapping(unsigned long gpaddr, unsigned long mfn, unsigned long new_gpaddr, unsigned int flags);
 
 // for grant transfer
xen/include/asm-powerpc/grant_table.h +2 -2
diff -r b9071cba1c2d xen/include/asm-powerpc/grant_table.h
@@ -33,8 +33,8 @@ extern long pte_remove(ulong flags, ulon
 extern long pte_remove(ulong flags, ulong ptex, ulong avpn,
                        ulong *hi, ulong *lo);
 
-int create_grant_host_mapping(
-    unsigned long addr, unsigned long frame, unsigned int flags);
+int create_grant_host_mapping(unsigned long addr, unsigned long frame, 
+			      unsigned int flags, unsigned int cache_flags);
 int replace_grant_host_mapping(
     unsigned long addr, unsigned long frame, unsigned long new_addr,
     unsigned int flags);
xen/include/asm-x86/grant_table.h +2 -2
diff -r b9071cba1c2d xen/include/asm-x86/grant_table.h
@@ -13,8 +13,8 @@
  * Caller must own caller's BIGLOCK, is responsible for flushing the TLB, and
  * must hold a reference to the page.
  */
-int create_grant_host_mapping(
-    uint64_t addr, unsigned long frame, unsigned int flags);
+int create_grant_host_mapping(uint64_t addr, unsigned long frame,
+			      unsigned int flags, unsigned int cache_flags);
 int replace_grant_host_mapping(
     uint64_t addr, unsigned long frame, uint64_t new_addr, unsigned int flags);
xen/include/public/grant_table.h +7 -0
diff -r b9071cba1c2d xen/include/public/grant_table.h
@@ -119,6 +119,7 @@ typedef struct grant_entry grant_entry_t
  *  GTF_readonly: Restrict @domid to read-only mappings and accesses. [GST]
  *  GTF_reading: Grant entry is currently mapped for reading by @domid. [XEN]
  *  GTF_writing: Grant entry is currently mapped for writing by @domid. [XEN]
+ *  GTF_PAT, GTF_PWT, GTF_PCD: Cache attribute flags for the grant [GST]
  */
 #define _GTF_readonly       (2)
 #define GTF_readonly        (1U<<_GTF_readonly)

@@ -126,6 +127,12 @@ typedef struct grant_entry grant_entry_t
 #define GTF_reading         (1U<<_GTF_reading)
 #define _GTF_writing        (4)
 #define GTF_writing         (1U<<_GTF_writing)
+#define _GTF_PAT            (5)
+#define GTF_PAT             (1U<<_GTF_PAT)
+#define _GTF_PWT            (6)
+#define GTF_PWT             (1U<<_GTF_PWT)
+#define _GTF_PCD            (7)
+#define GTF_PCD             (1U<<_GTF_PCD)
 
 /*
  * Subflags for GTF_accept_transfer:

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#10 lore raw

about parallex fs

In reply to Kieran Mansley

hi
I have read the paper about parallex FS in xen , and I am interested in it ,but I notice that parallex FS which is in the tree for earlier version ,has been dropped out in the later version, what is the reason for dropping it out, in my opinion,it is an interesting and promising technech,is it ? or are there some disadvantages in it ,or are there some even more interesting substituent tech for it?
I am confused about it ,could someone give me some exeplaination

thanks in advance

#11 lore raw

Re: about parallex fs

Quoted 7 lines

I have read the paper about parallex FS in xen , and I am interested in it ,but I notice that parallex FS which is in the tree for earlier version ,has been dropped out in the later version, what is the reason for dropping it out, in my opinion,it is an interesting and promising technech,is it ? or are there some disadvantages in it ,or are there some even more interesting substituent tech for it?
I am confused about it ,could someone give me some exeplaination

I don't think the parallax in the mainline tree was ever developed very heavily; it seems likely that the version written up in the paper was based on a modified source tree.

But the code went unmaintained for some time and was presumably removed from the source tree due to it getting old and not working properly... I agree it seemed like a really interesting technology and it's a shame not to see more done with it. I get the impression that most enterprises are using NAS/SAN solutions already, so perhaps there wasn't sufficient demand for Parallax as a competing solution...

Cheers,
Mark

Signature · 4 lines
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!
#12 lore raw

Re: about parallex fs

Actually, Parallax is currently under active development. Dutch Meyer recently presented a progress report on it at the Xen summit. I believe Dutch is planning a release of the updated Parallax source RSN, but I'll leave it to him (copied) to commit to a date. ;)

a.

Quoted 34 lines · Mark Williamson
Quoted 7 lines

I have read the paper about parallex FS in xen , and I am interested in it ,but I notice that parallex FS which is in the tree for earlier version ,has been dropped out in the later version, what is the reason for dropping it out, in my opinion,it is an interesting and promising technech,is it ? or are there some disadvantages in it ,or are there some even more interesting substituent tech for it?
I am confused about it ,could someone give me some exeplaination

I don't think the parallax in the mainline tree was ever developed very heavily; it seems likely that the version written up in the paper was based on a modified source tree.

But the code went unmaintained for some time and was presumably removed from the source tree due to it getting old and not working properly... I agree it seemed like a really interesting technology and it's a shame not to see more done with it. I get the impression that most enterprises are using NAS/SAN solutions already, so perhaps there wasn't sufficient demand for Parallax as a competing solution...

Cheers,
Mark

--
Dave: Just a question. What use is a unicyle with no seat? And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#13 lore raw

Re: about parallex fs

Quoted 4 lines

Actually, Parallax is currently under active development. Dutch Meyer recently presented a progress report on it at the Xen summit. I believe Dutch is planning a release of the updated Parallax source RSN, but I'll leave it to him (copied) to commit to a date. ;)

Ah, that's excellent news! I had the impression that you'd moved onto other things and was worried that it'd lost priority. It'll be awesome to see a parallax release.

I hope the summit proceedings get online soon so I can find out what's going on in Xenland ;-)

Cheers,
Mark

Quoted 3 lines

a.

On Dec 1, 2007 9:18 PM, Mark Williamson <[email protected]>

Quoted 33 lines · wrote:
Quoted 33 lines
Quoted 7 lines

I have read the paper about parallex FS in xen , and I am interested in it ,but I notice that parallex FS which is in the tree for earlier version ,has been dropped out in the later version, what is the reason for dropping it out, in my opinion,it is an interesting and promising technech,is it ? or are there some disadvantages in it ,or are there some even more interesting substituent tech for it?
I am confused about it ,could someone give me some exeplaination

I don't think the parallax in the mainline tree was ever developed very heavily; it seems likely that the version written up in the paper was based on a modified source tree.

But the code went unmaintained for some time and was presumably removed from the source tree due to it getting old and not working properly... I agree it seemed like a really interesting technology and it's a shame not to see more done with it. I get the impression that most enterprises are using NAS/SAN solutions already, so perhaps there wasn't sufficient demand for Parallax as a competing solution...

Cheers,
Mark

--
Dave: Just a question. What use is a unicyle with no seat? And no pedals! Mark: To answer a question with a question: What use is a skateboard? Dave: Skateboards have wheels.
Mark: My wheel has a wheel!

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Signature · 4 lines
Dave: Just a question. What use is a unicyle with no seat?  And no pedals!
Mark: To answer a question with a question: What use is a skateboard?
Dave: Skateboards have wheels.
Mark: My wheel has a wheel!