Re: [PATCH v7 25/36] swiotlb: Add warnings for use of bounce buffers with SME

2017-06-21 Thread Tom Lendacky

On 6/21/2017 5:50 AM, Borislav Petkov wrote:

On Fri, Jun 16, 2017 at 01:54:36PM -0500, Tom Lendacky wrote:

Add warnings to let the user know when bounce buffers are being used for
DMA when SME is active.  Since the bounce buffers are not in encrypted
memory, these notifications are to allow the user to determine some
appropriate action - if necessary.  Actions can range from utilizing an
IOMMU, replacing the device with another device that can support 64-bit
DMA, ignoring the message if the device isn't used much, etc.

Signed-off-by: Tom Lendacky 
---
  include/linux/dma-mapping.h |   11 +++
  include/linux/mem_encrypt.h |8 
  lib/swiotlb.c   |3 +++
  3 files changed, 22 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4f3eece..ee2307e 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -10,6 +10,7 @@
  #include 
  #include 
  #include 
+#include 
  
  /**

   * List of possible attributes associated with a DMA mapping. The semantics
@@ -577,6 +578,11 @@ static inline int dma_set_mask(struct device *dev, u64 
mask)
  
  	if (!dev->dma_mask || !dma_supported(dev, mask))

return -EIO;
+
+   /* Since mask is unsigned, this can only be true if SME is active */
+   if (mask < sme_dma_mask())
+   dev_warn(dev, "SME is active, device will require DMA bounce 
buffers\n");
+
*dev->dma_mask = mask;
return 0;
  }
