Re: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
On Mon, Jan 28, 2019 at 12:14:33PM -0500, Michael S. Tsirkin wrote: > On Mon, Jan 28, 2019 at 09:05:26AM +0100, Christoph Hellwig wrote: > > On Thu, Jan 24, 2019 at 10:51:51AM +0100, Joerg Roedel wrote: > > > On Thu, Jan 24, 2019 at 09:42:21AM +0100, Christoph Hellwig wrote: > > > > Yes. But more importantly it would fix the limit for all other block > > > > drivers that set large segment sizes when running over swiotlb. > > > > > > True, so it would be something like the diff below? I havn't worked on > > > the block layer, so I don't know if that needs additional checks for > > > ->dev or anything. > > > > Looks sensible. Maybe for now we'll just do the virtio-blk case > > that triggered it, and we'll do something like this patch for the > > next merge window. We'll also need to apply the same magic to the > > DMA boundary. > > So do I get an ack for this patch then? I'll wait for a resend of the series to review the whole thing. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/vt-d: Implement dma_[un]map_resource()
Looks good: Reviewed-by: Christoph Hellwig ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/amd: print reason for iommu_map_page failure in map_sg
Since there are multiple possible failures in iommu_map_page
it would be useful to know which case is being hit when the
error message is printed in map_sg. While here, fix up checkpatch
complaint about using function name in a string instead of
__func__.
Cc: Joerg Roedel
Signed-off-by: Jerry Snitselaar
---
drivers/iommu/amd_iommu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 87ba23a75b38..675f7027aa04 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -2562,6 +2562,7 @@ static int map_sg(struct device *dev, struct scatterlist
*sglist,
struct scatterlist *s;
unsigned long address;
u64 dma_mask;
+ int ret;
domain = get_domain(dev);
if (IS_ERR(domain))
@@ -2584,7 +2585,6 @@ static int map_sg(struct device *dev, struct scatterlist
*sglist,
for (j = 0; j < pages; ++j) {
unsigned long bus_addr, phys_addr;
- int ret;
bus_addr = address + s->dma_address + (j <<
PAGE_SHIFT);
phys_addr = (sg_phys(s) & PAGE_MASK) + (j <<
PAGE_SHIFT);
@@ -2605,8 +2605,8 @@ static int map_sg(struct device *dev, struct scatterlist
*sglist,
return nelems;
out_unmap:
- pr_err("%s: IOMMU mapping error in map_sg (io-pages: %d)\n",
- dev_name(dev), npages);
+ pr_err("%s: IOMMU mapping error in %s (io-pages: %d) reason: %d\n",
+ dev_name(dev), __func__, npages, ret);
for_each_sg(sglist, s, nelems, i) {
int j, pages = iommu_num_pages(sg_phys(s), s->length,
PAGE_SIZE);
--
2.20.1.98.gecbdaf0899
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] dcdbas: Fix Intel-IOMMU domain allocation failure
On Fri, Jan 25, 2019 at 5:12 AM Robin Murphy wrote: > > On 25/01/2019 11:58, Andy Shevchenko wrote: > > On Fri, Jan 25, 2019 at 3:50 AM Szabolcs Fruhwald > > wrote: > > > > First of all, please do not top post! > > > >> I took a quick look at arch_setup_pdev_archdata(), overridden it in > >> dcdbas.c and it works well (despite it's being called twice, since > >> it's called from platform_device_alloc and platform_device_register). > >> > >> However, as I am not super familiar with ELF weak method references, > >> especially with its scope resolution / versioning part, so as I see > >> this weak method was introduced mainly for arch/** specific hooks. > >> Is it safe to override this method from driver code, when let's say > >> there's another implementation in the x86 arch code (currently there > >> isn't)? > > > > No, it should be done somewhere in arch/x86. > > > > OTOH, Intel iommu driver can do it based on the check dev_is_pci(). > > For now I think it's better to solve this inside Intel iommu driver. > > Indeed - hacking code into individual device drivers which is entirely > specific to the current implementation of the intel-iommu driver sounds > like a recipe for a maintenance disaster (not to mention the extra fun > if any of those devices also end up in AMD-based systems). > > AFAICS, the VT-d spec accommodates non-PCI devices via DRHD and > corresponding ANDD entries, and the driver already has at least some > degree of support for those - see dmar_acpi_insert_dev_scope() - so it > may not need much more than just hooking up to the platform bus. If > these platform devices do actually master through the IOMMU but don't > have an appropriate device scope described, then frankly that's broken > firmware, but the place to work around that would still be in the DMAR code. > > Robin. It does, however, unfortunately, AFAK this 'device' is not listed in any of those ACPI tables. This is not even a real device, it's a CMOS chip with a fixed mem segment and it needs dma only to let it read out the cmd data from a buffer. So it's not really possible to support proper iommu for this driver. Same is true for the previously mentioned drm915 mock test device. Either way, there are device drivers which for various reasons may need to opt-out / force 1:1 iommu mapping to work properly when iommu is turned on. I agree that the current way of handling this (archdata.iommu) in intel_iommu driver is not nice, a better solution might be to create a generalized way through iommu.h (eg a method or constant value stored in main dev / pdev struct), which is well documented to be used only for legacy drivers / special cases where forced 1:1 mapping is truly justified / last resort. And other arch iommu drivers can support this, if and until necessary (legacy drivers don't need anymore). ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFC v3 02/21] iommu: Introduce cache_invalidate API
Hi Eric,
On 25/01/2019 16:49, Auger Eric wrote:
[...]
>>> diff --git a/include/uapi/linux/iommu.h b/include/uapi/linux/iommu.h
>>> index 7a7cf7a3de7c..4605f5cfac84 100644
>>> --- a/include/uapi/linux/iommu.h
>>> +++ b/include/uapi/linux/iommu.h
>>> @@ -47,4 +47,99 @@ struct iommu_pasid_table_config {
>>> };
>>> };
>>>
>>> +/**
>>> + * enum iommu_inv_granularity - Generic invalidation granularity
>>> + * @IOMMU_INV_GRANU_DOMAIN_ALL_PASID: TLB entries or PASID caches of
>>> all
>>> + * PASIDs associated with a domain ID
>>> + * @IOMMU_INV_GRANU_PASID_SEL: TLB entries or PASID cache
>>> associated
>>> + * with a PASID and a domain
>>> + * @IOMMU_INV_GRANU_PAGE_PASID:TLB entries of selected page
>>> range
>>> + * within a PASID
>>> + *
>>> + * When an invalidation request is passed down to IOMMU to flush
>>> translation
>>> + * caches, it may carry different granularity levels, which can be specific
>>> + * to certain types of translation caches.
>>> + * This enum is a collection of granularities for all types of translation
>>> + * caches. The idea is to make it easy for IOMMU model specific driver to
>>> + * convert from generic to model specific value. Each IOMMU driver
>>> + * can enforce check based on its own conversion table. The conversion is
>>> + * based on 2D look-up with inputs as follows:
>>> + * - translation cache types
>>> + * - granularity
>>> + *
>>> + * type | DTLB|TLB| PASID |
>>> + * granule | | | cache |
>>> + * -+---+---+---+
>>> + * DN_ALL_PASID| Y | Y | Y |
>>> + * PASID_SEL | Y | Y | Y |
>>> + * PAGE_PASID | Y | Y | N/A |
>>> + *
>>> + */
>>> +enum iommu_inv_granularity {
>>> + IOMMU_INV_GRANU_DOMAIN_ALL_PASID,
>>> + IOMMU_INV_GRANU_PASID_SEL,
>>> + IOMMU_INV_GRANU_PAGE_PASID,
>>> + IOMMU_INV_NR_GRANU,
>>> +};
>>> +
>>> +/**
>>> + * enum iommu_inv_type - Generic translation cache types for invalidation
>>> + *
>>> + * @IOMMU_INV_TYPE_DTLB: device IOTLB
>>> + * @IOMMU_INV_TYPE_TLB:IOMMU paging structure cache
>>> + * @IOMMU_INV_TYPE_PASID: PASID cache
>>> + * Invalidation requests sent to IOMMU for a given device need to indicate
>>> + * which type of translation cache to be operated on. Combined with enum
>>> + * iommu_inv_granularity, model specific driver can do a simple lookup to
>>> + * convert from generic to model specific value.
>>> + */
>>> +enum iommu_inv_type {
>>> + IOMMU_INV_TYPE_DTLB,
>>> + IOMMU_INV_TYPE_TLB,
>>> + IOMMU_INV_TYPE_PASID,
>>> + IOMMU_INV_NR_TYPE
>>> +};
>>> +
>>> +/**
>>> + * Translation cache invalidation header that contains mandatory meta data.
>>> + * @version: info format version, expecting future extesions
>>> + * @type: type of translation cache to be invalidated
>>> + */
>>> +struct iommu_cache_invalidate_hdr {
>>> + __u32 version;
>>> +#define TLB_INV_HDR_VERSION_1 1
>>> + enum iommu_inv_type type;
>>> +};
>>> +
>>> +/**
>>> + * Translation cache invalidation information, contains generic IOMMU
>>> + * data which can be parsed based on model ID by model specific drivers.
>>> + * Since the invalidation of second level page tables are included in the
>>> + * unmap operation, this info is only applicable to the first level
>>> + * translation caches, i.e. DMA request with PASID.
>>> + *
>>> + * @granularity: requested invalidation granularity, type dependent
>>> + * @size: 2^size of 4K pages, 0 for 4k, 9 for 2MB, etc.
>>
>> Why is this a 4K page centric interface?
> This matches the vt-d Address Mask (AM) field of the IOTLB Invalidate
> Descriptor. We can pass a log2size instead.
>>
>>> + * @nr_pages: number of pages to invalidate
>>> + * @pasid: processor address space ID value per PCI spec.
>>> + * @arch_id: architecture dependent id characterizing a
>>> context
>>> + * and tagging the caches, ie. domain Identfier on VTD,
>>> + * asid on ARM SMMU
>>> + * @addr: page address to be invalidated
>>> + * @flags IOMMU_INVALIDATE_ADDR_LEAF: leaf paging entries
>>> + * IOMMU_INVALIDATE_GLOBAL_PAGE: global pages
>>
>> Shouldn't some of these be tied the the granularity of the
>> invalidation? It seems like this should be more similar to
>> iommu_pasid_table_config where the granularity of the invalidation
>> defines which entry within a union at the end of the structure is valid
>> and populated. Otherwise we have fields that don't make sense for
>> certain invalidations.
>
> I am a little bit embarrassed here as this API version is the outcome of
> long discussions held by Jacob, jean-Philippe and many others. I don't
> want to hijack that work as I am "simply" reusing this API.
Re: [PATCH 2/5] swiotlb: Add is_swiotlb_active() function
On Thu, Jan 24, 2019 at 04:00:00PM +0100, Joerg Roedel wrote: > On Thu, Jan 24, 2019 at 09:41:07AM +0100, Christoph Hellwig wrote: > > On Thu, Jan 24, 2019 at 09:29:23AM +0100, Joerg Roedel wrote: > > > > As I've just introduced and fixed a bug in this area in the current > > > > cycle - I don't think no_iotlb_memory is what your want (and maybe > > > > not useful at all): if the arch valls swiotlb_exit after previously > > > > initializing a buffer it won't be set. You probably want to check > > > > for non-zero io_tlb_start and/or io_tlb_end. > > > > > > Okay, but that requires that I also set io_tlb_start and friends back to > > > zero in the failure path of swiotlb_init(). Otherwise it could be left > > > non-zero in case swiotlb_init_with_tbl() returns an error. > > > > Indeed, and we'll need to do that anyway as otherwise the dma mapping > > path might cause problems similar to the one when swiotlb_exit is > > called that I fixed. > > Turns out the the error path in swiotlb_init() is redundant because it > will never be executed. If the function returns it will always return 0 > because in case of failure it will just panic (through memblock_alloc). > > I'll clean that up in a separate patch-set. There are more users of that > function and all of them panic when the function fails. > > > Joerg OK so are you going to post a new version then? Time's running out for 5.0. This isn't a regression so maybe we should just defer it all to 5.1. -- MST ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
On Mon, Jan 28, 2019 at 09:05:26AM +0100, Christoph Hellwig wrote: > On Thu, Jan 24, 2019 at 10:51:51AM +0100, Joerg Roedel wrote: > > On Thu, Jan 24, 2019 at 09:42:21AM +0100, Christoph Hellwig wrote: > > > Yes. But more importantly it would fix the limit for all other block > > > drivers that set large segment sizes when running over swiotlb. > > > > True, so it would be something like the diff below? I havn't worked on > > the block layer, so I don't know if that needs additional checks for > > ->dev or anything. > > Looks sensible. Maybe for now we'll just do the virtio-blk case > that triggered it, and we'll do something like this patch for the > next merge window. We'll also need to apply the same magic to the > DMA boundary. So do I get an ack for this patch then? -- MST ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use generic DMA mapping code in powerpc V4
Thanks a lot! I will test it tomorrow. — Christian Sent from my iPhone > On 28. Jan 2019, at 17:22, Christoph Hellwig wrote: > >> On Mon, Jan 28, 2019 at 08:04:22AM +0100, Christoph Hellwig wrote: >>> On Sun, Jan 27, 2019 at 02:13:09PM +0100, Christian Zigotzky wrote: >>> Christoph, >>> >>> What shall I do next? >> >> I'll need to figure out what went wrong with the new zone selection >> on powerpc and give you another branch to test. > > Can you try the new powerpc-dma.6-debug.2 branch: > >git://git.infradead.org/users/hch/misc.git powerpc-dma.6-debug.2 > > Gitweb: > > > http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6-debug.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.4 35/80] iommu/arm-smmu-v3: Use explicit mb() when moving cons pointer
From: Will Deacon [ Upstream commit a868e8530441286342f90c1fd9c5f24de3aa2880 ] After removing an entry from a queue (e.g. reading an event in arm_smmu_evtq_thread()) it is necessary to advance the MMIO consumer pointer to free the queue slot back to the SMMU. A memory barrier is required here so that all reads targetting the queue entry have completed before the consumer pointer is updated. The implementation of queue_inc_cons() relies on a writel() to complete the previous reads, but this is incorrect because writel() is only guaranteed to complete prior writes. This patch replaces the call to writel() with an mb(); writel_relaxed() sequence, which gives us the read->write ordering which we require. Cc: Robin Murphy Signed-off-by: Will Deacon Signed-off-by: Sasha Levin --- drivers/iommu/arm-smmu-v3.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index fc6eb752ab35..eb9937225d64 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -683,7 +683,13 @@ static void queue_inc_cons(struct arm_smmu_queue *q) u32 cons = (Q_WRP(q, q->cons) | Q_IDX(q, q->cons)) + 1; q->cons = Q_OVF(q, q->cons) | Q_WRP(q, cons) | Q_IDX(q, cons); - writel(q->cons, q->cons_reg); + + /* +* Ensure that all CPU accesses (reads and writes) to the queue +* are complete before we update the cons pointer. +*/ + mb(); + writel_relaxed(q->cons, q->cons_reg); } static int queue_sync_prod(struct arm_smmu_queue *q) -- 2.19.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use generic DMA mapping code in powerpc V4
On Mon, Jan 28, 2019 at 08:04:22AM +0100, Christoph Hellwig wrote: > On Sun, Jan 27, 2019 at 02:13:09PM +0100, Christian Zigotzky wrote: > > Christoph, > > > > What shall I do next? > > I'll need to figure out what went wrong with the new zone selection > on powerpc and give you another branch to test. Can you try the new powerpc-dma.6-debug.2 branch: git://git.infradead.org/users/hch/misc.git powerpc-dma.6-debug.2 Gitweb: http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.6-debug.2 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.9 049/107] iommu/arm-smmu-v3: Use explicit mb() when moving cons pointer
From: Will Deacon [ Upstream commit a868e8530441286342f90c1fd9c5f24de3aa2880 ] After removing an entry from a queue (e.g. reading an event in arm_smmu_evtq_thread()) it is necessary to advance the MMIO consumer pointer to free the queue slot back to the SMMU. A memory barrier is required here so that all reads targetting the queue entry have completed before the consumer pointer is updated. The implementation of queue_inc_cons() relies on a writel() to complete the previous reads, but this is incorrect because writel() is only guaranteed to complete prior writes. This patch replaces the call to writel() with an mb(); writel_relaxed() sequence, which gives us the read->write ordering which we require. Cc: Robin Murphy Signed-off-by: Will Deacon Signed-off-by: Sasha Levin --- drivers/iommu/arm-smmu-v3.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index ff4be1174ff0..7bd98585d78d 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -697,7 +697,13 @@ static void queue_inc_cons(struct arm_smmu_queue *q) u32 cons = (Q_WRP(q, q->cons) | Q_IDX(q, q->cons)) + 1; q->cons = Q_OVF(q, q->cons) | Q_WRP(q, cons) | Q_IDX(q, cons); - writel(q->cons, q->cons_reg); + + /* +* Ensure that all CPU accesses (reads and writes) to the queue +* are complete before we update the cons pointer. +*/ + mb(); + writel_relaxed(q->cons, q->cons_reg); } static int queue_sync_prod(struct arm_smmu_queue *q) -- 2.19.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.9 043/107] iommu/amd: Fix amd_iommu=force_isolation
From: Yu Zhao
[ Upstream commit c12b08ebbe16f0d3a96a116d86709b04c1ee8e74 ]
The parameter is still there but it's ignored. We need to check its
value before deciding to go into passthrough mode for AMD IOMMU v2
capable device.
We occasionally use this parameter to force v2 capable device into
translation mode to debug memory corruption that we suspect is
caused by DMA writes.
To address the following comment from Joerg Roedel on the first
version, v2 capability of device is completely ignored.
> This breaks the iommu_v2 use-case, as it needs a direct mapping for the
> devices that support it.
And from Documentation/admin-guide/kernel-parameters.txt:
This option does not override iommu=pt
Fixes: aafd8ba0ca74 ("iommu/amd: Implement add_device and remove_device")
Signed-off-by: Yu Zhao
Signed-off-by: Joerg Roedel
Signed-off-by: Sasha Levin
---
drivers/iommu/amd_iommu.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index bba1b9f2f782..e984418ffa2a 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -464,7 +464,14 @@ static int iommu_init_device(struct device *dev)
dev_data->alias = get_alias(dev);
- if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
+ /*
+* By default we use passthrough mode for IOMMUv2 capable device.
+* But if amd_iommu=force_isolation is set (e.g. to debug DMA to
+* invalid address), we ignore the capability for the device so
+* it'll be forced to go into translation mode.
+*/
+ if ((iommu_pass_through || !amd_iommu_force_isolation) &&
+ dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
struct amd_iommu *iommu;
iommu = amd_iommu_rlookup_table[dev_data->devid];
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.9 048/107] iommu/arm-smmu: Add support for qcom, smmu-v2 variant
From: Vivek Gautam
[ Upstream commit 89cddc563743cb1e0068867ac97013b2a5bf86aa ]
qcom,smmu-v2 is an arm,smmu-v2 implementation with specific
clock and power requirements.
On msm8996, multiple cores, viz. mdss, video, etc. use this
smmu. On sdm845, this smmu is used with gpu.
Add bindings for the same.
Signed-off-by: Vivek Gautam
Reviewed-by: Rob Herring
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
Reviewed-by: Robin Murphy
Signed-off-by: Will Deacon
Signed-off-by: Sasha Levin
---
drivers/iommu/arm-smmu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 5a9a4416f467..f7ecb30a0bac 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -297,6 +297,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
struct arm_smmu_s2cr {
@@ -1894,6 +1895,7 @@ ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2,
GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
+ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = _generic_v1 },
@@ -1902,6 +1904,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-401", .data = _mmu401 },
{ .compatible = "arm,mmu-500", .data = _mmu500 },
{ .compatible = "cavium,smmu-v2", .data = _smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = _smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.14 084/170] iommu/arm-smmu: Add support for qcom, smmu-v2 variant
From: Vivek Gautam
[ Upstream commit 89cddc563743cb1e0068867ac97013b2a5bf86aa ]
qcom,smmu-v2 is an arm,smmu-v2 implementation with specific
clock and power requirements.
On msm8996, multiple cores, viz. mdss, video, etc. use this
smmu. On sdm845, this smmu is used with gpu.
Add bindings for the same.
Signed-off-by: Vivek Gautam
Reviewed-by: Rob Herring
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
Reviewed-by: Robin Murphy
Signed-off-by: Will Deacon
Signed-off-by: Sasha Levin
---
drivers/iommu/arm-smmu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 15b5856475fc..01a6a0ea2a4f 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -117,6 +117,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
/* Until ACPICA headers cover IORT rev. C */
@@ -1910,6 +1911,7 @@ ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2,
GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
+ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = _generic_v1 },
@@ -1918,6 +1920,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-401", .data = _mmu401 },
{ .compatible = "arm,mmu-500", .data = _mmu500 },
{ .compatible = "cavium,smmu-v2", .data = _smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = _smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.14 085/170] iommu/arm-smmu-v3: Use explicit mb() when moving cons pointer
From: Will Deacon [ Upstream commit a868e8530441286342f90c1fd9c5f24de3aa2880 ] After removing an entry from a queue (e.g. reading an event in arm_smmu_evtq_thread()) it is necessary to advance the MMIO consumer pointer to free the queue slot back to the SMMU. A memory barrier is required here so that all reads targetting the queue entry have completed before the consumer pointer is updated. The implementation of queue_inc_cons() relies on a writel() to complete the previous reads, but this is incorrect because writel() is only guaranteed to complete prior writes. This patch replaces the call to writel() with an mb(); writel_relaxed() sequence, which gives us the read->write ordering which we require. Cc: Robin Murphy Signed-off-by: Will Deacon Signed-off-by: Sasha Levin --- drivers/iommu/arm-smmu-v3.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 26e99c03390f..09eb258a9a7d 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -730,7 +730,13 @@ static void queue_inc_cons(struct arm_smmu_queue *q) u32 cons = (Q_WRP(q, q->cons) | Q_IDX(q, q->cons)) + 1; q->cons = Q_OVF(q, q->cons) | Q_WRP(q, cons) | Q_IDX(q, cons); - writel(q->cons, q->cons_reg); + + /* +* Ensure that all CPU accesses (reads and writes) to the queue +* are complete before we update the cons pointer. +*/ + mb(); + writel_relaxed(q->cons, q->cons_reg); } static int queue_sync_prod(struct arm_smmu_queue *q) -- 2.19.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.14 075/170] iommu/amd: Fix amd_iommu=force_isolation
From: Yu Zhao
[ Upstream commit c12b08ebbe16f0d3a96a116d86709b04c1ee8e74 ]
The parameter is still there but it's ignored. We need to check its
value before deciding to go into passthrough mode for AMD IOMMU v2
capable device.
We occasionally use this parameter to force v2 capable device into
translation mode to debug memory corruption that we suspect is
caused by DMA writes.
To address the following comment from Joerg Roedel on the first
version, v2 capability of device is completely ignored.
> This breaks the iommu_v2 use-case, as it needs a direct mapping for the
> devices that support it.
And from Documentation/admin-guide/kernel-parameters.txt:
This option does not override iommu=pt
Fixes: aafd8ba0ca74 ("iommu/amd: Implement add_device and remove_device")
Signed-off-by: Yu Zhao
Signed-off-by: Joerg Roedel
Signed-off-by: Sasha Levin
---
drivers/iommu/amd_iommu.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index efa6cd2500b9..766103ea237e 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -442,7 +442,14 @@ static int iommu_init_device(struct device *dev)
dev_data->alias = get_alias(dev);
- if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
+ /*
+* By default we use passthrough mode for IOMMUv2 capable device.
+* But if amd_iommu=force_isolation is set (e.g. to debug DMA to
+* invalid address), we ignore the capability for the device so
+* it'll be forced to go into translation mode.
+*/
+ if ((iommu_pass_through || !amd_iommu_force_isolation) &&
+ dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
struct amd_iommu *iommu;
iommu = amd_iommu_rlookup_table[dev_data->devid];
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.19 130/258] iommu/arm-smmu-v3: Use explicit mb() when moving cons pointer
From: Will Deacon [ Upstream commit a868e8530441286342f90c1fd9c5f24de3aa2880 ] After removing an entry from a queue (e.g. reading an event in arm_smmu_evtq_thread()) it is necessary to advance the MMIO consumer pointer to free the queue slot back to the SMMU. A memory barrier is required here so that all reads targetting the queue entry have completed before the consumer pointer is updated. The implementation of queue_inc_cons() relies on a writel() to complete the previous reads, but this is incorrect because writel() is only guaranteed to complete prior writes. This patch replaces the call to writel() with an mb(); writel_relaxed() sequence, which gives us the read->write ordering which we require. Cc: Robin Murphy Signed-off-by: Will Deacon Signed-off-by: Sasha Levin --- drivers/iommu/arm-smmu-v3.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 4afb9cb99ea3..9ae3678844eb 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -688,7 +688,13 @@ static void queue_inc_cons(struct arm_smmu_queue *q) u32 cons = (Q_WRP(q, q->cons) | Q_IDX(q, q->cons)) + 1; q->cons = Q_OVF(q, q->cons) | Q_WRP(q, cons) | Q_IDX(q, cons); - writel(q->cons, q->cons_reg); + + /* +* Ensure that all CPU accesses (reads and writes) to the queue +* are complete before we update the cons pointer. +*/ + mb(); + writel_relaxed(q->cons, q->cons_reg); } static int queue_sync_prod(struct arm_smmu_queue *q) -- 2.19.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.19 128/258] iommu/arm-smmu-v3: Avoid memory corruption from Hisilicon MSI payloads
From: Zhen Lei
[ Upstream commit 84a9a75774961612d0c7dd34a1777e8f98a65abd ]
The GITS_TRANSLATER MMIO doorbell register in the ITS hardware is
architected to be 4 bytes in size, yet on hi1620 and earlier, Hisilicon
have allocated the adjacent 4 bytes to carry some IMPDEF sideband
information which results in an 8-byte MSI payload being delivered when
signalling an interrupt:
MSIAddr:
|4bytes|4bytes|
|MSIData |IMPDEF|
This poses no problem for the ITS hardware because the adjacent 4 bytes
are reserved in the memory map. However, when delivering MSIs to memory,
as we do in the SMMUv3 driver for signalling the completion of a SYNC
command, the extended payload will corrupt the 4 bytes adjacent to the
"sync_count" member in struct arm_smmu_device. Fortunately, the current
layout allocates these bytes to padding, but this is fragile and we
should make this explicit.
Reviewed-by: Robin Murphy
Signed-off-by: Zhen Lei
[will: Rewrote commit message and comment]
Signed-off-by: Will Deacon
Signed-off-by: Sasha Levin
---
drivers/iommu/arm-smmu-v3.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 3e02aace38b1..4afb9cb99ea3 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -586,7 +586,11 @@ struct arm_smmu_device {
struct arm_smmu_strtab_cfg strtab_cfg;
- u32 sync_count;
+ /* Hi16xx adds an extra 32 bits of goodness to its MSI payload */
+ union {
+ u32 sync_count;
+ u64 padding;
+ };
/* IOMMU core code handle */
struct iommu_device iommu;
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.19 118/258] iommu/amd: Fix amd_iommu=force_isolation
From: Yu Zhao
[ Upstream commit c12b08ebbe16f0d3a96a116d86709b04c1ee8e74 ]
The parameter is still there but it's ignored. We need to check its
value before deciding to go into passthrough mode for AMD IOMMU v2
capable device.
We occasionally use this parameter to force v2 capable device into
translation mode to debug memory corruption that we suspect is
caused by DMA writes.
To address the following comment from Joerg Roedel on the first
version, v2 capability of device is completely ignored.
> This breaks the iommu_v2 use-case, as it needs a direct mapping for the
> devices that support it.
And from Documentation/admin-guide/kernel-parameters.txt:
This option does not override iommu=pt
Fixes: aafd8ba0ca74 ("iommu/amd: Implement add_device and remove_device")
Signed-off-by: Yu Zhao
Signed-off-by: Joerg Roedel
Signed-off-by: Sasha Levin
---
drivers/iommu/amd_iommu.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index bee0dfb7b93b..34c9aa76a7bd 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -438,7 +438,14 @@ static int iommu_init_device(struct device *dev)
dev_data->alias = get_alias(dev);
- if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
+ /*
+* By default we use passthrough mode for IOMMUv2 capable device.
+* But if amd_iommu=force_isolation is set (e.g. to debug DMA to
+* invalid address), we ignore the capability for the device so
+* it'll be forced to go into translation mode.
+*/
+ if ((iommu_pass_through || !amd_iommu_force_isolation) &&
+ dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
struct amd_iommu *iommu;
iommu = amd_iommu_rlookup_table[dev_data->devid];
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.19 129/258] iommu/arm-smmu: Add support for qcom, smmu-v2 variant
From: Vivek Gautam
[ Upstream commit 89cddc563743cb1e0068867ac97013b2a5bf86aa ]
qcom,smmu-v2 is an arm,smmu-v2 implementation with specific
clock and power requirements.
On msm8996, multiple cores, viz. mdss, video, etc. use this
smmu. On sdm845, this smmu is used with gpu.
Add bindings for the same.
Signed-off-by: Vivek Gautam
Reviewed-by: Rob Herring
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
Reviewed-by: Robin Murphy
Signed-off-by: Will Deacon
Signed-off-by: Sasha Levin
---
drivers/iommu/arm-smmu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index e7cbf4fcf61d..ce119cb279c3 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -118,6 +118,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
struct arm_smmu_s2cr {
@@ -1912,6 +1913,7 @@ ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2,
GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
+ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = _generic_v1 },
@@ -1920,6 +1922,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-401", .data = _mmu401 },
{ .compatible = "arm,mmu-500", .data = _mmu500 },
{ .compatible = "cavium,smmu-v2", .data = _smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = _smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/5 v3] Fix virtio-blk issue with SWIOTLB
On Mon, Jan 28, 2019 at 10:20:05AM -0500, Michael S. Tsirkin wrote: > On Wed, Jan 23, 2019 at 04:14:53PM -0500, Konrad Rzeszutek Wilk wrote: > > On Wed, Jan 23, 2019 at 01:51:29PM -0500, Michael S. Tsirkin wrote: > > > On Wed, Jan 23, 2019 at 05:30:44PM +0100, Joerg Roedel wrote: > > > > Hi, > > > > > > > > here is the third version of this patch-set. Previous > > > > versions can be found here: > > > > > > > > V1: > > > > https://lore.kernel.org/lkml/20190110134433.15672-1-j...@8bytes.org/ > > > > > > > > V2: > > > > https://lore.kernel.org/lkml/20190115132257.6426-1-j...@8bytes.org/ > > > > > > > > The problem solved here is a limitation of the SWIOTLB implementation, > > > > which does not support allocations larger than 256kb. When the > > > > virtio-blk driver tries to read/write a block larger than that, the > > > > allocation of the dma-handle fails and an IO error is reported. > > > > > > > > > OK looks good to me. > > > I will park this in my tree for now this way it will get > > > testing in linux-next. > > > Can I get an ack from DMA maintainers on the DMA bits for > > > merging this in 5.0? > > > > You got mine (SWIOTBL is my area). > > OK so > > Reviewed-by: Konrad Rzeszutek Wilk Yes :-) > > > > > > > > > > Changes to v2 are: > > > > > > > > * Check if SWIOTLB is active before returning its limit in > > > > dma_direct_max_mapping_size() > > > > > > > > * Only apply the maximum segment limit in virtio-blk when > > > > DMA-API is used for the vring > > > > > > > > Please review. > > > > > > > > Thanks, > > > > > > > > Joerg > > > > > > > > Joerg Roedel (5): > > > > swiotlb: Introduce swiotlb_max_mapping_size() > > > > swiotlb: Add is_swiotlb_active() function > > > > dma: Introduce dma_max_mapping_size() > > > > virtio: Introduce virtio_max_dma_size() > > > > virtio-blk: Consider virtio_max_dma_size() for maximum segment size > > > > > > > > drivers/block/virtio_blk.c | 10 ++ > > > > drivers/virtio/virtio_ring.c | 10 ++ > > > > include/linux/dma-mapping.h | 16 > > > > include/linux/swiotlb.h | 11 +++ > > > > include/linux/virtio.h | 2 ++ > > > > kernel/dma/direct.c | 11 +++ > > > > kernel/dma/swiotlb.c | 10 ++ > > > > 7 files changed, 66 insertions(+), 4 deletions(-) > > > > > > > > -- > > > > 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.20 154/304] iommu/arm-smmu-v3: Use explicit mb() when moving cons pointer
From: Will Deacon [ Upstream commit a868e8530441286342f90c1fd9c5f24de3aa2880 ] After removing an entry from a queue (e.g. reading an event in arm_smmu_evtq_thread()) it is necessary to advance the MMIO consumer pointer to free the queue slot back to the SMMU. A memory barrier is required here so that all reads targetting the queue entry have completed before the consumer pointer is updated. The implementation of queue_inc_cons() relies on a writel() to complete the previous reads, but this is incorrect because writel() is only guaranteed to complete prior writes. This patch replaces the call to writel() with an mb(); writel_relaxed() sequence, which gives us the read->write ordering which we require. Cc: Robin Murphy Signed-off-by: Will Deacon Signed-off-by: Sasha Levin --- drivers/iommu/arm-smmu-v3.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c index 62ef4afc9ee5..11f528e727a1 100644 --- a/drivers/iommu/arm-smmu-v3.c +++ b/drivers/iommu/arm-smmu-v3.c @@ -679,7 +679,13 @@ static void queue_inc_cons(struct arm_smmu_queue *q) u32 cons = (Q_WRP(q, q->cons) | Q_IDX(q, q->cons)) + 1; q->cons = Q_OVF(q, q->cons) | Q_WRP(q, cons) | Q_IDX(q, cons); - writel(q->cons, q->cons_reg); + + /* +* Ensure that all CPU accesses (reads and writes) to the queue +* are complete before we update the cons pointer. +*/ + mb(); + writel_relaxed(q->cons, q->cons_reg); } static int queue_sync_prod(struct arm_smmu_queue *q) -- 2.19.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.20 153/304] iommu/arm-smmu: Add support for qcom, smmu-v2 variant
From: Vivek Gautam
[ Upstream commit 89cddc563743cb1e0068867ac97013b2a5bf86aa ]
qcom,smmu-v2 is an arm,smmu-v2 implementation with specific
clock and power requirements.
On msm8996, multiple cores, viz. mdss, video, etc. use this
smmu. On sdm845, this smmu is used with gpu.
Add bindings for the same.
Signed-off-by: Vivek Gautam
Reviewed-by: Rob Herring
Reviewed-by: Tomasz Figa
Tested-by: Srinivas Kandagatla
Reviewed-by: Robin Murphy
Signed-off-by: Will Deacon
Signed-off-by: Sasha Levin
---
drivers/iommu/arm-smmu.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 5a28ae892504..e23aa7f6c4ad 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -119,6 +119,7 @@ enum arm_smmu_implementation {
GENERIC_SMMU,
ARM_MMU500,
CAVIUM_SMMUV2,
+ QCOM_SMMUV2,
};
struct arm_smmu_s2cr {
@@ -1954,6 +1955,7 @@ ARM_SMMU_MATCH_DATA(smmu_generic_v2, ARM_SMMU_V2,
GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu401, ARM_SMMU_V1_64K, GENERIC_SMMU);
ARM_SMMU_MATCH_DATA(arm_mmu500, ARM_SMMU_V2, ARM_MMU500);
ARM_SMMU_MATCH_DATA(cavium_smmuv2, ARM_SMMU_V2, CAVIUM_SMMUV2);
+ARM_SMMU_MATCH_DATA(qcom_smmuv2, ARM_SMMU_V2, QCOM_SMMUV2);
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v1", .data = _generic_v1 },
@@ -1962,6 +1964,7 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,mmu-401", .data = _mmu401 },
{ .compatible = "arm,mmu-500", .data = _mmu500 },
{ .compatible = "cavium,smmu-v2", .data = _smmuv2 },
+ { .compatible = "qcom,smmu-v2", .data = _smmuv2 },
{ },
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.20 152/304] iommu/arm-smmu-v3: Avoid memory corruption from Hisilicon MSI payloads
From: Zhen Lei
[ Upstream commit 84a9a75774961612d0c7dd34a1777e8f98a65abd ]
The GITS_TRANSLATER MMIO doorbell register in the ITS hardware is
architected to be 4 bytes in size, yet on hi1620 and earlier, Hisilicon
have allocated the adjacent 4 bytes to carry some IMPDEF sideband
information which results in an 8-byte MSI payload being delivered when
signalling an interrupt:
MSIAddr:
|4bytes|4bytes|
|MSIData |IMPDEF|
This poses no problem for the ITS hardware because the adjacent 4 bytes
are reserved in the memory map. However, when delivering MSIs to memory,
as we do in the SMMUv3 driver for signalling the completion of a SYNC
command, the extended payload will corrupt the 4 bytes adjacent to the
"sync_count" member in struct arm_smmu_device. Fortunately, the current
layout allocates these bytes to padding, but this is fragile and we
should make this explicit.
Reviewed-by: Robin Murphy
Signed-off-by: Zhen Lei
[will: Rewrote commit message and comment]
Signed-off-by: Will Deacon
Signed-off-by: Sasha Levin
---
drivers/iommu/arm-smmu-v3.c | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 71eda422c926..62ef4afc9ee5 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -576,7 +576,11 @@ struct arm_smmu_device {
struct arm_smmu_strtab_cfg strtab_cfg;
- u32 sync_count;
+ /* Hi16xx adds an extra 32 bits of goodness to its MSI payload */
+ union {
+ u32 sync_count;
+ u64 padding;
+ };
/* IOMMU core code handle */
struct iommu_device iommu;
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH AUTOSEL 4.20 139/304] iommu/amd: Fix amd_iommu=force_isolation
From: Yu Zhao
[ Upstream commit c12b08ebbe16f0d3a96a116d86709b04c1ee8e74 ]
The parameter is still there but it's ignored. We need to check its
value before deciding to go into passthrough mode for AMD IOMMU v2
capable device.
We occasionally use this parameter to force v2 capable device into
translation mode to debug memory corruption that we suspect is
caused by DMA writes.
To address the following comment from Joerg Roedel on the first
version, v2 capability of device is completely ignored.
> This breaks the iommu_v2 use-case, as it needs a direct mapping for the
> devices that support it.
And from Documentation/admin-guide/kernel-parameters.txt:
This option does not override iommu=pt
Fixes: aafd8ba0ca74 ("iommu/amd: Implement add_device and remove_device")
Signed-off-by: Yu Zhao
Signed-off-by: Joerg Roedel
Signed-off-by: Sasha Levin
---
drivers/iommu/amd_iommu.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1167ff0416cf..325f3bad118b 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -438,7 +438,14 @@ static int iommu_init_device(struct device *dev)
dev_data->alias = get_alias(dev);
- if (dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
+ /*
+* By default we use passthrough mode for IOMMUv2 capable device.
+* But if amd_iommu=force_isolation is set (e.g. to debug DMA to
+* invalid address), we ignore the capability for the device so
+* it'll be forced to go into translation mode.
+*/
+ if ((iommu_pass_through || !amd_iommu_force_isolation) &&
+ dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
struct amd_iommu *iommu;
iommu = amd_iommu_rlookup_table[dev_data->devid];
--
2.19.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/5 v3] Fix virtio-blk issue with SWIOTLB
On Wed, Jan 23, 2019 at 04:14:53PM -0500, Konrad Rzeszutek Wilk wrote: > On Wed, Jan 23, 2019 at 01:51:29PM -0500, Michael S. Tsirkin wrote: > > On Wed, Jan 23, 2019 at 05:30:44PM +0100, Joerg Roedel wrote: > > > Hi, > > > > > > here is the third version of this patch-set. Previous > > > versions can be found here: > > > > > > V1: https://lore.kernel.org/lkml/20190110134433.15672-1-j...@8bytes.org/ > > > > > > V2: https://lore.kernel.org/lkml/20190115132257.6426-1-j...@8bytes.org/ > > > > > > The problem solved here is a limitation of the SWIOTLB implementation, > > > which does not support allocations larger than 256kb. When the > > > virtio-blk driver tries to read/write a block larger than that, the > > > allocation of the dma-handle fails and an IO error is reported. > > > > > > OK looks good to me. > > I will park this in my tree for now this way it will get > > testing in linux-next. > > Can I get an ack from DMA maintainers on the DMA bits for > > merging this in 5.0? > > You got mine (SWIOTBL is my area). OK so Reviewed-by: Konrad Rzeszutek Wilk > > > > > Changes to v2 are: > > > > > > * Check if SWIOTLB is active before returning its limit in > > > dma_direct_max_mapping_size() > > > > > > * Only apply the maximum segment limit in virtio-blk when > > > DMA-API is used for the vring > > > > > > Please review. > > > > > > Thanks, > > > > > > Joerg > > > > > > Joerg Roedel (5): > > > swiotlb: Introduce swiotlb_max_mapping_size() > > > swiotlb: Add is_swiotlb_active() function > > > dma: Introduce dma_max_mapping_size() > > > virtio: Introduce virtio_max_dma_size() > > > virtio-blk: Consider virtio_max_dma_size() for maximum segment size > > > > > > drivers/block/virtio_blk.c | 10 ++ > > > drivers/virtio/virtio_ring.c | 10 ++ > > > include/linux/dma-mapping.h | 16 > > > include/linux/swiotlb.h | 11 +++ > > > include/linux/virtio.h | 2 ++ > > > kernel/dma/direct.c | 11 +++ > > > kernel/dma/swiotlb.c | 10 ++ > > > 7 files changed, 66 insertions(+), 4 deletions(-) > > > > > > -- > > > 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu/io-pgtable-arm: Add support for non-coherent page tables
On Mon, Jan 21, 2019 at 6:43 PM Robin Murphy wrote:
>
> On 17/01/2019 09:27, Vivek Gautam wrote:
> > From Robin's comment [1] about touching TCR configurations -
> >
> > "TBH if we're going to touch the TCR attributes at all then we should
> > probably correct that sloppiness first - there's an occasional argument
> > for using non-cacheable pagetables even on a coherent SMMU if reducing
> > snoop traffic/latency on walks outweighs the cost of cache maintenance
> > on PTE updates, but anyone thinking they can get that by overriding
> > dma-coherent silently gets the worst of both worlds thanks to this
> > current TCR value."
> >
> > We have IO_PGTABLE_QUIRK_NO_DMA quirk present, but we don't force
> > anybody _not_ using dma-coherent smmu to have non-cacheable page table
> > mappings.
> > Having another quirk flag can help in having non-cacheable memory for
> > page tables once and for all.
> >
> > [1] https://lore.kernel.org/patchwork/patch/1020906/
> >
> > Signed-off-by: Vivek Gautam
> > ---
> > drivers/iommu/io-pgtable-arm.c | 17 -
> > drivers/iommu/io-pgtable.h | 6 ++
> > 2 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> > index 237cacd4a62b..c76919c30f1a 100644
> > --- a/drivers/iommu/io-pgtable-arm.c
> > +++ b/drivers/iommu/io-pgtable-arm.c
> > @@ -780,7 +780,8 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg
> > *cfg, void *cookie)
> > struct arm_lpae_io_pgtable *data;
> >
> > if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS | IO_PGTABLE_QUIRK_NO_DMA
> > |
> > - IO_PGTABLE_QUIRK_NON_STRICT))
> > + IO_PGTABLE_QUIRK_NON_STRICT |
> > + IO_PGTABLE_QUIRK_NON_COHERENT))
> > return NULL;
> >
> > data = arm_lpae_alloc_pgtable(cfg);
> > @@ -788,9 +789,14 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg
> > *cfg, void *cookie)
> > return NULL;
> >
> > /* TCR */
> > - reg = (ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT) |
> > - (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT) |
> > - (ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT);
> > + reg = ARM_LPAE_TCR_SH_IS << ARM_LPAE_TCR_SH0_SHIFT;
> > +
> > + if (cfg->quirks & IO_PGTABLE_QUIRK_NON_COHERENT)
> > + reg |= ARM_LPAE_TCR_RGN_NC << ARM_LPAE_TCR_IRGN0_SHIFT |
> > +ARM_LPAE_TCR_RGN_NC << ARM_LPAE_TCR_ORGN0_SHIFT;
> > + else
> > + reg |= ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_IRGN0_SHIFT |
> > +ARM_LPAE_TCR_RGN_WBWA << ARM_LPAE_TCR_ORGN0_SHIFT;
> >
> > switch (ARM_LPAE_GRANULE(data)) {
> > case SZ_4K:
> > @@ -873,7 +879,8 @@ arm_64_lpae_alloc_pgtable_s2(struct io_pgtable_cfg
> > *cfg, void *cookie)
> >
> > /* The NS quirk doesn't apply at stage 2 */
> > if (cfg->quirks & ~(IO_PGTABLE_QUIRK_NO_DMA |
> > - IO_PGTABLE_QUIRK_NON_STRICT))
> > + IO_PGTABLE_QUIRK_NON_STRICT |
> > + IO_PGTABLE_QUIRK_NON_COHERENT))
> > return NULL;
> >
> > data = arm_lpae_alloc_pgtable(cfg);
> > diff --git a/drivers/iommu/io-pgtable.h b/drivers/iommu/io-pgtable.h
> > index 47d5ae559329..46604cf7b017 100644
> > --- a/drivers/iommu/io-pgtable.h
> > +++ b/drivers/iommu/io-pgtable.h
> > @@ -75,6 +75,11 @@ struct io_pgtable_cfg {
> >* IO_PGTABLE_QUIRK_NON_STRICT: Skip issuing synchronous leaf TLBIs
> >* on unmap, for DMA domains using the flush queue mechanism for
> >* delayed invalidation.
> > + *
> > + * IO_PGTABLE_QUIRK_NON_COHERENT: Enforce non-cacheable mappings for
> > + * pagetables even on a coherent SMMU for cases where reducing
> > + * snoop traffic/latency on walks outweighs the cost of cache
> > + * maintenance on PTE updates.
>
> Hmm, we can't actually "enforce" anything with this as-is - all we're
> doing is setting the attributes that the IOMMU will use for pagetable
> walks, and that has no impact on how the CPU actually writes PTEs to
> memory. In particular, in the case of a hardware-coherent IOMMU which is
> described as such, even if we make the dma_map/sync calls they still
> won't do anything since they 'know' that the IOMMU is coherent. Thus if
> we then set up a non-cacheable TCR we would have no proper means of
> making pagetables correctly visible to the walker.
Right, I get this point. With non-cacheable TCR, the PTW will always go to
the main memory rather then snooping in CPU caches for the latest page
tables.
>
> Aw crap, this is turning out to be a microcosm of the PCIe no-snoop
> mess... :(
>
> To start with, at least, what we want is to set a non-cacheable TCR if
> the IOMMU is *not* coherent (as far as Linux is concerned - that
> includes the firmware-lying-about-the-hardware situation I was alluding
> to before),
Re: [PATCH 0/3] iommu/arm-smmu: Add support to use Last level cache
Hi Ard, On Thu, Jan 24, 2019 at 1:25 PM Ard Biesheuvel wrote: > > On Thu, 24 Jan 2019 at 07:58, Vivek Gautam > wrote: > > > > On Mon, Jan 21, 2019 at 7:55 PM Ard Biesheuvel > > wrote: > > > > > > On Mon, 21 Jan 2019 at 14:56, Robin Murphy wrote: > > > > > > > > On 21/01/2019 13:36, Ard Biesheuvel wrote: > > > > > On Mon, 21 Jan 2019 at 14:25, Robin Murphy > > > > > wrote: > > > > >> > > > > >> On 21/01/2019 10:50, Ard Biesheuvel wrote: > > > > >>> On Mon, 21 Jan 2019 at 11:17, Vivek Gautam > > > > >>> wrote: > > > > > > > > Hi, > > > > > > > > > > > > On Mon, Jan 21, 2019 at 12:56 PM Ard Biesheuvel > > > > wrote: > > > > > > > > > > On Mon, 21 Jan 2019 at 06:54, Vivek Gautam > > > > > wrote: > > > > >> > > > > >> Qualcomm SoCs have an additional level of cache called as > > > > >> System cache, aka. Last level cache (LLC). This cache sits right > > > > >> before the DDR, and is tightly coupled with the memory > > > > >> controller. > > > > >> The clients using this cache 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 > > > > >> buffers and, related page tables [1], memory attributes need to > > > > >> be > > > > >> set accordingly. This series add the required support. > > > > >> > > > > > > > > > > Does this actually improve performance on reads from a device? The > > > > > non-cache coherent DMA routines perform an unconditional D-cache > > > > > invalidate by VA to the PoC before reading from the buffers > > > > > filled by > > > > > the device, and I would expect the PoC to be defined as lying > > > > > beyond > > > > > the LLC to still guarantee the architected behavior. > > > > > > > > We have seen performance improvements when running Manhattan > > > > GFXBench benchmarks. > > > > > > > > >>> > > > > >>> Ah ok, that makes sense, since in that case, the data flow is mostly > > > > >>> to the device, not from the device. > > > > >>> > > > > As for the PoC, from my knowledge on sdm845 the system cache, aka > > > > Last level cache (LLC) lies beyond the point of coherency. > > > > Non-cache coherent buffers will not be cached to system cache > > > > also, and > > > > no additional software cache maintenance ops are required for > > > > system cache. > > > > Pratik can add more if I am missing something. > > > > > > > > To take care of the memory attributes from DMA APIs side, we can > > > > add a > > > > DMA_ATTR definition to take care of any dma non-coherent APIs > > > > calls. > > > > > > > > >>> > > > > >>> So does the device use the correct inner non-cacheable, outer > > > > >>> writeback cacheable attributes if the SMMU is in pass-through? > > > > >>> > > > > >>> We have been looking into another use case where the fact that the > > > > >>> SMMU overrides memory attributes is causing issues (WC mappings used > > > > >>> by the radeon and amdgpu driver). So if the SMMU would honour the > > > > >>> existing attributes, would you still need the SMMU changes? > > > > >> > > > > >> Even if we could force a stage 2 mapping with the weakest pagetable > > > > >> attributes (such that combining would work), there would still need > > > > >> to > > > > >> be a way to set the TCR attributes appropriately if this behaviour is > > > > >> wanted for the SMMU's own table walks as well. > > > > >> > > > > > > > > > > Isn't that just a matter of implementing support for SMMUs that lack > > > > > the 'dma-coherent' attribute? > > > > > > > > Not quite - in general they need INC-ONC attributes in case there > > > > actually is something in the architectural outer-cacheable domain. > > > > > > But is it a problem to use INC-ONC attributes for the SMMU PTW on this > > > chip? AIUI, the reason for the SMMU changes is to avoid the > > > performance hit of snooping, which is more expensive than cache > > > maintenance of SMMU page tables. So are you saying the by-VA cache > > > maintenance is not relayed to this system cache, resulting in page > > > table updates to be invisible to masters using INC-ONC attributes? > > > > The reason for this SMMU changes is that the non-coherent devices > > can't access the inner caches at all. But they have a way to allocate > > and lookup in system cache. > > > > CPU will by default make use of system cache when the inner-cacheable > > and outer-cacheable memory attribute is set. > > > > So for SMMU page tables to be visible to PTW, > > -- For IO coherent clients, the CPU cache maintenance operations are not > > required for buffers marked Normal Cached to achieve a coherent view of > > memory. However, client-specific cache maintenance may still be > > required for devices > > with local caches (for example, compute DSP local L1
Re: [PATCH] iommu/io-pgtable-arm-v7s: only kmemleak_ignore L2 tables
On Mon, Jan 28, 2019 at 05:43:01PM +0800, Nicolas Boichat wrote:
> L1 tables are allocated with __get_dma_pages, and therefore already
> ignored by kmemleak.
>
> Without this, the kernel would print this error message on boot,
> when the first L1 table is allocated:
>
> [2.810533] kmemleak: Trying to color unknown object at 0xffd652388000
> as Black
> [2.818190] CPU: 5 PID: 39 Comm: kworker/5:0 Tainted: G S
> 4.19.16 #8
> [2.831227] Workqueue: events deferred_probe_work_func
> [2.836353] Call trace:
> ...
> [2.852532] paint_ptr+0xa0/0xa8
> [2.855750] kmemleak_ignore+0x38/0x6c
> [2.859490] __arm_v7s_alloc_table+0x168/0x1f4
> [2.863922] arm_v7s_alloc_pgtable+0x114/0x17c
> [2.868354] alloc_io_pgtable_ops+0x3c/0x78
> ...
>
> Fixes: e5fc9753b1a8314 ("iommu/io-pgtable: Add ARMv7 short descriptor
> support")
> Signed-off-by: Nicolas Boichat
> ---
>
> I only tested this on top of my other series
> (https://patchwork.kernel.org/patch/10720495/), but I think the same fix
> applies. I'm still a bit confused as to why this only shows up now, as IIUC,
> the kmemleak_ignore call was always wrong with L1 tables.
>
> drivers/iommu/io-pgtable-arm-v7s.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
This is not the correct way to submit patches for inclusion in the
stable kernel tree. Please read:
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] iommu/io-pgtable-arm-v7s: only kmemleak_ignore L2 tables
L1 tables are allocated with __get_dma_pages, and therefore already
ignored by kmemleak.
Without this, the kernel would print this error message on boot,
when the first L1 table is allocated:
[2.810533] kmemleak: Trying to color unknown object at 0xffd652388000
as Black
[2.818190] CPU: 5 PID: 39 Comm: kworker/5:0 Tainted: G S
4.19.16 #8
[2.831227] Workqueue: events deferred_probe_work_func
[2.836353] Call trace:
...
[2.852532] paint_ptr+0xa0/0xa8
[2.855750] kmemleak_ignore+0x38/0x6c
[2.859490] __arm_v7s_alloc_table+0x168/0x1f4
[2.863922] arm_v7s_alloc_pgtable+0x114/0x17c
[2.868354] alloc_io_pgtable_ops+0x3c/0x78
...
Fixes: e5fc9753b1a8314 ("iommu/io-pgtable: Add ARMv7 short descriptor support")
Signed-off-by: Nicolas Boichat
---
I only tested this on top of my other series
(https://patchwork.kernel.org/patch/10720495/), but I think the same fix
applies. I'm still a bit confused as to why this only shows up now, as IIUC,
the kmemleak_ignore call was always wrong with L1 tables.
drivers/iommu/io-pgtable-arm-v7s.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/io-pgtable-arm-v7s.c
b/drivers/iommu/io-pgtable-arm-v7s.c
index cec29bf45c9bd7..98a4a4a0dfb0c1 100644
--- a/drivers/iommu/io-pgtable-arm-v7s.c
+++ b/drivers/iommu/io-pgtable-arm-v7s.c
@@ -217,7 +217,8 @@ static void *__arm_v7s_alloc_table(int lvl, gfp_t gfp,
if (dma != phys)
goto out_unmap;
}
- kmemleak_ignore(table);
+ if (lvl == 2)
+ kmemleak_ignore(table);
return table;
out_unmap:
--
2.20.1.495.gaa96b0ce6b-goog
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/5] virtio-blk: Consider virtio_max_dma_size() for maximum segment size
On Thu, Jan 24, 2019 at 10:51:51AM +0100, Joerg Roedel wrote: > On Thu, Jan 24, 2019 at 09:42:21AM +0100, Christoph Hellwig wrote: > > Yes. But more importantly it would fix the limit for all other block > > drivers that set large segment sizes when running over swiotlb. > > True, so it would be something like the diff below? I havn't worked on > the block layer, so I don't know if that needs additional checks for > ->dev or anything. Looks sensible. Maybe for now we'll just do the virtio-blk case that triggered it, and we'll do something like this patch for the next merge window. We'll also need to apply the same magic to the DMA boundary. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
