Re: [PATCH] iommu: amd: Fix IOMMU perf counter clobbering during init

2020-01-20 Thread Suravee Suthikulpanit

On 1/17/2020 5:08 PM, Joerg Roedel wrote:

Adding Suravee, who wrote the IOMMU Perf Counter code.

On Tue, Jan 14, 2020 at 08:12:20AM -0700, Shuah Khan wrote:

init_iommu_perf_ctr() clobbers the register when it checks write access
to IOMMU perf counters and fails to restore when they are writable.

Add save and restore to fix it.

Signed-off-by: Shuah Khan
---
  drivers/iommu/amd_iommu_init.c | 22 --
  1 file changed, 16 insertions(+), 6 deletions(-)

Suravee, can you please review this patch?



This looks ok. Does this fix certain issues? Or is this just for sanity.

Reviewed-by: Suravee Suthikulpanit 

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


Re: [PATCH v4 4/7] iommu/vt-d: Use pci_real_dma_dev() for mapping

2020-01-20 Thread Derrick, Jonathan
Good catch. Thanks Baolu.
Will do v5 fixing this and Christoph's nit

On Tue, 2020-01-21 at 09:06 +0800, Lu Baolu wrote:
> Hi,
> 
> On 1/18/20 12:27 AM, Jon Derrick wrote:
> > The PCI device may have a DMA requester on another bus, such as VMD
> > subdevices needing to use the VMD endpoint. This case requires the real
> > DMA device when mapping to IOMMU.
> > 
> > Signed-off-by: Jon Derrick
> > ---
> >   drivers/iommu/intel-iommu.c | 9 +
> >   1 file changed, 9 insertions(+)
> > 
> > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> > index 0c8d81f..01a1b0f 100644
> > --- a/drivers/iommu/intel-iommu.c
> > +++ b/drivers/iommu/intel-iommu.c
> > @@ -782,6 +782,8 @@ static struct intel_iommu *device_to_iommu(struct 
> > device *dev, u8 *bus, u8 *devf
> > return NULL;
> >   #endif
> >   
> > +   pdev = pci_real_dma_dev(dev);
> 
> This isn't correct. It will result in a compiling error when bisect.
> 
> Best regards,
> baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH v4 4/7] iommu/vt-d: Use pci_real_dma_dev() for mapping

2020-01-20 Thread Lu Baolu

Hi,

On 1/18/20 12:27 AM, Jon Derrick wrote:

The PCI device may have a DMA requester on another bus, such as VMD
subdevices needing to use the VMD endpoint. This case requires the real
DMA device when mapping to IOMMU.

