linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xen-blkfront: set pages are FOREIGN_FRAME when sharing them
@ 2012-03-27 13:49 Stefano Stabellini
  2012-03-27 17:58 ` [Xen-devel] " Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 6+ messages in thread
From: Stefano Stabellini @ 2012-03-27 13:49 UTC (permalink / raw)
  To: konrad.wilk
  Cc: Stefano.Stabellini, linux-kernel, xen-devel, Stefano Stabellini

Set pages as FOREIGN_FRAME whenever blkfront shares them with another
domain. Then when blkfront un-share them, also removes the
FOREIGN_FRAME_BIT from the p2m.

We do it so that when the source and the destination domain are the same
(blkfront connected to blkback in the same domain) we can more easily
recognize which ones are the source pfns and which ones are the
destination pfns (both are going to be pointing to the same mfns).

Without this patch enstablishing a connection between blkfront and QEMU
qdisk in the same domain causes QEMU to hang and never return.
Considering that it fixes a serious bug, it should be backported to
previous releases.

Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
---
 drivers/block/xen-blkfront.c |   30 +++++++++++++++++++++++++-----
 1 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 2f22874..e027aa7 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -262,7 +262,7 @@ static int blkif_ioctl(struct block_device *bdev, fmode_t mode,
 static int blkif_queue_request(struct request *req)
 {
 	struct blkfront_info *info = req->rq_disk->private_data;
-	unsigned long buffer_mfn;
+	unsigned long buffer_mfn, buffer_pfn;
 	struct blkif_request *ring_req;
 	unsigned long id;
 	unsigned int fsect, lsect;
@@ -321,7 +321,8 @@ static int blkif_queue_request(struct request *req)
 		       BLKIF_MAX_SEGMENTS_PER_REQUEST);
 
 		for_each_sg(info->sg, sg, ring_req->u.rw.nr_segments, i) {
-			buffer_mfn = pfn_to_mfn(page_to_pfn(sg_page(sg)));
+			buffer_pfn = page_to_pfn(sg_page(sg));
+			buffer_mfn = pfn_to_mfn(buffer_pfn);
 			fsect = sg->offset >> 9;
 			lsect = fsect + (sg->length >> 9) - 1;
 			/* install a grant reference. */
@@ -340,6 +341,16 @@ static int blkif_queue_request(struct request *req)
 						.gref       = ref,
 						.first_sect = fsect,
 						.last_sect  = lsect };
+			/* 
+			 * Set the page as foreign, considering that we are giving
+			 * it to a foreign domain.
+			 * This is important in case the destination domain is
+			 * ourselves, so that we can more easily recognize the
+			 * source pfn from destination pfn, both mapping to the same
+			 * mfn.
+			 */
+			set_phys_to_machine(buffer_pfn,
+					FOREIGN_FRAME(buffer_mfn));
 		}
 	}
 
@@ -715,8 +726,11 @@ static void blkif_completion(struct blk_shadow *s)
 	int i;
 	/* Do not let BLKIF_OP_DISCARD as nr_segment is in the same place
 	 * flag. */
-	for (i = 0; i < s->req.u.rw.nr_segments; i++)
+	for (i = 0; i < s->req.u.rw.nr_segments; i++) {
 		gnttab_end_foreign_access(s->req.u.rw.seg[i].gref, 0, 0UL);
+		set_phys_to_machine(s->frame[i],
+				get_phys_to_machine(s->frame[i]) & ~FOREIGN_FRAME_BIT);
+	}
 }
 
 static irqreturn_t blkif_interrupt(int irq, void *dev_id)
@@ -1051,13 +1065,19 @@ static int blkif_recover(struct blkfront_info *info)
 		memcpy(&info->shadow[req->u.rw.id], &copy[i], sizeof(copy[i]));
 
 		if (req->operation != BLKIF_OP_DISCARD) {
+			unsigned long buffer_pfn;
+			unsigned long buffer_mfn;
 		/* Rewrite any grant references invalidated by susp/resume. */
-			for (j = 0; j < req->u.rw.nr_segments; j++)
+			for (j = 0; j < req->u.rw.nr_segments; j++) {
+				buffer_pfn = info->shadow[req->u.rw.id].frame[j];
+				buffer_mfn = pfn_to_mfn(buffer_pfn);
 				gnttab_grant_foreign_access_ref(
 					req->u.rw.seg[j].gref,
 					info->xbdev->otherend_id,
-					pfn_to_mfn(info->shadow[req->u.rw.id].frame[j]),
+					buffer_mfn,
 					rq_data_dir(info->shadow[req->u.rw.id].request));
+				set_phys_to_machine(buffer_pfn, FOREIGN_FRAME(buffer_mfn));
+			}
 		}
 		info->shadow[req->u.rw.id].req = *req;
 
-- 
1.7.2.5


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Xen-devel] [PATCH] xen-blkfront: set pages are FOREIGN_FRAME when sharing them
  2012-03-27 13:49 [PATCH] xen-blkfront: set pages are FOREIGN_FRAME when sharing them Stefano Stabellini
@ 2012-03-27 17:58 ` Konrad Rzeszutek Wilk
  2012-03-29 13:56   ` Stefano Stabellini
  0 siblings, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-03-27 17:58 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: konrad.wilk, xen-devel, linux-kernel

On Tue, Mar 27, 2012 at 02:49:58PM +0100, Stefano Stabellini wrote:
> Set pages as FOREIGN_FRAME whenever blkfront shares them with another
> domain. Then when blkfront un-share them, also removes the
> FOREIGN_FRAME_BIT from the p2m.
> 
> We do it so that when the source and the destination domain are the same
> (blkfront connected to blkback in the same domain) we can more easily
> recognize which ones are the source pfns and which ones are the
> destination pfns (both are going to be pointing to the same mfns).
> 
> Without this patch enstablishing a connection between blkfront and QEMU
> qdisk in the same domain causes QEMU to hang and never return.
> Considering that it fixes a serious bug, it should be backported to
> previous releases.


ERROR: "set_phys_to_machine" [drivers/block/xen-blkfront.ko] undefined!

> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  drivers/block/xen-blkfront.c |   30 +++++++++++++++++++++++++-----
>  1 files changed, 25 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
> index 2f22874..e027aa7 100644
> --- a/drivers/block/xen-blkfront.c
> +++ b/drivers/block/xen-blkfront.c
> @@ -262,7 +262,7 @@ static int blkif_ioctl(struct block_device *bdev, fmode_t mode,
>  static int blkif_queue_request(struct request *req)
>  {
>  	struct blkfront_info *info = req->rq_disk->private_data;
> -	unsigned long buffer_mfn;
> +	unsigned long buffer_mfn, buffer_pfn;
>  	struct blkif_request *ring_req;
>  	unsigned long id;
>  	unsigned int fsect, lsect;
> @@ -321,7 +321,8 @@ static int blkif_queue_request(struct request *req)
>  		       BLKIF_MAX_SEGMENTS_PER_REQUEST);
>  
>  		for_each_sg(info->sg, sg, ring_req->u.rw.nr_segments, i) {
> -			buffer_mfn = pfn_to_mfn(page_to_pfn(sg_page(sg)));
> +			buffer_pfn = page_to_pfn(sg_page(sg));
> +			buffer_mfn = pfn_to_mfn(buffer_pfn);
>  			fsect = sg->offset >> 9;
>  			lsect = fsect + (sg->length >> 9) - 1;
>  			/* install a grant reference. */
> @@ -340,6 +341,16 @@ static int blkif_queue_request(struct request *req)
>  						.gref       = ref,
>  						.first_sect = fsect,
>  						.last_sect  = lsect };
> +			/* 
> +			 * Set the page as foreign, considering that we are giving
> +			 * it to a foreign domain.
> +			 * This is important in case the destination domain is
> +			 * ourselves, so that we can more easily recognize the
> +			 * source pfn from destination pfn, both mapping to the same
> +			 * mfn.
> +			 */
> +			set_phys_to_machine(buffer_pfn,
> +					FOREIGN_FRAME(buffer_mfn));
>  		}
>  	}
>  
> @@ -715,8 +726,11 @@ static void blkif_completion(struct blk_shadow *s)
>  	int i;
>  	/* Do not let BLKIF_OP_DISCARD as nr_segment is in the same place
>  	 * flag. */
> -	for (i = 0; i < s->req.u.rw.nr_segments; i++)
> +	for (i = 0; i < s->req.u.rw.nr_segments; i++) {
>  		gnttab_end_foreign_access(s->req.u.rw.seg[i].gref, 0, 0UL);
> +		set_phys_to_machine(s->frame[i],
> +				get_phys_to_machine(s->frame[i]) & ~FOREIGN_FRAME_BIT);
> +	}
>  }
>  
>  static irqreturn_t blkif_interrupt(int irq, void *dev_id)
> @@ -1051,13 +1065,19 @@ static int blkif_recover(struct blkfront_info *info)
>  		memcpy(&info->shadow[req->u.rw.id], &copy[i], sizeof(copy[i]));
>  
>  		if (req->operation != BLKIF_OP_DISCARD) {
> +			unsigned long buffer_pfn;
> +			unsigned long buffer_mfn;
>  		/* Rewrite any grant references invalidated by susp/resume. */
> -			for (j = 0; j < req->u.rw.nr_segments; j++)
> +			for (j = 0; j < req->u.rw.nr_segments; j++) {
> +				buffer_pfn = info->shadow[req->u.rw.id].frame[j];
> +				buffer_mfn = pfn_to_mfn(buffer_pfn);
>  				gnttab_grant_foreign_access_ref(
>  					req->u.rw.seg[j].gref,
>  					info->xbdev->otherend_id,
> -					pfn_to_mfn(info->shadow[req->u.rw.id].frame[j]),
> +					buffer_mfn,
>  					rq_data_dir(info->shadow[req->u.rw.id].request));
> +				set_phys_to_machine(buffer_pfn, FOREIGN_FRAME(buffer_mfn));
> +			}
>  		}
>  		info->shadow[req->u.rw.id].req = *req;
>  
> -- 
> 1.7.2.5
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Xen-devel] [PATCH] xen-blkfront: set pages are FOREIGN_FRAME when sharing them
  2012-03-27 17:58 ` [Xen-devel] " Konrad Rzeszutek Wilk
