Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin

2020-12-10 Thread zhukeqian



On 2020/12/11 3:16, Alex Williamson wrote:
> On Thu, 10 Dec 2020 15:34:19 +0800
> Keqian Zhu  wrote:
> 
>> Currently we do not clear added dirty bit of bitmap when unwind
>> pin, so if pin failed at halfway, we set unnecessary dirty bit
>> in bitmap. Clearing added dirty bit when unwind pin, userspace
>> will see less dirty page, which can save much time to handle them.
>>
>> Note that we should distinguish the bits added by pin and the bits
>> already set before pin, so introduce bitmap_added to record this.
>>
>> Signed-off-by: Keqian Zhu 
>> ---
>>  drivers/vfio/vfio_iommu_type1.c | 33 ++---
>>  1 file changed, 22 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/vfio/vfio_iommu_type1.c 
>> b/drivers/vfio/vfio_iommu_type1.c
>> index 67e827638995..f129d24a6ec3 100644
>> --- a/drivers/vfio/vfio_iommu_type1.c
>> +++ b/drivers/vfio/vfio_iommu_type1.c
>> @@ -637,7 +637,11 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>>  struct vfio_iommu *iommu = iommu_data;
>>  struct vfio_group *group;
>>  int i, j, ret;
>> +unsigned long pgshift = __ffs(iommu->pgsize_bitmap);
>>  unsigned long remote_vaddr;
>> +unsigned long bitmap_offset;
>> +unsigned long *bitmap_added;
>> +dma_addr_t iova;
>>  struct vfio_dma *dma;
>>  bool do_accounting;
>>  
>> @@ -650,6 +654,12 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>>  
>>  mutex_lock(>lock);
>>  
>> +bitmap_added = bitmap_zalloc(npage, GFP_KERNEL);
>> +if (!bitmap_added) {
>> +ret = -ENOMEM;
>> +goto pin_done;
>> +}
> 
> 
> This is allocated regardless of whether dirty tracking is enabled, so
> this adds overhead to the common case in order to reduce user overhead
> (not correctness) in the rare condition that dirty tracking is enabled,
> and the even rarer condition that there's a fault during that case.
> This is not a good trade-off.  Thanks,

Hi Alex,

We can allocate the bitmap when dirty tracking is active, do you accept this?
Or we can set the dirty bitmap after all pins succeed, but this costs cpu time
to locate vfio_dma with iova.

Thanks,
Keqian

> 
> Alex
> 
> 
>> +
>>  /* Fail if notifier list is empty */
>>  if (!iommu->notifier.head) {
>>  ret = -EINVAL;
>> @@ -664,7 +674,6 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>>  do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu);
>>  
>>  for (i = 0; i < npage; i++) {
>> -dma_addr_t iova;
>>  struct vfio_pfn *vpfn;
>>  
>>  iova = user_pfn[i] << PAGE_SHIFT;
>> @@ -699,14 +708,10 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>>  }
>>  
>>  if (iommu->dirty_page_tracking) {
>> -unsigned long pgshift = __ffs(iommu->pgsize_bitmap);
>> -
>> -/*
>> - * Bitmap populated with the smallest supported page
>> - * size
>> - */
>> -bitmap_set(dma->bitmap,
>> -   (iova - dma->iova) >> pgshift, 1);
>> +/* Populated with the smallest supported page size */
>> +bitmap_offset = (iova - dma->iova) >> pgshift;
>> +if (!test_and_set_bit(bitmap_offset, dma->bitmap))
>> +set_bit(i, bitmap_added);
>>  }
>>  }
>>  ret = i;
>> @@ -722,14 +727,20 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>>  pin_unwind:
>>  phys_pfn[i] = 0;
>>  for (j = 0; j < i; j++) {
>> -dma_addr_t iova;
>> -
>>  iova = user_pfn[j] << PAGE_SHIFT;
>>  dma = vfio_find_dma(iommu, iova, PAGE_SIZE);
>>  vfio_unpin_page_external(dma, iova, do_accounting);
>>  phys_pfn[j] = 0;
>> +
>> +if (test_bit(j, bitmap_added)) {
>> +bitmap_offset = (iova - dma->iova) >> pgshift;
>> +clear_bit(bitmap_offset, dma->bitmap);
>> +}
>>  }
>>  pin_done:
>> +if (bitmap_added)
>> +bitmap_free(bitmap_added);
>> +
>>  mutex_unlock(>lock);
>>  return ret;
>>  }
> 
> .
> 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] dma-mapping: move hint unlikely for dma_mapping_error from drivers to core

2020-12-10 Thread Kalle Valo
Heiner Kallweit  writes:

> Zillions of drivers use the unlikely() hint when checking the result of
> dma_mapping_error(). This is an inline function anyway, so we can move
> the hint into the function and remove it from drivers.
>>From time to time discussions pop up how effective unlikely() is,
> and that it should be used only if something is really very unlikely.
> I think that's the case here.
>
> Patch was created with some help from coccinelle.
>
> @@
> expression dev, dma_addr;
> @@
>
> - unlikely(dma_mapping_error(dev, dma_addr))
> + dma_mapping_error(dev, dma_addr)
>
> Signed-off-by: Heiner Kallweit 
> ---
> If ok, then tbd through which tree this is supposed to go.
> Patch is based on linux-next-20201210.
> ---

[...]

>  drivers/net/wireless/ath/ath10k/htt_rx.c  |  2 +-
>  drivers/net/wireless/ath/ath10k/pci.c |  2 +-
>  drivers/net/wireless/ath/ath10k/snoc.c|  2 +-
>  drivers/net/wireless/ath/ath11k/ce.c  |  2 +-
>  drivers/net/wireless/ath/ath11k/dp_rx.c   |  2 +-
>  drivers/net/wireless/ath/ath5k/base.c |  2 +-
>  drivers/net/wireless/ath/ath9k/beacon.c   |  2 +-
>  drivers/net/wireless/ath/ath9k/recv.c | 21 +++-
>  drivers/net/wireless/ath/ath9k/xmit.c |  2 +-
>  drivers/net/wireless/ath/wil6210/txrx.c   | 10 
>  drivers/net/wireless/ath/wil6210/txrx_edma.c  |  4 +--
>  drivers/net/wireless/broadcom/b43/dma.c   |  2 +-
>  drivers/net/wireless/broadcom/b43legacy/dma.c |  2 +-
>  drivers/net/wireless/intel/iwlwifi/pcie/tx.c  | 10 
>  drivers/net/wireless/intel/iwlwifi/queue/tx.c | 10 
>  drivers/net/wireless/mediatek/mt76/dma.c  |  8 +++---
>  .../net/wireless/ralink/rt2x00/rt2x00queue.c  |  4 +--

For wireless drivers:

Acked-by: Kalle Valo 

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v5 05/27] dt-bindings: memory: mediatek: Rename header guard for SMI header file