@@ -596,6 +602,11 @@ static inline int dma_set_coherent_mask(struct device 
*dev, u64 mask)
  {
if (!dma_supported(dev, mask))
return -EIO;
+
+   /* Since mask is unsigned, this can only be true if SME is active */
+   if (mask < sme_dma_mask())
+   dev_warn(dev, "SME is active, device will require DMA bounce 
buffers\n");


Looks to me like those two checks above need to be a:

void sme_check_mask(struct device *dev, u64 mask)
{
 if (!sme_me_mask)
 return;

 /* Since mask is unsigned, this can only be true if SME is active */
 if (mask < (((u64)sme_me_mask << 1) - 1))
 dev_warn(dev, "SME is active, device will require DMA bounce 
buffers\n");
}

which gets called and sme_dma_mask() is not really needed.


Makes a lot of sense, I'll update the patch.

Thanks,
Tom




--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v7 25/36] swiotlb: Add warnings for use of bounce buffers with SME

2017-06-21 Thread Borislav Petkov
On Fri, Jun 16, 2017 at 01:54:36PM -0500, Tom Lendacky wrote:
> Add warnings to let the user know when bounce buffers are being used for
> DMA when SME is active.  Since the bounce buffers are not in encrypted
> memory, these notifications are to allow the user to determine some
> appropriate action - if necessary.  Actions can range from utilizing an
> IOMMU, replacing the device with another device that can support 64-bit
> DMA, ignoring the message if the device isn't used much, etc.
> 
> Signed-off-by: Tom Lendacky 
> ---
>  include/linux/dma-mapping.h |   11 +++
>  include/linux/mem_encrypt.h |8 
>  lib/swiotlb.c   |3 +++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
> index 4f3eece..ee2307e 100644
> --- a/include/linux/dma-mapping.h
> +++ b/include/linux/dma-mapping.h
> @@ -10,6 +10,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  /**
>   * List of possible attributes associated with a DMA mapping. The semantics
> @@ -577,6 +578,11 @@ static inline int dma_set_mask(struct device *dev, u64 
> mask)
>  
>   if (!dev->dma_mask || !dma_supported(dev, mask))
>   return -EIO;
> +
> + /* Since mask is unsigned, this can only be true if SME is active */
> + if (mask < sme_dma_mask())
> + dev_warn(dev, "SME is active, device will require DMA bounce 
> buffers\n");
> +
>   *dev->dma_mask = mask;
>   return 0;
>  }
> @@ -596,6 +602,11 @@ static inline int dma_set_coherent_mask(struct device 
> *dev, u64 mask)
>  {
>   if (!dma_supported(dev, mask))
>   return -EIO;
> +
> + /* Since mask is unsigned, this can only be true if SME is active */
> + if (mask < sme_dma_mask())
> + dev_warn(dev, "SME is active, device will require DMA bounce 
> buffers\n");

Looks to me like those two checks above need to be a:

void sme_check_mask(struct device *dev, u64 mask)
{
if (!sme_me_mask)
return;

/* Since mask is unsigned, this can only be true if SME is active */
if (mask < (((u64)sme_me_mask << 1) - 1))
dev_warn(dev, "SME is active, device will require DMA bounce 
buffers\n");
}

which gets called and sme_dma_mask() is not really needed.

-- 
Regards/Gruss,
Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v7 25/36] swiotlb: Add warnings for use of bounce buffers with SME

2017-06-16 Thread Tom Lendacky
Add warnings to let the user know when bounce buffers are being used for
DMA when SME is active.  Since the bounce buffers are not in encrypted
memory, these notifications are to allow the user to determine some
appropriate action - if necessary.  Actions can range from utilizing an
IOMMU, replacing the device with another device that can support 64-bit
DMA, ignoring the message if the device isn't used much, etc.

Signed-off-by: Tom Lendacky 
---
 include/linux/dma-mapping.h |   11 +++
 include/linux/mem_encrypt.h |8 
 lib/swiotlb.c   |3 +++
 3 files changed, 22 insertions(+)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 4f3eece..ee2307e 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * List of possible attributes associated with a DMA mapping. The semantics
@@ -577,6 +578,11 @@ static inline int dma_set_mask(struct device *dev, u64 
mask)
 
if (!dev->dma_mask || !dma_supported(dev, mask))
return -EIO;
+
+   /* Since mask is unsigned, this can only be true if SME is active */
+   if (mask < sme_dma_mask())
+   dev_warn(dev, "SME is active, device will require DMA bounce 
buffers\n");
+
*dev->dma_mask = mask;
return 0;
 }
@@ -596,6 +602,11 @@ static inline int dma_set_coherent_mask(struct device 
*dev, u64 mask)
 {
if (!dma_supported(dev, mask))
return -EIO;
+
+   /* Since mask is unsigned, this can only be true if SME is active */
+   if (mask < sme_dma_mask())
+   dev_warn(dev, "SME is active, device will require DMA bounce 
buffers\n");
+
dev->coherent_dma_mask = mask;
return 0;
 }
diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
index 837c66b..2168002 100644
--- a/include/linux/mem_encrypt.h
+++ b/include/linux/mem_encrypt.h
@@ -30,6 +30,14 @@ static inline bool sme_active(void)
return !!sme_me_mask;
 }
 
+static inline u64 sme_dma_mask(void)
+{
+   if (!sme_me_mask)
+   return 0ULL;
+
+   return ((u64)sme_me_mask << 1) - 1;
+}
+
 /*
  * The __sme_set() and __sme_clr() macros are useful for adding or removing
  * the encryption mask from a value (e.g. when dealing with pagetable
diff --git a/lib/swiotlb.c b/lib/swiotlb.c
index 04ac91a..8c6c83e 100644
--- a/lib/swiotlb.c
+++ b/lib/swiotlb.c
@@ -507,6 +507,9 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev,
if (no_iotlb_memory)
panic("Can not allocate SWIOTLB buffer earlier and can't now 
provide you with the DMA bounce buffer");
 
+   if (sme_active())
+   pr_warn_once("SME is active and system is using DMA bounce 
buffers\n");
+
mask = dma_get_seg_boundary(hwdev);
 
tbl_dma_addr &= mask;

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html