Signed-off-by: Jon Derrick
---
  drivers/iommu/intel-iommu.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 0c8d81f..01a1b0f 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -782,6 +782,8 @@ static struct intel_iommu *device_to_iommu(struct device 
*dev, u8 *bus, u8 *devf
return NULL;
  #endif
  
+		pdev = pci_real_dma_dev(dev);


This isn't correct. It will result in a compiling error when bisect.

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


Re: [RFC PATCH 0/4] iommu: Per-group default domain type

2020-01-20 Thread Lu Baolu

Hi John,

On 1/20/20 5:44 PM, John Garry wrote:

On 01/01/2020 05:26, Lu Baolu wrote:

An IOMMU group represents the smallest set of devices that are considered
to be isolated. All devices belonging to an IOMMU group share a default
domain for DMA APIs. There are two types of default domain: 
IOMMU_DOMAIN_DMA

and IOMMU_DOMAIN_IDENTITY. The former means IOMMU translation, while the
latter means IOMMU by-pass.

Currently, the default domain type for the IOMMU groups is determined
globally. All IOMMU groups use a single default domain type. The global
default domain type can be adjusted by kernel build configuration or
kernel parameters.

More and more users are looking forward to a fine grained default domain
type. For example, with the global default domain type set to 
translation,
the OEM verndors or end users might want some trusted and fast-speed 
devices

to bypass IOMMU for performance gains. On the other hand, with global
default domain type set to by-pass, some devices with limited system
memory addressing capability might want IOMMU translation to remove the
bounce buffer overhead.


Hi Lu Baolu,

Do you think that it would be a more common usecase to want 
kernel-managed devices to be passthrough for performance reasons and 
some select devices to be in DMA domain, like those with limited address 
cap or whose drivers request huge amounts of memory?


I just think it would be more manageable to set kernel commandline 
parameters for this, i.e. those select few which want DMA domain.




It's just two sides of a coin. Currently, iommu subsystem make DMA
domain by default, that's the reason why I selected to let user set
which devices are willing to use identity domains.



Thanks,
John


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


Re: [Patch v3 0/3] iommu: reduce spinlock contention on fast path

2020-01-20 Thread Cong Wang
On Tue, Dec 17, 2019 at 8:40 PM Cong Wang  wrote:
>
> This patchset contains three small optimizations for the global spinlock
> contention in IOVA cache. Our memcache perf test shows this reduced its
> p999 latency down by 45% on AMD when IOMMU is enabled.
>
> (Resending v3 on Joerg's request.)

Hi, Joerg

Can you take these patches?

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


Re: [PATCH v4 0/7] Clean up VMD DMA Map Ops

2020-01-20 Thread Lorenzo Pieralisi
On Sun, Jan 19, 2020 at 11:25:23PM +0100, Christoph Hellwig wrote:
> This series looks good to me (modulo the one minor nitpick which isn't
> all that important):
> 
> Reviewed-by: Christoph Hellwig 

Hi Bjorn,

are you picking this up ? I can merge it too but since it is mostly
x86 changes I reckon you should take it, I acked patch (6) to that
end.

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


Re: [PATCH v4 6/7] PCI: vmd: Stop overriding dma_map_ops

2020-01-20 Thread Lorenzo Pieralisi
On Fri, Jan 17, 2020 at 09:27:28AM -0700, Jon Derrick wrote:
> Devices on the VMD domain use the VMD endpoint's requester ID and have
> been relying on the VMD endpoint's DMA operations. The problem with this
> was that VMD domain devices would use the VMD endpoint's attributes when
> doing DMA and IOMMU mapping. We can be smarter about this by only using
> the VMD endpoint when mapping and providing the correct child device's
> attributes during DMA operations.
> 
> This patch removes the dma_map_ops redirect.
> 
> Signed-off-by: Jon Derrick 
> ---
>  drivers/pci/controller/Kconfig |   1 -
>  drivers/pci/controller/vmd.c   | 150 
> -
>  2 files changed, 151 deletions(-)

Acked-by: Lorenzo Pieralisi 

> diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
> index 918e283..20bf00f 100644
> --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -239,7 +239,6 @@ config PCIE_TANGO_SMP8759
>  
>  config VMD
>   depends on PCI_MSI && X86_64 && SRCU
> - select X86_DEV_DMA_OPS
>   tristate "Intel Volume Management Device Driver"
>   ---help---
> Adds support for the Intel Volume Management Device (VMD). VMD is a
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index d67ad56..fe1acb0 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -98,9 +98,6 @@ struct vmd_dev {
>   struct irq_domain   *irq_domain;
>   struct pci_bus  *bus;
>   u8  busn_start;
> -
> - struct dma_map_ops  dma_ops;
> - struct dma_domain   dma_domain;
>  };
>  
>  static inline struct vmd_dev *vmd_from_bus(struct pci_bus *bus)
> @@ -295,151 +292,6 @@ static void vmd_set_desc(msi_alloc_info_t *arg, struct 
> msi_desc *desc)
>   .chip   = _msi_controller,
>  };
>  
> -/*
> - * VMD replaces the requester ID with its own.  DMA mappings for devices in a
> - * VMD domain need to be mapped for the VMD, not the device requiring
> - * the mapping.
> - */
> -static struct device *to_vmd_dev(struct device *dev)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev);
> - struct vmd_dev *vmd = vmd_from_bus(pdev->bus);
> -
> - return >dev->dev;
> -}
> -
> -static void *vmd_alloc(struct device *dev, size_t size, dma_addr_t *addr,
> -gfp_t flag, unsigned long attrs)
> -{
> - return dma_alloc_attrs(to_vmd_dev(dev), size, addr, flag, attrs);
> -}
> -
> -static void vmd_free(struct device *dev, size_t size, void *vaddr,
> -  dma_addr_t addr, unsigned long attrs)
> -{
> - return dma_free_attrs(to_vmd_dev(dev), size, vaddr, addr, attrs);
> -}
> -
> -static int vmd_mmap(struct device *dev, struct vm_area_struct *vma,
> - void *cpu_addr, dma_addr_t addr, size_t size,
> - unsigned long attrs)
> -{
> - return dma_mmap_attrs(to_vmd_dev(dev), vma, cpu_addr, addr, size,
> - attrs);
> -}
> -
> -static int vmd_get_sgtable(struct device *dev, struct sg_table *sgt,
> -void *cpu_addr, dma_addr_t addr, size_t size,
> -unsigned long attrs)
> -{
> - return dma_get_sgtable_attrs(to_vmd_dev(dev), sgt, cpu_addr, addr, size,
> - attrs);
> -}
> -
> -static dma_addr_t vmd_map_page(struct device *dev, struct page *page,
> -unsigned long offset, size_t size,
> -enum dma_data_direction dir,
> -unsigned long attrs)
> -{
> - return dma_map_page_attrs(to_vmd_dev(dev), page, offset, size, dir,
> - attrs);
> -}
> -
> -static void vmd_unmap_page(struct device *dev, dma_addr_t addr, size_t size,
> -enum dma_data_direction dir, unsigned long attrs)
> -{
> - dma_unmap_page_attrs(to_vmd_dev(dev), addr, size, dir, attrs);
> -}
> -
> -static int vmd_map_sg(struct device *dev, struct scatterlist *sg, int nents,
> -   enum dma_data_direction dir, unsigned long attrs)
> -{
> - return dma_map_sg_attrs(to_vmd_dev(dev), sg, nents, dir, attrs);
> -}
> -
> -static void vmd_unmap_sg(struct device *dev, struct scatterlist *sg, int 
> nents,
> -  enum dma_data_direction dir, unsigned long attrs)
> -{
> - dma_unmap_sg_attrs(to_vmd_dev(dev), sg, nents, dir, attrs);
> -}
> -
> -static void vmd_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
> - size_t size, enum dma_data_direction dir)
> -{
> - dma_sync_single_for_cpu(to_vmd_dev(dev), addr, size, dir);
> -}
> -
> -static void vmd_sync_single_for_device(struct device *dev, dma_addr_t addr,
> -size_t size, enum dma_data_direction dir)
> -{
> - dma_sync_single_for_device(to_vmd_dev(dev), addr, size, dir);
> -}
> -
> -static void vmd_sync_sg_for_cpu(struct device *dev, struct scatterlist *sg,

Re: [RFC PATCH 0/4] iommu: Per-group default domain type

2020-01-20 Thread John Garry

On 01/01/2020 05:26, Lu Baolu wrote:

An IOMMU group represents the smallest set of devices that are considered
to be isolated. All devices belonging to an IOMMU group share a default
domain for DMA APIs. There are two types of default domain: IOMMU_DOMAIN_DMA
and IOMMU_DOMAIN_IDENTITY. The former means IOMMU translation, while the
latter means IOMMU by-pass.

Currently, the default domain type for the IOMMU groups is determined
globally. All IOMMU groups use a single default domain type. The global
default domain type can be adjusted by kernel build configuration or
kernel parameters.

More and more users are looking forward to a fine grained default domain
type. For example, with the global default domain type set to translation,
the OEM verndors or end users might want some trusted and fast-speed devices
to bypass IOMMU for performance gains. On the other hand, with global
default domain type set to by-pass, some devices with limited system
memory addressing capability might want IOMMU translation to remove the
bounce buffer overhead.


Hi Lu Baolu,

Do you think that it would be a more common usecase to want 
kernel-managed devices to be passthrough for performance reasons and 
some select devices to be in DMA domain, like those with limited address 
cap or whose drivers request huge amounts of memory?


I just think it would be more manageable to set kernel commandline 
parameters for this, i.e. those select few which want DMA domain.


Thanks,
John



This series proposes per-group default domain type to meet these demands.
It adds a per-device iommu_passthrough attribute. By setting this
attribute, end users or device vendors are able to tell the IOMMU subsystem
that this device is willing to use a default domain of IOMMU_DOMAIN_IDENTITY.
The IOMMU device probe procedure is reformed to pre-allocate groups for
all devices on a specific bus before adding the devices into the groups.
This enables the IOMMU device probe precedure to determine a per-group
default domain type before allocating IOMMU domains and attaching them
to devices.

Please help to review it. Your comments and suggestions are appricated.

Best regards,
baolu

Lu Baolu (4):
   driver core: Add iommu_passthrough to struct device
   PCI: Add "pci=iommu_passthrough=" parameter for iommu passthrough
   iommu: Preallocate iommu group when probing devices
   iommu: Determine default domain type before allocating domain

  .../admin-guide/kernel-parameters.txt |   5 +
  drivers/iommu/iommu.c | 127 ++
  drivers/pci/pci.c |  34 +
  drivers/pci/pci.h |   1 +
  drivers/pci/probe.c   |   2 +
  include/linux/device.h|   3 +
  6 files changed, 143 insertions(+), 29 deletions(-)



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