@ 2012-03-29 13:56   ` Stefano Stabellini
  2012-03-29 14:13     ` Wei Liu
  2012-03-29 15:17     ` Konrad Rzeszutek Wilk
  0 siblings, 2 replies; 6+ messages in thread
From: Stefano Stabellini @ 2012-03-29 13:56 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: Stefano Stabellini, konrad.wilk, xen-devel, linux-kernel

On Tue, 27 Mar 2012, Konrad Rzeszutek Wilk wrote:
> On Tue, Mar 27, 2012 at 02:49:58PM +0100, Stefano Stabellini wrote:
> > Set pages as FOREIGN_FRAME whenever blkfront shares them with another
> > domain. Then when blkfront un-share them, also removes the
> > FOREIGN_FRAME_BIT from the p2m.
> > 
> > We do it so that when the source and the destination domain are the same
> > (blkfront connected to blkback in the same domain) we can more easily
> > recognize which ones are the source pfns and which ones are the
> > destination pfns (both are going to be pointing to the same mfns).
> > 
> > Without this patch enstablishing a connection between blkfront and QEMU
> > qdisk in the same domain causes QEMU to hang and never return.
> > Considering that it fixes a serious bug, it should be backported to
> > previous releases.
> 
> 
> ERROR: "set_phys_to_machine" [drivers/block/xen-blkfront.ko] undefined!

I cannot reproduce this error: set_phys_to_machine is defined in
arch/x86/include/asm/xen/page.h, that is included by include/xen/page.h,
that is included by blkfront).
What CS did you apply this patch to? What is your config?

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Xen-devel] [PATCH] xen-blkfront: set pages are FOREIGN_FRAME when sharing them
  2012-03-29 13:56   ` Stefano Stabellini