2020-12-10 Thread Rob Herring
On Wed, 09 Dec 2020 16:00:40 +0800, Yong Wu wrote:
> Only rename the header guard for all the SoC larb port header file.
> No funtional change.
> 
> Suggested-by: Krzysztof Kozlowski 
> Signed-off-by: Yong Wu 
> ---
>  include/dt-bindings/memory/mt2701-larb-port.h | 4 ++--
>  include/dt-bindings/memory/mt2712-larb-port.h | 4 ++--
>  include/dt-bindings/memory/mt6779-larb-port.h | 4 ++--
>  include/dt-bindings/memory/mt8167-larb-port.h | 4 ++--
>  include/dt-bindings/memory/mt8173-larb-port.h | 4 ++--
>  include/dt-bindings/memory/mt8183-larb-port.h | 4 ++--
>  6 files changed, 12 insertions(+), 12 deletions(-)
> 

Acked-by: Rob Herring 
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 1/1] vfio/type1: Add vfio_group_iommu_domain()

2020-12-10 Thread Alex Williamson
On Wed,  9 Dec 2020 09:44:44 +0800
Lu Baolu  wrote:

> Add the API for getting the domain from a vfio group. This could be used
> by the physical device drivers which rely on the vfio/mdev framework for
> mediated device user level access. The typical use case like below:
> 
>   unsigned int pasid;
>   struct vfio_group *vfio_group;
>   struct iommu_domain *iommu_domain;
>   struct device *dev = mdev_dev(mdev);
>   struct device *iommu_device = mdev_get_iommu_device(dev);
> 
>   if (!iommu_device ||
>   !iommu_dev_feature_enabled(iommu_device, IOMMU_DEV_FEAT_AUX))
>   return -EINVAL;
> 
>   vfio_group = vfio_group_get_external_user_from_dev(dev);
>   if (IS_ERR_OR_NULL(vfio_group))
>   return -EFAULT;
> 
>   iommu_domain = vfio_group_iommu_domain(vfio_group);
>   if (IS_ERR_OR_NULL(iommu_domain)) {
>   vfio_group_put_external_user(vfio_group);
>   return -EFAULT;
>   }
> 
>   pasid = iommu_aux_get_pasid(iommu_domain, iommu_device);
>   if (pasid < 0) {
>   vfio_group_put_external_user(vfio_group);
>   return -EFAULT;
>   }
> 
>   /* Program device context with pasid value. */
>   ...
> 
> Signed-off-by: Lu Baolu 
> ---
>  drivers/vfio/vfio.c | 18 ++
>  drivers/vfio/vfio_iommu_type1.c | 24 
>  include/linux/vfio.h|  4 
>  3 files changed, 46 insertions(+)


Applied to vfio next branch for v5.11.  Thanks,

Alex


