Re: [PATCH v7 3/5] memory: mediatek: Add SMI driver

[Philipp Zabel]

Hi,

Am Montag, den 04.01.2016, 14:56 +0800 schrieb Yong Wu:
> On Fri, 2015-12-18 at 16:09 +0800, Yong Wu wrote:
> > This patch add SMI(Smart Multimedia Interface) driver. This driver
> > is responsible to enable/disable iommu and control the power domain
> > and clocks of each local arbiter.
> 
> Hi Matthias,
> What's your opinion about the SMI in this version?
> Sorry to disturb you again since SMI need your review before Joerg
> accept it.

How is this going to be merged, eventually? If it could be put on a
separate branch, that would allow me to also include it in the DRM pull
request that will build-depend on the SMI driver.

> > Signed-off-by: Yong Wu 

Tested-by: Philipp Zabel 

regards
Philipp

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

Re: [patch] iommu/amd: remove an unneeded condition

[Joerg Roedel]

On Thu, Jan 07, 2016 at 12:36:06PM +0300, Dan Carpenter wrote:
> get_device_id() returns an unsigned short device id.  It never fails and
> it never returns a negative so we can remove this condition.
> 
> Signed-off-by: Dan Carpenter 

Applied, thanks.

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

[PATCH] iommu/vt-d: Fix up error handling in alloc_iommu successfully in alloc_iommu

[Joerg Roedel]

On Mon, Jan 04, 2016 at 06:27:57PM -0500, Nicholas Krause wrote:
> This adds the proper check to alloc_iommu to make sure that the call
> to iommu_device_create has completed successfully and if not return
> to the caller the error code returned after freeing up resources
> allocated previously by alloc_iommu.
> 
> Signed-off-by: Nicholas Krause 

Applied with this fixup on top:

>From 3108f03ce20adf6429cb130b44da528ae85fc68d Mon Sep 17 00:00:00 2001
From: Joerg Roedel 
Date: Thu, 7 Jan 2016 12:16:51 +0100
Subject: [PATCH] iommu/vt-d: Fix up error handling in alloc_iommu

Only check for error when iommu->iommu_dev has been assigned
and only assign drhd->iommu when the function can't fail
anymore.

Signed-off-by: Joerg Roedel 
---
 drivers/iommu/dmar.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/iommu/dmar.c b/drivers/iommu/dmar.c
index add177a..62a400c 100644
--- a/drivers/iommu/dmar.c
+++ b/drivers/iommu/dmar.c
@@ -1063,19 +1063,19 @@ static int alloc_iommu(struct dmar_drhd_unit *drhd)
 
raw_spin_lock_init(>register_lock);
 
-   drhd->iommu = iommu;
-
-   if (intel_iommu_enabled)
+   if (intel_iommu_enabled) {
iommu->iommu_dev = iommu_device_create(NULL, iommu,
   intel_iommu_groups,
   "%s", iommu->name);
 
-   if (IS_ERR(iommu->iommu_dev)) {
-   drhd->iommu = NULL;
-   err = PTR_ERR(iommu->iommu_dev);
-   goto err_unmap;
+   if (IS_ERR(iommu->iommu_dev)) {
+   err = PTR_ERR(iommu->iommu_dev);
+   goto err_unmap;
+   }
}
 
+   drhd->iommu = iommu;
+
return 0;
 
 err_unmap:
-- 
1.8.4.5

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

Re: [PATCH v2 3/6] iommu/amd: Introduce amd_iommu_get_num_iommus()

[Joerg Roedel]

On Fri, Jan 01, 2016 at 12:13:37PM -0600, Suravee Suthikulpanit wrote:
> +int amd_iommu_get_num_iommus(void)
> +{
> + return amd_iommus_present;
> +}
> +EXPORT_SYMBOL(amd_iommu_get_num_iommus);

Is this export needed? The perf code can't be build as a module.

Otherwise it looks good.

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

Re: [PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices

[Joerg Roedel]

On Tue, Jan 05, 2016 at 05:07:24AM -0500, Wan Zongshun wrote:
> +static struct iommu_group *amd_iommu_device_group(struct device *dev)
> +{
> + if (dev_is_pci(dev))
> + return pci_device_group(dev);
> +
> + return acpihid_device_group(dev);
> +}
> +
>  
> /*
>   *
>   * The next functions belong to the dma_ops mapping/unmapping code.
> @@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
>   .iova_to_phys = amd_iommu_iova_to_phys,
>   .add_device = amd_iommu_add_device,
>   .remove_device = amd_iommu_remove_device,
> - .device_group = pci_device_group,
> + .device_group = amd_iommu_device_group,

Does this work? Which bus do the ACPIHID devices belong to (what does
dev->bus point to)? If it is not _bus_type, then the iommu core code
will not create an iommu group for the devices.


Joerg

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

Re: [PATCH 5/6] iommu/amd: Add support for non-pci devices

[Wan ZongShun]

2016-01-07 20:04 GMT+08:00 Joerg Roedel :
> On Tue, Jan 05, 2016 at 05:07:23AM -0500, Wan Zongshun wrote:
>> -static inline u16 get_device_id(struct device *dev)
>> +static inline int match_hid_uid(struct device *dev,
>> + struct acpihid_map_entry *entry)
>> +{
>> + const char *hid, *uid;
>> +
>> + hid = acpi_device_hid(ACPI_COMPANION(dev));
>> + uid = acpi_device_uid(ACPI_COMPANION(dev));
>> +
>> + if (!hid || !(*hid))
>> + return -ENODEV;
>> +
>> + if (!uid || !(*uid))
>> + return strcmp(hid, entry->hid);
>> +
>> + if (!(*entry->uid))
>> + return strcmp(hid, entry->hid);
>> +
>> + return -ENODEV;
>> +}
>> +
>> +static inline u16 get_pci_device_id(struct device *dev)
>>  {
>>   struct pci_dev *pdev = to_pci_dev(dev);
>>
>>   return PCI_DEVID(pdev->bus->number, pdev->devfn);
>>  }
>>
>> +static inline int get_acpihid_device_id(struct device *dev,
>> + struct acpihid_map_entry **entry)
>> +{
>> + struct acpihid_map_entry *p;
>> +
>> + list_for_each_entry(p, _map, list) {
>> + if (!match_hid_uid(dev, p)) {
>> + if (entry)
>> + *entry = p;
>> + return p->devid;
>> + }
>> + }
>> + return -EINVAL;
>> +}
>> +
>> +static inline u16 get_device_id(struct device *dev)
>> +{
>> + if (dev_is_pci(dev))
>> + return get_pci_device_id(dev);
>> + else
>> + return get_acpihid_device_id(dev, NULL);
>> +}
>
> This is not robust, get_acpihid_device_id() returns int and can return a
> negative value. This gets lost when converting it to u16 here. So either
> you add error handling for get_acpihid_device_id() in get_device_id() or
> you change get_device_id() to return int too and handle the error at the
> callers of get_device_id().

Joerg,

Please see the following function, since I judge this
'get_acpihid_device_id(dev, NULL) < 0'  in the front of
'get_device_id', so your concern should not exist. I have already
filtered the negative situation in check_device firstly, do you think
it is ok?


static bool check_device(struct device *dev)
{
u16 devid;
..

/* No PCI device */
if (!dev_is_pci(dev) && (get_acpihid_device_id(dev, NULL) < 0))
return false;

devid = get_device_id(dev);

.

return true;
}


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



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH 6/6] iommu/amd: Manage iommu_group for non-pci devices