@ 2012-03-29 14:13     ` Wei Liu
  2012-03-29 14:21       ` Stefano Stabellini
  2012-03-29 15:17     ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 6+ messages in thread
From: Wei Liu @ 2012-03-29 14:13 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: wei.liu2, Konrad Rzeszutek Wilk, linux-kernel, konrad.wilk, xen-devel

On Thu, 2012-03-29 at 14:56 +0100, Stefano Stabellini wrote:
> On Tue, 27 Mar 2012, Konrad Rzeszutek Wilk wrote:
> > On Tue, Mar 27, 2012 at 02:49:58PM +0100, Stefano Stabellini wrote:
> > > Set pages as FOREIGN_FRAME whenever blkfront shares them with another
> > > domain. Then when blkfront un-share them, also removes the
> > > FOREIGN_FRAME_BIT from the p2m.
> > > 
> > > We do it so that when the source and the destination domain are the same
> > > (blkfront connected to blkback in the same domain) we can more easily
> > > recognize which ones are the source pfns and which ones are the
> > > destination pfns (both are going to be pointing to the same mfns).
> > > 
> > > Without this patch enstablishing a connection between blkfront and QEMU
> > > qdisk in the same domain causes QEMU to hang and never return.
> > > Considering that it fixes a serious bug, it should be backported to
> > > previous releases.
> > 
> > 
> > ERROR: "set_phys_to_machine" [drivers/block/xen-blkfront.ko] undefined!
> 
> I cannot reproduce this error: set_phys_to_machine is defined in
> arch/x86/include/asm/xen/page.h, that is included by include/xen/page.h,
> that is included by blkfront).
> What CS did you apply this patch to? What is your config?
> 

In arch/x86/xen/p2m.c, set_phys_to_machine is not exported. You need
another patch to do that.

It might be that you were using a monolithic kernel so that you didn't
see this error.


