Re: [PATCH v9 2/5] iommu/dma: Add a new dma_map_ops of get_merge_boundary()

2019-08-19 Thread Robin Murphy

On 26/07/2019 09:31, Yoshihiro Shimoda wrote:

This patch adds a new dma_map_ops of get_merge_boundary() to
expose the DMA merge boundary if the domain type is IOMMU_DOMAIN_DMA.

Signed-off-by: Yoshihiro Shimoda 
Reviewed-by: Simon Horman 
---
  drivers/iommu/dma-iommu.c | 11 +++
  1 file changed, 11 insertions(+)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index a7f9c3e..2992ce4 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -1085,6 +1085,16 @@ static int iommu_dma_get_sgtable(struct device *dev, 
struct sg_table *sgt,
return ret;
  }
  
+static unsigned long iommu_dma_get_merge_boundary(struct device *dev)

+{
+   struct iommu_domain *domain = iommu_get_dma_domain(dev);
+
+   if (domain->type != IOMMU_DOMAIN_DMA)


Did you actually need this check? In principle, if the default domain is 
not of type IOMMU_DOMAIN_DMA then the device should be assigned 
dma_direct_ops rather than iommu_dma_ops, thus it shouldn't be possible 
to get here. If you did manage to hit a case where the domain type 
didn't match the ops it would be interesting to figure out how.


Robin.


+   return 0;   /* can't merge */
+
+   return (1UL << __ffs(domain->pgsize_bitmap)) - 1;
+}
+
  static const struct dma_map_ops iommu_dma_ops = {
.alloc  = iommu_dma_alloc,
.free   = iommu_dma_free,
@@ -1100,6 +1110,7 @@ static const struct dma_map_ops iommu_dma_ops = {
.sync_sg_for_device = iommu_dma_sync_sg_for_device,
.map_resource   = iommu_dma_map_resource,
.unmap_resource = iommu_dma_unmap_resource,
+   .get_merge_boundary = iommu_dma_get_merge_boundary,
  };
  
  /*




Re: [PATCH 0/3] Fix virtio-blk issue with SWIOTLB

2019-01-14 Thread Robin Murphy

On 14/01/2019 18:20, Michael S. Tsirkin wrote:

On Mon, Jan 14, 2019 at 08:41:37PM +0800, Jason Wang wrote:


On 2019/1/14 下午5:50, Christoph Hellwig wrote:

On Mon, Jan 14, 2019 at 05:41:56PM +0800, Jason Wang wrote:

On 2019/1/11 下午5:15, Joerg Roedel wrote:

On Fri, Jan 11, 2019 at 11:29:31AM +0800, Jason Wang wrote:

Just wonder if my understanding is correct IOMMU_PLATFORM must be set for
all virtio devices under AMD-SEV guests?

Yes, that is correct. Emulated DMA can only happen on the SWIOTLB
aperture, because that memory is not encrypted. The guest bounces the
data then to its encrypted memory.

Regards,

Joerg


Thanks, have you tested vhost-net in this case. I suspect it may not work

Which brings me back to my pet pevee that we need to take actions
that virtio uses the proper dma mapping API by default with quirks
for legacy cases.  The magic bypass it uses is just causing problems
over problems.



Yes, I fully agree with you. This is probably an exact example of such
problem.

Thanks


I don't think so - the issue is really that DMA API does not yet handle
the SEV case 100% correctly. I suspect passthrough devices would have
the same issue.


Huh? Regardless of which virtio devices use it or not, the DMA API is 
handling the SEV case as correctly as it possibly can, by forcing 
everything through the unencrypted bounce buffer. If the segments being 
mapped are too big for that bounce buffer in the first place, there's 
nothing it can possibly do except fail, gracefully or otherwise.


Now, in theory, yes, the real issue at hand is not unique to virtio-blk 
nor SEV - any driver whose device has a sufficiently large DMA segment 
size and who manages to get sufficient physically-contiguous memory 
could technically generate a scatterlist segment longer than SWIOTLB can 
handle. However, in practice that basically never happens, not least 
because very few drivers ever override the default 64K DMA segment 
limit. AFAICS nothing in drivers/virtio is calling 
dma_set_max_seg_size() or otherwise assigning any dma_parms to replace 
the defaults either, so the really interesting question here is how are 
these apparently-out-of-spec 256K segments getting generated at all?


Robin.


Re: [PATCH 08/12] mmc: reduce use of block bounce buffers

2018-04-16 Thread Robin Murphy

On 16/04/18 09:50, Christoph Hellwig wrote:

We can rely on the dma-mapping code to handle any DMA limits that is
bigger than the ISA DMA mask for us (either using an iommu or swiotlb),
so remove setting the block layer bounce limit for anything but bouncing
for highmem pages.

Signed-off-by: Christoph Hellwig 
---
  drivers/mmc/core/queue.c | 7 ++-
  1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/core/queue.c b/drivers/mmc/core/queue.c
index 56e9a803db21..60a02a763d01 100644
--- a/drivers/mmc/core/queue.c
+++ b/drivers/mmc/core/queue.c
@@ -351,17 +351,14 @@ static const struct blk_mq_ops mmc_mq_ops = {
  static void mmc_setup_queue(struct mmc_queue *mq, struct mmc_card *card)
  {
struct mmc_host *host = card->host;
-   u64 limit = BLK_BOUNCE_HIGH;
-
-   if (mmc_dev(host)->dma_mask && *mmc_dev(host)->dma_mask)
-   limit = (u64)dma_max_pfn(mmc_dev(host)) << PAGE_SHIFT;
  
  	blk_queue_flag_set(QUEUE_FLAG_NONROT, mq->queue);

blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, mq->queue);
if (mmc_can_erase(card))
mmc_queue_setup_discard(mq->queue, card);
  
-	blk_queue_bounce_limit(mq->queue, limit);

+   if (!mmc_dev(host)->dma_mask || !mmc_dev(host)->dma_mask)


I'm almost surprised that GCC doesn't warn about "x || x", but 
nevertheless I think you've lost a "*" here...


Robin.


+   blk_queue_bounce_limit(mq->queue, BLK_BOUNCE_HIGH);
blk_queue_max_hw_sectors(mq->queue,
min(host->max_blk_count, host->max_req_size / 512));
blk_queue_max_segments(mq->queue, host->max_segs);