[Wan ZongShun]

2016-01-07 20:06 GMT+08:00 Joerg Roedel :
> On Tue, Jan 05, 2016 at 05:07:24AM -0500, Wan Zongshun wrote:
>> +static struct iommu_group *amd_iommu_device_group(struct device *dev)
>> +{
>> + if (dev_is_pci(dev))
>> + return pci_device_group(dev);
>> +
>> + return acpihid_device_group(dev);
>> +}
>> +
>>  
>> /*
>>   *
>>   * The next functions belong to the dma_ops mapping/unmapping code.
>> @@ -3202,7 +3233,7 @@ static const struct iommu_ops amd_iommu_ops = {
>>   .iova_to_phys = amd_iommu_iova_to_phys,
>>   .add_device = amd_iommu_add_device,
>>   .remove_device = amd_iommu_remove_device,
>> - .device_group = pci_device_group,
>> + .device_group = amd_iommu_device_group,
>
> Does this work? Which bus do the ACPIHID devices belong to (what does
> dev->bus point to)? If it is not _bus_type, then the iommu core code
> will not create an iommu group for the devices.

Yes, it works, we have already done validation on AMD platform.
Please refer to my previous patch :
[PATCH] iommu/amd: set AMD iommu-callbacks for the amba bus.

Currently, The UART DMA is the use case of ACPIHID device, the
dev->bus is amba_bus, so I add bus_set_iommu for this bus type.

We will do create iommu group in acpihid_device_group which will call
generic_device_group to alloc group.

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



-- 
---
Vincent Wan(Zongshun)
www.mcuos.com
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

[git pull] IOMMU Fixes for Linux v4.4-rc8

[Joerg Roedel]

Hi Linus,

The following changes since commit 74bf8efb5fa6e958d2d7c7917b8bb672085ec0c6:

  Linux 4.4-rc7 (2015-12-27 18:17:37 -0800)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/joro/iommu.git 
tags/iommu-fixes-v4.4-rc8

for you to fetch changes up to 164afb1d85b872907cfac048b46c094db596d529:

  iommu/dma: Use correct offset in map_sg (2016-01-07 13:36:41 +0100)


IOMMU Fixes for Linux v4.4-rc8

The patches include fixes for:

* Two build issues, one in the ipmmu-vmsa driver and one for the
  new generic dma-api implemention used on arm64

* A performance fix for said dma-api implemention

* An issue caused by a wrong offset in map_sg in the same code
  as above


Geert Uytterhoeven (1):
  iommu/ipmmu-vmsa: Don't truncate ttbr if LPAE is not enabled

Robin Murphy (3):
  iommu/dma: Add some missing #includes
  iommu/dma: Avoid unlikely high-order allocations
  iommu/dma: Use correct offset in map_sg

 drivers/iommu/dma-iommu.c  | 11 ---
 drivers/iommu/ipmmu-vmsa.c |  2 +-
 2 files changed, 9 insertions(+), 4 deletions(-)

Please pull.

Thanks,

Joerg



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