Wei.

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



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Xen-devel] [PATCH] xen-blkfront: set pages are FOREIGN_FRAME when sharing them
  2012-03-29 14:13     ` Wei Liu
@ 2012-03-29 14:21       ` Stefano Stabellini
  0 siblings, 0 replies; 6+ messages in thread
From: Stefano Stabellini @ 2012-03-29 14:21 UTC (permalink / raw)
  To: Wei Liu (Intern)
  Cc: Stefano Stabellini, Konrad Rzeszutek Wilk, linux-kernel,
	konrad.wilk, xen-devel

On Thu, 29 Mar 2012, Wei Liu (Intern) wrote:
> On Thu, 2012-03-29 at 14:56 +0100, Stefano Stabellini wrote:
> > On Tue, 27 Mar 2012, Konrad Rzeszutek Wilk wrote:
> > > On Tue, Mar 27, 2012 at 02:49:58PM +0100, Stefano Stabellini wrote:
> > > > Set pages as FOREIGN_FRAME whenever blkfront shares them with another
> > > > domain. Then when blkfront un-share them, also removes the
> > > > FOREIGN_FRAME_BIT from the p2m.
> > > > 
> > > > We do it so that when the source and the destination domain are the same
> > > > (blkfront connected to blkback in the same domain) we can more easily
> > > > recognize which ones are the source pfns and which ones are the
> > > > destination pfns (both are going to be pointing to the same mfns).
> > > > 
> > > > Without this patch enstablishing a connection between blkfront and QEMU
> > > > qdisk in the same domain causes QEMU to hang and never return.
> > > > Considering that it fixes a serious bug, it should be backported to
> > > > previous releases.
> > > 
> > > 
> > > ERROR: "set_phys_to_machine" [drivers/block/xen-blkfront.ko] undefined!
> > 
> > I cannot reproduce this error: set_phys_to_machine is defined in
> > arch/x86/include/asm/xen/page.h, that is included by include/xen/page.h,
> > that is included by blkfront).
> > What CS did you apply this patch to? What is your config?
> > 
> 
> In arch/x86/xen/p2m.c, set_phys_to_machine is not exported. You need
> another patch to do that.
> 
> It might be that you were using a monolithic kernel so that you didn't
> see this error.

Yes, you are right. Thanks!
I'll write another patch to export set_phys_to_machine.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Xen-devel] [PATCH] xen-blkfront: set pages are FOREIGN_FRAME when sharing them
  2012-03-29 13:56   ` Stefano Stabellini
  2012-03-29 14:13     ` Wei Liu
@ 2012-03-29 15:17     ` Konrad Rzeszutek Wilk
  1 sibling, 0 replies; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-03-29 15:17 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: Konrad Rzeszutek Wilk, linux-kernel, xen-devel

On Thu, Mar 29, 2012 at 02:56:10PM +0100, Stefano Stabellini wrote:
> On Tue, 27 Mar 2012, Konrad Rzeszutek Wilk wrote:
> > On Tue, Mar 27, 2012 at 02:49:58PM +0100, Stefano Stabellini wrote:
> > > Set pages as FOREIGN_FRAME whenever blkfront shares them with another
> > > domain. Then when blkfront un-share them, also removes the
> > > FOREIGN_FRAME_BIT from the p2m.
> > > 
> > > We do it so that when the source and the destination domain are the same
> > > (blkfront connected to blkback in the same domain) we can more easily
> > > recognize which ones are the source pfns and which ones are the
> > > destination pfns (both are going to be pointing to the same mfns).
> > > 
> > > Without this patch enstablishing a connection between blkfront and QEMU
> > > qdisk in the same domain causes QEMU to hang and never return.
> > > Considering that it fixes a serious bug, it should be backported to
> > > previous releases.
> > 
> > 
> > ERROR: "set_phys_to_machine" [drivers/block/xen-blkfront.ko] undefined!
> 
> I cannot reproduce this error: set_phys_to_machine is defined in
> arch/x86/include/asm/xen/page.h, that is included by include/xen/page.h,
> that is included by blkfront).
> What CS did you apply this patch to? What is your config?

To upstream Linus's tree.
The culprit was that I try to compile everything as modules.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-03-29 15:23 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-03-27 13:49 [PATCH] xen-blkfront: set pages are FOREIGN_FRAME when sharing them Stefano Stabellini
2012-03-27 17:58 ` [Xen-devel] " Konrad Rzeszutek Wilk
2012-03-29 13:56   ` Stefano Stabellini
2012-03-29 14:13     ` Wei Liu
2012-03-29 14:21       ` Stefano Stabellini
2012-03-29 15:17     ` Konrad Rzeszutek Wilk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).