Re: [PATCH 1/3] hexagon: remove the sync_single_for_cpu DMA operation
On Tue, Jul 24, 2018 at 10:29:48PM -0500, Richard Kuo wrote: > Patch series looks good. Definitely appreciate the cleanup. > > I can take it through my tree, or if not: > > Acked-by: Richard Kuo Please take it through your tree, thanks! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/3] hexagon: remove the sync_single_for_cpu DMA operation
On Thu, Jul 19, 2018 at 05:56:33AM -0700, Christoph Hellwig wrote: > hexagon does all the required cache maintainance at dma map time, and none > at unmap time. It thus has to implement sync_single_for_device to match > the map cace for buffer reuse, but there is no point in doing another > invalidation in the sync_single_cpu_case, which in terms of cache > maintainance is equivalent to the unmap case. > > Signed-off-by: Christoph Hellwig > --- > arch/hexagon/kernel/dma.c | 8 > 1 file changed, 8 deletions(-) > Patch series looks good. Definitely appreciate the cleanup. I can take it through my tree, or if not: Acked-by: Richard Kuo Thanks! -- Employee of Qualcomm Innovation Center, Inc. Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH 03/10] iommu/vt-d: Allocate groups for mediated devices
Hi Jean, > From: Jean-Philippe Brucker [mailto:jean-philippe.bruc...@arm.com] > Sent: Tuesday, July 24, 2018 7:31 PM > > Hi Baolu, > > On 24/07/18 03:22, Lu Baolu wrote: > > Hi, > > > > On 07/23/2018 12:44 PM, Liu, Yi L wrote: > >>> From: Lu Baolu [mailto:baolu...@linux.intel.com] > >>> Sent: Sunday, July 22, 2018 2:09 PM > >>> > >>> With the Intel IOMMU supporting PASID granularity isolation and > >>> protection, a > >>> mediated device could be isolated and protected by an IOMMU unit. We need > >>> to > >>> allocate a new group instead of a PCI group. > >> Existing vfio mdev framework also allocates an iommu group for mediate > >> device. > >> > >> mdev_probe() > >> |_ mdev_attach_iommu() > >>|_ iommu_group_alloc() > > > > When external components ask iommu to allocate a group for a device, > > it will call pci_device_group in Intel IOMMU driver's @device_group > > callback. In another word, current Intel IOMMU driver doesn't aware > > the mediated device and treat all devices as PCI ones. This patch > > extends the @device_group call back to make it aware of a mediated > > device. > > I agree that allocating two groups for an mdev seems strange, and in my There will not be two groups for a mdev. Pls refer to Patch 08/10 of this series. Baolu added iommu_ops check when doing group allocation in mdev_attach_iommu(). [RFC PATCH 08/10] vfio/mdev: Set iommu ops for mdev bus @@ -21,6 +21,13 @@ static int mdev_attach_iommu(struct mdev_device *mdev) int ret; struct iommu_group *group; + /* +* If iommu_ops is set for bus, add_device() will allocate +* a group and add the device in the group. +*/ + if (iommu_present(mdev->dev.bus)) + return 0; + > opinion we shouldn't export the notion of mdev to IOMMU drivers. The key idea of this RFC is to tag iommu domain with PASID, if any mdev is added to such a domain, it would get the PASID and config in its parent. Thus the transactions from mdev can be isolated in iommu hardware. Based on this idea, mdevs can be managed in a flexible manner. e.g. if two mdevs are assigned to same VM, they can share PASID. This share can be easily achieve by adding them to the same domain. If we default allocate a PASID for each mdev, it may be a waste. With vendor-specific iommu driver handle the mdev difference, it can largely keep the fundamental iommu concepts in current software implementation. > Otherwise each driver will have to add its own "dev_is_mdev()" special > cases, which will get messy in the long run. Besides, the macro is > currently private, and to be exported it should be wrapped in > symbol_get/put(mdev_bus_type). Agreed. Should figure out a better manner. > There is another way: as we're planning to add a generic pasid_alloc() > function to the IOMMU API, the mdev module itself could allocate a > default PASID for each mdev by calling pasid_alloc() on the mdev's > parent, and then do map()/unmap() with that PASID. This way we don't so far, map/unmap is per-domain operation. In this way, passing PASID makes it be kind of per-device operation. This may affect too much of existing software implementation. > have to add IOMMU ops to the mdev bus, everything can still be done > using the ops of the parent. And IOMMU drivers "only" have to implement > PASID ops, which will be reused by drivers other than mdev. > > The allocated PASID also needs to be installed into the parent device. > If the mdev module knows the PASID, we can do that by adding > set_pasid(mdev, pasid) and clear_pasid(mdev, pasid) operations to > mdev_parent_ops. Your idea is fascinating. Pls feel free let us know if we missed any from you. :) > Thanks, > Jean Thanks, Yi Liu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [RFC PATCH 03/10] iommu/vt-d: Allocate groups for mediated devices
> From: Jean-Philippe Brucker > Sent: Tuesday, July 24, 2018 7:31 PM > > Hi Baolu, > > On 24/07/18 03:22, Lu Baolu wrote: > > Hi, > > > > On 07/23/2018 12:44 PM, Liu, Yi L wrote: > >>> From: Lu Baolu [mailto:baolu...@linux.intel.com] > >>> Sent: Sunday, July 22, 2018 2:09 PM > >>> > >>> With the Intel IOMMU supporting PASID granularity isolation and > protection, a > >>> mediated device could be isolated and protected by an IOMMU unit. > We need to > >>> allocate a new group instead of a PCI group. > >> Existing vfio mdev framework also allocates an iommu group for > mediate device. > >> > >> mdev_probe() > >> |_ mdev_attach_iommu() > >>|_ iommu_group_alloc() > > > > When external components ask iommu to allocate a group for a device, > > it will call pci_device_group in Intel IOMMU driver's @device_group > > callback. In another word, current Intel IOMMU driver doesn't aware > > the mediated device and treat all devices as PCI ones. This patch > > extends the @device_group call back to make it aware of a mediated > > device. > > I agree that allocating two groups for an mdev seems strange, and in my > opinion we shouldn't export the notion of mdev to IOMMU drivers. > Otherwise each driver will have to add its own "dev_is_mdev()" special > cases, which will get messy in the long run. Besides, the macro is > currently private, and to be exported it should be wrapped in > symbol_get/put(mdev_bus_type). There is only one group per mdev, but I agree that avoiding mdev awareness in IOMMU driver is definitely good... We didn't find a good way before, which is why current RFC was implemented that way. > > There is another way: as we're planning to add a generic pasid_alloc() > function to the IOMMU API, the mdev module itself could allocate a > default PASID for each mdev by calling pasid_alloc() on the mdev's > parent, and then do map()/unmap() with that PASID. This way we don't > have to add IOMMU ops to the mdev bus, everything can still be done > using the ops of the parent. And IOMMU drivers "only" have to implement > PASID ops, which will be reused by drivers other than mdev. yes, PASID is more general abstraction than mdev. However there is one problem. Please note with new scalable mode now PASID is not used just for SVA. You can do 1st-level, 2nd-level and nested in PASID granularity, meaning each PASID can be bound to an unique iommu domain. However today IOMMU driver allows only one iommu_domain per device. If we simply install allocated PASID to parent device, we should also remove that iommu_domain limitation. Is it the way that we want to pursue? mdev avoids such problem as it introduces a separate device... > > The allocated PASID also needs to be installed into the parent device. > If the mdev module knows the PASID, we can do that by adding > set_pasid(mdev, pasid) and clear_pasid(mdev, pasid) operations to > mdev_parent_ops. > Thanks Kevin ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 03/10] iommu/vt-d: Allocate groups for mediated devices
Hi Jean, Pull Kevin in. Not sure why he was removed from cc list. On 07/24/2018 07:30 PM, Jean-Philippe Brucker wrote: > Hi Baolu, > > On 24/07/18 03:22, Lu Baolu wrote: >> Hi, >> >> On 07/23/2018 12:44 PM, Liu, Yi L wrote: From: Lu Baolu [mailto:baolu...@linux.intel.com] Sent: Sunday, July 22, 2018 2:09 PM With the Intel IOMMU supporting PASID granularity isolation and protection, a mediated device could be isolated and protected by an IOMMU unit. We need to allocate a new group instead of a PCI group. >>> Existing vfio mdev framework also allocates an iommu group for mediate >>> device. >>> >>> mdev_probe() >>> |_ mdev_attach_iommu() >>>|_ iommu_group_alloc() >> When external components ask iommu to allocate a group for a device, >> it will call pci_device_group in Intel IOMMU driver's @device_group >> callback. In another word, current Intel IOMMU driver doesn't aware >> the mediated device and treat all devices as PCI ones. This patch >> extends the @device_group call back to make it aware of a mediated >> device. > I agree that allocating two groups for an mdev seems strange, and in my > opinion we shouldn't export the notion of mdev to IOMMU drivers. > Otherwise each driver will have to add its own "dev_is_mdev()" special > cases, which will get messy in the long run. Besides, the macro is > currently private, and to be exported it should be wrapped in > symbol_get/put(mdev_bus_type). I agree with you. It should be better if we can make it in a driver agnostic way. > > There is another way: as we're planning to add a generic pasid_alloc() > function to the IOMMU API, the mdev module itself could allocate a > default PASID for each mdev by calling pasid_alloc() on the mdev's > parent, and then do map()/unmap() with that PASID. This way we don't > have to add IOMMU ops to the mdev bus, everything can still be done > using the ops of the parent. And IOMMU drivers "only" have to implement > PASID ops, which will be reused by drivers other than mdev. A quick question when I thinking about this is how to allocate a domain for the mediated device who uses the default pasid for DMA transactions? Best regards, Lu Baolu > > The allocated PASID also needs to be installed into the parent device. > If the mdev module knows the PASID, we can do that by adding > set_pasid(mdev, pasid) and clear_pasid(mdev, pasid) operations to > mdev_parent_ops. > > Thanks, > Jean > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 01/10] iommu/vt-d: Get iommu device for a mediated device
Hi Alex,
On 07/25/2018 02:50 AM, Alex Williamson wrote:
> On Sun, 22 Jul 2018 14:09:24 +0800
> Lu Baolu wrote:
>
>> This patch adds the support to get the iommu device for a mediated
>> device. The assumption is that each mediated device is a minimal
>> assignable set of a physical PCI device. Hence, we should use the
>> iommu device of the parent PCI device to manage the translation.
> Hmm, is this absolutely a valid assumption? I'm afraid there's an
> assumption throughout this series that the only way an mdev device
> could be interacting with the IOMMU is via S-IOV, but we could choose
> today to make an mdev wrapper for any device, which might provide basic
> RID granularity to the IOMMU. So if I make an mdev wrapper for a PF
> such that I can implement migration for that device, is it valid for
> the IOMMU driver to flag me as an mdev device and base mappings on my
> parent device?
You are right. We should not make it SIOV centric. Let me look into the
patches and identify/fix the unreasonable assumptions.
> Perhaps in this patch we can assume that the parent of
> such an mdev device would be the PF backing it and that results in the
> correct drhd,
Okay.
> but in the next patch we start imposing the assumption
> that because the device is an mdev, the only valid association is via
> pasid, which I'd say is incorrect.
You are right. I will find and fix it.
>
>> Cc: Ashok Raj
>> Cc: Jacob Pan
>> Cc: Kevin Tian
>> Cc: Liu Yi L
>> Signed-off-by: Sanjay Kumar
>> Signed-off-by: Lu Baolu
>> ---
>> drivers/iommu/intel-iommu.c | 6 ++
>> drivers/iommu/intel-pasid.h | 16
>> 2 files changed, 22 insertions(+)
>>
>> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
>> index 7d198ea..fc3ac1c 100644
>> --- a/drivers/iommu/intel-iommu.c
>> +++ b/drivers/iommu/intel-iommu.c
>> @@ -767,6 +767,12 @@ static struct intel_iommu *device_to_iommu(struct
>> device *dev, u8 *bus, u8 *devf
>> if (iommu_dummy(dev))
>> return NULL;
>>
>> +if (dev_is_mdev(dev)) {
>> +dev = dev_mdev_parent(dev);
>> +if (WARN_ON(!dev_is_pci(dev)))
>> +return NULL;
>> +}
>> +
>> if (dev_is_pci(dev)) {
>> struct pci_dev *pf_pdev;
>>
>> diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h
>> index 518df72..46cde66 100644
>> --- a/drivers/iommu/intel-pasid.h
>> +++ b/drivers/iommu/intel-pasid.h
>> @@ -35,6 +35,22 @@ struct pasid_table {
>> struct list_headdev;/* device list */
>> };
>>
>> +static inline bool dev_is_mdev(struct device *dev)
>> +{
>> +if (!dev)
>> +return false;
>> +
>> +return !strcmp(dev->bus->name, "mdev");
>> +}
> I assume we're not using mdev_bus_type because mdev is a loadable
> module and that symbol doesn't exist in this statically loaded driver,
Yes.
> but strcmp is a pretty ugly alternative. Could we use symbol_get() so
> that we can use mdev_bus_type? Thanks,
Sure.
Best regards,
Lu Baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 6/6] iommu/arm-smmu-v3: add bootup option "iommu_strict_mode"
On 2018-07-12 7:18 AM, Zhen Lei wrote:
Because the non-strict mode introduces a vulnerability window, so add a
bootup option to make the manager can choose which mode to be used. The
default mode is IOMMU_STRICT.
Signed-off-by: Zhen Lei
---
Documentation/admin-guide/kernel-parameters.txt | 12 ++
drivers/iommu/arm-smmu-v3.c | 32 ++---
2 files changed, 41 insertions(+), 3 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt
b/Documentation/admin-guide/kernel-parameters.txt
index efc7aa7..0cc80bc 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1720,6 +1720,18 @@
nobypass[PPC/POWERNV]
Disable IOMMU bypass, using IOMMU for PCI devices.
+ iommu_strict_mode= [arm-smmu-v3]
If anything this should belong to iommu-dma, as that's where the actual
decision of whether to use a flush queue or not happens. Also it would
be nice to stick to the iommu.* option namespace in the hope of
maintaining some consistency.
+ 0 - strict mode
+ Make sure all related TLBs to be invalidated before the
+ memory released.
+ 1 - non-strict mode
+ Put off TLBs invalidation and release memory first. This
mode
+ introduces a vlunerability window, an untrusted device can
+ access the reused memory because the TLBs may still valid.
+ Please take full consideration before choosing this mode.
+ Note that, VFIO is always use strict mode.
+ others - strict mode
+
iommu.passthrough=
[ARM64] Configure DMA to bypass the IOMMU by default.
Format: { "0" | "1" }
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 4a198a0..9b72fc4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -631,6 +631,24 @@ struct arm_smmu_option_prop {
{ 0, NULL},
};
+static u32 iommu_strict_mode __read_mostly = IOMMU_STRICT;
+
+static int __init setup_iommu_strict_mode(char *str)
+{
+ u32 strict_mode = IOMMU_STRICT;
+
+ get_option(, _mode);
+ if (strict_mode == IOMMU_NON_STRICT) {
+ iommu_strict_mode = strict_mode;
+ pr_warn("WARNING: iommu non-strict mode is chose.\n"
+ "It's good for scatter-gather performance but lacks full
isolation\n");
+ add_taint(TAINT_WARN, LOCKDEP_STILL_OK);
+ }
+
+ return 0;
+}
+early_param("iommu_strict_mode", setup_iommu_strict_mode);
+
static inline void __iomem *arm_smmu_page1_fixup(unsigned long offset,
struct arm_smmu_device *smmu)
{
@@ -1441,7 +1459,7 @@ static bool arm_smmu_capable(enum iommu_cap cap)
case IOMMU_CAP_NOEXEC:
return true;
case IOMMU_CAP_NON_STRICT:
- return true;
+ return (iommu_strict_mode == IOMMU_NON_STRICT) ? true : false;
Ugh. The "completely redundant ternany" idiom hurts my soul :(
Robin.
default:
return false;
}
@@ -1750,6 +1768,14 @@ static int arm_smmu_attach_dev(struct iommu_domain
*domain, struct device *dev)
return ret;
}
+static u32 arm_smmu_strict_mode(struct iommu_domain *domain)
+{
+ if (iommu_strict_mode == IOMMU_NON_STRICT)
+ return IOMMU_DOMAIN_STRICT_MODE(domain);
+
+ return IOMMU_STRICT;
+}
+
static int arm_smmu_map(struct iommu_domain *domain, unsigned long iova,
phys_addr_t paddr, size_t size, int prot)
{
@@ -1769,7 +1795,7 @@ static int arm_smmu_map(struct iommu_domain *domain,
unsigned long iova,
if (!ops)
return 0;
- return ops->unmap(ops, iova | IOMMU_DOMAIN_STRICT_MODE(domain), size);
+ return ops->unmap(ops, iova | arm_smmu_strict_mode(domain), size);
}
static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain)
@@ -1784,7 +1810,7 @@ static void arm_smmu_iotlb_sync(struct iommu_domain
*domain)
{
struct arm_smmu_device *smmu = to_smmu_domain(domain)->smmu;
- if (smmu && (IOMMU_DOMAIN_STRICT_MODE(domain) == IOMMU_STRICT))
+ if (smmu && (arm_smmu_strict_mode(domain) == IOMMU_STRICT))
__arm_smmu_tlb_sync(smmu);
}
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 4/6] iommu/io-pgtable-arm: add support for non-strict mode
On 2018-07-12 7:18 AM, Zhen Lei wrote:
To support the non-strict mode, now we only tlbi and sync for the strict
mode. But for the non-leaf case, always follow strict mode.
Use the lowest bit of the iova parameter to pass the strict mode:
0, IOMMU_STRICT;
1, IOMMU_NON_STRICT;
Treat 0 as IOMMU_STRICT, so that the unmap operation can compatible with
other IOMMUs which still use strict mode.
Signed-off-by: Zhen Lei
---
drivers/iommu/io-pgtable-arm.c | 23 ++-
1 file changed, 14 insertions(+), 9 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index 010a254..9234db3 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -292,7 +292,7 @@ static void __arm_lpae_set_pte(arm_lpae_iopte *ptep,
arm_lpae_iopte pte,
static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
unsigned long iova, size_t size, int lvl,
- arm_lpae_iopte *ptep);
+ arm_lpae_iopte *ptep, int strict);
static void __arm_lpae_init_pte(struct arm_lpae_io_pgtable *data,
phys_addr_t paddr, arm_lpae_iopte prot,
@@ -334,7 +334,7 @@ static int arm_lpae_init_pte(struct arm_lpae_io_pgtable
*data,
size_t sz = ARM_LPAE_BLOCK_SIZE(lvl, data);
tblp = ptep - ARM_LPAE_LVL_IDX(iova, lvl, data);
- if (WARN_ON(__arm_lpae_unmap(data, iova, sz, lvl, tblp) != sz))
+ if (WARN_ON(__arm_lpae_unmap(data, iova, sz, lvl, tblp,
IOMMU_STRICT) != sz))
return -EINVAL;
}
@@ -531,7 +531,7 @@ static void arm_lpae_free_pgtable(struct io_pgtable *iop)
static size_t arm_lpae_split_blk_unmap(struct arm_lpae_io_pgtable *data,
unsigned long iova, size_t size,
arm_lpae_iopte blk_pte, int lvl,
- arm_lpae_iopte *ptep)
+ arm_lpae_iopte *ptep, int strict)
DMA code should never ever be splitting blocks anyway, and frankly the
TLB maintenance here is dodgy enough (since we can't reasonably do
break-before make as VMSA says we should) that I *really* don't want to
introduce any possibility of making it more asynchronous. I'd much
rather just hard-code the expectation of strict == true for this.
Robin.
{
struct io_pgtable_cfg *cfg = >iop.cfg;
arm_lpae_iopte pte, *tablep;
@@ -576,15 +576,18 @@ static size_t arm_lpae_split_blk_unmap(struct
arm_lpae_io_pgtable *data,
}
if (unmap_idx < 0)
- return __arm_lpae_unmap(data, iova, size, lvl, tablep);
+ return __arm_lpae_unmap(data, iova, size, lvl, tablep, strict);
io_pgtable_tlb_add_flush(>iop, iova, size, size, true);
+ if (!strict)
+ io_pgtable_tlb_sync(>iop);
+
return size;
}
static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
unsigned long iova, size_t size, int lvl,
- arm_lpae_iopte *ptep)
+ arm_lpae_iopte *ptep, int strict)
{
arm_lpae_iopte pte;
struct io_pgtable *iop = >iop;
@@ -609,7 +612,7 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable
*data,
io_pgtable_tlb_sync(iop);
ptep = iopte_deref(pte, data);
__arm_lpae_free_pgtable(data, lvl + 1, ptep);
- } else {
+ } else if (strict) {
io_pgtable_tlb_add_flush(iop, iova, size, size, true);
}
@@ -620,25 +623,27 @@ static size_t __arm_lpae_unmap(struct arm_lpae_io_pgtable *data,
* minus the part we want to unmap
*/
return arm_lpae_split_blk_unmap(data, iova, size, pte,
- lvl + 1, ptep);
+ lvl + 1, ptep, strict);
}
/* Keep on walkin' */
ptep = iopte_deref(pte, data);
- return __arm_lpae_unmap(data, iova, size, lvl + 1, ptep);
+ return __arm_lpae_unmap(data, iova, size, lvl + 1, ptep, strict);
}
static size_t arm_lpae_unmap(struct io_pgtable_ops *ops, unsigned long iova,
size_t size)
{
+ int strict = ((iova & IOMMU_STRICT_MODE_MASK) == IOMMU_STRICT);
struct arm_lpae_io_pgtable *data = io_pgtable_ops_to_data(ops);
arm_lpae_iopte *ptep = data->pgd;
int lvl = ARM_LPAE_START_LVL(data);
+ iova &= ~IOMMU_STRICT_MODE_MASK;
if (WARN_ON(iova >= (1ULL << data->iop.cfg.ias)))
return 0;
- return __arm_lpae_unmap(data, iova, size, lvl, ptep);
+ return __arm_lpae_unmap(data, iova, size, lvl, ptep, strict);
}
static phys_addr_t arm_lpae_iova_to_phys(struct io_pgtable_ops
Re: [PATCH 1/3] Docs: dt: arm-smmu: Add optional clock parameter
Hi Rob, On 07/20/2018 11:15 AM, Rob Herring wrote: On Fri, Jul 13, 2018 at 11:27:56AM -0500, thor.tha...@linux.intel.com wrote: From: Thor Thayer Add a clock to the SMMU node bindings. Signed-off-by: Thor Thayer --- Documentation/devicetree/bindings/iommu/arm,smmu.txt | 16 1 file changed, 16 insertions(+) diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt index 8a6ffce12af5..356fd9f41e1b 100644 --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt @@ -71,6 +71,8 @@ conditions. or using stream matching with #iommu-cells = <2>, and may be ignored if present in such cases. +- clock: clock provider specifier + The TRM says there is a TCU clock and clock per TBU. Rob Yes, good point. I'm abandoning this review and will use the bulk clock suggested in [1]. In our case, the TCU clock is always on and we have 1 clock for the TBU masters. Thanks for reviewing! Thor [1] https://patchwork.kernel.org/patch/10534089/ ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 2/6] iommu/dma: add support for non-strict mode
On 2018-07-12 7:18 AM, Zhen Lei wrote:
1. Save the related domain pointer in struct iommu_dma_cookie, make iovad
capable call domain->ops->flush_iotlb_all to flush TLB.
2. Add a new iommu capability: IOMMU_CAP_NON_STRICT, which used to indicate
that the iommu domain support non-strict mode.
3. During the iommu domain initialization phase, call capable() to check
whether it support non-strcit mode. If so, call init_iova_flush_queue
to register iovad->flush_cb callback.
4. All unmap(contains iova-free) APIs will finally invoke __iommu_dma_unmap
-->iommu_dma_free_iova. If the domain is non-strict, call queue_iova to
put off iova freeing.
Signed-off-by: Zhen Lei
---
drivers/iommu/dma-iommu.c | 25 +
include/linux/iommu.h | 7 +++
2 files changed, 32 insertions(+)
diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index ddcbbdb..9f0c77a 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -55,6 +55,7 @@ struct iommu_dma_cookie {
};
struct list_headmsi_page_list;
spinlock_t msi_lock;
+ struct iommu_domain *domain_non_strict;
};
static inline size_t cookie_msi_granule(struct iommu_dma_cookie *cookie)
@@ -257,6 +258,17 @@ static int iova_reserve_iommu_regions(struct device *dev,
return ret;
}
+static void iommu_dma_flush_iotlb_all(struct iova_domain *iovad)
+{
+ struct iommu_dma_cookie *cookie;
+ struct iommu_domain *domain;
+
+ cookie = container_of(iovad, struct iommu_dma_cookie, iovad);
+ domain = cookie->domain_non_strict;
+
+ domain->ops->flush_iotlb_all(domain);
+}
+
/**
* iommu_dma_init_domain - Initialise a DMA mapping domain
* @domain: IOMMU domain previously prepared by iommu_get_dma_cookie()
@@ -272,6 +284,7 @@ static int iova_reserve_iommu_regions(struct device *dev,
int iommu_dma_init_domain(struct iommu_domain *domain, dma_addr_t base,
u64 size, struct device *dev)
{
+ const struct iommu_ops *ops = domain->ops;
struct iommu_dma_cookie *cookie = domain->iova_cookie;
struct iova_domain *iovad = >iovad;
unsigned long order, base_pfn, end_pfn;
@@ -308,6 +321,15 @@ int iommu_dma_init_domain(struct iommu_domain *domain,
dma_addr_t base,
}
init_iova_domain(iovad, 1UL << order, base_pfn);
+
+ if ((ops->capable && ops->capable(IOMMU_CAP_NON_STRICT)) &&
+ (IOMMU_DOMAIN_STRICT_MODE(domain) == IOMMU_NON_STRICT)) {
+ BUG_ON(!ops->flush_iotlb_all);
+
+ cookie->domain_non_strict = domain;
+ init_iova_flush_queue(iovad, iommu_dma_flush_iotlb_all, NULL);
+ }
+
if (!dev)
return 0;
@@ -390,6 +412,9 @@ static void iommu_dma_free_iova(struct iommu_dma_cookie *cookie,
/* The MSI case is only ever cleaning up its most recent allocation */
if (cookie->type == IOMMU_DMA_MSI_COOKIE)
cookie->msi_iova -= size;
+ else if (cookie->domain_non_strict)
+ queue_iova(iovad, iova_pfn(iovad, iova),
+ size >> iova_shift(iovad), 0);
else
free_iova_fast(iovad, iova_pfn(iovad, iova),
size >> iova_shift(iovad));
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 19938ee..82ed979 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -86,6 +86,12 @@ struct iommu_domain_geometry {
#define IOMMU_DOMAIN_DMA (__IOMMU_DOMAIN_PAGING |\
__IOMMU_DOMAIN_DMA_API)
+#define IOMMU_STRICT 0
+#define IOMMU_NON_STRICT 1
+#define IOMMU_STRICT_MODE_MASK 1UL
+#define IOMMU_DOMAIN_STRICT_MODE(domain) \
+ (domain->type != IOMMU_DOMAIN_UNMANAGED)
+
struct iommu_domain {
unsigned type;
const struct iommu_ops *ops;
@@ -101,6 +107,7 @@ enum iommu_cap {
transactions */
IOMMU_CAP_INTR_REMAP, /* IOMMU supports interrupt isolation */
IOMMU_CAP_NOEXEC, /* IOMMU_NOEXEC flag */
+ IOMMU_CAP_NON_STRICT, /* IOMMU supports non-strict mode */
This still isn't a capability of the hardware. Non-strict mode is pure
software policy; *every IOMMU ever* supports it.
If you want to have this kind of per-driver control, make it a domain
attribute - that's not only more logical, but should also work out a lot
cleaner overall.
Robin.
};
/*
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3 0/6] add non-strict mode support for arm-smmu-v3
On 2018-07-12 7:18 AM, Zhen Lei wrote: v2 -> v3: Add a bootup option "iommu_strict_mode" to make the manager can choose which mode to be used. The first 5 patches have not changed. + iommu_strict_mode= [arm-smmu-v3] + 0 - strict mode (default) + 1 - non-strict mode v1 -> v2: Use the lowest bit of the io_pgtable_ops.unmap's iova parameter to pass the strict mode: 0, IOMMU_STRICT; 1, IOMMU_NON_STRICT; Treat 0 as IOMMU_STRICT, so that the unmap operation can compatible with other IOMMUs which still use strict mode. In other words, this patch series will not impact other IOMMU drivers. I tried add a new quirk IO_PGTABLE_QUIRK_NON_STRICT in io_pgtable_cfg.quirks, but it can not pass the strict mode of the domain from SMMUv3 driver to io-pgtable module. What exactly is the issue there? We don't have any problem with other quirks like NO_DMA, and as I said before, by the time we're allocating the io-pgtable in arm_smmu_domain_finalise() we already know everything there is to know about the domain. Add a new member domain_non_strict in struct iommu_dma_cookie, this member will only be initialized when the related domain and IOMMU driver support non-strict mode. v1: In common, a IOMMU unmap operation follow the below steps: 1. remove the mapping in page table of the specified iova range 2. execute tlbi command to invalid the mapping which is cached in TLB 3. wait for the above tlbi operation to be finished 4. free the IOVA resource 5. free the physical memory resource This maybe a problem when unmap is very frequently, the combination of tlbi and wait operation will consume a lot of time. A feasible method is put off tlbi and iova-free operation, when accumulating to a certain number or reaching a specified time, execute only one tlbi_all command to clean up TLB, then free the backup IOVAs. Mark as non-strict mode. But it must be noted that, although the mapping has already been removed in the page table, it maybe still exist in TLB. And the freed physical memory may also be reused for others. So a attacker can persistent access to memory based on the just freed IOVA, to obtain sensible data or corrupt memory. So the VFIO should always choose the strict mode. Some may consider put off physical memory free also, that will still follow strict mode. But for the map_sg cases, the memory allocation is not controlled by IOMMU APIs, so it is not enforceable. Fortunately, Intel and AMD have already applied the non-strict mode, and put queue_iova() operation into the common file dma-iommu.c., and my work is based on it. The difference is that arm-smmu-v3 driver will call IOMMU common APIs to unmap, but Intel and AMD IOMMU drivers are not. Below is the performance data of strict vs non-strict for NVMe device: Randomly Read IOPS: 146K(strict) vs 573K(non-strict) Randomly Write IOPS: 143K(strict) vs 513K(non-strict) How does that compare to passthrough performance? One thing I'm not entirely clear about is what the realistic use-case for this is - even if invalidation were infinitely fast, enabling translation still typically has a fair impact on overall system performance in terms of latency, power, memory bandwidth, etc., so I can't help wonder what devices exist today for which performance is critical and robustness* is unimportant, yet have crippled addressing capabilities such that they can't just use passthrough. Robin. * I don't want to say "security" here, since I'm actually a lot less concerned about the theoretical malicious endpoint/wild write scenarios than the the much more straightforward malfunctioning device and/or buggy driver causing use-after-free style memory corruption. Also, I'm sick of the word "security"... Zhen Lei (6): iommu/arm-smmu-v3: fix the implementation of flush_iotlb_all hook iommu/dma: add support for non-strict mode iommu/amd: use default branch to deal with all non-supported capabilities iommu/io-pgtable-arm: add support for non-strict mode iommu/arm-smmu-v3: add support for non-strict mode iommu/arm-smmu-v3: add bootup option "iommu_strict_mode" Documentation/admin-guide/kernel-parameters.txt | 12 +++ drivers/iommu/amd_iommu.c | 4 +-- drivers/iommu/arm-smmu-v3.c | 42 +++-- drivers/iommu/dma-iommu.c | 25 +++ drivers/iommu/io-pgtable-arm.c | 23 -- include/linux/iommu.h | 7 + 6 files changed, 98 insertions(+), 15 deletions(-) ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use the generic dma-noncoherent code for sh V2
Ok, there is one more issue with this version. Wait for a new one tomorrow. On Tue, Jul 24, 2018 at 02:01:42PM +0200, Christoph Hellwig wrote: > Hi all, > > can you review these patches to switch sh to use the generic > dma-noncoherent code? All the requirements are in mainline already > and we've switched various architectures over to it already. > > Changes since V1: > - fixed two stupid compile errors and verified them using a local >cross toolchain instead of the 0day buildbot > ___ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu ---end quoted text--- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 03/10] iommu/vt-d: Allocate groups for mediated devices
On Tue, 24 Jul 2018 12:30:35 +0100 Jean-Philippe Brucker wrote: > Hi Baolu, > > On 24/07/18 03:22, Lu Baolu wrote: > > Hi, > > > > On 07/23/2018 12:44 PM, Liu, Yi L wrote: > >>> From: Lu Baolu [mailto:baolu...@linux.intel.com] > >>> Sent: Sunday, July 22, 2018 2:09 PM > >>> > >>> With the Intel IOMMU supporting PASID granularity isolation and > >>> protection, a > >>> mediated device could be isolated and protected by an IOMMU unit. We need > >>> to > >>> allocate a new group instead of a PCI group. > >> Existing vfio mdev framework also allocates an iommu group for mediate > >> device. > >> > >> mdev_probe() > >> |_ mdev_attach_iommu() > >>|_ iommu_group_alloc() > > > > When external components ask iommu to allocate a group for a device, > > it will call pci_device_group in Intel IOMMU driver's @device_group > > callback. In another word, current Intel IOMMU driver doesn't aware > > the mediated device and treat all devices as PCI ones. This patch > > extends the @device_group call back to make it aware of a mediated > > device. > > I agree that allocating two groups for an mdev seems strange, and in my > opinion we shouldn't export the notion of mdev to IOMMU drivers. Yep, I was just thinking the same thing. This is too highly integrated into VT-d and too narrowly focused on PASID being the only way that an mdev could make use of the IOMMU. Thanks, Alex ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 01/10] iommu/vt-d: Get iommu device for a mediated device
On Sun, 22 Jul 2018 14:09:24 +0800
Lu Baolu wrote:
> This patch adds the support to get the iommu device for a mediated
> device. The assumption is that each mediated device is a minimal
> assignable set of a physical PCI device. Hence, we should use the
> iommu device of the parent PCI device to manage the translation.
Hmm, is this absolutely a valid assumption? I'm afraid there's an
assumption throughout this series that the only way an mdev device
could be interacting with the IOMMU is via S-IOV, but we could choose
today to make an mdev wrapper for any device, which might provide basic
RID granularity to the IOMMU. So if I make an mdev wrapper for a PF
such that I can implement migration for that device, is it valid for
the IOMMU driver to flag me as an mdev device and base mappings on my
parent device? Perhaps in this patch we can assume that the parent of
such an mdev device would be the PF backing it and that results in the
correct drhd, but in the next patch we start imposing the assumption
that because the device is an mdev, the only valid association is via
pasid, which I'd say is incorrect.
> Cc: Ashok Raj
> Cc: Jacob Pan
> Cc: Kevin Tian
> Cc: Liu Yi L
> Signed-off-by: Sanjay Kumar
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel-iommu.c | 6 ++
> drivers/iommu/intel-pasid.h | 16
> 2 files changed, 22 insertions(+)
>
> diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
> index 7d198ea..fc3ac1c 100644
> --- a/drivers/iommu/intel-iommu.c
> +++ b/drivers/iommu/intel-iommu.c
> @@ -767,6 +767,12 @@ static struct intel_iommu *device_to_iommu(struct device
> *dev, u8 *bus, u8 *devf
> if (iommu_dummy(dev))
> return NULL;
>
> + if (dev_is_mdev(dev)) {
> + dev = dev_mdev_parent(dev);
> + if (WARN_ON(!dev_is_pci(dev)))
> + return NULL;
> + }
> +
> if (dev_is_pci(dev)) {
> struct pci_dev *pf_pdev;
>
> diff --git a/drivers/iommu/intel-pasid.h b/drivers/iommu/intel-pasid.h
> index 518df72..46cde66 100644
> --- a/drivers/iommu/intel-pasid.h
> +++ b/drivers/iommu/intel-pasid.h
> @@ -35,6 +35,22 @@ struct pasid_table {
> struct list_headdev;/* device list */
> };
>
> +static inline bool dev_is_mdev(struct device *dev)
> +{
> + if (!dev)
> + return false;
> +
> + return !strcmp(dev->bus->name, "mdev");
> +}
I assume we're not using mdev_bus_type because mdev is a loadable
module and that symbol doesn't exist in this statically loaded driver,
but strcmp is a pretty ugly alternative. Could we use symbol_get() so
that we can use mdev_bus_type? Thanks,
Alex
> +
> +static inline struct device *dev_mdev_parent(struct device *dev)
> +{
> + if (WARN_ON(!dev_is_mdev(dev)))
> + return NULL;
> +
> + return dev->parent;
> +}
> +
> extern u32 intel_pasid_max_id;
> int intel_pasid_alloc_id(void *ptr, int start, int end, gfp_t gfp);
> void intel_pasid_free_id(int pasid);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v13 1/4] iommu/arm-smmu: Add pm_runtime/sleep ops
On 19/07/18 11:15, Vivek Gautam wrote:
From: Sricharan R
The smmu needs to be functional only when the respective
master's using it are active. The device_link feature
helps to track such functional dependencies, so that the
iommu gets powered when the master device enables itself
using pm_runtime. So by adapting the smmu driver for
runtime pm, above said dependency can be addressed.
This patch adds the pm runtime/sleep callbacks to the
driver and also the functions to parse the smmu clocks
from DT and enable them in resume/suspend.
Also, while we enable the runtime pm add a pm sleep suspend
callback that pushes devices to low power state by turning
the clocks off in a system sleep.
Also add corresponding clock enable path in resume callback.
Signed-off-by: Sricharan R
Signed-off-by: Archit Taneja
[vivek: rework for clock and pm ops]
Signed-off-by: Vivek Gautam
Reviewed-by: Tomasz Figa
---
Changes since v12:
- Added pm sleep .suspend callback. This disables the clocks.
- Added corresponding change to enable clocks in .resume
pm sleep callback.
drivers/iommu/arm-smmu.c | 75 ++--
1 file changed, 73 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index c73cfce1ccc0..9138a6fffe04 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -48,6 +48,7 @@
#include
#include
#include
+#include
#include
#include
@@ -205,6 +206,8 @@ struct arm_smmu_device {
u32 num_global_irqs;
u32 num_context_irqs;
unsigned int*irqs;
+ struct clk_bulk_data*clks;
+ int num_clks;
u32cavium_id_base; /* Specific to Cavium */
@@ -1897,10 +1900,12 @@ static int arm_smmu_device_cfg_probe(struct arm_smmu_device *smmu)
struct arm_smmu_match_data {
enum arm_smmu_arch_version version;
enum arm_smmu_implementation model;
+ const char * const *clks;
+ int num_clks;
};
#define ARM_SMMU_MATCH_DATA(name, ver, imp) \
-static struct arm_smmu_match_data name = { .version = ver, .model = imp }
+static const struct arm_smmu_match_data name = { .version = ver, .model = imp }
ARM_SMMU_MATCH_DATA(smmu_generic_v1, ARM_SMMU_V1, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2, GENERIC_SMMU);
@@ -1919,6 +1924,23 @@ static const struct of_device_id arm_smmu_of_match[] = {
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
+static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
+ const char * const *clks)
+{
+ int i;
+
+ if (smmu->num_clks < 1)
+ return;
+
+ smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
+ sizeof(*smmu->clks), GFP_KERNEL);
+ if (!smmu->clks)
+ return;
+
+ for (i = 0; i < smmu->num_clks; i++)
+ smmu->clks[i].id = clks[i];
+}
+
#ifdef CONFIG_ACPI
static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
{
@@ -2001,6 +2023,9 @@ static int arm_smmu_device_dt_probe(struct
platform_device *pdev,
data = of_device_get_match_data(dev);
smmu->version = data->version;
smmu->model = data->model;
+ smmu->num_clks = data->num_clks;
+
+ arm_smmu_fill_clk_data(smmu, data->clks);
parse_driver_options(smmu);
@@ -2099,6 +2124,14 @@ static int arm_smmu_device_probe(struct platform_device *pdev)
smmu->irqs[i] = irq;
}
+ err = devm_clk_bulk_get(smmu->dev, smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
+ err = clk_bulk_prepare(smmu->num_clks, smmu->clks);
+ if (err)
+ return err;
+
err = arm_smmu_device_cfg_probe(smmu);
if (err)
return err;
@@ -2181,6 +2214,9 @@ static int arm_smmu_device_remove(struct platform_device
*pdev)
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
+
+ clk_bulk_unprepare(smmu->num_clks, smmu->clks);
+
return 0;
}
@@ -2189,15 +2225,50 @@ static void arm_smmu_device_shutdown(struct platform_device *pdev)
arm_smmu_device_remove(pdev);
}
+static int __maybe_unused arm_smmu_runtime_resume(struct device *dev)
+{
+ struct arm_smmu_device *smmu = dev_get_drvdata(dev);
+
+ return clk_bulk_enable(smmu->num_clks, smmu->clks);
If there's a power domain being automatically switched by genpd then we
need a reset here because we may have lost state entirely. Since I
remembered the otherwise-useless GPU SMMU on Juno is in a separate power
domain, I gave it a poking via sysfs with some debug stuff to dump sCR0
in these callbacks, and the problem is clear:
...
[4.625551] arm-smmu 2b40.iommu: genpd_runtime_suspend()
[4.631163] arm-smmu
RE: [PATCH 4/5] sh: split arch/sh/mm/consistent.c
> Subject: Re: [PATCH 4/5] sh: split arch/sh/mm/consistent.c
>
> On Mon, Jul 23, 2018 at 10:49:39AM +0200, Geert Uytterhoeven wrote:
> > > > + *dma_handle = virt_to_phys(ret);
> > > > + if (!WARN_ON(!dev))
> > > > + *dma_handle - PFN_PHYS(dev->dma_pfn_offset);
> > > vs
> > > > - *dma_handle = virt_to_phys(ret);
> > > > - if (!WARN_ON(!dev))
> > > > - *dma_handle -= PFN_PHYS(dev->dma_pfn_offset);
> >
> > Doesn't look right to me, neither.
> >
> > No complaints for 0day? My gcc says:
> >
> > error: statement with no effect [-Werror=unused-value]
>
> For some reason 0day has failed me multiple times for sh. Does the
> 0day bot even cover sh at all?
yes, we do cover the sh arch. Sorry for missing the reports for sh arch.
After check, for unknown reason, certain commit leads to large build error and
makes
disc full, so that other sh builds are impacted (blocked). I will follow up
this to see
how to solve the issue.
kernel/sh-allnoconfig/gcc-7# find -mindepth 1 -maxdepth 1|xargs -i du -sh {}
30G ./f41ccf64b487381388c2b5ef8c13d79509dde76e
kernel/sh-rsk7269_defconfig/gcc-7# find -mindepth 1 -maxdepth 1|xargs -i du -sh
{}
32G ./f41ccf64b487381388c2b5ef8c13d79509dde76e
kernel/sh-rsk7269_defconfig/gcc-7/f41ccf64b487381388c2b5ef8c13d79509dde76e/build-error#
ls
make-oldnoconfig
make-prepare
make-prepare.0
make-prepare.1
make-prepare.10
make-prepare.100
make-prepare.1000
Thanks
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/5] sh: use generic dma_noncoherent_ops
Switch to the generic noncoherent direct mapping implementation.
Signed-off-by: Christoph Hellwig
---
arch/sh/Kconfig | 3 +-
arch/sh/include/asm/Kbuild| 1 +
arch/sh/include/asm/dma-mapping.h | 26 ---
arch/sh/kernel/Makefile | 2 +-
arch/sh/kernel/dma-coherent.c | 23 +
arch/sh/kernel/dma-nommu.c| 78 ---
6 files changed, 15 insertions(+), 118 deletions(-)
delete mode 100644 arch/sh/include/asm/dma-mapping.h
delete mode 100644 arch/sh/kernel/dma-nommu.c
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index c9993a0cdc7e..da4db4b5359f 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -51,7 +51,6 @@ config SUPERH
select HAVE_ARCH_AUDITSYSCALL
select HAVE_FUTEX_CMPXCHG if FUTEX
select HAVE_NMI
- select NEED_DMA_MAP_STATE
select NEED_SG_DMA_LENGTH
help
@@ -164,6 +163,8 @@ config DMA_COHERENT
config DMA_NONCOHERENT
def_bool !DMA_COHERENT
+ select ARCH_HAS_SYNC_DMA_FOR_DEVICE
+ select DMA_NONCOHERENT_OPS
config PGTABLE_LEVELS
default 3 if X2TLB
diff --git a/arch/sh/include/asm/Kbuild b/arch/sh/include/asm/Kbuild
index 46dd82ab2c29..6a5609a55965 100644
--- a/arch/sh/include/asm/Kbuild
+++ b/arch/sh/include/asm/Kbuild
@@ -2,6 +2,7 @@ generic-y += compat.h
generic-y += current.h
generic-y += delay.h
generic-y += div64.h
+generic-y += dma-mapping.h
generic-y += emergency-restart.h
generic-y += exec.h
generic-y += irq_regs.h
diff --git a/arch/sh/include/asm/dma-mapping.h
b/arch/sh/include/asm/dma-mapping.h
deleted file mode 100644
index 1ebc6a4eb1c5..
--- a/arch/sh/include/asm/dma-mapping.h
+++ /dev/null
@@ -1,26 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-#ifndef __ASM_SH_DMA_MAPPING_H
-#define __ASM_SH_DMA_MAPPING_H
-
-extern const struct dma_map_ops nommu_dma_ops;
-
-static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
-{
-#ifdef CONFIG_DMA_NONCOHERENT
- return _dma_ops;
-#else
- return _direct_ops;
-#endif
-}
-
-extern void *dma_generic_alloc_coherent(struct device *dev, size_t size,
- dma_addr_t *dma_addr, gfp_t flag,
- unsigned long attrs);
-extern void dma_generic_free_coherent(struct device *dev, size_t size,
- void *vaddr, dma_addr_t dma_handle,
- unsigned long attrs);
-
-void sh_sync_dma_for_device(void *vaddr, size_t size,
- enum dma_data_direction dir);
-
-#endif /* __ASM_SH_DMA_MAPPING_H */
diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
index d5ddb64bfffe..59673f8a3379 100644
--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -45,7 +45,7 @@ obj-$(CONFIG_DUMP_CODE) += disassemble.o
obj-$(CONFIG_HIBERNATION) += swsusp.o
obj-$(CONFIG_DWARF_UNWINDER) += dwarf.o
obj-$(CONFIG_PERF_EVENTS) += perf_event.o perf_callchain.o
-obj-$(CONFIG_DMA_NONCOHERENT) += dma-nommu.o dma-coherent.o
+obj-$(CONFIG_DMA_NONCOHERENT) += dma-coherent.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
ccflags-y := -Werror
diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
index 763ba10fbd3e..9199b5523f7c 100644
--- a/arch/sh/kernel/dma-coherent.c
+++ b/arch/sh/kernel/dma-coherent.c
@@ -7,14 +7,13 @@
*/
#include
#include
-#include
+#include
#include
#include
#include
-void *dma_generic_alloc_coherent(struct device *dev, size_t size,
-dma_addr_t *dma_handle, gfp_t gfp,
-unsigned long attrs)
+void *arch_dma_alloc(struct device *dev, size_t size, dma_addr_t *dma_handle,
+ gfp_t gfp, unsigned long attrs)
{
void *ret, *ret_nocache;
int order = get_order(size);
@@ -29,7 +28,8 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t
size,
* Pages from the page allocator may have data present in
* cache. So flush the cache before using uncached memory.
*/
- sh_sync_dma_for_device(ret, size, DMA_BIDIRECTIONAL);
+ arch_sync_dma_for_device(dev, virt_to_phys(ret), size,
+ DMA_BIDIRECTIONAL);
ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
if (!ret_nocache) {
@@ -46,9 +46,8 @@ void *dma_generic_alloc_coherent(struct device *dev, size_t
size,
return ret_nocache;
}
-void dma_generic_free_coherent(struct device *dev, size_t size,
- void *vaddr, dma_addr_t dma_handle,
- unsigned long attrs)
+void arch_dma_free(struct device *dev, size_t size, void *vaddr,
+ dma_addr_t dma_handle, unsigned long attrs)
{
int order = get_order(size);
unsigned long pfn = (dma_handle >> PAGE_SHIFT);
@@ -63,12 +62,12 @@ void dma_generic_free_coherent(struct
[PATCH 4/5] sh: split arch/sh/mm/consistent.c
Half of the file just contains platform device memory setup code which
is required for all builds, and half contains helpers for dma coherent
allocation, which is only needed if CONFIG_DMA_NONCOHERENT is enabled.
Signed-off-by: Christoph Hellwig
---
arch/sh/kernel/Makefile | 2 +-
arch/sh/kernel/dma-coherent.c | 85 +++
arch/sh/mm/consistent.c | 80 -
3 files changed, 86 insertions(+), 81 deletions(-)
create mode 100644 arch/sh/kernel/dma-coherent.c
diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
index cb5f1bfb52de..d5ddb64bfffe 100644
--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -45,7 +45,7 @@ obj-$(CONFIG_DUMP_CODE) += disassemble.o
obj-$(CONFIG_HIBERNATION) += swsusp.o
obj-$(CONFIG_DWARF_UNWINDER) += dwarf.o
obj-$(CONFIG_PERF_EVENTS) += perf_event.o perf_callchain.o
-obj-$(CONFIG_DMA_NONCOHERENT) += dma-nommu.o
+obj-$(CONFIG_DMA_NONCOHERENT) += dma-nommu.o dma-coherent.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
ccflags-y := -Werror
diff --git a/arch/sh/kernel/dma-coherent.c b/arch/sh/kernel/dma-coherent.c
new file mode 100644
index ..763ba10fbd3e
--- /dev/null
+++ b/arch/sh/kernel/dma-coherent.c
@@ -0,0 +1,85 @@
+/*
+ * Copyright (C) 2004 - 2007 Paul Mundt
+ *
+ * This file is subject to the terms and conditions of the GNU General Public
+ * License. See the file "COPYING" in the main directory of this archive
+ * for more details.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+void *dma_generic_alloc_coherent(struct device *dev, size_t size,
+dma_addr_t *dma_handle, gfp_t gfp,
+unsigned long attrs)
+{
+ void *ret, *ret_nocache;
+ int order = get_order(size);
+
+ gfp |= __GFP_ZERO;
+
+ ret = (void *)__get_free_pages(gfp, order);
+ if (!ret)
+ return NULL;
+
+ /*
+* Pages from the page allocator may have data present in
+* cache. So flush the cache before using uncached memory.
+*/
+ sh_sync_dma_for_device(ret, size, DMA_BIDIRECTIONAL);
+
+ ret_nocache = (void __force *)ioremap_nocache(virt_to_phys(ret), size);
+ if (!ret_nocache) {
+ free_pages((unsigned long)ret, order);
+ return NULL;
+ }
+
+ split_page(pfn_to_page(virt_to_phys(ret) >> PAGE_SHIFT), order);
+
+ *dma_handle = virt_to_phys(ret);
+ if (!WARN_ON(!dev))
+ *dma_handle -= PFN_PHYS(dev->dma_pfn_offset);
+
+ return ret_nocache;
+}
+
+void dma_generic_free_coherent(struct device *dev, size_t size,
+ void *vaddr, dma_addr_t dma_handle,
+ unsigned long attrs)
+{
+ int order = get_order(size);
+ unsigned long pfn = (dma_handle >> PAGE_SHIFT);
+ int k;
+
+ if (!WARN_ON(!dev))
+ pfn += dev->dma_pfn_offset;
+
+ for (k = 0; k < (1 << order); k++)
+ __free_pages(pfn_to_page(pfn + k), 0);
+
+ iounmap(vaddr);
+}
+
+void sh_sync_dma_for_device(void *vaddr, size_t size,
+ enum dma_data_direction direction)
+{
+ void *addr = sh_cacheop_vaddr(vaddr);
+
+ switch (direction) {
+ case DMA_FROM_DEVICE: /* invalidate only */
+ __flush_invalidate_region(addr, size);
+ break;
+ case DMA_TO_DEVICE: /* writeback only */
+ __flush_wback_region(addr, size);
+ break;
+ case DMA_BIDIRECTIONAL: /* writeback and invalidate */
+ __flush_purge_region(addr, size);
+ break;
+ default:
+ BUG();
+ }
+}
+EXPORT_SYMBOL(sh_sync_dma_for_device);
diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
index 1622ae6b9dbd..792f36129062 100644
--- a/arch/sh/mm/consistent.c
+++ b/arch/sh/mm/consistent.c
@@ -1,10 +1,6 @@
/*
- * arch/sh/mm/consistent.c
- *
* Copyright (C) 2004 - 2007 Paul Mundt
*
- * Declared coherent memory functions based on arch/x86/kernel/pci-dma_32.c
- *
* This file is subject to the terms and conditions of the GNU General Public
* License. See the file "COPYING" in the main directory of this archive
* for more details.
@@ -13,83 +9,7 @@
#include
#include
#include
-#include
#include
-#include
-#include
-#include
-#include
-
-void *dma_generic_alloc_coherent(struct device *dev, size_t size,
-dma_addr_t *dma_handle, gfp_t gfp,
-unsigned long attrs)
-{
- void *ret, *ret_nocache;
- int order = get_order(size);
-
- gfp |= __GFP_ZERO;
-
- ret = (void *)__get_free_pages(gfp, order);
- if (!ret)
- return NULL;
-
- /*
-* Pages from the page allocator may have data present in
-*
[PATCH 3/5] sh: use dma_direct_ops for the CONFIG_DMA_COHERENT case
This is a slight change in behavior as we avoid the detour through the
virtual mapping for the coherent allocator, but if this CPU really is
coherent that should be the right thing to do.
Signed-off-by: Christoph Hellwig
---
arch/sh/Kconfig | 1 +
arch/sh/include/asm/dma-mapping.h | 4
arch/sh/kernel/Makefile | 4 ++--
arch/sh/kernel/dma-nommu.c| 4
4 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/arch/sh/Kconfig b/arch/sh/Kconfig
index dd4f3d3e644f..c9993a0cdc7e 100644
--- a/arch/sh/Kconfig
+++ b/arch/sh/Kconfig
@@ -159,6 +159,7 @@ config SWAP_IO_SPACE
bool
config DMA_COHERENT
+ select DMA_DIRECT_OPS
bool
config DMA_NONCOHERENT
diff --git a/arch/sh/include/asm/dma-mapping.h
b/arch/sh/include/asm/dma-mapping.h
index 149e71f95be7..1ebc6a4eb1c5 100644
--- a/arch/sh/include/asm/dma-mapping.h
+++ b/arch/sh/include/asm/dma-mapping.h
@@ -6,7 +6,11 @@ extern const struct dma_map_ops nommu_dma_ops;
static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
{
+#ifdef CONFIG_DMA_NONCOHERENT
return _dma_ops;
+#else
+ return _direct_ops;
+#endif
}
extern void *dma_generic_alloc_coherent(struct device *dev, size_t size,
diff --git a/arch/sh/kernel/Makefile b/arch/sh/kernel/Makefile
index dc80041f7363..cb5f1bfb52de 100644
--- a/arch/sh/kernel/Makefile
+++ b/arch/sh/kernel/Makefile
@@ -12,7 +12,7 @@ endif
CFLAGS_REMOVE_return_address.o = -pg
-obj-y := debugtraps.o dma-nommu.o dumpstack.o \
+obj-y := debugtraps.o dumpstack.o \
idle.o io.o irq.o irq_$(BITS).o kdebugfs.o \
machvec.o nmi_debug.o process.o \
process_$(BITS).o ptrace.o ptrace_$(BITS).o \
@@ -45,7 +45,7 @@ obj-$(CONFIG_DUMP_CODE) += disassemble.o
obj-$(CONFIG_HIBERNATION) += swsusp.o
obj-$(CONFIG_DWARF_UNWINDER) += dwarf.o
obj-$(CONFIG_PERF_EVENTS) += perf_event.o perf_callchain.o
-
+obj-$(CONFIG_DMA_NONCOHERENT) += dma-nommu.o
obj-$(CONFIG_HAVE_HW_BREAKPOINT) += hw_breakpoint.o
ccflags-y := -Werror
diff --git a/arch/sh/kernel/dma-nommu.c b/arch/sh/kernel/dma-nommu.c
index 79a9edafa5b0..d8689b1cb743 100644
--- a/arch/sh/kernel/dma-nommu.c
+++ b/arch/sh/kernel/dma-nommu.c
@@ -51,7 +51,6 @@ static int nommu_map_sg(struct device *dev, struct
scatterlist *sg,
return nents;
}
-#ifdef CONFIG_DMA_NONCOHERENT
static void nommu_sync_single_for_device(struct device *dev, dma_addr_t addr,
size_t size, enum dma_data_direction dir)
{
@@ -67,16 +66,13 @@ static void nommu_sync_sg_for_device(struct device *dev,
struct scatterlist *sg,
for_each_sg(sg, s, nelems, i)
sh_sync_dma_for_device(sg_virt(s), s->length, dir);
}
-#endif
const struct dma_map_ops nommu_dma_ops = {
.alloc = dma_generic_alloc_coherent,
.free = dma_generic_free_coherent,
.map_page = nommu_map_page,
.map_sg = nommu_map_sg,
-#ifdef CONFIG_DMA_NONCOHERENT
.sync_single_for_device = nommu_sync_single_for_device,
.sync_sg_for_device = nommu_sync_sg_for_device,
-#endif
};
EXPORT_SYMBOL(nommu_dma_ops);
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/5] sh: introduce a sh_cacheop_vaddr helper
And use it in the maple bus code to avoid a dma API dependency.
Signed-off-by: Christoph Hellwig
---
arch/sh/include/asm/cacheflush.h | 7 +++
arch/sh/mm/consistent.c | 6 +-
drivers/sh/maple/maple.c | 7 ---
3 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/arch/sh/include/asm/cacheflush.h b/arch/sh/include/asm/cacheflush.h
index d103ab5a4e4b..b932e42ef028 100644
--- a/arch/sh/include/asm/cacheflush.h
+++ b/arch/sh/include/asm/cacheflush.h
@@ -101,5 +101,12 @@ void kunmap_coherent(void *kvaddr);
void cpu_cache_init(void);
+static inline void *sh_cacheop_vaddr(void *vaddr)
+{
+ if (__in_29bit_mode())
+ vaddr = (void *)CAC_ADDR((unsigned long)vaddr);
+ return vaddr;
+}
+
#endif /* __KERNEL__ */
#endif /* __ASM_SH_CACHEFLUSH_H */
diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
index e9d422bd42a5..1622ae6b9dbd 100644
--- a/arch/sh/mm/consistent.c
+++ b/arch/sh/mm/consistent.c
@@ -74,10 +74,7 @@ void dma_generic_free_coherent(struct device *dev, size_t
size,
void sh_sync_dma_for_device(void *vaddr, size_t size,
enum dma_data_direction direction)
{
- void *addr;
-
- addr = __in_29bit_mode() ?
- (void *)CAC_ADDR((unsigned long)vaddr) : vaddr;
+ void *addr = sh_cacheop_vaddr(vaddr);
switch (direction) {
case DMA_FROM_DEVICE: /* invalidate only */
@@ -93,7 +90,6 @@ void sh_sync_dma_for_device(void *vaddr, size_t size,
BUG();
}
}
-EXPORT_SYMBOL(sh_sync_dma_for_device);
static int __init memchunk_setup(char *str)
{
diff --git a/drivers/sh/maple/maple.c b/drivers/sh/maple/maple.c
index 2e45988d1259..e5d7fb81ad66 100644
--- a/drivers/sh/maple/maple.c
+++ b/drivers/sh/maple/maple.c
@@ -300,8 +300,8 @@ static void maple_send(void)
mutex_unlock(_wlist_lock);
if (maple_packets > 0) {
for (i = 0; i < (1 << MAPLE_DMA_PAGES); i++)
- sh_sync_dma_for_device(maple_sendbuf + i * PAGE_SIZE,
- PAGE_SIZE, DMA_BIDIRECTIONAL);
+ __flush_purge_region(maple_sendbuf + i * PAGE_SIZE,
+ PAGE_SIZE);
}
finish:
@@ -642,7 +642,8 @@ static void maple_dma_handler(struct work_struct *work)
list_for_each_entry_safe(mq, nmq, _sentq, list) {
mdev = mq->dev;
recvbuf = mq->recvbuf->buf;
- sh_sync_dma_for_device(recvbuf, 0x400, DMA_FROM_DEVICE);
+ __flush_invalidate_region(sh_cacheop_vaddr(recvbuf),
+ 0x400);
code = recvbuf[0];
kfree(mq->sendbuf);
list_del_init(>list);
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/5] sh: simplify get_arch_dma_ops
Remove the indirection through the dma_ops variable, and just return
nommu_dma_ops directly from get_arch_dma_ops.
Signed-off-by: Christoph Hellwig
---
arch/sh/include/asm/dma-mapping.h | 5 ++---
arch/sh/kernel/dma-nommu.c| 8 +---
arch/sh/mm/consistent.c | 3 ---
arch/sh/mm/init.c | 10 --
4 files changed, 3 insertions(+), 23 deletions(-)
diff --git a/arch/sh/include/asm/dma-mapping.h
b/arch/sh/include/asm/dma-mapping.h
index 41167931e5d9..149e71f95be7 100644
--- a/arch/sh/include/asm/dma-mapping.h
+++ b/arch/sh/include/asm/dma-mapping.h
@@ -2,12 +2,11 @@
#ifndef __ASM_SH_DMA_MAPPING_H
#define __ASM_SH_DMA_MAPPING_H
-extern const struct dma_map_ops *dma_ops;
-extern void no_iommu_init(void);
+extern const struct dma_map_ops nommu_dma_ops;
static inline const struct dma_map_ops *get_arch_dma_ops(struct bus_type *bus)
{
- return dma_ops;
+ return _dma_ops;
}
extern void *dma_generic_alloc_coherent(struct device *dev, size_t size,
diff --git a/arch/sh/kernel/dma-nommu.c b/arch/sh/kernel/dma-nommu.c
index 3e3a32fc676e..79a9edafa5b0 100644
--- a/arch/sh/kernel/dma-nommu.c
+++ b/arch/sh/kernel/dma-nommu.c
@@ -79,10 +79,4 @@ const struct dma_map_ops nommu_dma_ops = {
.sync_sg_for_device = nommu_sync_sg_for_device,
#endif
};
-
-void __init no_iommu_init(void)
-{
- if (dma_ops)
- return;
- dma_ops = _dma_ops;
-}
+EXPORT_SYMBOL(nommu_dma_ops);
diff --git a/arch/sh/mm/consistent.c b/arch/sh/mm/consistent.c
index fceb2adfcac7..e9d422bd42a5 100644
--- a/arch/sh/mm/consistent.c
+++ b/arch/sh/mm/consistent.c
@@ -20,9 +20,6 @@
#include
#include
-const struct dma_map_ops *dma_ops;
-EXPORT_SYMBOL(dma_ops);
-
void *dma_generic_alloc_coherent(struct device *dev, size_t size,
dma_addr_t *dma_handle, gfp_t gfp,
unsigned long attrs)
diff --git a/arch/sh/mm/init.c b/arch/sh/mm/init.c
index 4034035fbede..7713c084d040 100644
--- a/arch/sh/mm/init.c
+++ b/arch/sh/mm/init.c
@@ -339,22 +339,12 @@ void __init paging_init(void)
free_area_init_nodes(max_zone_pfns);
}
-/*
- * Early initialization for any I/O MMUs we might have.
- */
-static void __init iommu_init(void)
-{
- no_iommu_init();
-}
-
unsigned int mem_init_done = 0;
void __init mem_init(void)
{
pg_data_t *pgdat;
- iommu_init();
-
high_memory = NULL;
for_each_online_pgdat(pgdat)
high_memory = max_t(void *, high_memory,
--
2.18.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
use the generic dma-noncoherent code for sh V2
Hi all, can you review these patches to switch sh to use the generic dma-noncoherent code? All the requirements are in mainline already and we've switched various architectures over to it already. Changes since V1: - fixed two stupid compile errors and verified them using a local cross toolchain instead of the 0day buildbot ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 4/5] sh: split arch/sh/mm/consistent.c
On Mon, Jul 23, 2018 at 10:49:39AM +0200, Geert Uytterhoeven wrote: > > > + *dma_handle = virt_to_phys(ret); > > > + if (!WARN_ON(!dev)) > > > + *dma_handle - PFN_PHYS(dev->dma_pfn_offset); > > vs > > > - *dma_handle = virt_to_phys(ret); > > > - if (!WARN_ON(!dev)) > > > - *dma_handle -= PFN_PHYS(dev->dma_pfn_offset); > > Doesn't look right to me, neither. > > No complaints for 0day? My gcc says: > > error: statement with no effect [-Werror=unused-value] For some reason 0day has failed me multiple times for sh. Does the 0day bot even cover sh at all? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC PATCH 03/10] iommu/vt-d: Allocate groups for mediated devices
Hi Baolu, On 24/07/18 03:22, Lu Baolu wrote: > Hi, > > On 07/23/2018 12:44 PM, Liu, Yi L wrote: >>> From: Lu Baolu [mailto:baolu...@linux.intel.com] >>> Sent: Sunday, July 22, 2018 2:09 PM >>> >>> With the Intel IOMMU supporting PASID granularity isolation and protection, >>> a >>> mediated device could be isolated and protected by an IOMMU unit. We need to >>> allocate a new group instead of a PCI group. >> Existing vfio mdev framework also allocates an iommu group for mediate >> device. >> >> mdev_probe() >> |_ mdev_attach_iommu() >>|_ iommu_group_alloc() > > When external components ask iommu to allocate a group for a device, > it will call pci_device_group in Intel IOMMU driver's @device_group > callback. In another word, current Intel IOMMU driver doesn't aware > the mediated device and treat all devices as PCI ones. This patch > extends the @device_group call back to make it aware of a mediated > device. I agree that allocating two groups for an mdev seems strange, and in my opinion we shouldn't export the notion of mdev to IOMMU drivers. Otherwise each driver will have to add its own "dev_is_mdev()" special cases, which will get messy in the long run. Besides, the macro is currently private, and to be exported it should be wrapped in symbol_get/put(mdev_bus_type). There is another way: as we're planning to add a generic pasid_alloc() function to the IOMMU API, the mdev module itself could allocate a default PASID for each mdev by calling pasid_alloc() on the mdev's parent, and then do map()/unmap() with that PASID. This way we don't have to add IOMMU ops to the mdev bus, everything can still be done using the ops of the parent. And IOMMU drivers "only" have to implement PASID ops, which will be reused by drivers other than mdev. The allocated PASID also needs to be installed into the parent device. If the mdev module knows the PASID, we can do that by adding set_pasid(mdev, pasid) and clear_pasid(mdev, pasid) operations to mdev_parent_ops. Thanks, Jean ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/1] iommu/arm-smmu: Add support to use Last level cache
Hi Will, On Wed, Jun 27, 2018 at 10:07 PM, Will Deacon wrote: > Hi Vivek, > > On Tue, Jun 19, 2018 at 02:04:44PM +0530, Vivek Gautam wrote: >> On Fri, Jun 15, 2018 at 10:22 PM, Will Deacon wrote: >> > On Fri, Jun 15, 2018 at 04:23:29PM +0530, Vivek Gautam wrote: >> >> Qualcomm SoCs have an additional level of cache called as >> >> System cache or Last level cache[1]. This cache sits right >> >> before the DDR, and is tightly coupled with the memory >> >> controller. >> >> The cache is available to all the clients present in the >> >> SoC system. The clients request their slices from this system >> >> cache, make it active, and can then start using it. For these >> >> clients with smmu, to start using the system cache for >> >> dma buffers and related page tables [2], few of the memory >> >> attributes need to be set accordingly. >> >> This change makes the related memory Outer-Shareable, and >> >> updates the MAIR with necessary protection. >> >> >> >> The MAIR attribute requirements are: >> >> Inner Cacheablity = 0 >> >> Outer Cacheablity = 1, Write-Back Write Allocate >> >> Outer Shareablity = 1 >> > >> > Hmm, so is this cache coherent with the CPU or not? >> >> Thanks for reviewing. >> Yes, this LLC is cache coherent with CPU, so we mark for Outer-cacheable. >> The different masters such as GPU as able to allocated and activate a slice >> in this Last Level Cache. > > What I mean is, for example, if the CPU writes some data using Normal, Inner > Shareable, Inner/Outer Cacheable, Inner/Outer Write-back, Non-transient > Read/Write-allocate and a device reads that data using your MAIR encoding > above, is the device guaranteed to see the CPU writes after the CPU has > executed a DSB instruction? > > I don't think so, because the ARM ARM would say that there's a mismatch on > the Inner Cacheability attribute. > >> > Why don't normal >> > non-cacheable mappings allocated in the LLC by default? >> >> Sorry, I couldn't fully understand your question here. >> Few of the masters on qcom socs are not io-coherent, so for them >> the IC has to be marked as 0. > > By IC you mean Inner Cacheability? In your MAIR encoding above, it is zero > so I don't understand the problem. What goes wrong if non-coherent devices > use your MAIR encoding for their DMA buffers? > >> But they are able to use the LLC with OC marked as 1. > > The issue here is that whatever attributes we put in the SMMU need to align > with the attributes used by the CPU in order to avoid introducing mismatched > aliases. Currently, we support three types of mapping in the SMMU: > > 1. DMA non-coherent (e.g. "dma-coherent" is not set on the device) > Normal, Inner Shareable, Inner/Outer Non-Cacheable > > 2. DMA coherent (e.g. "dma-coherent" is set on the device) [IOMMU_CACHE] > Normal, Inner Shareable, Inner/Outer Cacheable, Inner/Outer > Write-back, Non-transient Read/Write-allocate > > 3. MMIO (e.g. MSI doorbell) [IOMMU_MMIO] > Device-nGnRE (Outer Shareable) > > So either you override one of these types (I was suggesting (1)) or you need > to create a new memory type, along with the infrastructure for it to be > recognised on a per-device basis and used by the DMA API so that we don't > get mismatched aliases on the CPU. My apologies for delay in responding to this thread. I have been digging and getting in touch with internal tech teams to get more information on this. I will update as soon as I have enough details. Thanks. Best regards Vivek > > Will > ___ > iommu mailing list > iommu@lists.linux-foundation.org > https://lists.linuxfoundation.org/mailman/listinfo/iommu -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/3] iommu/arm-smmu: Error out only if not enough context interrupts
Hi Will,
On 7/24/2018 2:06 PM, Will Deacon wrote:
On Thu, Jul 19, 2018 at 11:23:56PM +0530, Vivek Gautam wrote:
Currently we check if the number of context banks is not equal to
num_context_interrupts. However, there are booloaders such as, one
on sdm845 that reserves few context banks and thus kernel views
less than the total available context banks.
So, although the hardware definition in device tree would mention
the correct number of context interrupts, this number can be
greater than the number of context banks visible to smmu in kernel.
We should therefore error out only when the number of context banks
is greater than the available number of context interrupts.
Signed-off-by: Vivek Gautam
Suggested-by: Tomasz Figa
Cc: Robin Murphy
Cc: Will Deacon
---
drivers/iommu/arm-smmu.c | 19 +--
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 7c69736a30f8..4cb53bf4f423 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2229,12 +2229,19 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
if (err)
return err;
- if (smmu->version == ARM_SMMU_V2 &&
- smmu->num_context_banks != smmu->num_context_irqs) {
- dev_err(dev,
- "found only %d context interrupt(s) but %d required\n",
- smmu->num_context_irqs, smmu->num_context_banks);
- return -ENODEV;
+ if (smmu->version == ARM_SMMU_V2) {
+ if (smmu->num_context_banks > smmu->num_context_irqs) {
+ dev_err(dev,
+ "found only %d context irq(s) but %d required\n",
+ smmu->num_context_irqs, smmu->num_context_banks);
+ return -ENODEV;
+ } else if (smmu->num_context_banks < smmu->num_context_irqs) {
+ /* loose extra context interrupts */
+ dev_notice(dev,
+ "found %d context irq(s) but only %d required\n",
+ smmu->num_context_irqs, smmu->num_context_banks);
+ smmu->num_context_irqs = smmu->num_context_banks;
+ }
I don't see the utility in the new message. Can you simplify with the patch
below on top? It's a bit weird that we only decide to ignore the extra irqs
after calling platform_get_irq() on them, but that seems to be harmless.
Thanks. I will modify as suggested below and respin.
Best regards
Vivek
Will
--->8
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index aa46c1ed5bf9..5349e22b5c78 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2109,13 +2109,10 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
"found only %d context irq(s) but %d required\n",
smmu->num_context_irqs, smmu->num_context_banks);
return -ENODEV;
- } else if (smmu->num_context_banks < smmu->num_context_irqs) {
- /* loose extra context interrupts */
- dev_notice(dev,
- "found %d context irq(s) but only %d required\n",
- smmu->num_context_irqs, smmu->num_context_banks);
- smmu->num_context_irqs = smmu->num_context_banks;
}
+
+ /* Ignore superfluous interrupts */
+ smmu->num_context_irqs = smmu->num_context_banks;
}
for (i = 0; i < smmu->num_global_irqs; ++i) {
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/3] iommu/arm-smmu: Error out only if not enough context interrupts
On Thu, Jul 19, 2018 at 11:23:56PM +0530, Vivek Gautam wrote:
> Currently we check if the number of context banks is not equal to
> num_context_interrupts. However, there are booloaders such as, one
> on sdm845 that reserves few context banks and thus kernel views
> less than the total available context banks.
> So, although the hardware definition in device tree would mention
> the correct number of context interrupts, this number can be
> greater than the number of context banks visible to smmu in kernel.
> We should therefore error out only when the number of context banks
> is greater than the available number of context interrupts.
>
> Signed-off-by: Vivek Gautam
> Suggested-by: Tomasz Figa
> Cc: Robin Murphy
> Cc: Will Deacon
> ---
> drivers/iommu/arm-smmu.c | 19 +--
> 1 file changed, 13 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
> index 7c69736a30f8..4cb53bf4f423 100644
> --- a/drivers/iommu/arm-smmu.c
> +++ b/drivers/iommu/arm-smmu.c
> @@ -2229,12 +2229,19 @@ static int arm_smmu_device_probe(struct
> platform_device *pdev)
> if (err)
> return err;
>
> - if (smmu->version == ARM_SMMU_V2 &&
> - smmu->num_context_banks != smmu->num_context_irqs) {
> - dev_err(dev,
> - "found only %d context interrupt(s) but %d required\n",
> - smmu->num_context_irqs, smmu->num_context_banks);
> - return -ENODEV;
> + if (smmu->version == ARM_SMMU_V2) {
> + if (smmu->num_context_banks > smmu->num_context_irqs) {
> + dev_err(dev,
> + "found only %d context irq(s) but %d required\n",
> + smmu->num_context_irqs, smmu->num_context_banks);
> + return -ENODEV;
> + } else if (smmu->num_context_banks < smmu->num_context_irqs) {
> + /* loose extra context interrupts */
> + dev_notice(dev,
> + "found %d context irq(s) but only %d required\n",
> + smmu->num_context_irqs, smmu->num_context_banks);
> + smmu->num_context_irqs = smmu->num_context_banks;
> + }
I don't see the utility in the new message. Can you simplify with the patch
below on top? It's a bit weird that we only decide to ignore the extra irqs
after calling platform_get_irq() on them, but that seems to be harmless.
Will
--->8
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index aa46c1ed5bf9..5349e22b5c78 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -2109,13 +2109,10 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
"found only %d context irq(s) but %d required\n",
smmu->num_context_irqs, smmu->num_context_banks);
return -ENODEV;
- } else if (smmu->num_context_banks < smmu->num_context_irqs) {
- /* loose extra context interrupts */
- dev_notice(dev,
- "found %d context irq(s) but only %d required\n",
- smmu->num_context_irqs, smmu->num_context_banks);
- smmu->num_context_irqs = smmu->num_context_banks;
}
+
+ /* Ignore superfluous interrupts */
+ smmu->num_context_irqs = smmu->num_context_banks;
}
for (i = 0; i < smmu->num_global_irqs; ++i) {
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
