[PATCH] swiotlb-xen: update dev_addr after swapping pages

[PATCH] swiotlb-xen: update dev_addr after swapping pages

[Stefano Stabellini]

In xen_swiotlb_map_page and xen_swiotlb_map_sg_attrs, if the original
page is not suitable, we swap it for another page from the swiotlb 
pool.

In these cases, we don't update the previously calculated dma address
for the page before calling xen_dma_map_page. Thus, we end up calling
xen_dma_map_page passing the wrong dev_addr, resulting in
xen_dma_map_page mistakenly assuming that the page is foreign when it is
local.

Fix the bug by updating dev_addr appropriately.

This change has no effect on x86, because xen_dma_map_page is a stub
there.

Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>
Tested-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>

diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
index f905d6e..f8afc6d 100644
--- a/drivers/xen/swiotlb-xen.c
+++ b/drivers/xen/swiotlb-xen.c
@@ -414,9 +414,9 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
 	if (map == SWIOTLB_MAP_ERROR)
 		return DMA_ERROR_CODE;
 
+	dev_addr = xen_phys_to_bus(map);
 	xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT),
 					dev_addr, map & ~PAGE_MASK, size, dir, attrs);
-	dev_addr = xen_phys_to_bus(map);
 
 	/*
 	 * Ensure that the address returned is DMA'ble
@@ -575,13 +575,14 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,
 				sg_dma_len(sgl) = 0;
 				return 0;
 			}
+			dev_addr = xen_phys_to_bus(map);
 			xen_dma_map_page(hwdev, pfn_to_page(map >> PAGE_SHIFT),
 						dev_addr,
 						map & ~PAGE_MASK,
 						sg->length,
 						dir,
 						attrs);
-			sg->dma_address = xen_phys_to_bus(map);
+			sg->dma_address = dev_addr;
 		} else {
 			/* we are not interested in the dma_addr returned by
 			 * xen_dma_map_page, only in the potential cache flushes executed

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

Re: [PATCH] swiotlb-xen: update dev_addr after swapping pages

[Boris Ostrovsky]

On 01/19/2017 01:39 PM, Stefano Stabellini wrote:
> In xen_swiotlb_map_page and xen_swiotlb_map_sg_attrs, if the original
> page is not suitable, we swap it for another page from the swiotlb 
> pool.
>
> In these cases, we don't update the previously calculated dma address
> for the page before calling xen_dma_map_page. Thus, we end up calling
> xen_dma_map_page passing the wrong dev_addr, resulting in
> xen_dma_map_page mistakenly assuming that the page is foreign when it is
> local.
>
> Fix the bug by updating dev_addr appropriately.
>
> This change has no effect on x86, because xen_dma_map_page is a stub
> there.
>
> Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> Signed-off-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>
> Tested-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>

Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

>
> diff --git a/drivers/xen/swiotlb-xen.c b/drivers/xen/swiotlb-xen.c
> index f905d6e..f8afc6d 100644
> --- a/drivers/xen/swiotlb-xen.c
> +++ b/drivers/xen/swiotlb-xen.c
> @@ -414,9 +414,9 @@ dma_addr_t xen_swiotlb_map_page(struct device *dev, struct page *page,
>  	if (map == SWIOTLB_MAP_ERROR)
>  		return DMA_ERROR_CODE;
>  
> +	dev_addr = xen_phys_to_bus(map);
>  	xen_dma_map_page(dev, pfn_to_page(map >> PAGE_SHIFT),
>  					dev_addr, map & ~PAGE_MASK, size, dir, attrs);
> -	dev_addr = xen_phys_to_bus(map);
>  
>  	/*
>  	 * Ensure that the address returned is DMA'ble
> @@ -575,13 +575,14 @@ void xen_swiotlb_unmap_page(struct device *hwdev, dma_addr_t dev_addr,

Interesting that git diff picked xen_swiotlb_unmap_page() here instead
of xen_swiotlb_map_sg_attrs()

-boris

 

>  				sg_dma_len(sgl) = 0;
>  				return 0;
>  			}
> +			dev_addr = xen_phys_to_bus(map);
>  			xen_dma_map_page(hwdev, pfn_to_page(map >> PAGE_SHIFT),
>  						dev_addr,
>  						map & ~PAGE_MASK,
>  						sg->length,
>  						dir,
>  						attrs);
> -			sg->dma_address = xen_phys_to_bus(map);
> +			sg->dma_address = dev_addr;
>  		} else {
>  			/* we are not interested in the dma_addr returned by
>  			 * xen_dma_map_page, only in the potential cache flushes executed


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

Re: [PATCH] swiotlb-xen: update dev_addr after swapping pages

[Konrad Rzeszutek Wilk]

On Thu, Jan 19, 2017 at 06:58:46PM -0500, Boris Ostrovsky wrote:
> On 01/19/2017 01:39 PM, Stefano Stabellini wrote:
> > In xen_swiotlb_map_page and xen_swiotlb_map_sg_attrs, if the original
> > page is not suitable, we swap it for another page from the swiotlb 
> > pool.
> >
> > In these cases, we don't update the previously calculated dma address
> > for the page before calling xen_dma_map_page. Thus, we end up calling
> > xen_dma_map_page passing the wrong dev_addr, resulting in
> > xen_dma_map_page mistakenly assuming that the page is foreign when it is
> > local.
> >
> > Fix the bug by updating dev_addr appropriately.
> >
> > This change has no effect on x86, because xen_dma_map_page is a stub
> > there.
> >
> > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > Signed-off-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>
> > Tested-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>
> 
> Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>

Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

I can carry it via my swiotlb.git tree or if there are some extra
things on the Xen tree - it can go through that?

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

Re: [PATCH] swiotlb-xen: update dev_addr after swapping pages

[Stefano Stabellini]

On Thu, 19 Jan 2017, Konrad Rzeszutek Wilk wrote:
> On Thu, Jan 19, 2017 at 06:58:46PM -0500, Boris Ostrovsky wrote:
> > On 01/19/2017 01:39 PM, Stefano Stabellini wrote:
> > > In xen_swiotlb_map_page and xen_swiotlb_map_sg_attrs, if the original
> > > page is not suitable, we swap it for another page from the swiotlb 
> > > pool.
> > >
> > > In these cases, we don't update the previously calculated dma address
> > > for the page before calling xen_dma_map_page. Thus, we end up calling
> > > xen_dma_map_page passing the wrong dev_addr, resulting in
> > > xen_dma_map_page mistakenly assuming that the page is foreign when it is
> > > local.
> > >
> > > Fix the bug by updating dev_addr appropriately.
> > >
> > > This change has no effect on x86, because xen_dma_map_page is a stub
> > > there.
> > >
> > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > Signed-off-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>
> > > Tested-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>
> > 
> > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> 
> Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> I can carry it via my swiotlb.git tree or if there are some extra
> things on the Xen tree - it can go through that?

It might be best to go via swiotlb.git: I don't think we have anything
on the Xen tree at the moment -- Juergen has just sent a pull request
for it.

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

Re: [PATCH] swiotlb-xen: update dev_addr after swapping pages

[Konrad Rzeszutek Wilk]

On Thu, Jan 19, 2017 at 04:29:19PM -0800, Stefano Stabellini wrote:
> On Thu, 19 Jan 2017, Konrad Rzeszutek Wilk wrote:
> > On Thu, Jan 19, 2017 at 06:58:46PM -0500, Boris Ostrovsky wrote:
> > > On 01/19/2017 01:39 PM, Stefano Stabellini wrote:
> > > > In xen_swiotlb_map_page and xen_swiotlb_map_sg_attrs, if the original
> > > > page is not suitable, we swap it for another page from the swiotlb 
> > > > pool.
> > > >
> > > > In these cases, we don't update the previously calculated dma address
> > > > for the page before calling xen_dma_map_page. Thus, we end up calling
> > > > xen_dma_map_page passing the wrong dev_addr, resulting in
> > > > xen_dma_map_page mistakenly assuming that the page is foreign when it is
> > > > local.
> > > >
> > > > Fix the bug by updating dev_addr appropriately.
> > > >
> > > > This change has no effect on x86, because xen_dma_map_page is a stub
> > > > there.
> > > >
> > > > Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
> > > > Signed-off-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>
> > > > Tested-by: Pooya Keshavarzi <Pooya.Keshavarzi@de.bosch.com>
> > > 
> > > Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> > 
> > Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> > 
> > I can carry it via my swiotlb.git tree or if there are some extra
> > things on the Xen tree - it can go through that?
> 
> It might be best to go via swiotlb.git: I don't think we have anything
> on the Xen tree at the moment -- Juergen has just sent a pull request
> for it.

OK, let me queue it up and run it through the tests.
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> https://lists.xen.org/xen-devel

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