> Change log:
>  - v3: 
> https://lore.kernel.org/linux-iommu/20201201012328.2465735-1-baolu...@linux.intel.com/
>  - Changed according to comments @ 
> https://lore.kernel.org/linux-iommu/20201202144834.1dd09...@w520.home/
>- Rename group_domain to group_iommu_domain;
>- Remove an unnecessary else branch.
> 
> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> index 2151bc7f87ab..4ad8a35667a7 100644
> --- a/drivers/vfio/vfio.c
> +++ b/drivers/vfio/vfio.c
> @@ -2331,6 +2331,24 @@ int vfio_unregister_notifier(struct device *dev, enum 
> vfio_notify_type type,
>  }
>  EXPORT_SYMBOL(vfio_unregister_notifier);
>  
> +struct iommu_domain *vfio_group_iommu_domain(struct vfio_group *group)
> +{
> + struct vfio_container *container;
> + struct vfio_iommu_driver *driver;
> +
> + if (!group)
> + return ERR_PTR(-EINVAL);
> +
> + container = group->container;
> + driver = container->iommu_driver;
> + if (likely(driver && driver->ops->group_iommu_domain))
> + return driver->ops->group_iommu_domain(container->iommu_data,
> +group->iommu_group);
> +
> + return ERR_PTR(-ENOTTY);
> +}
> +EXPORT_SYMBOL_GPL(vfio_group_iommu_domain);
> +
>  /**
>   * Module/class support
>   */
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 67e827638995..0b4dedaa9128 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -2980,6 +2980,29 @@ static int vfio_iommu_type1_dma_rw(void *iommu_data, 
> dma_addr_t user_iova,
>   return ret;
>  }
>  
> +static struct iommu_domain *
> +vfio_iommu_type1_group_iommu_domain(void *iommu_data,
> + struct iommu_group *iommu_group)
> +{
> + struct iommu_domain *domain = ERR_PTR(-ENODEV);
> + struct vfio_iommu *iommu = iommu_data;
> + struct vfio_domain *d;
> +
> + if (!iommu || !iommu_group)
> + return ERR_PTR(-EINVAL);
> +
> + mutex_lock(>lock);
> + list_for_each_entry(d, >domain_list, next) {
> + if (find_iommu_group(d, iommu_group)) {
> + domain = d->domain;
> + break;
> + }
> + }
> + mutex_unlock(>lock);
> +
> + return domain;
> +}
> +
>  static const struct vfio_iommu_driver_ops vfio_iommu_driver_ops_type1 = {
>   .name   = "vfio-iommu-type1",
>   .owner  = THIS_MODULE,
> @@ -2993,6 +3016,7 @@ static const struct vfio_iommu_driver_ops 
> vfio_iommu_driver_ops_type1 = {
>   .register_notifier  = vfio_iommu_type1_register_notifier,
>   .unregister_notifier= vfio_iommu_type1_unregister_notifier,
>   .dma_rw = vfio_iommu_type1_dma_rw,
> + .group_iommu_domain = vfio_iommu_type1_group_iommu_domain,
>  };
>  
>  static int __init vfio_iommu_type1_init(void)
> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> index 38d3c6a8dc7e..f45940b38a02 100644
> --- a/include/linux/vfio.h
> +++ b/include/linux/vfio.h
> @@ -90,6 +90,8 @@ struct vfio_iommu_driver_ops {
>  struct notifier_block *nb);
>   int (*dma_rw)(void *iommu_data, dma_addr_t user_iova,
> void *data, size_t count, bool write);
> + struct iommu_domain *(*group_iommu_domain)(void *iommu_data,
> +

Re: [PATCH] dma-mapping: move hint unlikely for dma_mapping_error from drivers to core

2020-12-10 Thread Wolfram Sang
On Thu, Dec 10, 2020 at 03:47:50PM +0100, Heiner Kallweit wrote:
> Zillions of drivers use the unlikely() hint when checking the result of
> dma_mapping_error(). This is an inline function anyway, so we can move
> the hint into the function and remove it from drivers.
> From time to time discussions pop up how effective unlikely() is,
> and that it should be used only if something is really very unlikely.
> I think that's the case here.
> 
> Patch was created with some help from coccinelle.
> 
> @@
> expression dev, dma_addr;
> @@
> 
> - unlikely(dma_mapping_error(dev, dma_addr))
> + dma_mapping_error(dev, dma_addr)
> 
> Signed-off-by: Heiner Kallweit 

Acked-by: Wolfram Sang  # for I2C



signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 1/7] vfio: iommu_type1: Clear added dirty bit when unwind pin

2020-12-10 Thread Alex Williamson
On Thu, 10 Dec 2020 15:34:19 +0800
Keqian Zhu  wrote:

> Currently we do not clear added dirty bit of bitmap when unwind
> pin, so if pin failed at halfway, we set unnecessary dirty bit
> in bitmap. Clearing added dirty bit when unwind pin, userspace
> will see less dirty page, which can save much time to handle them.
> 
> Note that we should distinguish the bits added by pin and the bits
> already set before pin, so introduce bitmap_added to record this.
> 
> Signed-off-by: Keqian Zhu 
> ---
>  drivers/vfio/vfio_iommu_type1.c | 33 ++---
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 67e827638995..f129d24a6ec3 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -637,7 +637,11 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>   struct vfio_iommu *iommu = iommu_data;
>   struct vfio_group *group;
>   int i, j, ret;
> + unsigned long pgshift = __ffs(iommu->pgsize_bitmap);
>   unsigned long remote_vaddr;
> + unsigned long bitmap_offset;
> + unsigned long *bitmap_added;
> + dma_addr_t iova;
>   struct vfio_dma *dma;
>   bool do_accounting;
>  
> @@ -650,6 +654,12 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>  
>   mutex_lock(>lock);
>  
> + bitmap_added = bitmap_zalloc(npage, GFP_KERNEL);
> + if (!bitmap_added) {
> + ret = -ENOMEM;
> + goto pin_done;
> + }


This is allocated regardless of whether dirty tracking is enabled, so
this adds overhead to the common case in order to reduce user overhead
(not correctness) in the rare condition that dirty tracking is enabled,
and the even rarer condition that there's a fault during that case.
This is not a good trade-off.  Thanks,

Alex


> +
>   /* Fail if notifier list is empty */
>   if (!iommu->notifier.head) {
>   ret = -EINVAL;
> @@ -664,7 +674,6 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>   do_accounting = !IS_IOMMU_CAP_DOMAIN_IN_CONTAINER(iommu);
>  
>   for (i = 0; i < npage; i++) {
> - dma_addr_t iova;
>   struct vfio_pfn *vpfn;
>  
>   iova = user_pfn[i] << PAGE_SHIFT;
> @@ -699,14 +708,10 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>   }
>  
>   if (iommu->dirty_page_tracking) {
> - unsigned long pgshift = __ffs(iommu->pgsize_bitmap);
> -
> - /*
> -  * Bitmap populated with the smallest supported page
> -  * size
> -  */
> - bitmap_set(dma->bitmap,
> -(iova - dma->iova) >> pgshift, 1);
> + /* Populated with the smallest supported page size */
> + bitmap_offset = (iova - dma->iova) >> pgshift;
> + if (!test_and_set_bit(bitmap_offset, dma->bitmap))
> + set_bit(i, bitmap_added);
>   }
>   }
>   ret = i;
> @@ -722,14 +727,20 @@ static int vfio_iommu_type1_pin_pages(void *iommu_data,
>  pin_unwind:
>   phys_pfn[i] = 0;
>   for (j = 0; j < i; j++) {
> - dma_addr_t iova;
> -
>   iova = user_pfn[j] << PAGE_SHIFT;
>   dma = vfio_find_dma(iommu, iova, PAGE_SIZE);
>   vfio_unpin_page_external(dma, iova, do_accounting);
>   phys_pfn[j] = 0;
> +
> + if (test_bit(j, bitmap_added)) {
> + bitmap_offset = (iova - dma->iova) >> pgshift;
> + clear_bit(bitmap_offset, dma->bitmap);
> + }
>   }
>  pin_done:
> + if (bitmap_added)
> + bitmap_free(bitmap_added);
> +
>   mutex_unlock(>lock);
>   return ret;
>  }

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu/amd: Add sanity check for interrupt remapping table length macros

2020-12-10 Thread Jerry Snitselaar


Suravee Suthikulpanit @ 2020-12-10 09:24 MST:

> Currently, macros related to the interrupt remapping table length are
> defined separately. This has resulted in an oversight in which one of
> the macros were missed when changing the length. To prevent this,
> redefine the macros to add built-in sanity check.
>
> Also, rename macros to use the name of the DTE[IntTabLen] field as
> specified in the AMD IOMMU specification. There is no functional change.
>
> Suggested-by: Linus Torvalds 
> Reviewed-by: Tom Lendacky 
> Signed-off-by: Suravee Suthikulpanit 
> Cc: Will Deacon 
> Cc: Jerry Snitselaar 
> Cc: Joerg Roedel 
> ---
>  drivers/iommu/amd/amd_iommu_types.h | 19 ++-
>  drivers/iommu/amd/init.c|  6 +++---
>  drivers/iommu/amd/iommu.c   |  2 +-
>  3 files changed, 14 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h 
> b/drivers/iommu/amd/amd_iommu_types.h
> index 494b42a31b7a..899ce62df3f0 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -255,11 +255,19 @@
>  /* Bit value definition for dte irq remapping fields*/
>  #define DTE_IRQ_PHYS_ADDR_MASK   (((1ULL << 45)-1) << 6)
>  #define DTE_IRQ_REMAP_INTCTL_MASK(0x3ULL << 60)
> -#define DTE_IRQ_TABLE_LEN_MASK   (0xfULL << 1)
>  #define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
> -#define DTE_IRQ_TABLE_LEN   (9ULL << 1)
>  #define DTE_IRQ_REMAP_ENABLE1ULL
>  
> +/*
> + * AMD IOMMU hardware only support 512 IRTEs despite
> + * the architectural limitation of 2048 entries.
> + */
> +#define DTE_INTTAB_ALIGNMENT128
> +#define DTE_INTTABLEN_VALUE 9ULL
> +#define DTE_INTTABLEN   (DTE_INTTABLEN_VALUE << 1)
> +#define DTE_INTTABLEN_MASK  (0xfULL << 1)
> +#define MAX_IRQS_PER_TABLE  (1 << DTE_INTTABLEN_VALUE)
> +
>  #define PAGE_MODE_NONE0x00
>  #define PAGE_MODE_1_LEVEL 0x01
>  #define PAGE_MODE_2_LEVEL 0x02
> @@ -409,13 +417,6 @@ extern bool amd_iommu_np_cache;
>  /* Only true if all IOMMUs support device IOTLBs */
>  extern bool amd_iommu_iotlb_sup;
>  
> -/*
> - * AMD IOMMU hardware only support 512 IRTEs despite
> - * the architectural limitation of 2048 entries.
> - */
> -#define MAX_IRQS_PER_TABLE   512
> -#define IRQ_TABLE_ALIGNMENT  128
> -
>  struct irq_remap_table {
>   raw_spinlock_t lock;
>   unsigned min_index;
> diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
> index 23a790f8f550..6bec8913d064 100644
> --- a/drivers/iommu/amd/init.c
> +++ b/drivers/iommu/amd/init.c
> @@ -989,10 +989,10 @@ static bool copy_device_table(void)
>  
>   irq_v = old_devtb[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
>   int_ctl = old_devtb[devid].data[2] & DTE_IRQ_REMAP_INTCTL_MASK;
> - int_tab_len = old_devtb[devid].data[2] & DTE_IRQ_TABLE_LEN_MASK;
> + int_tab_len = old_devtb[devid].data[2] & DTE_INTTABLEN_MASK;
>   if (irq_v && (int_ctl || int_tab_len)) {
>   if ((int_ctl != DTE_IRQ_REMAP_INTCTL) ||
> - (int_tab_len != DTE_IRQ_TABLE_LEN)) {
> + (int_tab_len != DTE_INTTABLEN)) {
>   pr_err("Wrong old irq remapping flag: %#x\n", 
> devid);
>   return false;
>   }
> @@ -2674,7 +2674,7 @@ static int __init early_amd_iommu_init(void)
>   remap_cache_sz = MAX_IRQS_PER_TABLE * (sizeof(u64) * 2);
>   amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
>   remap_cache_sz,
> - IRQ_TABLE_ALIGNMENT,
> + DTE_INTTAB_ALIGNMENT,
>   0, NULL);
>   if (!amd_iommu_irq_cache)
>   goto out;
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index b9cf59443843..f7abf16d1e3a 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -3191,7 +3191,7 @@ static void set_dte_irq_entry(u16 devid, struct 
> irq_remap_table *table)
>   dte &= ~DTE_IRQ_PHYS_ADDR_MASK;
>   dte |= iommu_virt_to_phys(table->table);
>   dte |= DTE_IRQ_REMAP_INTCTL;
> - dte |= DTE_IRQ_TABLE_LEN;
> + dte |= DTE_INTTABLEN;
>   dte |= DTE_IRQ_REMAP_ENABLE;
>  
>   amd_iommu_dev_table[devid].data[2] = dte;


Reviewed-by: Jerry Snitselaar 

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [RFC PATCH 04/15] lib/scatterlist: Add flag for indicating P2PDMA segments in an SGL

2020-12-10 Thread Logan Gunthorpe



On 2020-12-09 9:04 p.m., Dan Williams wrote:
> On Wed, Dec 9, 2020 at 6:07 PM Logan Gunthorpe  wrote:
>>
>>
>>
>> On 2020-12-09 6:22 p.m., Dan Williams wrote:
>>> On Mon, Nov 9, 2020 at 8:47 AM Logan Gunthorpe  wrote:



 On 2020-11-09 2:12 a.m., Christoph Hellwig wrote:
> On Fri, Nov 06, 2020 at 10:00:25AM -0700, Logan Gunthorpe wrote:
>> We make use of the top bit of the dma_length to indicate a P2PDMA
>> segment.
>
> I don't think "we" can.  There is nothing limiting the size of a SGL
> segment.

 Yes, I expected this would be the unacceptable part. Any alternative ideas?
>>>
>>> Why is the SG_P2PDMA_FLAG needed as compared to checking the SGL
>>> segment-pages for is_pci_p2pdma_page()?
>>
>> Because the DMA and page segments in the SGL aren't necessarily aligned...
>>
>> The IOMMU implementations can coalesce multiple pages into fewer DMA
>> address ranges, so the page pointed to by sg->page_link may not be the
>> one that corresponds to the address in sg->dma_address for a given segment.
>>
>> If that makes sense -- it's not the easiest thing to explain.
> 
> It does...
> 
> Did someone already grab, or did you already consider the 3rd
> available bit in page_link? AFAICS only SG_CHAIN and SG_END are
> reserved. However, if you have a CONFIG_64BIT dependency for
> user-directed p2pdma that would seem to allow SG_P2PDMA_FLAG to be
> (0x4) in page_link.

Hmm, I half considered that, but I had came to the conclusion that given
the mis-alignment I shouldn't be using the page side of the SGL.
However, reconsidering now, that might actually be a reasonable option.

However, the CONFIG_64BIT dependency would have to be on all P2PDMA,
because we'd need to replace pci_p2pdma_map_sg() in all cases. I'm not
sure if this would be a restriction people care about.

Thanks,

Logan

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken

2020-12-10 Thread Bjorn Helgaas
On Thu, Dec 10, 2020 at 03:36:36PM +, Deucher, Alexander wrote:
> [AMD Public Use]
> 
> > -Original Message-
> > From: Merger, Edgar [AUTOSOL/MAS/AUGS] 
> > Sent: Thursday, December 10, 2020 5:48 AM
> > To: Deucher, Alexander ; Huang, Ray 
> > ; Kuehling, Felix 
> > Cc: Will Deacon ; linux-ker...@vger.kernel.org; 
> > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > Helgaas ; Joerg Roedel ; Zhu, 
> > Changfeng 
> > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > broken
> > 
> > Alright. Done that.
> > This should be it finally I believe.
> > Which will be the initial kernel-version that incorporates that?
> 
> Looks good to me.  Bjorn, can you pick this up for PCI?

Didn't apply cleanly, but I applied it by hand to pci/misc for v5.11.
If all goes well it should appear in v5.11-rc1.

https://git.kernel.org/pub/scm/linux/kernel/git/helgaas/pci.git/commit/?h=pci/misc=23bb0d9a9fe70a8ff23f53af822f2c6e6f261818

> > -Original Message-
> > From: Deucher, Alexander 
> > Sent: Mittwoch, 9. Dezember 2020 15:24
> > To: Merger, Edgar [AUTOSOL/MAS/AUGS] ; 
> > Huang, Ray ; Kuehling, Felix 
> > 
> > Cc: Will Deacon ; linux-ker...@vger.kernel.org; 
> > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > Helgaas ; Joerg Roedel ; Zhu, 
> > Changfeng 
> > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > broken
> > 
> > [AMD Public Use]
> > 
> > > -Original Message-
> > > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> > 
> > > Sent: Wednesday, December 9, 2020 2:59 AM
> > > To: Deucher, Alexander ; Huang, Ray 
> > > ; Kuehling, Felix 
> > > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > > Helgaas ; Joerg Roedel ; Zhu, 
> > > Changfeng 
> > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > > broken
> > >
> > > Alex,
> > >
> > > I had to revise the patch. Please see attachment. It is actually two 
> > > more SSIDs affected to that.
> > 
> > Other than some minor whitespace issues, the patch looks fine to me.
> > Please align the subsystem_device lines and put the closing 
> > parenthesis on the same line as the last check.
> > 
> > Thanks!
> > 
> > Alex
> > 
> > >
> > > Best regards,
> > > Edgar
> > >
> > > -Original Message-
> > > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> > > Sent: Dienstag, 8. Dezember 2020 09:23
> > > To: 'Deucher, Alexander' ; 'Huang, Ray'
> > > ; 'Kuehling, Felix' 
> > > Cc: 'Will Deacon' ; 'linux-ker...@vger.kernel.org'
> > > ; 'linux-...@vger.kernel.org'  > > p...@vger.kernel.org>; 'iommu@lists.linux-foundation.org'
> > > ; 'Bjorn Helgaas'
> > > ; 'Joerg Roedel' ; 'Zhu, 
> > > Changfeng' 
> > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > > broken
> > >
> > > Applied the patch as in attachment. Verified that ATS for GPU-Device 
> > > had been disabled. See attachment "dmesg_ATS.log".
> > >
> > > Was running that build over night successfully.
> > >
> > > -Original Message-
> > > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> > > Sent: Montag, 7. Dezember 2020 05:53
> > > To: Deucher, Alexander ; Huang, Ray 
> > > ; Kuehling, Felix 
> > > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > > Helgaas ; Joerg Roedel ; Zhu, 
> > > Changfeng 
> > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > > broken
> > >
> > > Hi Alex,
> > >
> > > I believe in the patch file, this
> > > + (pdev->subsystem_device == 0x0c19 ||
> > > +  pdev->subsystem_device == 0x0c10))
> > >
> > > Has to be changed to:
> > > + (pdev->subsystem_device == 0xce19 ||
> > > +  pdev->subsystem_device == 0xcc10))
> > >
> > > Because our SSIDs are "ea50:ce19" and "ea50:cc10" respectively and 
> > > another one would "ea50:cc08".
> > >
> > > I will apply that patch and feedback the results soon plus the patch 
> > > file that I actually had applied.
> > >
> > >
> > > -Original Message-
> > > From: Deucher, Alexander 
> > > Sent: Montag, 30. November 2020 19:36
> > > To: Merger, Edgar [AUTOSOL/MAS/AUGS]
> > ;
> > > Huang, Ray ; Kuehling, Felix 
> > > 
> > > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > > Helgaas ; Joerg Roedel ; Zhu, 
> > > Changfeng 
> > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > > broken
> > >
> > > [AMD Public Use]
> > >
> > > > -Original Message-
> > > > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> > > 
> > > > Sent: Thursday, November 26, 2020 4:24 AM
> > > > To: Deucher, Alexander ; Huang, Ray 
> > > > ; Kuehling, Felix 
> > > > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; 
> > > > Bjorn Helgaas ; Joerg Roedel 
> > > > ; Zhu, 

[PATCH] iommu/amd: Add sanity check for interrupt remapping table length macros

2020-12-10 Thread Suravee Suthikulpanit
Currently, macros related to the interrupt remapping table length are
defined separately. This has resulted in an oversight in which one of
the macros were missed when changing the length. To prevent this,
redefine the macros to add built-in sanity check.

Also, rename macros to use the name of the DTE[IntTabLen] field as
specified in the AMD IOMMU specification. There is no functional change.

Suggested-by: Linus Torvalds 
Reviewed-by: Tom Lendacky 
Signed-off-by: Suravee Suthikulpanit 
Cc: Will Deacon 
Cc: Jerry Snitselaar 
Cc: Joerg Roedel 
---
 drivers/iommu/amd/amd_iommu_types.h | 19 ++-
 drivers/iommu/amd/init.c|  6 +++---
 drivers/iommu/amd/iommu.c   |  2 +-
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu_types.h 
b/drivers/iommu/amd/amd_iommu_types.h
index 494b42a31b7a..899ce62df3f0 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -255,11 +255,19 @@
 /* Bit value definition for dte irq remapping fields*/
 #define DTE_IRQ_PHYS_ADDR_MASK (((1ULL << 45)-1) << 6)
 #define DTE_IRQ_REMAP_INTCTL_MASK  (0x3ULL << 60)
-#define DTE_IRQ_TABLE_LEN_MASK (0xfULL << 1)
 #define DTE_IRQ_REMAP_INTCTL(2ULL << 60)
-#define DTE_IRQ_TABLE_LEN   (9ULL << 1)
 #define DTE_IRQ_REMAP_ENABLE1ULL
 
+/*
+ * AMD IOMMU hardware only support 512 IRTEs despite
+ * the architectural limitation of 2048 entries.
+ */
+#define DTE_INTTAB_ALIGNMENT128
+#define DTE_INTTABLEN_VALUE 9ULL
+#define DTE_INTTABLEN   (DTE_INTTABLEN_VALUE << 1)
+#define DTE_INTTABLEN_MASK  (0xfULL << 1)
+#define MAX_IRQS_PER_TABLE  (1 << DTE_INTTABLEN_VALUE)
+
 #define PAGE_MODE_NONE0x00
 #define PAGE_MODE_1_LEVEL 0x01
 #define PAGE_MODE_2_LEVEL 0x02
@@ -409,13 +417,6 @@ extern bool amd_iommu_np_cache;
 /* Only true if all IOMMUs support device IOTLBs */
 extern bool amd_iommu_iotlb_sup;
 
-/*
- * AMD IOMMU hardware only support 512 IRTEs despite
- * the architectural limitation of 2048 entries.
- */
-#define MAX_IRQS_PER_TABLE 512
-#define IRQ_TABLE_ALIGNMENT128
-
 struct irq_remap_table {
raw_spinlock_t lock;
unsigned min_index;
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 23a790f8f550..6bec8913d064 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -989,10 +989,10 @@ static bool copy_device_table(void)
 
irq_v = old_devtb[devid].data[2] & DTE_IRQ_REMAP_ENABLE;
int_ctl = old_devtb[devid].data[2] & DTE_IRQ_REMAP_INTCTL_MASK;
-   int_tab_len = old_devtb[devid].data[2] & DTE_IRQ_TABLE_LEN_MASK;
+   int_tab_len = old_devtb[devid].data[2] & DTE_INTTABLEN_MASK;
if (irq_v && (int_ctl || int_tab_len)) {
if ((int_ctl != DTE_IRQ_REMAP_INTCTL) ||
-   (int_tab_len != DTE_IRQ_TABLE_LEN)) {
+   (int_tab_len != DTE_INTTABLEN)) {
pr_err("Wrong old irq remapping flag: %#x\n", 
devid);
return false;
}
@@ -2674,7 +2674,7 @@ static int __init early_amd_iommu_init(void)
remap_cache_sz = MAX_IRQS_PER_TABLE * (sizeof(u64) * 2);
amd_iommu_irq_cache = kmem_cache_create("irq_remap_cache",
remap_cache_sz,
-   IRQ_TABLE_ALIGNMENT,
+   DTE_INTTAB_ALIGNMENT,
0, NULL);
if (!amd_iommu_irq_cache)
goto out;
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index b9cf59443843..f7abf16d1e3a 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -3191,7 +3191,7 @@ static void set_dte_irq_entry(u16 devid, struct 
irq_remap_table *table)
dte &= ~DTE_IRQ_PHYS_ADDR_MASK;
dte |= iommu_virt_to_phys(table->table);
dte |= DTE_IRQ_REMAP_INTCTL;
-   dte |= DTE_IRQ_TABLE_LEN;
+   dte |= DTE_INTTABLEN;
dte |= DTE_IRQ_REMAP_ENABLE;
 
amd_iommu_dev_table[devid].data[2] = dte;
-- 
2.17.1

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken

2020-12-10 Thread Deucher, Alexander
[AMD Public Use]

> -Original Message-
> From: Merger, Edgar [AUTOSOL/MAS/AUGS] 
> Sent: Thursday, December 10, 2020 5:48 AM
> To: Deucher, Alexander ; Huang, Ray 
> ; Kuehling, Felix 
> Cc: Will Deacon ; linux-ker...@vger.kernel.org; 
> linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> Helgaas ; Joerg Roedel ; Zhu, 
> Changfeng 
> Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> broken
> 
> Alright. Done that.
> This should be it finally I believe.
> Which will be the initial kernel-version that incorporates that?

Looks good to me.  Bjorn, can you pick this up for PCI?

Alex

> 
> -Original Message-
> From: Deucher, Alexander 
> Sent: Mittwoch, 9. Dezember 2020 15:24
> To: Merger, Edgar [AUTOSOL/MAS/AUGS] ; 
> Huang, Ray ; Kuehling, Felix 
> 
> Cc: Will Deacon ; linux-ker...@vger.kernel.org; 
> linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> Helgaas ; Joerg Roedel ; Zhu, 
> Changfeng 
> Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> broken
> 
> [AMD Public Use]
> 
> > -Original Message-
> > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> 
> > Sent: Wednesday, December 9, 2020 2:59 AM
> > To: Deucher, Alexander ; Huang, Ray 
> > ; Kuehling, Felix 
> > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > Helgaas ; Joerg Roedel ; Zhu, 
> > Changfeng 
> > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > broken
> >
> > Alex,
> >
> > I had to revise the patch. Please see attachment. It is actually two 
> > more SSIDs affected to that.
> 
> Other than some minor whitespace issues, the patch looks fine to me.
> Please align the subsystem_device lines and put the closing 
> parenthesis on the same line as the last check.
> 
> Thanks!
> 
> Alex
> 
> >
> > Best regards,
> > Edgar
> >
> > -Original Message-
> > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> > Sent: Dienstag, 8. Dezember 2020 09:23
> > To: 'Deucher, Alexander' ; 'Huang, Ray'
> > ; 'Kuehling, Felix' 
> > Cc: 'Will Deacon' ; 'linux-ker...@vger.kernel.org'
> > ; 'linux-...@vger.kernel.org'  > p...@vger.kernel.org>; 'iommu@lists.linux-foundation.org'
> > ; 'Bjorn Helgaas'
> > ; 'Joerg Roedel' ; 'Zhu, 
> > Changfeng' 
> > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > broken
> >
> > Applied the patch as in attachment. Verified that ATS for GPU-Device 
> > had been disabled. See attachment "dmesg_ATS.log".
> >
> > Was running that build over night successfully.
> >
> > -Original Message-
> > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> > Sent: Montag, 7. Dezember 2020 05:53
> > To: Deucher, Alexander ; Huang, Ray 
> > ; Kuehling, Felix 
> > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > Helgaas ; Joerg Roedel ; Zhu, 
> > Changfeng 
> > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > broken
> >
> > Hi Alex,
> >
> > I believe in the patch file, this
> > +   (pdev->subsystem_device == 0x0c19 ||
> > +pdev->subsystem_device == 0x0c10))
> >
> > Has to be changed to:
> > +   (pdev->subsystem_device == 0xce19 ||
> > +pdev->subsystem_device == 0xcc10))
> >
> > Because our SSIDs are "ea50:ce19" and "ea50:cc10" respectively and 
> > another one would "ea50:cc08".
> >
> > I will apply that patch and feedback the results soon plus the patch 
> > file that I actually had applied.
> >
> >
> > -Original Message-
> > From: Deucher, Alexander 
> > Sent: Montag, 30. November 2020 19:36
> > To: Merger, Edgar [AUTOSOL/MAS/AUGS]
> ;
> > Huang, Ray ; Kuehling, Felix 
> > 
> > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > Helgaas ; Joerg Roedel ; Zhu, 
> > Changfeng 
> > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > broken
> >
> > [AMD Public Use]
> >
> > > -Original Message-
> > > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> > 
> > > Sent: Thursday, November 26, 2020 4:24 AM
> > > To: Deucher, Alexander ; Huang, Ray 
> > > ; Kuehling, Felix 
> > > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; 
> > > Bjorn Helgaas ; Joerg Roedel 
> > > ; Zhu, Changfeng 
> > > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS 
> > > as broken
> > >
> > > Alex,
> > >
> > > This is pretty much the same patch as what I have received from 
> > > Joerg previously, except that it is tied to the particular Emerson 
> > > platform and its derivatives (listed with Subsystem IDs).
> >
> > Right.  As per my original point, I don't want to disable ATS on all 
> > Picasso chips because doing so would break GPU compute on them, so 
> > I'd like to apply this quirk as narrowly as possible.
> >
> > >
> > > Below patch 

[PATCH] dma-mapping: move hint unlikely for dma_mapping_error from drivers to core

2020-12-10 Thread Heiner Kallweit
Zillions of drivers use the unlikely() hint when checking the result of
dma_mapping_error(). This is an inline function anyway, so we can move
the hint into the function and remove it from drivers.
>From time to time discussions pop up how effective unlikely() is,
and that it should be used only if something is really very unlikely.
I think that's the case here.

Patch was created with some help from coccinelle.

@@
expression dev, dma_addr;
@@

- unlikely(dma_mapping_error(dev, dma_addr))
+ dma_mapping_error(dev, dma_addr)

Signed-off-by: Heiner Kallweit 
---
If ok, then tbd through which tree this is supposed to go.
Patch is based on linux-next-20201210.
---
 drivers/crypto/cavium/cpt/cptvf_reqmanager.c  |  3 +--
 drivers/crypto/hisilicon/hpre/hpre_crypto.c   |  2 +-
 .../marvell/octeontx/otx_cptvf_reqmgr.c   |  5 ++--
 drivers/crypto/mediatek/mtk-aes.c |  2 +-
 drivers/crypto/mediatek/mtk-sha.c |  6 ++---
 drivers/crypto/qat/qat_common/qat_algs.c  |  8 +++---
 drivers/crypto/qat/qat_common/qat_asym_algs.c | 25 +--
 drivers/i2c/busses/i2c-amd-mp2-plat.c |  2 +-
 drivers/infiniband/hw/hfi1/sdma.c |  2 +-
 drivers/net/ethernet/aeroflex/greth.c |  4 +--
 drivers/net/ethernet/amazon/ena/ena_netdev.c  |  8 +++---
 .../net/ethernet/apm/xgene/xgene_enet_main.c  |  2 +-
 .../net/ethernet/aquantia/atlantic/aq_nic.c   |  5 ++--
 .../net/ethernet/aquantia/atlantic/aq_ring.c  |  2 +-
 drivers/net/ethernet/arc/emac_main.c  |  2 +-
 .../net/ethernet/atheros/atl1c/atl1c_main.c   |  6 ++---
 drivers/net/ethernet/broadcom/bgmac.c |  4 +--
 .../net/ethernet/broadcom/bnx2x/bnx2x_cmn.c   | 10 
 .../ethernet/broadcom/bnx2x/bnx2x_ethtool.c   |  2 +-
 drivers/net/ethernet/broadcom/bnxt/bnxt.c |  4 +--
 drivers/net/ethernet/chelsio/cxgb4/sge.c  |  4 +--
 drivers/net/ethernet/chelsio/cxgb4vf/sge.c|  4 +--
 drivers/net/ethernet/faraday/ftgmac100.c  |  2 +-
 drivers/net/ethernet/faraday/ftmac100.c   |  4 +--
 .../net/ethernet/freescale/dpaa/dpaa_eth.c| 13 +-
 .../net/ethernet/freescale/dpaa2/dpaa2-eth.c  | 12 -
 drivers/net/ethernet/freescale/enetc/enetc.c  |  4 +--
 drivers/net/ethernet/freescale/gianfar.c  |  6 ++---
 drivers/net/ethernet/google/gve/gve_tx.c  |  4 +--
 drivers/net/ethernet/hisilicon/hisi_femac.c   |  2 +-
 drivers/net/ethernet/hisilicon/hix5hd2_gmac.c |  4 +--
 .../net/ethernet/hisilicon/hns3/hns3_enet.c   |  4 +--
 drivers/net/ethernet/lantiq_xrx200.c  |  5 ++--
 drivers/net/ethernet/marvell/mv643xx_eth.c|  3 +--
 drivers/net/ethernet/marvell/mvneta.c |  9 +++
 drivers/net/ethernet/marvell/mvneta_bm.c  |  2 +-
 .../net/ethernet/marvell/mvpp2/mvpp2_main.c   |  8 +++---
 .../marvell/octeontx2/nic/otx2_common.c   |  2 +-
 drivers/net/ethernet/mediatek/mtk_eth_soc.c   | 10 
 drivers/net/ethernet/mellanox/mlx4/en_rx.c|  2 +-
 .../mellanox/mlx5/core/diag/rsc_dump.c|  2 +-
 .../net/ethernet/mellanox/mlx5/core/en/xdp.c  |  2 +-
 .../mellanox/mlx5/core/en_accel/ktls_rx.c |  2 +-
 .../mellanox/mlx5/core/en_accel/ktls_tx.c |  2 +-
 .../net/ethernet/mellanox/mlx5/core/en_rx.c   |  2 +-
 .../net/ethernet/mellanox/mlx5/core/en_tx.c   |  6 ++---
 .../net/ethernet/neterion/vxge/vxge-config.c  |  6 ++---
 .../net/ethernet/neterion/vxge/vxge-main.c|  6 ++---
 drivers/net/ethernet/nvidia/forcedeth.c   | 21 ++--
 .../net/ethernet/pensando/ionic/ionic_txrx.c  |  2 +-
 drivers/net/ethernet/qlogic/qed/qed_ll2.c |  4 +--
 .../net/ethernet/qlogic/qede/qede_ethtool.c   |  2 +-
 drivers/net/ethernet/qlogic/qede/qede_fp.c|  8 +++---
 drivers/net/ethernet/realtek/r8169_main.c |  2 +-
 drivers/net/ethernet/rocker/rocker_main.c |  2 +-
 drivers/net/ethernet/sfc/falcon/rx.c  |  3 +--
 drivers/net/ethernet/sfc/falcon/tx.c  |  4 +--
 drivers/net/ethernet/sfc/rx_common.c  |  3 +--
 drivers/net/ethernet/sfc/tx_common.c  |  4 +--
 drivers/net/ethernet/sfc/tx_tso.c |  2 +-
 drivers/net/ethernet/sis/sis900.c | 24 --
 drivers/net/ethernet/socionext/sni_ave.c  |  2 +-
 drivers/net/ethernet/sun/sunhme.c |  8 +++---
 drivers/net/ethernet/ti/am65-cpsw-nuss.c  |  6 ++---
 drivers/net/ethernet/ti/netcp_core.c  |  4 +--
 drivers/net/ethernet/via/via-rhine.c  |  2 +-
 .../net/ethernet/xilinx/xilinx_axienet_main.c |  6 ++---
 drivers/net/wireless/ath/ath10k/htt_rx.c  |  2 +-
 drivers/net/wireless/ath/ath10k/pci.c |  2 +-
 drivers/net/wireless/ath/ath10k/snoc.c|  2 +-
 drivers/net/wireless/ath/ath11k/ce.c  |  2 +-
 drivers/net/wireless/ath/ath11k/dp_rx.c   |  2 +-
 drivers/net/wireless/ath/ath5k/base.c |  2 +-
 drivers/net/wireless/ath/ath9k/beacon.c   |  2 +-
 drivers/net/wireless/ath/ath9k/recv.c | 21 +++-
 drivers/net/wireless/ath/ath9k/xmi

Re: [GIT PULL] IOMMU fix for 5.10 (-final)

2020-12-10 Thread Suravee Suthikulpanit

Hi All,

On 12/10/20 1:50 AM, Will Deacon wrote:

On Wed, Dec 09, 2020 at 10:07:46AM -0800, Linus Torvalds wrote:

On Wed, Dec 9, 2020 at 6:12 AM Will Deacon  wrote:


Please pull this one-liner AMD IOMMU fix for 5.10. It's actually a fix
for a fix, where the size of the interrupt remapping table was increased
but a related constant for the size of the interrupt table was forgotten.


Pulled.


Thanks.


However, why didn't this then add some sanity checking for the two
different #defines to be in sync?

IOW, something like

#define AMD_IOMMU_IRQ_TABLE_SHIFT 9

#define MAX_IRQS_PER_TABLE (1 << AMD_IOMMU_IRQ_TABLE_SHIFT)
#define DTE_IRQ_TABLE_LEN ((u64)AMD_IOMMU_IRQ_TABLE_SHIFT << 1)

or whatever. Hmm?


This looks like a worthwhile change to me, but I don't have any hardware
so I've been very reluctant to make even "obvious" driver changes here.

Suravee -- please can you post a patch implementing the above?


I'll send one out ASAP.




That way this won't happen again, but perhaps equally importantly the
linkage will be more clear, and there won't be those random constants.

Naming above is probably garbage - I assume there's some actual
architectural name for that irq table length field in the DTE?


The one in the spec is even better: "IntTabLen".

Will


Thanks,
Suravee
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken

2020-12-10 Thread Merger, Edgar [AUTOSOL/MAS/AUGS]
Alright. Done that. 
This should be it finally I believe.
Which will be the initial kernel-version that incorporates that?

-Original Message-
From: Deucher, Alexander  
Sent: Mittwoch, 9. Dezember 2020 15:24
To: Merger, Edgar [AUTOSOL/MAS/AUGS] ; Huang, Ray 
; Kuehling, Felix 
Cc: Will Deacon ; linux-ker...@vger.kernel.org; 
linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn Helgaas 
; Joerg Roedel ; Zhu, Changfeng 

Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken

[AMD Public Use]

> -Original Message-
> From: Merger, Edgar [AUTOSOL/MAS/AUGS] 
> Sent: Wednesday, December 9, 2020 2:59 AM
> To: Deucher, Alexander ; Huang, Ray 
> ; Kuehling, Felix 
> Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> Helgaas ; Joerg Roedel ; Zhu, 
> Changfeng 
> Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> broken
> 
> Alex,
> 
> I had to revise the patch. Please see attachment. It is actually two 
> more SSIDs affected to that.

Other than some minor whitespace issues, the patch looks fine to me.  Please 
align the subsystem_device lines and put the closing parenthesis on the same 
line as the last check.

Thanks!

Alex

> 
> Best regards,
> Edgar
> 
> -Original Message-
> From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> Sent: Dienstag, 8. Dezember 2020 09:23
> To: 'Deucher, Alexander' ; 'Huang, Ray'
> ; 'Kuehling, Felix' 
> Cc: 'Will Deacon' ; 'linux-ker...@vger.kernel.org' 
> ; 'linux-...@vger.kernel.org'  p...@vger.kernel.org>; 'iommu@lists.linux-foundation.org'
> ; 'Bjorn Helgaas'
> ; 'Joerg Roedel' ; 'Zhu, 
> Changfeng' 
> Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> broken
> 
> Applied the patch as in attachment. Verified that ATS for GPU-Device 
> had been disabled. See attachment "dmesg_ATS.log".
> 
> Was running that build over night successfully.
> 
> -Original Message-
> From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> Sent: Montag, 7. Dezember 2020 05:53
> To: Deucher, Alexander ; Huang, Ray 
> ; Kuehling, Felix 
> Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> Helgaas ; Joerg Roedel ; Zhu, 
> Changfeng 
> Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> broken
> 
> Hi Alex,
> 
> I believe in the patch file, this
> + (pdev->subsystem_device == 0x0c19 ||
> +  pdev->subsystem_device == 0x0c10))
> 
> Has to be changed to:
> + (pdev->subsystem_device == 0xce19 ||
> +  pdev->subsystem_device == 0xcc10))
> 
> Because our SSIDs are "ea50:ce19" and "ea50:cc10" respectively and 
> another one would "ea50:cc08".
> 
> I will apply that patch and feedback the results soon plus the patch 
> file that I actually had applied.
> 
> 
> -Original Message-
> From: Deucher, Alexander 
> Sent: Montag, 30. November 2020 19:36
> To: Merger, Edgar [AUTOSOL/MAS/AUGS] ; 
> Huang, Ray ; Kuehling, Felix 
> 
> Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> Helgaas ; Joerg Roedel ; Zhu, 
> Changfeng 
> Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> broken
> 
> [AMD Public Use]
> 
> > -Original Message-
> > From: Merger, Edgar [AUTOSOL/MAS/AUGS]
> 
> > Sent: Thursday, November 26, 2020 4:24 AM
> > To: Deucher, Alexander ; Huang, Ray 
> > ; Kuehling, Felix 
> > Cc: Will Deacon ; linux-ker...@vger.kernel.org;
> > linux- p...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn 
> > Helgaas ; Joerg Roedel ; Zhu, 
> > Changfeng 
> > Subject: RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as 
> > broken
> >
> > Alex,
> >
> > This is pretty much the same patch as what I have received from 
> > Joerg previously, except that it is tied to the particular Emerson 
> > platform and its derivatives (listed with Subsystem IDs).
> 
> Right.  As per my original point, I don't want to disable ATS on all 
> Picasso chips because doing so would break GPU compute on them, so I'd 
> like to apply this quirk as narrowly as possible.
> 
> >
> > Below patch was what Joerg provided me and I successfully tested.
> >
> > This diff to the kernel should do that:
> >
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index 
> > f70692ac79c5..3911b0ec57ba 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -5176,6 +5176,8 @@
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI,
> > 0x6900, quirk_amd_harvest_no_ats);
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7312, 
> > quirk_amd_harvest_no_ats);
> >  /* AMD Navi14 dGPU */
> >  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7340, 
> > quirk_amd_harvest_no_ats);
> > +/* AMD Raven platform iGPU */
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x15d8, 
> > +quirk_amd_harvest_no_ats);
> >  #endif /* CONFIG_PCI_ATS */
> >
> >  /* 

Re: [RFC PATCH v2 0/8] ACPI/IORT: Support for IORT RMR node

2020-12-10 Thread Steven Price

On 19/11/2020 12:11, Shameer Kolothum wrote:

RFC v1 --> v2:
  - Added a generic interface for IOMMU drivers to retrieve all the
    RMR info associated with a given IOMMU.
  - SMMUv3 driver gets the RMR list during probe() and installs
    bypass STEs for all the SIDs in the RMR list. This is to keep
    the ongoing traffic alive(if any) during SMMUv3 reset. This is
based on the suggestions received for v1 to take care of the
EFI framebuffer use case. Only sanity tested for now.


Hi Shameer,

Sorry for not looking at this before.

Do you have any plans to implement support in the SMMUv2 driver? The 
platform I've been testing the EFI framebuffer support on has the 
display controller behind SMMUv2, so as it stands this series doesn't 
work. I did hack something up for SMMUv2 so I was able to test the first 
4 patches.



  - During the probe/attach device, SMMUv3 driver reserves any
    RMR region associated with the device such that there is a unity
    mapping for them in SMMU.


For the EFI framebuffer use case there is no device to attach so I 
believe we are left with just the stream ID in bypass mode - which is 
definitely an improvement (the display works!) but not actually a unity 
mapping of the RMR range. I'm not sure whether it's worth fixing this or 
not, but I just wanted to point out there's still a need for a driver 
for the device before the bypass mode is replaced with the unity mapping.


Thanks,

Steve
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] drivers/iommu: fix a null-ptr-deref bug in fsl_pamu_domain.c

2020-12-10 Thread Will Deacon
On Wed, Dec 09, 2020 at 04:34:30AM -0800, tangzhenhao wrote:
> At line 362 in drivers/iommu/fsl_pamu_domain.c, the ret-val of 
> kmem_cache_zalloc should be checked to avoid null-ptr-deref bug.
> 
> Signed-off-by: tangzhenhao 
> ---
>  drivers/iommu/fsl_pamu_domain.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
> index b2110767caf4..9ebd5135f4a8 100644
> --- a/drivers/iommu/fsl_pamu_domain.c
> +++ b/drivers/iommu/fsl_pamu_domain.c
> @@ -360,6 +360,10 @@ static void attach_device(struct fsl_dma_domain 
> *dma_domain, int liodn, struct d
>   }
>  
>   info = kmem_cache_zalloc(iommu_devinfo_cache, GFP_ATOMIC);
> + if (!info) {
> + pr_debug("device_domain_info allocation failed\n");
> + return;
> + }

All the comments I made on your other patch [1] apply to this one too.

Will

[1] https://lore.kernel.org/r/20201210091326.GA9633@willie-the-truck
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] drivers/iommu: fix a null-ptr-deref bug in msm_iommu.c

2020-12-10 Thread Will Deacon
On Sun, Dec 06, 2020 at 01:43:51AM -0800, tangzhenhao wrote:
> At line 600 in drivers/iommu/msm_iommu.c, the ret-val of kzalloc should be 
> checked to avoid null-ptr-deref bug.

There's no need to mention the line number of the file name in the commit
message -- that information is already available in the diff. Instead,
please try to describe the problem that you're solving.

Have a look at Documentation/process/submitting-patches.rst.

> Signed-off-by: tangzhenhao 
> ---
>  drivers/iommu/msm_iommu.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/iommu/msm_iommu.c b/drivers/iommu/msm_iommu.c
> index 3615cd6241c4..e3c576e5babb 100644
> --- a/drivers/iommu/msm_iommu.c
> +++ b/drivers/iommu/msm_iommu.c
> @@ -598,6 +598,10 @@ static void insert_iommu_master(struct device *dev,
>  
>   if (list_empty(&(*iommu)->ctx_list)) {
>   master = kzalloc(sizeof(*master), GFP_ATOMIC);
> + if (!master) {
> + dev_err(dev, "Failed to allocate IOMMU context bank 
> instance\n");

No need to print an error here -- kzalloc should be plenty noisy enough
if an atomic allocation fails.

> + return;

Hmm, and then what? We haven't propagated the error, so how much further do
we get?

Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu