RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
Module Version : PiccasoCpu 10 AGESA Version : PiccasoPI 100A I did not try to enter the system in any other way (like via ssh) than via Desktop. -Original Message- From: Huang Rui Sent: Dienstag, 24. November 2020 07:43 To: Kuehling, Felix Cc: Will Deacon ; Deucher, Alexander ; linux-ker...@vger.kernel.org; linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn Helgaas ; Merger, Edgar [AUTOSOL/MAS/AUGS] ; Joerg Roedel ; Changfeng Zhu Subject: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken On Tue, Nov 24, 2020 at 06:51:11AM +0800, Kuehling, Felix wrote: > On 2020-11-23 5:33 p.m., Will Deacon wrote: > > On Mon, Nov 23, 2020 at 09:04:14PM +, Deucher, Alexander wrote: > >> [AMD Public Use] > >> > >>> -Original Message- > >>> From: Will Deacon > >>> Sent: Monday, November 23, 2020 8:44 AM > >>> To: linux-ker...@vger.kernel.org > >>> Cc: linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; > >>> Will Deacon ; Bjorn Helgaas > >>> ; Deucher, Alexander > >>> ; Edgar Merger > >>> ; Joerg Roedel > >>> Subject: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken > >>> > >>> Edgar Merger reports that the AMD Raven GPU does not work reliably > >>> on his system when the IOMMU is enabled: > >>> > >>>| [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > >>> signaled seq=1, emitted seq=3 > >>>| [...] > >>>| amdgpu :0b:00.0: GPU reset begin! > >>>| AMD-Vi: Completion-Wait loop timed out > >>>| iommu ivhd0: AMD-Vi: Event logged [IOTLB_INV_TIMEOUT > >>> device=0b:00.0 address=0x38edc0970] > >>> > >>> This is indicative of a hardware/platform configuration issue so, > >>> since disabling ATS has been shown to resolve the problem, add a > >>> quirk to match this particular device while Edgar follows-up with AMD for > >>> more information. > >>> > >>> Cc: Bjorn Helgaas > >>> Cc: Alex Deucher > >>> Reported-by: Edgar Merger > >>> Suggested-by: Joerg Roedel > >>> Link: > >>> https://urldefense.proofpoint.com/v2/url?u=https-3A__lore=DwIDAw=jOURTkCZzT8tVB5xPEYIm3YJGoxoTaQsQPzPKJGaWbo=BJxhacqqa4K1PJGm6_-862rdSP13_P6LVp7j_9l1xmg=lNXu2xwvyxEZ3PzoVmXMBXXS55jsmfDicuQFJqkIOH4=_5VDNCRQdA7AhsvvZ3TJJtQZ2iBp9c9tFHIleTYT_ZM= > >>> . > >>> kernel.org/linux- > >>> iommu/MWHPR10MB1310F042A30661D4158520B589FC0@MWHPR10M > >>> B1310.namprd10.prod.outlook.com > >>> her%40amd.com%7C1a883fe14d0c408e7d9508d88fb5df4e%7C3dd8961fe488 > >>> 4e608e11a82d994e183d%7C0%7C0%7C637417358593629699%7CUnknown%7 > >>> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi > >>> LCJXVCI6Mn0%3D%7C1000sdata=TMgKldWzsX8XZ0l7q3%2BszDWXQJJ > >>> LOUfX5oGaoLN8n%2B8%3Dreserved=0 > >>> Signed-off-by: Will Deacon > >>> --- > >>> > >>> Hi all, > >>> > >>> Since Joerg is away at the moment, I'm posting this to try to make > >>> some progress with the thread in the Link: tag. > >> + Felix > >> > >> What system is this? Can you provide more details? Does a sbios > >> update fix this? Disabling ATS for all Ravens will break GPU > >> compute for a lot of people. I'd prefer to just black list this > >> particular system (e.g., just SSIDs or revision) if possible. > > +Ray > > There are already many systems where the IOMMU is disabled in the > BIOS, or the CRAT table reporting the APU compute capabilities is > broken. Ray has been working on a fallback to make APUs behave like > dGPUs on such systems. That should also cover this case where ATS is > blacklisted. That said, it affects the programming model, because we > don't support the unified and coherent memory model on dGPUs like we > do on APUs with IOMMUv2. So it would be good to make the conditions > for this workaround as narrow as possible. Yes, besides the comments from Alex and Felix, may we get your firmware version (SMC firmware which is from SBIOS) and device id? > >>>| [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > >>> signaled seq=1, emitted seq=3 It looks only gfx ib test passed, and fails to lanuch desktop, am I right? We would like to see whether it is Raven, Raven kicker (new Raven), or Picasso. In our side, per the internal test result, we didn't see the similiar issue on Raven kicker and Picasso platform. Thanks, Ray > > These are the relevant changes in KFD and Thunk for reference: > > ### KFD ### > > commit 914913ab04dfbcd0226ecb6bc99d276832ea2908 > Author: Huang Rui > Date: Tue Aug 18 14:54:23 2020 +0800 > > drm/amdkfd: implement the dGPU fallback path for apu (v6) > > We still have a few iommu issues which need to address, so force > raven > as "dgpu" path for the moment. > > This is to add the fallback path to bypass IOMMU if IOMMU v2 is > disabled > or ACPI CRAT table not correct. > > v2: Use ignore_crat parameter to decide whether it will go with > IOMMUv2. > v3: Align with existed thunk, don't change the way of raven, only > renoir > will use "dgpu" path by default. >
Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
On Tue, Nov 24, 2020 at 06:51:11AM +0800, Kuehling, Felix wrote: > On 2020-11-23 5:33 p.m., Will Deacon wrote: > > On Mon, Nov 23, 2020 at 09:04:14PM +, Deucher, Alexander wrote: > >> [AMD Public Use] > >> > >>> -Original Message- > >>> From: Will Deacon > >>> Sent: Monday, November 23, 2020 8:44 AM > >>> To: linux-ker...@vger.kernel.org > >>> Cc: linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; Will > >>> Deacon ; Bjorn Helgaas ; > >>> Deucher, Alexander ; Edgar Merger > >>> ; Joerg Roedel > >>> Subject: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken > >>> > >>> Edgar Merger reports that the AMD Raven GPU does not work reliably on his > >>> system when the IOMMU is enabled: > >>> > >>>| [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > >>> signaled seq=1, emitted seq=3 > >>>| [...] > >>>| amdgpu :0b:00.0: GPU reset begin! > >>>| AMD-Vi: Completion-Wait loop timed out > >>>| iommu ivhd0: AMD-Vi: Event logged [IOTLB_INV_TIMEOUT > >>> device=0b:00.0 address=0x38edc0970] > >>> > >>> This is indicative of a hardware/platform configuration issue so, since > >>> disabling ATS has been shown to resolve the problem, add a quirk to match > >>> this particular device while Edgar follows-up with AMD for more > >>> information. > >>> > >>> Cc: Bjorn Helgaas > >>> Cc: Alex Deucher > >>> Reported-by: Edgar Merger > >>> Suggested-by: Joerg Roedel > >>> Link: > >>> https://lore. > >>> kernel.org/linux- > >>> iommu/MWHPR10MB1310F042A30661D4158520B589FC0@MWHPR10M > >>> B1310.namprd10.prod.outlook.com > >>> her%40amd.com%7C1a883fe14d0c408e7d9508d88fb5df4e%7C3dd8961fe488 > >>> 4e608e11a82d994e183d%7C0%7C0%7C637417358593629699%7CUnknown%7 > >>> CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi > >>> LCJXVCI6Mn0%3D%7C1000sdata=TMgKldWzsX8XZ0l7q3%2BszDWXQJJ > >>> LOUfX5oGaoLN8n%2B8%3Dreserved=0 > >>> Signed-off-by: Will Deacon > >>> --- > >>> > >>> Hi all, > >>> > >>> Since Joerg is away at the moment, I'm posting this to try to make some > >>> progress with the thread in the Link: tag. > >> + Felix > >> > >> What system is this? Can you provide more details? Does a sbios update > >> fix this? Disabling ATS for all Ravens will break GPU compute for a lot > >> of people. I'd prefer to just black list this particular system (e.g., > >> just SSIDs or revision) if possible. > > +Ray > > There are already many systems where the IOMMU is disabled in the BIOS, > or the CRAT table reporting the APU compute capabilities is broken. Ray > has been working on a fallback to make APUs behave like dGPUs on such > systems. That should also cover this case where ATS is blacklisted. That > said, it affects the programming model, because we don't support the > unified and coherent memory model on dGPUs like we do on APUs with > IOMMUv2. So it would be good to make the conditions for this workaround > as narrow as possible. Yes, besides the comments from Alex and Felix, may we get your firmware version (SMC firmware which is from SBIOS) and device id? > >>>| [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > >>> signaled seq=1, emitted seq=3 It looks only gfx ib test passed, and fails to lanuch desktop, am I right? We would like to see whether it is Raven, Raven kicker (new Raven), or Picasso. In our side, per the internal test result, we didn't see the similiar issue on Raven kicker and Picasso platform. Thanks, Ray > > These are the relevant changes in KFD and Thunk for reference: > > ### KFD ### > > commit 914913ab04dfbcd0226ecb6bc99d276832ea2908 > Author: Huang Rui > Date: Tue Aug 18 14:54:23 2020 +0800 > > drm/amdkfd: implement the dGPU fallback path for apu (v6) > > We still have a few iommu issues which need to address, so force raven > as "dgpu" path for the moment. > > This is to add the fallback path to bypass IOMMU if IOMMU v2 is > disabled > or ACPI CRAT table not correct. > > v2: Use ignore_crat parameter to decide whether it will go with > IOMMUv2. > v3: Align with existed thunk, don't change the way of raven, only > renoir > will use "dgpu" path by default. > v4: don't update global ignore_crat in the driver, and revise fallback > function if CRAT is broken. > v5: refine acpi crat good but no iommu support case, and rename the > title. > v6: fix the issue of dGPU initialized firstly, just modify the report > value in the node_show(). > > Signed-off-by: Huang Rui > Reviewed-by: Felix Kuehling > Signed-off-by: Alex Deucher > > ### Thunk ### > > commit e32482fa4b9ca398c8bdc303920abfd672592764 > Author: Huang Rui > Date: Tue Aug 18 18:54:05 2020 +0800 > > libhsakmt: remove is_dgpu flag in the hsa_gfxip_table > > Whether use dgpu path will check the props which exposed from kernel. > We won't need hard code in the ASIC table. > > Signed-off-by: Huang Rui > Change-Id:
RE: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
This is a board developed by my company. Subsystem-ID is ea50:0c19 or ea50:cc10 (depending on which particular carrier board the compute module is attached to), however we haven´t managed yet to enter this Subsystem-ID to every PCI-Device in the system, because of missing means to do that by our UEFI-FW. This might will change if we update to latest AGESA version. -Original Message- From: Will Deacon Sent: Montag, 23. November 2020 23:34 To: Deucher, Alexander Cc: linux-ker...@vger.kernel.org; linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; Bjorn Helgaas ; Merger, Edgar [AUTOSOL/MAS/AUGS] ; Joerg Roedel ; Kuehling, Felix Subject: [EXTERNAL] Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken On Mon, Nov 23, 2020 at 09:04:14PM +, Deucher, Alexander wrote: > [AMD Public Use] > > > -Original Message- > > From: Will Deacon > > Sent: Monday, November 23, 2020 8:44 AM > > To: linux-ker...@vger.kernel.org > > Cc: linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; > > Will Deacon ; Bjorn Helgaas ; > > Deucher, Alexander ; Edgar Merger > > ; Joerg Roedel > > Subject: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken > > > > Edgar Merger reports that the AMD Raven GPU does not work reliably > > on his system when the IOMMU is enabled: > > > > | [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > > signaled seq=1, emitted seq=3 > > | [...] > > | amdgpu :0b:00.0: GPU reset begin! > > | AMD-Vi: Completion-Wait loop timed out > > | iommu ivhd0: AMD-Vi: Event logged [IOTLB_INV_TIMEOUT > > device=0b:00.0 address=0x38edc0970] > > > > This is indicative of a hardware/platform configuration issue so, > > since disabling ATS has been shown to resolve the problem, add a > > quirk to match this particular device while Edgar follows-up with AMD for > > more information. > > > > Cc: Bjorn Helgaas > > Cc: Alex Deucher > > Reported-by: Edgar Merger > > Suggested-by: Joerg Roedel > > Link: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__nam11.safelinks.protection.outlook.com_-3Furl-3Dhttps-253A-252F-252Flore=DwIBAg=jOURTkCZzT8tVB5xPEYIm3YJGoxoTaQsQPzPKJGaWbo=BJxhacqqa4K1PJGm6_-862rdSP13_P6LVp7j_9l1xmg=WjiRGepDgI7voSyaAJcvnvZb6gsvZ1fvcnR2tm6bGXg=O1nU-RafBXMAS7Mao5Gtu6o1Xkuj8fg4oHQs74TssuA= > > . > > kernel.org%2Flinux- > > iommu%2FMWHPR10MB1310F042A30661D4158520B589FC0%40MWHPR10M > > B1310.namprd10.prod.outlook.comdata=04%7C01%7Calexander.deuc > > her%40amd.com%7C1a883fe14d0c408e7d9508d88fb5df4e%7C3dd8961fe488 > > 4e608e11a82d994e183d%7C0%7C0%7C637417358593629699%7CUnknown%7 > > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi > > LCJXVCI6Mn0%3D%7C1000sdata=TMgKldWzsX8XZ0l7q3%2BszDWXQJJ > > LOUfX5oGaoLN8n%2B8%3Dreserved=0 > > Signed-off-by: Will Deacon > > --- > > > > Hi all, > > > > Since Joerg is away at the moment, I'm posting this to try to make > > some progress with the thread in the Link: tag. > > + Felix > > What system is this? Can you provide more details? Does a sbios > update fix this? Disabling ATS for all Ravens will break GPU compute > for a lot of people. I'd prefer to just black list this particular > system (e.g., just SSIDs or revision) if possible. Cheers, Alex. I'll have to defer to Edgar for the details, as my understanding from the original thread over at: https://urldefense.proofpoint.com/v2/url?u=https-3A__lore.kernel.org_linux-2Diommu_MWHPR10MB1310CDB6829DDCF5EA84A14689150-40MWHPR10MB1310.namprd10.prod.outlook.com_=DwIBAg=jOURTkCZzT8tVB5xPEYIm3YJGoxoTaQsQPzPKJGaWbo=BJxhacqqa4K1PJGm6_-862rdSP13_P6LVp7j_9l1xmg=WjiRGepDgI7voSyaAJcvnvZb6gsvZ1fvcnR2tm6bGXg=9qyuCqHeOGaY1sKjkzNN5A6ks6PNF7V2M2PPckHyFKk= is that this is a board developed by his company. Edgar -- please can you answer Alex's questions? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv8 0/8] System Cache support for GPU and required SMMU support
On 2020-11-24 00:52, Rob Clark wrote: On Mon, Nov 23, 2020 at 9:01 AM Sai Prakash Ranjan wrote: On 2020-11-23 20:51, Will Deacon wrote: > On Tue, Nov 17, 2020 at 08:00:39PM +0530, Sai Prakash Ranjan wrote: >> Some hardware variants contain a system cache or the last level >> cache(llc). This cache is typically a large block which is shared >> by multiple clients on the SOC. GPU uses the system cache to cache >> both the GPU data buffers(like textures) as well the SMMU pagetables. >> This helps with improved render performance as well as lower power >> consumption by reducing the bus traffic to the system memory. >> >> The system cache architecture allows the cache to be split into slices >> which then be used by multiple SOC clients. This patch series is an >> effort to enable and use two of those slices preallocated for the GPU, >> one for the GPU data buffers and another for the GPU SMMU hardware >> pagetables. >> >> Patch 1 - Patch 6 adds system cache support in SMMU and GPU driver. >> Patch 7 and 8 are minor cleanups for arm-smmu impl. >> >> Changes in v8: >> * Introduce a generic domain attribute for pagetable config (Will) >> * Rename quirk to more generic IO_PGTABLE_QUIRK_ARM_OUTER_WBWA (Will) >> * Move non-strict mode to use new struct domain_attr_io_pgtbl_config >> (Will) > > Modulo some minor comments I've made, this looks good to me. What is > the > plan for merging it? I can take the IOMMU parts, but patches 4-6 touch > the > MSM GPU driver and I'd like to avoid conflicts with that. > SMMU bits are pretty much independent and GPU relies on the domain attribute and the quirk exposed, so as long as SMMU changes go in first it should be good. Rob? I suppose one option would be to split out the patch that adds the attribute into it's own patch, and merge that both thru drm and iommu? Ok I can split out domain attr and quirk into its own patch if Will is fine with that approach. If Will/Robin dislike that approach, I'll pick up the parts of the drm patches which don't depend on the new attribute for v5.11 and the rest for v5.12.. or possibly a second late v5.11 pull req if airlied doesn't hate me too much for it. Going forward, I think we will have one or two more co-dependent series, like the smmu iova fault handler improvements that Jordan posted. So I would like to hear how Will and Robin prefer to handle those. BR, -R Thanks, Sai -- 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
[PATCH] [PATCH] Adding offset keeping option when mapping data via SWIOTLB.
NVMe driver and other applications may depend on the data offset to operate correctly. Currently when unaligned data is mapped via SWIOTLB, the data is mapped as slab aligned with the SWIOTLB. When booting with --swiotlb=force option and using NVMe as interface, running mkfs.xfs on Rhel fails because of the unalignment issue. This patch adds an option to make sure the mapped data preserves its offset of the orginal addrss. Tested on latest kernel that this patch fixes the issue. Signed-off-by: Jianxiong Gao Acked-by: David Rientjes --- drivers/nvme/host/pci.c | 3 ++- include/linux/dma-mapping.h | 8 kernel/dma/swiotlb.c| 13 + 3 files changed, 23 insertions(+), 1 deletion(-) diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c index 0578ff253c47..a366fb8a1ff0 100644 --- a/drivers/nvme/host/pci.c +++ b/drivers/nvme/host/pci.c @@ -833,7 +833,8 @@ static blk_status_t nvme_map_data(struct nvme_dev *dev, struct request *req, iod->nents, rq_dma_dir(req), DMA_ATTR_NO_WARN); else nr_mapped = dma_map_sg_attrs(dev->dev, iod->sg, iod->nents, -rq_dma_dir(req), DMA_ATTR_NO_WARN); + rq_dma_dir(req), + DMA_ATTR_NO_WARN|DMA_ATTR_SWIOTLB_KEEP_OFFSET); if (!nr_mapped) goto out; diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h index 956151052d45..e46d23d9fa20 100644 --- a/include/linux/dma-mapping.h +++ b/include/linux/dma-mapping.h @@ -61,6 +61,14 @@ */ #define DMA_ATTR_PRIVILEGED(1UL << 9) +/* + * DMA_ATTR_SWIOTLB_KEEP_OFFSET: used to indicate that the buffer has to keep + * its offset when mapped via SWIOTLB. Some application functionality depends + * on the address offset, thus when buffers are mapped via SWIOTLB, the offset + * needs to be preserved. + */ +#define DMA_ATTR_SWIOTLB_KEEP_OFFSET (1UL << 10) + /* * A dma_addr_t can hold any valid DMA or bus address for the platform. It can * be given to a device to use as a DMA source or target. It is specific to a diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c index 781b9dca197c..f43d7be1342d 100644 --- a/kernel/dma/swiotlb.c +++ b/kernel/dma/swiotlb.c @@ -483,6 +483,13 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t orig_addr, max_slots = mask + 1 ? ALIGN(mask + 1, 1 << IO_TLB_SHIFT) >> IO_TLB_SHIFT : 1UL << (BITS_PER_LONG - IO_TLB_SHIFT); + + /* +* If we need to keep the offset when mapping, we need to add the offset +* to the total set we need to allocate in SWIOTLB +*/ + if (attrs & DMA_ATTR_SWIOTLB_KEEP_OFFSET) + alloc_size += offset_in_page(orig_addr); /* * For mappings greater than or equal to a page, we limit the stride @@ -567,6 +574,12 @@ phys_addr_t swiotlb_tbl_map_single(struct device *hwdev, phys_addr_t orig_addr, */ for (i = 0; i < nslots; i++) io_tlb_orig_addr[index+i] = orig_addr + (i << IO_TLB_SHIFT); + /* +* When keeping the offset of the original data, we need to advance +* the tlb_addr by the offset of orig_addr. +*/ + if (attrs & DMA_ATTR_SWIOTLB_KEEP_OFFSET) + tlb_addr += orig_addr & (PAGE_SIZE - 1); if (!(attrs & DMA_ATTR_SKIP_CPU_SYNC) && (dir == DMA_TO_DEVICE || dir == DMA_BIDIRECTIONAL)) swiotlb_bounce(orig_addr, tlb_addr, mapping_size, DMA_TO_DEVICE); -- 2.27.0 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
On 2020-11-23 5:33 p.m., Will Deacon wrote: On Mon, Nov 23, 2020 at 09:04:14PM +, Deucher, Alexander wrote: [AMD Public Use] -Original Message- From: Will Deacon Sent: Monday, November 23, 2020 8:44 AM To: linux-ker...@vger.kernel.org Cc: linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; Will Deacon ; Bjorn Helgaas ; Deucher, Alexander ; Edgar Merger ; Joerg Roedel Subject: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken Edgar Merger reports that the AMD Raven GPU does not work reliably on his system when the IOMMU is enabled: | [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=1, emitted seq=3 | [...] | amdgpu :0b:00.0: GPU reset begin! | AMD-Vi: Completion-Wait loop timed out | iommu ivhd0: AMD-Vi: Event logged [IOTLB_INV_TIMEOUT device=0b:00.0 address=0x38edc0970] This is indicative of a hardware/platform configuration issue so, since disabling ATS has been shown to resolve the problem, add a quirk to match this particular device while Edgar follows-up with AMD for more information. Cc: Bjorn Helgaas Cc: Alex Deucher Reported-by: Edgar Merger Suggested-by: Joerg Roedel Link: https://lore. kernel.org/linux- iommu/MWHPR10MB1310F042A30661D4158520B589FC0@MWHPR10M B1310.namprd10.prod.outlook.com her%40amd.com%7C1a883fe14d0c408e7d9508d88fb5df4e%7C3dd8961fe488 4e608e11a82d994e183d%7C0%7C0%7C637417358593629699%7CUnknown%7 CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi LCJXVCI6Mn0%3D%7C1000sdata=TMgKldWzsX8XZ0l7q3%2BszDWXQJJ LOUfX5oGaoLN8n%2B8%3Dreserved=0 Signed-off-by: Will Deacon --- Hi all, Since Joerg is away at the moment, I'm posting this to try to make some progress with the thread in the Link: tag. + Felix What system is this? Can you provide more details? Does a sbios update fix this? Disabling ATS for all Ravens will break GPU compute for a lot of people. I'd prefer to just black list this particular system (e.g., just SSIDs or revision) if possible. +Ray There are already many systems where the IOMMU is disabled in the BIOS, or the CRAT table reporting the APU compute capabilities is broken. Ray has been working on a fallback to make APUs behave like dGPUs on such systems. That should also cover this case where ATS is blacklisted. That said, it affects the programming model, because we don't support the unified and coherent memory model on dGPUs like we do on APUs with IOMMUv2. So it would be good to make the conditions for this workaround as narrow as possible. These are the relevant changes in KFD and Thunk for reference: ### KFD ### commit 914913ab04dfbcd0226ecb6bc99d276832ea2908 Author: Huang Rui Date: Tue Aug 18 14:54:23 2020 +0800 drm/amdkfd: implement the dGPU fallback path for apu (v6) We still have a few iommu issues which need to address, so force raven as "dgpu" path for the moment. This is to add the fallback path to bypass IOMMU if IOMMU v2 is disabled or ACPI CRAT table not correct. v2: Use ignore_crat parameter to decide whether it will go with IOMMUv2. v3: Align with existed thunk, don't change the way of raven, only renoir will use "dgpu" path by default. v4: don't update global ignore_crat in the driver, and revise fallback function if CRAT is broken. v5: refine acpi crat good but no iommu support case, and rename the title. v6: fix the issue of dGPU initialized firstly, just modify the report value in the node_show(). Signed-off-by: Huang Rui Reviewed-by: Felix Kuehling Signed-off-by: Alex Deucher ### Thunk ### commit e32482fa4b9ca398c8bdc303920abfd672592764 Author: Huang Rui Date: Tue Aug 18 18:54:05 2020 +0800 libhsakmt: remove is_dgpu flag in the hsa_gfxip_table Whether use dgpu path will check the props which exposed from kernel. We won't need hard code in the ASIC table. Signed-off-by: Huang Rui Change-Id: I0c018a26b219914a41197ff36dbec7a75945d452 commit 7c60f6d912034aa67ed27b47a29221422423f5cc Author: Huang Rui Date: Thu Jul 30 10:22:23 2020 +0800 libhsakmt: implement the method that using flag which exposed by kfd to configure is_dgpu KFD already implemented the fallback path for APU. Thunk will use flag which exposed by kfd to configure is_dgpu instead of hardcode before. Signed-off-by: Huang Rui Change-Id: I445f6cf668f9484dd06cd9ae1bb3cfe7428ec7eb Regards, Felix Cheers, Alex. I'll have to defer to Edgar for the details, as my understanding from the original thread over at: https://lore.kernel.org/linux-iommu/mwhpr10mb1310cdb6829ddcf5ea84a14689...@mwhpr10mb1310.namprd10.prod.outlook.com/ is that this is a board developed by his company. Edgar -- please can you answer Alex's questions? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6] swiotlb: Adjust SWIOTBL bounce buffer size for SEV guests.
Hello Konrad,
On Mon, Nov 23, 2020 at 12:56:32PM -0500, Konrad Rzeszutek Wilk wrote:
> On Mon, Nov 23, 2020 at 06:06:47PM +0100, Borislav Petkov wrote:
> > On Thu, Nov 19, 2020 at 09:42:05PM +, Ashish Kalra wrote:
> > > From: Ashish Kalra
> > >
> > > For SEV, all DMA to and from guest has to use shared (un-encrypted) pages.
> > > SEV uses SWIOTLB to make this happen without requiring changes to device
> > > drivers. However, depending on workload being run, the default 64MB of
> > > SWIOTLB might not be enough and SWIOTLB may run out of buffers to use
> > > for DMA, resulting in I/O errors and/or performance degradation for
> > > high I/O workloads.
> > >
> > > Increase the default size of SWIOTLB for SEV guests using a minimum
> > > value of 128MB and a maximum value of 512MB, determining on amount
> > > of provisioned guest memory.
> >
> > That sentence needs massaging.
> >
> > > Using late_initcall() interface to invoke swiotlb_adjust() does not
> > > work as the size adjustment needs to be done before mem_encrypt_init()
> > > and reserve_crashkernel() which use the allocated SWIOTLB buffer size,
> > > hence calling it explicitly from setup_arch().
> >
> > "hence call it ... "
> >
> > >
> > > The SWIOTLB default size adjustment is added as an architecture specific
> >
> > "... is added... " needs to be "Add ..."
> >
> > > interface/callback to allow architectures such as those supporting memory
> > > encryption to adjust/expand SWIOTLB size for their use.
> > >
> > > v5 fixed build errors and warnings as
> > > Reported-by: kbuild test robot
> > >
> > > Signed-off-by: Ashish Kalra
> > > ---
> > > arch/x86/kernel/setup.c | 2 ++
> > > arch/x86/mm/mem_encrypt.c | 32
> > > include/linux/swiotlb.h | 6 ++
> > > kernel/dma/swiotlb.c | 24
> > > 4 files changed, 64 insertions(+)
> > >
> > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > > index 3511736fbc74..b073d58dd4a3 100644
> > > --- a/arch/x86/kernel/setup.c
> > > +++ b/arch/x86/kernel/setup.c
> > > @@ -1166,6 +1166,8 @@ void __init setup_arch(char **cmdline_p)
> > > if (boot_cpu_has(X86_FEATURE_GBPAGES))
> > > hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT);
> > >
> > > + swiotlb_adjust();
> > > +
> > > /*
> > >* Reserve memory for crash kernel after SRAT is parsed so that it
> > >* won't consume hotpluggable memory.
> > > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> > > index 3f248f0d0e07..c79a0d761db5 100644
> > > --- a/arch/x86/mm/mem_encrypt.c
> > > +++ b/arch/x86/mm/mem_encrypt.c
> > > @@ -490,6 +490,38 @@ static void print_mem_encrypt_feature_info(void)
> > > }
> > >
> > > /* Architecture __weak replacement functions */
> > > +unsigned long __init arch_swiotlb_adjust(unsigned long
> > > iotlb_default_size)
> > > +{
> > > + unsigned long size = 0;
> >
> > unsigned long size = iotlb_default_size;
> >
> > > +
> > > + /*
> > > + * For SEV, all DMA has to occur via shared/unencrypted pages.
> > > + * SEV uses SWOTLB to make this happen without changing device
> > > + * drivers. However, depending on the workload being run, the
> > > + * default 64MB of SWIOTLB may not be enough & SWIOTLB may
> > ^
> >
> > Use words pls, not "&".
> >
> >
> > > + * run out of buffers for DMA, resulting in I/O errors and/or
> > > + * performance degradation especially with high I/O workloads.
> > > + * Increase the default size of SWIOTLB for SEV guests using
> > > + * a minimum value of 128MB and a maximum value of 512MB,
> > > + * depending on amount of provisioned guest memory.
> > > + */
> > > + if (sev_active()) {
> > > + phys_addr_t total_mem = memblock_phys_mem_size();
> > > +
> > > + if (total_mem <= SZ_1G)
> > > + size = max(iotlb_default_size, (unsigned long) SZ_128M);
> > > + else if (total_mem <= SZ_4G)
> > > + size = max(iotlb_default_size, (unsigned long) SZ_256M);
>
> That is eating 128MB for 1GB, aka 12% of the guest memory allocated
> statically for this.
>
> And for guests that are 2GB, that is 12% until it gets to 3GB when it is 8%
> and then 6% at 4GB.
>
> I would prefer this to be based on your memory count, that is 6% of total
> memory. And then going forward we can allocate memory _after_ boot and then
> stich
> the late SWIOTLB pool and allocate on demand.
>
>
Ok.
As i mentioned earlier, the patch was initially based on using a % of guest
memory,
as below:
+#define SEV_ADJUST_SWIOTLB_SIZE_PERCENT5
+#define SEV_ADJUST_SWIOTLB_SIZE_MAX(1UL << 30)
...
...
+ if (sev_active() && !io_tlb_nslabs) {
+ unsigned long total_mem = get_num_physpages() <<
+ PAGE_SHIFT;
+
+ default_size = total_mem *
+ SEV_ADJUST_SWIOTLB_SIZE_PERCENT / 100;
+
+ default_size = ALIGN(default_size, 1 <<
Re: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
On Mon, Nov 23, 2020 at 09:04:14PM +, Deucher, Alexander wrote: > [AMD Public Use] > > > -Original Message- > > From: Will Deacon > > Sent: Monday, November 23, 2020 8:44 AM > > To: linux-ker...@vger.kernel.org > > Cc: linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; Will > > Deacon ; Bjorn Helgaas ; > > Deucher, Alexander ; Edgar Merger > > ; Joerg Roedel > > Subject: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken > > > > Edgar Merger reports that the AMD Raven GPU does not work reliably on his > > system when the IOMMU is enabled: > > > > | [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > > signaled seq=1, emitted seq=3 > > | [...] > > | amdgpu :0b:00.0: GPU reset begin! > > | AMD-Vi: Completion-Wait loop timed out > > | iommu ivhd0: AMD-Vi: Event logged [IOTLB_INV_TIMEOUT > > device=0b:00.0 address=0x38edc0970] > > > > This is indicative of a hardware/platform configuration issue so, since > > disabling ATS has been shown to resolve the problem, add a quirk to match > > this particular device while Edgar follows-up with AMD for more information. > > > > Cc: Bjorn Helgaas > > Cc: Alex Deucher > > Reported-by: Edgar Merger > > Suggested-by: Joerg Roedel > > Link: > > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore. > > kernel.org%2Flinux- > > iommu%2FMWHPR10MB1310F042A30661D4158520B589FC0%40MWHPR10M > > B1310.namprd10.prod.outlook.comdata=04%7C01%7Calexander.deuc > > her%40amd.com%7C1a883fe14d0c408e7d9508d88fb5df4e%7C3dd8961fe488 > > 4e608e11a82d994e183d%7C0%7C0%7C637417358593629699%7CUnknown%7 > > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi > > LCJXVCI6Mn0%3D%7C1000sdata=TMgKldWzsX8XZ0l7q3%2BszDWXQJJ > > LOUfX5oGaoLN8n%2B8%3Dreserved=0 > > Signed-off-by: Will Deacon > > --- > > > > Hi all, > > > > Since Joerg is away at the moment, I'm posting this to try to make some > > progress with the thread in the Link: tag. > > + Felix > > What system is this? Can you provide more details? Does a sbios update > fix this? Disabling ATS for all Ravens will break GPU compute for a lot > of people. I'd prefer to just black list this particular system (e.g., > just SSIDs or revision) if possible. Cheers, Alex. I'll have to defer to Edgar for the details, as my understanding from the original thread over at: https://lore.kernel.org/linux-iommu/mwhpr10mb1310cdb6829ddcf5ea84a14689...@mwhpr10mb1310.namprd10.prod.outlook.com/ is that this is a board developed by his company. Edgar -- please can you answer Alex's questions? Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken
[AMD Public Use] > -Original Message- > From: Will Deacon > Sent: Monday, November 23, 2020 8:44 AM > To: linux-ker...@vger.kernel.org > Cc: linux-...@vger.kernel.org; iommu@lists.linux-foundation.org; Will > Deacon ; Bjorn Helgaas ; > Deucher, Alexander ; Edgar Merger > ; Joerg Roedel > Subject: [PATCH] PCI: Mark AMD Raven iGPU ATS as broken > > Edgar Merger reports that the AMD Raven GPU does not work reliably on his > system when the IOMMU is enabled: > > | [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, > signaled seq=1, emitted seq=3 > | [...] > | amdgpu :0b:00.0: GPU reset begin! > | AMD-Vi: Completion-Wait loop timed out > | iommu ivhd0: AMD-Vi: Event logged [IOTLB_INV_TIMEOUT > device=0b:00.0 address=0x38edc0970] > > This is indicative of a hardware/platform configuration issue so, since > disabling ATS has been shown to resolve the problem, add a quirk to match > this particular device while Edgar follows-up with AMD for more information. > > Cc: Bjorn Helgaas > Cc: Alex Deucher > Reported-by: Edgar Merger > Suggested-by: Joerg Roedel > Link: > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore. > kernel.org%2Flinux- > iommu%2FMWHPR10MB1310F042A30661D4158520B589FC0%40MWHPR10M > B1310.namprd10.prod.outlook.comdata=04%7C01%7Calexander.deuc > her%40amd.com%7C1a883fe14d0c408e7d9508d88fb5df4e%7C3dd8961fe488 > 4e608e11a82d994e183d%7C0%7C0%7C637417358593629699%7CUnknown%7 > CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwi > LCJXVCI6Mn0%3D%7C1000sdata=TMgKldWzsX8XZ0l7q3%2BszDWXQJJ > LOUfX5oGaoLN8n%2B8%3Dreserved=0 > Signed-off-by: Will Deacon > --- > > Hi all, > > Since Joerg is away at the moment, I'm posting this to try to make some > progress with the thread in the Link: tag. + Felix What system is this? Can you provide more details? Does a sbios update fix this? Disabling ATS for all Ravens will break GPU compute for a lot of people. I'd prefer to just black list this particular system (e.g., just SSIDs or revision) if possible. Alex > > Cheers, > > Will > > drivers/pci/quirks.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index > f70692ac79c5..3911b0ec57ba 100644 > --- a/drivers/pci/quirks.c > +++ b/drivers/pci/quirks.c > @@ -5176,6 +5176,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, > 0x6900, quirk_amd_harvest_no_ats); > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7312, > quirk_amd_harvest_no_ats); > /* AMD Navi14 dGPU */ > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7340, > quirk_amd_harvest_no_ats); > +/* AMD Raven platform iGPU */ > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x15d8, > +quirk_amd_harvest_no_ats); > #endif /* CONFIG_PCI_ATS */ > > /* Freescale PCIe doesn't support MSI in RC mode */ > -- > 2.29.2.454.gaff20da3a2-goog ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv8 0/8] System Cache support for GPU and required SMMU support
On Mon, Nov 23, 2020 at 9:01 AM Sai Prakash Ranjan wrote: > > On 2020-11-23 20:51, Will Deacon wrote: > > On Tue, Nov 17, 2020 at 08:00:39PM +0530, Sai Prakash Ranjan wrote: > >> Some hardware variants contain a system cache or the last level > >> cache(llc). This cache is typically a large block which is shared > >> by multiple clients on the SOC. GPU uses the system cache to cache > >> both the GPU data buffers(like textures) as well the SMMU pagetables. > >> This helps with improved render performance as well as lower power > >> consumption by reducing the bus traffic to the system memory. > >> > >> The system cache architecture allows the cache to be split into slices > >> which then be used by multiple SOC clients. This patch series is an > >> effort to enable and use two of those slices preallocated for the GPU, > >> one for the GPU data buffers and another for the GPU SMMU hardware > >> pagetables. > >> > >> Patch 1 - Patch 6 adds system cache support in SMMU and GPU driver. > >> Patch 7 and 8 are minor cleanups for arm-smmu impl. > >> > >> Changes in v8: > >> * Introduce a generic domain attribute for pagetable config (Will) > >> * Rename quirk to more generic IO_PGTABLE_QUIRK_ARM_OUTER_WBWA (Will) > >> * Move non-strict mode to use new struct domain_attr_io_pgtbl_config > >> (Will) > > > > Modulo some minor comments I've made, this looks good to me. What is > > the > > plan for merging it? I can take the IOMMU parts, but patches 4-6 touch > > the > > MSM GPU driver and I'd like to avoid conflicts with that. > > > > SMMU bits are pretty much independent and GPU relies on the domain > attribute > and the quirk exposed, so as long as SMMU changes go in first it should > be good. > Rob? I suppose one option would be to split out the patch that adds the attribute into it's own patch, and merge that both thru drm and iommu? If Will/Robin dislike that approach, I'll pick up the parts of the drm patches which don't depend on the new attribute for v5.11 and the rest for v5.12.. or possibly a second late v5.11 pull req if airlied doesn't hate me too much for it. Going forward, I think we will have one or two more co-dependent series, like the smmu iova fault handler improvements that Jordan posted. So I would like to hear how Will and Robin prefer to handle those. BR, -R > Thanks, > Sai > > -- > 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 v6] swiotlb: Adjust SWIOTBL bounce buffer size for SEV guests.
On Mon, Nov 23, 2020 at 07:02:15PM +0100, Borislav Petkov wrote: > On Mon, Nov 23, 2020 at 12:56:32PM -0500, Konrad Rzeszutek Wilk wrote: > > This is not going to work for TDX. I think having a registration > > to SWIOTLB to have this function would be better going forward. > > > > As in there will be a swiotlb_register_adjuster() which AMD SEV > > code can call at start, also TDX can do it (and other platforms). > > Oh do tell. It doesn't need to adjust size? I am assuming that TDX is going to have the same exact issue that AMD SEV will have. Are you recommending to have an unified x86 specific callback where we check if it: - CPUID_AMD_SEV or CPUID_INTEL_TDX is set, and - No vIOMMU present, then we adjust the size? > > -- > Regards/Gruss, > Boris. > > https://people.kernel.org/tglx/notes-about-netiquette ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6] swiotlb: Adjust SWIOTBL bounce buffer size for SEV guests.
On Mon, Nov 23, 2020 at 12:56:32PM -0500, Konrad Rzeszutek Wilk wrote: > This is not going to work for TDX. I think having a registration > to SWIOTLB to have this function would be better going forward. > > As in there will be a swiotlb_register_adjuster() which AMD SEV > code can call at start, also TDX can do it (and other platforms). Oh do tell. It doesn't need to adjust size? -- Regards/Gruss, Boris. https://people.kernel.org/tglx/notes-about-netiquette ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6] swiotlb: Adjust SWIOTBL bounce buffer size for SEV guests.
On Mon, Nov 23, 2020 at 06:06:47PM +0100, Borislav Petkov wrote:
> On Thu, Nov 19, 2020 at 09:42:05PM +, Ashish Kalra wrote:
> > From: Ashish Kalra
> >
> > For SEV, all DMA to and from guest has to use shared (un-encrypted) pages.
> > SEV uses SWIOTLB to make this happen without requiring changes to device
> > drivers. However, depending on workload being run, the default 64MB of
> > SWIOTLB might not be enough and SWIOTLB may run out of buffers to use
> > for DMA, resulting in I/O errors and/or performance degradation for
> > high I/O workloads.
> >
> > Increase the default size of SWIOTLB for SEV guests using a minimum
> > value of 128MB and a maximum value of 512MB, determining on amount
> > of provisioned guest memory.
>
> That sentence needs massaging.
>
> > Using late_initcall() interface to invoke swiotlb_adjust() does not
> > work as the size adjustment needs to be done before mem_encrypt_init()
> > and reserve_crashkernel() which use the allocated SWIOTLB buffer size,
> > hence calling it explicitly from setup_arch().
>
> "hence call it ... "
>
> >
> > The SWIOTLB default size adjustment is added as an architecture specific
>
> "... is added... " needs to be "Add ..."
>
> > interface/callback to allow architectures such as those supporting memory
> > encryption to adjust/expand SWIOTLB size for their use.
> >
> > v5 fixed build errors and warnings as
> > Reported-by: kbuild test robot
> >
> > Signed-off-by: Ashish Kalra
> > ---
> > arch/x86/kernel/setup.c | 2 ++
> > arch/x86/mm/mem_encrypt.c | 32
> > include/linux/swiotlb.h | 6 ++
> > kernel/dma/swiotlb.c | 24
> > 4 files changed, 64 insertions(+)
> >
> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> > index 3511736fbc74..b073d58dd4a3 100644
> > --- a/arch/x86/kernel/setup.c
> > +++ b/arch/x86/kernel/setup.c
> > @@ -1166,6 +1166,8 @@ void __init setup_arch(char **cmdline_p)
> > if (boot_cpu_has(X86_FEATURE_GBPAGES))
> > hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT);
> >
> > + swiotlb_adjust();
> > +
> > /*
> > * Reserve memory for crash kernel after SRAT is parsed so that it
> > * won't consume hotpluggable memory.
> > diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> > index 3f248f0d0e07..c79a0d761db5 100644
> > --- a/arch/x86/mm/mem_encrypt.c
> > +++ b/arch/x86/mm/mem_encrypt.c
> > @@ -490,6 +490,38 @@ static void print_mem_encrypt_feature_info(void)
> > }
> >
> > /* Architecture __weak replacement functions */
> > +unsigned long __init arch_swiotlb_adjust(unsigned long iotlb_default_size)
> > +{
> > + unsigned long size = 0;
>
> unsigned long size = iotlb_default_size;
>
> > +
> > + /*
> > +* For SEV, all DMA has to occur via shared/unencrypted pages.
> > +* SEV uses SWOTLB to make this happen without changing device
> > +* drivers. However, depending on the workload being run, the
> > +* default 64MB of SWIOTLB may not be enough & SWIOTLB may
>^
>
> Use words pls, not "&".
>
>
> > +* run out of buffers for DMA, resulting in I/O errors and/or
> > +* performance degradation especially with high I/O workloads.
> > +* Increase the default size of SWIOTLB for SEV guests using
> > +* a minimum value of 128MB and a maximum value of 512MB,
> > +* depending on amount of provisioned guest memory.
> > +*/
> > + if (sev_active()) {
> > + phys_addr_t total_mem = memblock_phys_mem_size();
> > +
> > + if (total_mem <= SZ_1G)
> > + size = max(iotlb_default_size, (unsigned long) SZ_128M);
> > + else if (total_mem <= SZ_4G)
> > + size = max(iotlb_default_size, (unsigned long) SZ_256M);
That is eating 128MB for 1GB, aka 12% of the guest memory allocated statically
for this.
And for guests that are 2GB, that is 12% until it gets to 3GB when it is 8%
and then 6% at 4GB.
I would prefer this to be based on your memory count, that is 6% of total
memory. And then going forward we can allocate memory _after_ boot and then
stich
the late SWIOTLB pool and allocate on demand.
> > + else
> > + size = max(iotlb_default_size, (unsigned long) SZ_512M);
> > +
> > + pr_info("SWIOTLB bounce buffer size adjusted to %luMB for SEV
> > platform",
>
> just "... for SEV" - no need for "platform".
>
> ...
>
> > diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> > index c19379fabd20..3be9a19ea0a5 100644
> > --- a/kernel/dma/swiotlb.c
> > +++ b/kernel/dma/swiotlb.c
> > @@ -163,6 +163,30 @@ unsigned long swiotlb_size_or_default(void)
> > return size ? size : (IO_TLB_DEFAULT_SIZE);
> > }
> >
> > +unsigned long __init __weak arch_swiotlb_adjust(unsigned long size)
> > +{
> > + return 0;
>
> That, of course, needs to return size, not 0.
This is not going to work for TDX. I think
[PATCHv9 8/8] iommu: arm-smmu-impl: Add a space before open parenthesis
Fix the checkpatch warning for space required before the open
parenthesis.
Signed-off-by: Sai Prakash Ranjan
Acked-by: Will Deacon
---
drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
index 26e2734eb4d7..136872e77195 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
@@ -12,7 +12,7 @@
static int arm_smmu_gr0_ns(int offset)
{
- switch(offset) {
+ switch (offset) {
case ARM_SMMU_GR0_sCR0:
case ARM_SMMU_GR0_sACR:
case ARM_SMMU_GR0_sGFSR:
--
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
[PATCHv9 6/8] drm/msm/a6xx: Add support for using system cache on MMU500 based targets
From: Jordan Crouse
GPU targets with an MMU-500 attached have a slightly different process for
enabling system cache. Use the compatible string on the IOMMU phandle
to see if an MMU-500 is attached and modify the programming sequence
accordingly.
Signed-off-by: Jordan Crouse
Signed-off-by: Sai Prakash Ranjan
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 46 +--
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 1 +
2 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 95c98c642876..3f8b92da8cba 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1042,6 +1042,8 @@ static void a6xx_llc_deactivate(struct a6xx_gpu *a6xx_gpu)
static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu)
{
+ struct adreno_gpu *adreno_gpu = _gpu->base;
+ struct msm_gpu *gpu = _gpu->base;
u32 cntl1_regval = 0;
if (IS_ERR(a6xx_gpu->llc_mmio))
@@ -1055,11 +1057,17 @@ static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu)
(gpu_scid << 15) | (gpu_scid << 20);
}
+ /*
+* For targets with a MMU500, activate the slice but don't program the
+* register. The XBL will take care of that.
+*/
if (!llcc_slice_activate(a6xx_gpu->htw_llc_slice)) {
- u32 gpuhtw_scid = llcc_get_slice_id(a6xx_gpu->htw_llc_slice);
+ if (!a6xx_gpu->have_mmu500) {
+ u32 gpuhtw_scid =
llcc_get_slice_id(a6xx_gpu->htw_llc_slice);
- gpuhtw_scid &= 0x1f;
- cntl1_regval |= FIELD_PREP(GENMASK(29, 25), gpuhtw_scid);
+ gpuhtw_scid &= 0x1f;
+ cntl1_regval |= FIELD_PREP(GENMASK(29, 25),
gpuhtw_scid);
+ }
}
if (cntl1_regval) {
@@ -1067,13 +1075,20 @@ static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu)
* Program the slice IDs for the various GPU blocks and GPU MMU
* pagetables
*/
- a6xx_llc_write(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1,
cntl1_regval);
-
- /*
-* Program cacheability overrides to not allocate cache lines on
-* a write miss
-*/
- a6xx_llc_rmw(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0,
0xF, 0x03);
+ if (a6xx_gpu->have_mmu500)
+ gpu_rmw(gpu, REG_A6XX_GBIF_SCACHE_CNTL1, GENMASK(24, 0),
+ cntl1_regval);
+ else {
+ a6xx_llc_write(a6xx_gpu,
+ REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1,
cntl1_regval);
+
+ /*
+* Program cacheability overrides to not allocate cache
+* lines on a write miss
+*/
+ a6xx_llc_rmw(a6xx_gpu,
+ REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0, 0xF,
0x03);
+ }
}
}
@@ -1086,10 +1101,21 @@ static void a6xx_llc_slices_destroy(struct a6xx_gpu
*a6xx_gpu)
static void a6xx_llc_slices_init(struct platform_device *pdev,
struct a6xx_gpu *a6xx_gpu)
{
+ struct device_node *phandle;
+
a6xx_gpu->llc_mmio = msm_ioremap(pdev, "cx_mem", "gpu_cx");
if (IS_ERR(a6xx_gpu->llc_mmio))
return;
+ /*
+* There is a different programming path for targets with an mmu500
+* attached, so detect if that is the case
+*/
+ phandle = of_parse_phandle(pdev->dev.of_node, "iommus", 0);
+ a6xx_gpu->have_mmu500 = (phandle &&
+ of_device_is_compatible(phandle, "arm,mmu-500"));
+ of_node_put(phandle);
+
a6xx_gpu->llc_slice = llcc_slice_getd(LLCC_GPU);
a6xx_gpu->htw_llc_slice = llcc_slice_getd(LLCC_GPUHTW);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index 9e6079af679c..e793d329e77b 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -32,6 +32,7 @@ struct a6xx_gpu {
void __iomem *llc_mmio;
void *llc_slice;
void *htw_llc_slice;
+ bool have_mmu500;
};
#define to_a6xx_gpu(x) container_of(x, struct a6xx_gpu, base)
--
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
[PATCHv9 7/8] iommu: arm-smmu-impl: Use table to list QCOM implementations
Use table and of_match_node() to match qcom implementation
instead of multiple of_device_compatible() calls for each
QCOM SMMU implementation.
Signed-off-by: Sai Prakash Ranjan
Acked-by: Will Deacon
---
drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 9 +
drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 21 -
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 -
3 files changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
index 7fed89c9d18a..26e2734eb4d7 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-impl.c
@@ -214,14 +214,7 @@ struct arm_smmu_device *arm_smmu_impl_init(struct
arm_smmu_device *smmu)
if (of_device_is_compatible(np, "nvidia,tegra194-smmu"))
return nvidia_smmu_impl_init(smmu);
- if (of_device_is_compatible(np, "qcom,sdm845-smmu-500") ||
- of_device_is_compatible(np, "qcom,sc7180-smmu-500") ||
- of_device_is_compatible(np, "qcom,sm8150-smmu-500") ||
- of_device_is_compatible(np, "qcom,sm8250-smmu-500"))
- return qcom_smmu_impl_init(smmu);
-
- if (of_device_is_compatible(smmu->dev->of_node, "qcom,adreno-smmu"))
- return qcom_adreno_smmu_impl_init(smmu);
+ smmu = qcom_smmu_impl_init(smmu);
if (of_device_is_compatible(np, "marvell,ap806-smmu-500"))
smmu->impl = _mmu500_impl;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
index d0636c803a36..add1859b2899 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c
@@ -318,12 +318,23 @@ static struct arm_smmu_device *qcom_smmu_create(struct
arm_smmu_device *smmu,
return >smmu;
}
+static const struct of_device_id __maybe_unused qcom_smmu_impl_of_match[] = {
+ { .compatible = "qcom,sc7180-smmu-500" },
+ { .compatible = "qcom,sdm845-smmu-500" },
+ { .compatible = "qcom,sm8150-smmu-500" },
+ { .compatible = "qcom,sm8250-smmu-500" },
+ { }
+};
+
struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu)
{
- return qcom_smmu_create(smmu, _smmu_impl);
-}
+ const struct device_node *np = smmu->dev->of_node;
-struct arm_smmu_device *qcom_adreno_smmu_impl_init(struct arm_smmu_device
*smmu)
-{
- return qcom_smmu_create(smmu, _adreno_smmu_impl);
+ if (of_match_node(qcom_smmu_impl_of_match, np))
+ return qcom_smmu_create(smmu, _smmu_impl);
+
+ if (of_device_is_compatible(np, "qcom,adreno-smmu"))
+ return qcom_smmu_create(smmu, _adreno_smmu_impl);
+
+ return smmu;
}
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index cb7ca3a444c9..d2a2d1bc58ba 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -523,7 +523,6 @@ static inline void arm_smmu_writeq(struct arm_smmu_device
*smmu, int page,
struct arm_smmu_device *arm_smmu_impl_init(struct arm_smmu_device *smmu);
struct arm_smmu_device *nvidia_smmu_impl_init(struct arm_smmu_device *smmu);
struct arm_smmu_device *qcom_smmu_impl_init(struct arm_smmu_device *smmu);
-struct arm_smmu_device *qcom_adreno_smmu_impl_init(struct arm_smmu_device
*smmu);
void arm_smmu_write_context_bank(struct arm_smmu_device *smmu, int idx);
int arm_mmu500_reset(struct arm_smmu_device *smmu);
--
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
[PATCHv9 5/8] drm/msm/a6xx: Add support for using system cache(LLC)
From: Sharat Masetty
The last level system cache can be partitioned to 32 different
slices of which GPU has two slices preallocated. One slice is
used for caching GPU buffers and the other slice is used for
caching the GPU SMMU pagetables. This talks to the core system
cache driver to acquire the slice handles, configure the SCID's
to those slices and activates and deactivates the slices upon
GPU power collapse and restore.
Some support from the IOMMU driver is also needed to make use
of the system cache to set the right TCR attributes. GPU then
has the ability to override a few cacheability parameters which
it does to override write-allocate to write-no-allocate as the
GPU hardware does not benefit much from it.
DOMAIN_ATTR_IO_PGTABLE_CFG is another domain level attribute used
by the IOMMU driver for pagetable configuration which will be used
to set a quirk initially to set the right attributes to cache the
hardware pagetables into the system cache.
Signed-off-by: Sharat Masetty
[saiprakash.ranjan: fix to set attr before device attach to iommu and rebase]
Signed-off-by: Sai Prakash Ranjan
---
drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 83 +
drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 4 ++
drivers/gpu/drm/msm/adreno/adreno_gpu.c | 17 +
3 files changed, 104 insertions(+)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 948f3656c20c..95c98c642876 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -8,7 +8,9 @@
#include "a6xx_gpu.h"
#include "a6xx_gmu.xml.h"
+#include
#include
+#include
#define GPU_PAS_ID 13
@@ -1022,6 +1024,79 @@ static irqreturn_t a6xx_irq(struct msm_gpu *gpu)
return IRQ_HANDLED;
}
+static void a6xx_llc_rmw(struct a6xx_gpu *a6xx_gpu, u32 reg, u32 mask, u32 or)
+{
+ return msm_rmw(a6xx_gpu->llc_mmio + (reg << 2), mask, or);
+}
+
+static void a6xx_llc_write(struct a6xx_gpu *a6xx_gpu, u32 reg, u32 value)
+{
+ return msm_writel(value, a6xx_gpu->llc_mmio + (reg << 2));
+}
+
+static void a6xx_llc_deactivate(struct a6xx_gpu *a6xx_gpu)
+{
+ llcc_slice_deactivate(a6xx_gpu->llc_slice);
+ llcc_slice_deactivate(a6xx_gpu->htw_llc_slice);
+}
+
+static void a6xx_llc_activate(struct a6xx_gpu *a6xx_gpu)
+{
+ u32 cntl1_regval = 0;
+
+ if (IS_ERR(a6xx_gpu->llc_mmio))
+ return;
+
+ if (!llcc_slice_activate(a6xx_gpu->llc_slice)) {
+ u32 gpu_scid = llcc_get_slice_id(a6xx_gpu->llc_slice);
+
+ gpu_scid &= 0x1f;
+ cntl1_regval = (gpu_scid << 0) | (gpu_scid << 5) | (gpu_scid <<
10) |
+ (gpu_scid << 15) | (gpu_scid << 20);
+ }
+
+ if (!llcc_slice_activate(a6xx_gpu->htw_llc_slice)) {
+ u32 gpuhtw_scid = llcc_get_slice_id(a6xx_gpu->htw_llc_slice);
+
+ gpuhtw_scid &= 0x1f;
+ cntl1_regval |= FIELD_PREP(GENMASK(29, 25), gpuhtw_scid);
+ }
+
+ if (cntl1_regval) {
+ /*
+* Program the slice IDs for the various GPU blocks and GPU MMU
+* pagetables
+*/
+ a6xx_llc_write(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_1,
cntl1_regval);
+
+ /*
+* Program cacheability overrides to not allocate cache lines on
+* a write miss
+*/
+ a6xx_llc_rmw(a6xx_gpu, REG_A6XX_CX_MISC_SYSTEM_CACHE_CNTL_0,
0xF, 0x03);
+ }
+}
+
+static void a6xx_llc_slices_destroy(struct a6xx_gpu *a6xx_gpu)
+{
+ llcc_slice_putd(a6xx_gpu->llc_slice);
+ llcc_slice_putd(a6xx_gpu->htw_llc_slice);
+}
+
+static void a6xx_llc_slices_init(struct platform_device *pdev,
+ struct a6xx_gpu *a6xx_gpu)
+{
+ a6xx_gpu->llc_mmio = msm_ioremap(pdev, "cx_mem", "gpu_cx");
+ if (IS_ERR(a6xx_gpu->llc_mmio))
+ return;
+
+ a6xx_gpu->llc_slice = llcc_slice_getd(LLCC_GPU);
+ a6xx_gpu->htw_llc_slice = llcc_slice_getd(LLCC_GPUHTW);
+
+ if (IS_ERR(a6xx_gpu->llc_slice) && IS_ERR(a6xx_gpu->htw_llc_slice))
+ a6xx_gpu->llc_mmio = ERR_PTR(-EINVAL);
+}
+
static int a6xx_pm_resume(struct msm_gpu *gpu)
{
struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
@@ -1038,6 +1113,8 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
msm_gpu_resume_devfreq(gpu);
+ a6xx_llc_activate(a6xx_gpu);
+
return 0;
}
@@ -1048,6 +1125,8 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
trace_msm_gpu_suspend(0);
+ a6xx_llc_deactivate(a6xx_gpu);
+
devfreq_suspend_device(gpu->devfreq.devfreq);
return a6xx_gmu_stop(a6xx_gpu);
@@ -1091,6 +1170,8 @@ static void a6xx_destroy(struct msm_gpu *gpu)
drm_gem_object_put(a6xx_gpu->shadow_bo);
}
+ a6xx_llc_slices_destroy(a6xx_gpu);
+
a6xx_gmu_remove(a6xx_gpu);
[PATCHv9 4/8] drm/msm: rearrange the gpu_rmw() function
From: Sharat Masetty
The register read-modify-write construct is generic enough
that it can be used by other subsystems as needed, create
a more generic rmw() function and have the gpu_rmw() use
this new function.
Signed-off-by: Sharat Masetty
Reviewed-by: Jordan Crouse
Signed-off-by: Sai Prakash Ranjan
---
drivers/gpu/drm/msm/msm_drv.c | 8
drivers/gpu/drm/msm/msm_drv.h | 1 +
drivers/gpu/drm/msm/msm_gpu.h | 5 +
3 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 49685571dc0e..a1e22b974b77 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -180,6 +180,14 @@ u32 msm_readl(const void __iomem *addr)
return val;
}
+void msm_rmw(void __iomem *addr, u32 mask, u32 or)
+{
+ u32 val = msm_readl(addr);
+
+ val &= ~mask;
+ msm_writel(val | or, addr);
+}
+
struct msm_vblank_work {
struct work_struct work;
int crtc_id;
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index b9dd8f8f4887..655b3b0424a1 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -478,6 +478,7 @@ void __iomem *msm_ioremap_quiet(struct platform_device
*pdev, const char *name,
const char *dbgname);
void msm_writel(u32 data, void __iomem *addr);
u32 msm_readl(const void __iomem *addr);
+void msm_rmw(void __iomem *addr, u32 mask, u32 or);
struct msm_gpu_submitqueue;
int msm_submitqueue_init(struct drm_device *drm, struct msm_file_private *ctx);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 6c9e1fdc1a76..b2b419277953 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -246,10 +246,7 @@ static inline u32 gpu_read(struct msm_gpu *gpu, u32 reg)
static inline void gpu_rmw(struct msm_gpu *gpu, u32 reg, u32 mask, u32 or)
{
- uint32_t val = gpu_read(gpu, reg);
-
- val &= ~mask;
- gpu_write(gpu, reg, val | or);
+ msm_rmw(gpu->mmio + (reg << 2), mask, or);
}
static inline u64 gpu_read64(struct msm_gpu *gpu, u32 lo, u32 hi)
--
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 v6] swiotlb: Adjust SWIOTBL bounce buffer size for SEV guests.
On Thu, Nov 19, 2020 at 09:42:05PM +, Ashish Kalra wrote:
> From: Ashish Kalra
>
> For SEV, all DMA to and from guest has to use shared (un-encrypted) pages.
> SEV uses SWIOTLB to make this happen without requiring changes to device
> drivers. However, depending on workload being run, the default 64MB of
> SWIOTLB might not be enough and SWIOTLB may run out of buffers to use
> for DMA, resulting in I/O errors and/or performance degradation for
> high I/O workloads.
>
> Increase the default size of SWIOTLB for SEV guests using a minimum
> value of 128MB and a maximum value of 512MB, determining on amount
> of provisioned guest memory.
That sentence needs massaging.
> Using late_initcall() interface to invoke swiotlb_adjust() does not
> work as the size adjustment needs to be done before mem_encrypt_init()
> and reserve_crashkernel() which use the allocated SWIOTLB buffer size,
> hence calling it explicitly from setup_arch().
"hence call it ... "
>
> The SWIOTLB default size adjustment is added as an architecture specific
"... is added... " needs to be "Add ..."
> interface/callback to allow architectures such as those supporting memory
> encryption to adjust/expand SWIOTLB size for their use.
>
> v5 fixed build errors and warnings as
> Reported-by: kbuild test robot
>
> Signed-off-by: Ashish Kalra
> ---
> arch/x86/kernel/setup.c | 2 ++
> arch/x86/mm/mem_encrypt.c | 32
> include/linux/swiotlb.h | 6 ++
> kernel/dma/swiotlb.c | 24
> 4 files changed, 64 insertions(+)
>
> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
> index 3511736fbc74..b073d58dd4a3 100644
> --- a/arch/x86/kernel/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1166,6 +1166,8 @@ void __init setup_arch(char **cmdline_p)
> if (boot_cpu_has(X86_FEATURE_GBPAGES))
> hugetlb_cma_reserve(PUD_SHIFT - PAGE_SHIFT);
>
> + swiotlb_adjust();
> +
> /*
>* Reserve memory for crash kernel after SRAT is parsed so that it
>* won't consume hotpluggable memory.
> diff --git a/arch/x86/mm/mem_encrypt.c b/arch/x86/mm/mem_encrypt.c
> index 3f248f0d0e07..c79a0d761db5 100644
> --- a/arch/x86/mm/mem_encrypt.c
> +++ b/arch/x86/mm/mem_encrypt.c
> @@ -490,6 +490,38 @@ static void print_mem_encrypt_feature_info(void)
> }
>
> /* Architecture __weak replacement functions */
> +unsigned long __init arch_swiotlb_adjust(unsigned long iotlb_default_size)
> +{
> + unsigned long size = 0;
unsigned long size = iotlb_default_size;
> +
> + /*
> + * For SEV, all DMA has to occur via shared/unencrypted pages.
> + * SEV uses SWOTLB to make this happen without changing device
> + * drivers. However, depending on the workload being run, the
> + * default 64MB of SWIOTLB may not be enough & SWIOTLB may
^
Use words pls, not "&".
> + * run out of buffers for DMA, resulting in I/O errors and/or
> + * performance degradation especially with high I/O workloads.
> + * Increase the default size of SWIOTLB for SEV guests using
> + * a minimum value of 128MB and a maximum value of 512MB,
> + * depending on amount of provisioned guest memory.
> + */
> + if (sev_active()) {
> + phys_addr_t total_mem = memblock_phys_mem_size();
> +
> + if (total_mem <= SZ_1G)
> + size = max(iotlb_default_size, (unsigned long) SZ_128M);
> + else if (total_mem <= SZ_4G)
> + size = max(iotlb_default_size, (unsigned long) SZ_256M);
> + else
> + size = max(iotlb_default_size, (unsigned long) SZ_512M);
> +
> + pr_info("SWIOTLB bounce buffer size adjusted to %luMB for SEV
> platform",
just "... for SEV" - no need for "platform".
...
> diff --git a/kernel/dma/swiotlb.c b/kernel/dma/swiotlb.c
> index c19379fabd20..3be9a19ea0a5 100644
> --- a/kernel/dma/swiotlb.c
> +++ b/kernel/dma/swiotlb.c
> @@ -163,6 +163,30 @@ unsigned long swiotlb_size_or_default(void)
> return size ? size : (IO_TLB_DEFAULT_SIZE);
> }
>
> +unsigned long __init __weak arch_swiotlb_adjust(unsigned long size)
> +{
> + return 0;
That, of course, needs to return size, not 0.
> +}
> +
> +void __init swiotlb_adjust(void)
> +{
> + unsigned long size;
> +
> + /*
> + * If swiotlb parameter has not been specified, give a chance to
> + * architectures such as those supporting memory encryption to
> + * adjust/expand SWIOTLB size for their use.
> + */
And when you preset the function-local argument "size" with the size
coming in as the size argument of arch_swiotlb_adjust()...
> + if (!io_tlb_nslabs) {
> + size = arch_swiotlb_adjust(IO_TLB_DEFAULT_SIZE);
> + if (size) {
... you don't have to do if (size) here either but simply use size to
compute io_tlb_nslabs, I'd say.
Thx.
--
[PATCHv9 3/8] iommu/arm-smmu: Move non-strict mode to use io_pgtable_domain_attr
Now that we have a struct io_pgtable_domain_attr with quirks,
use that for non_strict mode as well thereby removing the need
for more members of arm_smmu_domain in the future.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 8 +++-
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 -
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 4b9b10fe50ed..f56f266ebdf7 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -786,9 +786,6 @@ static int arm_smmu_init_domain_context(struct iommu_domain
*domain,
goto out_clear_smmu;
}
- if (smmu_domain->non_strict)
- pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
-
if (smmu_domain->pgtbl_cfg.quirks)
pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
@@ -1527,7 +1524,8 @@ static int arm_smmu_domain_get_attr(struct iommu_domain
*domain,
case IOMMU_DOMAIN_DMA:
switch (attr) {
case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- *(int *)data = smmu_domain->non_strict;
+ if (smmu_domain->pgtbl_cfg.quirks &
IO_PGTABLE_QUIRK_NON_STRICT)
+ *(int *)data = smmu_domain->pgtbl_cfg.quirks;
return 0;
default:
return -ENODEV;
@@ -1578,7 +1576,7 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
case IOMMU_DOMAIN_DMA:
switch (attr) {
case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- smmu_domain->non_strict = *(int *)data;
+ smmu_domain->pgtbl_cfg.quirks |=
IO_PGTABLE_QUIRK_NON_STRICT;
break;
default:
ret = -ENODEV;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index bb5a419f240f..cb7ca3a444c9 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -368,7 +368,6 @@ struct arm_smmu_domain {
const struct iommu_flush_ops*flush_ops;
struct arm_smmu_cfg cfg;
enum arm_smmu_domain_stage stage;
- boolnon_strict;
struct mutexinit_mutex; /* Protects smmu pointer */
spinlock_t cb_lock; /* Serialises ATS1* ops and
TLB syncs */
struct iommu_domain domain;
--
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
[PATCHv9 0/8] System Cache support for GPU and required SMMU support
Some hardware variants contain a system cache or the last level cache(llc). This cache is typically a large block which is shared by multiple clients on the SOC. GPU uses the system cache to cache both the GPU data buffers(like textures) as well the SMMU pagetables. This helps with improved render performance as well as lower power consumption by reducing the bus traffic to the system memory. The system cache architecture allows the cache to be split into slices which then be used by multiple SOC clients. This patch series is an effort to enable and use two of those slices preallocated for the GPU, one for the GPU data buffers and another for the GPU SMMU hardware pagetables. Patch 1 - Patch 6 adds system cache support in SMMU and GPU driver. Patch 7 and 8 are minor cleanups for arm-smmu impl. Changes in v9: * Change name from domain_attr_io_pgtbl_cfg to io_pgtable_domain_attr (Will) * Modify comment for the quirk as suggested (Will) * Compare with IO_PGTABLE_QUIRK_NON_STRICT for non-strict mode (Will) Changes in v8: * Introduce a generic domain attribute for pagetable config (Will) * Rename quirk to more generic IO_PGTABLE_QUIRK_ARM_OUTER_WBWA (Will) * Move non-strict mode to use new struct domain_attr_io_pgtbl_config (Will) Changes in v7: * Squash Jordan's patch to support MMU500 targets * Rebase on top of for-joerg/arm-smmu/updates and Jordan's short series for adreno-smmu impl Changes in v6: * Move table to arm-smmu-qcom (Robin) Changes in v5: * Drop cleanup of blank lines since it was intentional (Robin) * Rebase again on top of msm-next-pgtables as it moves pretty fast Changes in v4: * Drop IOMMU_SYS_CACHE prot flag * Rebase on top of https://gitlab.freedesktop.org/drm/msm/-/tree/msm-next-pgtables Changes in v3: * Fix domain attribute setting to before iommu_attach_device() * Fix few code style and checkpatch warnings * Rebase on top of Jordan's latest split pagetables and per-instance pagetables support Changes in v2: * Addressed review comments and rebased on top of Jordan's split pagetables series Jordan Crouse (1): drm/msm/a6xx: Add support for using system cache on MMU500 based targets Sai Prakash Ranjan (5): iommu/io-pgtable-arm: Add support to use system cache iommu/arm-smmu: Add domain attribute for pagetable configuration iommu/arm-smmu: Move non-strict mode to use io_pgtable_domain_attr iommu: arm-smmu-impl: Use table to list QCOM implementations iommu: arm-smmu-impl: Add a space before open parenthesis Sharat Masetty (2): drm/msm: rearrange the gpu_rmw() function drm/msm/a6xx: Add support for using system cache(LLC) drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 109 + drivers/gpu/drm/msm/adreno/a6xx_gpu.h | 5 + drivers/gpu/drm/msm/adreno/adreno_gpu.c| 17 drivers/gpu/drm/msm/msm_drv.c | 8 ++ drivers/gpu/drm/msm/msm_drv.h | 1 + drivers/gpu/drm/msm/msm_gpu.h | 5 +- drivers/iommu/arm/arm-smmu/arm-smmu-impl.c | 11 +-- drivers/iommu/arm/arm-smmu/arm-smmu-qcom.c | 21 +++- drivers/iommu/arm/arm-smmu/arm-smmu.c | 26 - drivers/iommu/arm/arm-smmu/arm-smmu.h | 3 +- drivers/iommu/io-pgtable-arm.c | 10 +- include/linux/io-pgtable.h | 8 ++ include/linux/iommu.h | 1 + 13 files changed, 199 insertions(+), 26 deletions(-) base-commit: a29bbb0861f487a5e144dc997a9f71a36c7a2404 -- 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
[PATCHv9 2/8] iommu/arm-smmu: Add domain attribute for pagetable configuration
Add iommu domain attribute for pagetable configuration which
initially will be used to set quirks like for system cache aka
last level cache to be used by client drivers like GPU to set
right attributes for caching the hardware pagetables into the
system cache and later can be extended to include other page
table configuration data.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 20
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
include/linux/io-pgtable.h| 4
include/linux/iommu.h | 1 +
4 files changed, 26 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 0f28a8614da3..4b9b10fe50ed 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -789,6 +789,9 @@ static int arm_smmu_init_domain_context(struct iommu_domain
*domain,
if (smmu_domain->non_strict)
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+ if (smmu_domain->pgtbl_cfg.quirks)
+ pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
+
pgtbl_ops = alloc_io_pgtable_ops(fmt, _cfg, smmu_domain);
if (!pgtbl_ops) {
ret = -ENOMEM;
@@ -1511,6 +1514,12 @@ static int arm_smmu_domain_get_attr(struct iommu_domain
*domain,
case DOMAIN_ATTR_NESTING:
*(int *)data = (smmu_domain->stage ==
ARM_SMMU_DOMAIN_NESTED);
return 0;
+ case DOMAIN_ATTR_IO_PGTABLE_CFG: {
+ struct io_pgtable_domain_attr *pgtbl_cfg = data;
+ *pgtbl_cfg = smmu_domain->pgtbl_cfg;
+
+ return 0;
+ }
default:
return -ENODEV;
}
@@ -1551,6 +1560,17 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
else
smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
break;
+ case DOMAIN_ATTR_IO_PGTABLE_CFG: {
+ struct io_pgtable_domain_attr *pgtbl_cfg = data;
+
+ if (smmu_domain->smmu) {
+ ret = -EPERM;
+ goto out_unlock;
+ }
+
+ smmu_domain->pgtbl_cfg = *pgtbl_cfg;
+ break;
+ }
default:
ret = -ENODEV;
}
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 04288b6fc619..bb5a419f240f 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -364,6 +364,7 @@ enum arm_smmu_domain_stage {
struct arm_smmu_domain {
struct arm_smmu_device *smmu;
struct io_pgtable_ops *pgtbl_ops;
+ struct io_pgtable_domain_attr pgtbl_cfg;
const struct iommu_flush_ops*flush_ops;
struct arm_smmu_cfg cfg;
enum arm_smmu_domain_stage stage;
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index 6b8bb4f4afef..fb4d5a763e0c 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -212,6 +212,10 @@ struct io_pgtable {
#define io_pgtable_ops_to_pgtable(x) container_of((x), struct io_pgtable, ops)
+struct io_pgtable_domain_attr {
+ unsigned long quirks;
+};
+
static inline void io_pgtable_tlb_flush_all(struct io_pgtable *iop)
{
iop->cfg.tlb->tlb_flush_all(iop->cookie);
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index b95a6f8db6ff..ffaa389ea128 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -118,6 +118,7 @@ enum iommu_attr {
DOMAIN_ATTR_FSL_PAMUV1,
DOMAIN_ATTR_NESTING,/* two stages of translation */
DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE,
+ DOMAIN_ATTR_IO_PGTABLE_CFG,
DOMAIN_ATTR_MAX,
};
--
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
[PATCHv9 1/8] iommu/io-pgtable-arm: Add support to use system cache
Add a quirk IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to override
the outer-cacheability attributes set in the TCR for a
non-coherent page table walker when using system cache.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/io-pgtable-arm.c | 10 --
include/linux/io-pgtable.h | 4
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index a7a9bc08dcd1..7c9ea9d7874a 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -761,7 +761,8 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg,
void *cookie)
if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NON_STRICT |
- IO_PGTABLE_QUIRK_ARM_TTBR1))
+ IO_PGTABLE_QUIRK_ARM_TTBR1 |
+ IO_PGTABLE_QUIRK_ARM_OUTER_WBWA))
return NULL;
data = arm_lpae_alloc_pgtable(cfg);
@@ -773,10 +774,15 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg,
void *cookie)
tcr->sh = ARM_LPAE_TCR_SH_IS;
tcr->irgn = ARM_LPAE_TCR_RGN_WBWA;
tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
+ if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA)
+ goto out_free_data;
} else {
tcr->sh = ARM_LPAE_TCR_SH_OS;
tcr->irgn = ARM_LPAE_TCR_RGN_NC;
- tcr->orgn = ARM_LPAE_TCR_RGN_NC;
+ if (!(cfg->quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA))
+ tcr->orgn = ARM_LPAE_TCR_RGN_NC;
+ else
+ tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
}
tg1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1;
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index 4cde111e425b..6b8bb4f4afef 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -86,6 +86,9 @@ struct io_pgtable_cfg {
*
* IO_PGTABLE_QUIRK_ARM_TTBR1: (ARM LPAE format) Configure the table
* for use in the upper half of a split address space.
+*
+* IO_PGTABLE_QUIRK_ARM_OUTER_WBWA: Override the outer-cacheability
+* attributes set in the TCR for a non-coherent page-table walker.
*/
#define IO_PGTABLE_QUIRK_ARM_NS BIT(0)
#define IO_PGTABLE_QUIRK_NO_PERMS BIT(1)
@@ -93,6 +96,7 @@ struct io_pgtable_cfg {
#define IO_PGTABLE_QUIRK_ARM_MTK_EXTBIT(3)
#define IO_PGTABLE_QUIRK_NON_STRICT BIT(4)
#define IO_PGTABLE_QUIRK_ARM_TTBR1 BIT(5)
+ #define IO_PGTABLE_QUIRK_ARM_OUTER_WBWA BIT(6)
unsigned long quirks;
unsigned long pgsize_bitmap;
unsigned intias;
--
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: [PATCHv8 0/8] System Cache support for GPU and required SMMU support
On 2020-11-23 20:51, Will Deacon wrote: On Tue, Nov 17, 2020 at 08:00:39PM +0530, Sai Prakash Ranjan wrote: Some hardware variants contain a system cache or the last level cache(llc). This cache is typically a large block which is shared by multiple clients on the SOC. GPU uses the system cache to cache both the GPU data buffers(like textures) as well the SMMU pagetables. This helps with improved render performance as well as lower power consumption by reducing the bus traffic to the system memory. The system cache architecture allows the cache to be split into slices which then be used by multiple SOC clients. This patch series is an effort to enable and use two of those slices preallocated for the GPU, one for the GPU data buffers and another for the GPU SMMU hardware pagetables. Patch 1 - Patch 6 adds system cache support in SMMU and GPU driver. Patch 7 and 8 are minor cleanups for arm-smmu impl. Changes in v8: * Introduce a generic domain attribute for pagetable config (Will) * Rename quirk to more generic IO_PGTABLE_QUIRK_ARM_OUTER_WBWA (Will) * Move non-strict mode to use new struct domain_attr_io_pgtbl_config (Will) Modulo some minor comments I've made, this looks good to me. What is the plan for merging it? I can take the IOMMU parts, but patches 4-6 touch the MSM GPU driver and I'd like to avoid conflicts with that. SMMU bits are pretty much independent and GPU relies on the domain attribute and the quirk exposed, so as long as SMMU changes go in first it should be good. Rob? Thanks, Sai -- 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: [PATCHv8 3/8] iommu/arm-smmu: Move non-strict mode to use domain_attr_io_pgtbl_cfg
On 2020-11-23 20:49, Will Deacon wrote:
On Tue, Nov 17, 2020 at 08:00:42PM +0530, Sai Prakash Ranjan wrote:
Now that we have a struct domain_attr_io_pgtbl_cfg with quirks,
use that for non_strict mode as well thereby removing the need
for more members of arm_smmu_domain in the future.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 7 ++-
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 -
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 7b05782738e2..5f066a1b7221 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -786,9 +786,6 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
goto out_clear_smmu;
}
- if (smmu_domain->non_strict)
- pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
-
if (smmu_domain->pgtbl_cfg.quirks)
pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
@@ -1527,7 +1524,7 @@ static int arm_smmu_domain_get_attr(struct
iommu_domain *domain,
case IOMMU_DOMAIN_DMA:
switch (attr) {
case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
- *(int *)data = smmu_domain->non_strict;
+ *(int *)data = smmu_domain->pgtbl_cfg.quirks;
Probably better to compare with IO_PGTABLE_QUIRK_NON_STRICT here even
though
we only support this one quirk for DMA domains atm.
Ok will do, thanks.
--
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: [PATCHv8 2/8] iommu/arm-smmu: Add domain attribute for pagetable configuration
On 2020-11-23 20:48, Will Deacon wrote:
On Tue, Nov 17, 2020 at 08:00:41PM +0530, Sai Prakash Ranjan wrote:
Add iommu domain attribute for pagetable configuration which
initially will be used to set quirks like for system cache aka
last level cache to be used by client drivers like GPU to set
right attributes for caching the hardware pagetables into the
system cache and later can be extended to include other page
table configuration data.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/arm/arm-smmu/arm-smmu.c | 25 +
drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
include/linux/io-pgtable.h| 4
include/linux/iommu.h | 1 +
4 files changed, 31 insertions(+)
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 0f28a8614da3..7b05782738e2 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -789,6 +789,9 @@ static int arm_smmu_init_domain_context(struct
iommu_domain *domain,
if (smmu_domain->non_strict)
pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
+ if (smmu_domain->pgtbl_cfg.quirks)
+ pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
+
pgtbl_ops = alloc_io_pgtable_ops(fmt, _cfg, smmu_domain);
if (!pgtbl_ops) {
ret = -ENOMEM;
@@ -1511,6 +1514,12 @@ static int arm_smmu_domain_get_attr(struct
iommu_domain *domain,
case DOMAIN_ATTR_NESTING:
*(int *)data = (smmu_domain->stage ==
ARM_SMMU_DOMAIN_NESTED);
return 0;
+ case DOMAIN_ATTR_IO_PGTABLE_CFG: {
+ struct domain_attr_io_pgtbl_cfg *pgtbl_cfg = data;
+ *pgtbl_cfg = smmu_domain->pgtbl_cfg;
+
+ return 0;
+ }
default:
return -ENODEV;
}
@@ -1551,6 +1560,22 @@ static int arm_smmu_domain_set_attr(struct
iommu_domain *domain,
else
smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
break;
+ case DOMAIN_ATTR_IO_PGTABLE_CFG: {
+ struct domain_attr_io_pgtbl_cfg *pgtbl_cfg = data;
+
+ if (smmu_domain->smmu) {
+ ret = -EPERM;
+ goto out_unlock;
+ }
+
+ if (!pgtbl_cfg) {
Do we really need to check this? If somebody passed us a NULL pointer
then
they have a bug and we don't check this for other domain attributes
afaict.
True, I'll drop it.
+ ret = -ENODEV;
+ goto out_unlock;
+ }
+
+ smmu_domain->pgtbl_cfg = *pgtbl_cfg;
+ break;
+ }
default:
ret = -ENODEV;
}
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
b/drivers/iommu/arm/arm-smmu/arm-smmu.h
index 04288b6fc619..18fbed376afb 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
@@ -364,6 +364,7 @@ enum arm_smmu_domain_stage {
struct arm_smmu_domain {
struct arm_smmu_device *smmu;
struct io_pgtable_ops *pgtbl_ops;
+ struct domain_attr_io_pgtbl_cfg pgtbl_cfg;
const struct iommu_flush_ops*flush_ops;
struct arm_smmu_cfg cfg;
enum arm_smmu_domain_stage stage;
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index a9a2c59fab37..686b37d48743 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -212,6 +212,10 @@ struct io_pgtable {
#define io_pgtable_ops_to_pgtable(x) container_of((x), struct
io_pgtable, ops)
+struct domain_attr_io_pgtbl_cfg {
+ unsigned long quirks;
+};
nit: Can you rename this to 'struct io_pgtable_domain_attr' please?
Done, thanks.
--
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: [PATCHv8 1/8] iommu/io-pgtable-arm: Add support to use system cache
On 2020-11-23 20:36, Will Deacon wrote:
On Tue, Nov 17, 2020 at 08:00:40PM +0530, Sai Prakash Ranjan wrote:
Add a quirk IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to override
the attributes set in TCR for the page table walker when
using system cache.
Signed-off-by: Sai Prakash Ranjan
---
drivers/iommu/io-pgtable-arm.c | 10 --
include/linux/io-pgtable.h | 4
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/drivers/iommu/io-pgtable-arm.c
b/drivers/iommu/io-pgtable-arm.c
index a7a9bc08dcd1..7c9ea9d7874a 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -761,7 +761,8 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg
*cfg, void *cookie)
if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
IO_PGTABLE_QUIRK_NON_STRICT |
- IO_PGTABLE_QUIRK_ARM_TTBR1))
+ IO_PGTABLE_QUIRK_ARM_TTBR1 |
+ IO_PGTABLE_QUIRK_ARM_OUTER_WBWA))
return NULL;
data = arm_lpae_alloc_pgtable(cfg);
@@ -773,10 +774,15 @@ arm_64_lpae_alloc_pgtable_s1(struct
io_pgtable_cfg *cfg, void *cookie)
tcr->sh = ARM_LPAE_TCR_SH_IS;
tcr->irgn = ARM_LPAE_TCR_RGN_WBWA;
tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
+ if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA)
+ goto out_free_data;
} else {
tcr->sh = ARM_LPAE_TCR_SH_OS;
tcr->irgn = ARM_LPAE_TCR_RGN_NC;
- tcr->orgn = ARM_LPAE_TCR_RGN_NC;
+ if (!(cfg->quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA))
+ tcr->orgn = ARM_LPAE_TCR_RGN_NC;
+ else
+ tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
}
tg1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1;
diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
index 4cde111e425b..a9a2c59fab37 100644
--- a/include/linux/io-pgtable.h
+++ b/include/linux/io-pgtable.h
@@ -86,6 +86,9 @@ struct io_pgtable_cfg {
*
* IO_PGTABLE_QUIRK_ARM_TTBR1: (ARM LPAE format) Configure the table
* for use in the upper half of a split address space.
+*
+ * IO_PGTABLE_QUIRK_ARM_OUTER_WBWA: Override the attributes set in
TCR for
+* the page table walker when using system cache.
Please can you reword this to say:
"Override the outer-cacheability attributes set in the TCR for a
non-coherent
page-table walker."
Sure, thanks.
--
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 1/2] arm-smmu-qcom: Ensure the qcom_scm driver has finished probing
On Thu, 12 Nov 2020 22:05:19 +, John Stultz wrote: > Robin Murphy pointed out that if the arm-smmu driver probes before > the qcom_scm driver, we may call qcom_scm_qsmmu500_wait_safe_toggle() > before the __scm is initialized. > > Now, getting this to happen is a bit contrived, as in my efforts it > required enabling asynchronous probing for both drivers, moving the > firmware dts node to the end of the dtsi file, as well as forcing a > long delay in the qcom_scm_probe function. > > [...] Applied only the first patch to arm64 (for-next/iommu/fixes), thanks! [1/2] arm-smmu-qcom: Ensure the qcom_scm driver has finished probing https://git.kernel.org/arm64/c/72b55c96f3a5 Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Check return of __iommu_attach_device()
On Thu, 19 Nov 2020 16:58:46 +, Shameer Kolothum wrote: > Currently iommu_create_device_direct_mappings() is called > without checking the return of __iommu_attach_device(). This > may result in failures in iommu driver if dev attach returns > error. Applied to arm64 (for-next/iommu/fixes), thanks! [1/1] iommu: Check return of __iommu_attach_device() https://git.kernel.org/arm64/c/77c38c8cf52e Cheers, -- Will https://fixes.arm64.dev https://next.arm64.dev https://will.arm64.dev ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH RESEND v10 0/4] iommu: Shared Virtual Addressing for SMMUv3 (PT sharing part)
On Fri, 6 Nov 2020 16:50:46 +0100, Jean-Philippe Brucker wrote:
> These are the remaining bits implementing iommu_sva_bind_device() for
> SMMUv3. They didn't make it into v5.10 because an Ack was missing for
> adding the PASID field to mm_struct. That is now upstream, in commit
> 52ad9bc64c74 ("mm: Add a pasid member to struct mm_struct"). No change
> since last posting [1], only rebased onto v5.10-rc2.
>
> Note that full SVA support for SMMUv3 still needs IOPF and DVM support,
> coming soon.
>
> [...]
Applied to arm64 (for-next/iommu/svm), thanks!
[1/4] iommu/ioasid: Add ioasid references
https://git.kernel.org/arm64/c/cb4789b0d19f
[2/4] iommu/sva: Add PASID helpers
https://git.kernel.org/arm64/c/cfc78dfd9b36
[3/4] iommu/arm-smmu-v3: Implement iommu_sva_bind/unbind()
https://git.kernel.org/arm64/c/32784a9562fb
[4/4] iommu/arm-smmu-v3: Hook up ATC invalidation to mm ops
https://git.kernel.org/arm64/c/2f7e8c553e98
Cheers,
--
Will
https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v3] swiotlb: Adjust SWIOTBL bounce buffer size for SEV guests.
Hi Ashish, non-technical comment: in the subject, you might want to s/SWIOTBL/SWIOTLB . cheers, Guilherme ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv8 0/8] System Cache support for GPU and required SMMU support
On Tue, Nov 17, 2020 at 08:00:39PM +0530, Sai Prakash Ranjan wrote: > Some hardware variants contain a system cache or the last level > cache(llc). This cache is typically a large block which is shared > by multiple clients on the SOC. GPU uses the system cache to cache > both the GPU data buffers(like textures) as well the SMMU pagetables. > This helps with improved render performance as well as lower power > consumption by reducing the bus traffic to the system memory. > > The system cache architecture allows the cache to be split into slices > which then be used by multiple SOC clients. This patch series is an > effort to enable and use two of those slices preallocated for the GPU, > one for the GPU data buffers and another for the GPU SMMU hardware > pagetables. > > Patch 1 - Patch 6 adds system cache support in SMMU and GPU driver. > Patch 7 and 8 are minor cleanups for arm-smmu impl. > > Changes in v8: > * Introduce a generic domain attribute for pagetable config (Will) > * Rename quirk to more generic IO_PGTABLE_QUIRK_ARM_OUTER_WBWA (Will) > * Move non-strict mode to use new struct domain_attr_io_pgtbl_config (Will) Modulo some minor comments I've made, this looks good to me. What is the plan for merging it? I can take the IOMMU parts, but patches 4-6 touch the MSM GPU driver and I'd like to avoid conflicts with that. Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv8 3/8] iommu/arm-smmu: Move non-strict mode to use domain_attr_io_pgtbl_cfg
On Tue, Nov 17, 2020 at 08:00:42PM +0530, Sai Prakash Ranjan wrote:
> Now that we have a struct domain_attr_io_pgtbl_cfg with quirks,
> use that for non_strict mode as well thereby removing the need
> for more members of arm_smmu_domain in the future.
>
> Signed-off-by: Sai Prakash Ranjan
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 7 ++-
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 -
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 7b05782738e2..5f066a1b7221 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -786,9 +786,6 @@ static int arm_smmu_init_domain_context(struct
> iommu_domain *domain,
> goto out_clear_smmu;
> }
>
> - if (smmu_domain->non_strict)
> - pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
> -
> if (smmu_domain->pgtbl_cfg.quirks)
> pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
>
> @@ -1527,7 +1524,7 @@ static int arm_smmu_domain_get_attr(struct iommu_domain
> *domain,
> case IOMMU_DOMAIN_DMA:
> switch (attr) {
> case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
> - *(int *)data = smmu_domain->non_strict;
> + *(int *)data = smmu_domain->pgtbl_cfg.quirks;
Probably better to compare with IO_PGTABLE_QUIRK_NON_STRICT here even though
we only support this one quirk for DMA domains atm.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv8 2/8] iommu/arm-smmu: Add domain attribute for pagetable configuration
On Tue, Nov 17, 2020 at 08:00:41PM +0530, Sai Prakash Ranjan wrote:
> Add iommu domain attribute for pagetable configuration which
> initially will be used to set quirks like for system cache aka
> last level cache to be used by client drivers like GPU to set
> right attributes for caching the hardware pagetables into the
> system cache and later can be extended to include other page
> table configuration data.
>
> Signed-off-by: Sai Prakash Ranjan
> ---
> drivers/iommu/arm/arm-smmu/arm-smmu.c | 25 +
> drivers/iommu/arm/arm-smmu/arm-smmu.h | 1 +
> include/linux/io-pgtable.h| 4
> include/linux/iommu.h | 1 +
> 4 files changed, 31 insertions(+)
>
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> index 0f28a8614da3..7b05782738e2 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
> @@ -789,6 +789,9 @@ static int arm_smmu_init_domain_context(struct
> iommu_domain *domain,
> if (smmu_domain->non_strict)
> pgtbl_cfg.quirks |= IO_PGTABLE_QUIRK_NON_STRICT;
>
> + if (smmu_domain->pgtbl_cfg.quirks)
> + pgtbl_cfg.quirks |= smmu_domain->pgtbl_cfg.quirks;
> +
> pgtbl_ops = alloc_io_pgtable_ops(fmt, _cfg, smmu_domain);
> if (!pgtbl_ops) {
> ret = -ENOMEM;
> @@ -1511,6 +1514,12 @@ static int arm_smmu_domain_get_attr(struct
> iommu_domain *domain,
> case DOMAIN_ATTR_NESTING:
> *(int *)data = (smmu_domain->stage ==
> ARM_SMMU_DOMAIN_NESTED);
> return 0;
> + case DOMAIN_ATTR_IO_PGTABLE_CFG: {
> + struct domain_attr_io_pgtbl_cfg *pgtbl_cfg = data;
> + *pgtbl_cfg = smmu_domain->pgtbl_cfg;
> +
> + return 0;
> + }
> default:
> return -ENODEV;
> }
> @@ -1551,6 +1560,22 @@ static int arm_smmu_domain_set_attr(struct
> iommu_domain *domain,
> else
> smmu_domain->stage = ARM_SMMU_DOMAIN_S1;
> break;
> + case DOMAIN_ATTR_IO_PGTABLE_CFG: {
> + struct domain_attr_io_pgtbl_cfg *pgtbl_cfg = data;
> +
> + if (smmu_domain->smmu) {
> + ret = -EPERM;
> + goto out_unlock;
> + }
> +
> + if (!pgtbl_cfg) {
Do we really need to check this? If somebody passed us a NULL pointer then
they have a bug and we don't check this for other domain attributes afaict.
> + ret = -ENODEV;
> + goto out_unlock;
> + }
> +
> + smmu_domain->pgtbl_cfg = *pgtbl_cfg;
> + break;
> + }
> default:
> ret = -ENODEV;
> }
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> index 04288b6fc619..18fbed376afb 100644
> --- a/drivers/iommu/arm/arm-smmu/arm-smmu.h
> +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.h
> @@ -364,6 +364,7 @@ enum arm_smmu_domain_stage {
> struct arm_smmu_domain {
> struct arm_smmu_device *smmu;
> struct io_pgtable_ops *pgtbl_ops;
> + struct domain_attr_io_pgtbl_cfg pgtbl_cfg;
> const struct iommu_flush_ops*flush_ops;
> struct arm_smmu_cfg cfg;
> enum arm_smmu_domain_stage stage;
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index a9a2c59fab37..686b37d48743 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -212,6 +212,10 @@ struct io_pgtable {
>
> #define io_pgtable_ops_to_pgtable(x) container_of((x), struct io_pgtable,
> ops)
>
> +struct domain_attr_io_pgtbl_cfg {
> + unsigned long quirks;
> +};
nit: Can you rename this to 'struct io_pgtable_domain_attr' please?
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCHv8 1/8] iommu/io-pgtable-arm: Add support to use system cache
On Tue, Nov 17, 2020 at 08:00:40PM +0530, Sai Prakash Ranjan wrote:
> Add a quirk IO_PGTABLE_QUIRK_ARM_OUTER_WBWA to override
> the attributes set in TCR for the page table walker when
> using system cache.
>
> Signed-off-by: Sai Prakash Ranjan
> ---
> drivers/iommu/io-pgtable-arm.c | 10 --
> include/linux/io-pgtable.h | 4
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
> index a7a9bc08dcd1..7c9ea9d7874a 100644
> --- a/drivers/iommu/io-pgtable-arm.c
> +++ b/drivers/iommu/io-pgtable-arm.c
> @@ -761,7 +761,8 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg *cfg,
> void *cookie)
>
> if (cfg->quirks & ~(IO_PGTABLE_QUIRK_ARM_NS |
> IO_PGTABLE_QUIRK_NON_STRICT |
> - IO_PGTABLE_QUIRK_ARM_TTBR1))
> + IO_PGTABLE_QUIRK_ARM_TTBR1 |
> + IO_PGTABLE_QUIRK_ARM_OUTER_WBWA))
> return NULL;
>
> data = arm_lpae_alloc_pgtable(cfg);
> @@ -773,10 +774,15 @@ arm_64_lpae_alloc_pgtable_s1(struct io_pgtable_cfg
> *cfg, void *cookie)
> tcr->sh = ARM_LPAE_TCR_SH_IS;
> tcr->irgn = ARM_LPAE_TCR_RGN_WBWA;
> tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
> + if (cfg->quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA)
> + goto out_free_data;
> } else {
> tcr->sh = ARM_LPAE_TCR_SH_OS;
> tcr->irgn = ARM_LPAE_TCR_RGN_NC;
> - tcr->orgn = ARM_LPAE_TCR_RGN_NC;
> + if (!(cfg->quirks & IO_PGTABLE_QUIRK_ARM_OUTER_WBWA))
> + tcr->orgn = ARM_LPAE_TCR_RGN_NC;
> + else
> + tcr->orgn = ARM_LPAE_TCR_RGN_WBWA;
> }
>
> tg1 = cfg->quirks & IO_PGTABLE_QUIRK_ARM_TTBR1;
> diff --git a/include/linux/io-pgtable.h b/include/linux/io-pgtable.h
> index 4cde111e425b..a9a2c59fab37 100644
> --- a/include/linux/io-pgtable.h
> +++ b/include/linux/io-pgtable.h
> @@ -86,6 +86,9 @@ struct io_pgtable_cfg {
>*
>* IO_PGTABLE_QUIRK_ARM_TTBR1: (ARM LPAE format) Configure the table
>* for use in the upper half of a split address space.
> + *
> + * IO_PGTABLE_QUIRK_ARM_OUTER_WBWA: Override the attributes set in TCR
> for
> + * the page table walker when using system cache.
Please can you reword this to say:
"Override the outer-cacheability attributes set in the TCR for a non-coherent
page-table walker."
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] iommu/amd: Enforce 4k mapping for certain IOMMU data structures
On Thu, Nov 19, 2020 at 10:19:01PM -0600, Brijesh Singh wrote:
> On 11/19/20 8:30 PM, Suravee Suthikulpanit wrote:
> > On 11/18/20 5:57 AM, Will Deacon wrote:
> > > I think I'm missing something here. set_memory_4k() will break the
> > kernel
> > > linear mapping up into page granular mappings, but the IOMMU isn't
> > using
> > > that mapping, right?
> >
> > That's correct. This does not affect the IOMMU, but it affects the PSP
> > FW.
> >
> > > It's just using the physical address returned by
> > iommu_virt_to_phys(), so why does it matter?
> > >
> > > Just be nice to capture some of this rationale in the log,
> > especially as
> > > I'm not familiar with this device.
> >
> > According to the AMD SEV-SNP white paper
> > (https://www.amd.com/system/files/TechDocs/SEV-SNP-strengthening-vm-isolation-with-integrity-protection-and-more.pdf),
> > the Reverse Map Table (RMP) contains one entry for every 4K page of
> > DRAM that may be used by the VM. In this case, the pages allocated by
> > the IOMMU driver are added as 4K entries in the RMP table by the
> > SEV-SNP FW.
> >
> > During the page table walk, the RMP checks if the page is owned by the
> > hypervisor. Without calling set_memory_4k() to break the mapping up
> > into 4K pages, pages could end up being part of large mapping (e.g. 2M
> > page), in which the page access would be denied and result in #PF.
>
>
> Since the page is added as a 4K page in the RMP table by the SEV-SNP FW,
> so we need to split the physmap to ensure that this page will be access
> with a 4K mapping from the x86. If the page is part of large page then
> write access will cause a RMP violation (i.e #PF), this is because SNP
> hardware enforce that the CPU page level walk must match with page-level
> programmed in the RMP table.
Got it; thanks.
> > >> Fixes: commit c69d89aff393 ("iommu/amd: Use 4K page for completion
> > wait write-back semaphore")
> > >
> > > I couldn't figure out how that commit could cause this problem.
> > Please can
> > > you explain that to me?
> >
> > Hope this helps clarify. If so, I'll update the commit log and send
> > out V3.
Cheers. No need for a v2, as I've queued this up with a Link: tag.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: Regression due to commit 1161db6776bd1c35257e1e362e91bcb1b3fe4347 (media: usb: pwc: Don't use coherent DMA buffers for ISO transfer)
Sorry for the empty message. I would like to add Joerg Roedel and IOMMU related maillist in case they can help on this issue. пн, 23 нояб. 2020 г. в 16:32, Matwey V. Kornilov : > > пн, 23 нояб. 2020 г. в 14:19, Thimo Emmerich : > > > > Hi, > > > > thanks for your prompt answer. Unfortunately I was out of office on Friday. > > > > Here the updates: > > - Tracing showed the lines you mentioned. > > - Updating the kernel to the latest stable (v5.9.10) did not change > > anything. > > - Adding "amd_iommu=off" fixes the issue (of course including > > dysfunctional IOMMU) > > > > Best regards, > > Thimo > > > > On 20.11.20 08:55, Matwey V. Kornilov wrote: > > > Hi, > > > > > > As far as I understand, the driver itself works correctly. It > > > communicates with the device and receives the data. Only difference is > > > that the data is zeroed. If so, it means that the memory is allocated > > > without errors and the DMA is also mapped without errors. Otherwise, > > > you would get an error when trying to open the device/start stream. To > > > finally prove this, could you please use tracing points to see what is > > > going on while the video stream is on? > > > > > > mount -ttracefs tracefs /sys/kernel/tracing > > > cd /sys/kernel/tracing > > > echo 1 > events/pwc/enable > > > echo 1 > tracing_on > > > cat trace > > > > > > And then you should see lots of lines like the following when video > > > stream is running: > > > > > > -0 [002] ..s. 34645.185239: pwc_handler_enter: dev=Philips webcam > > > 1-1.4:1.0 (fbuf=303a809e filled=31581) urb=7e3783c4 > > > (status=0 actual_length=9570) > > > -0 [002] ..s. 34645.185244: pwc_handler_exit: dev=Philips webcam > > > 1-1.4:1.0 (fbuf=303a809e filled=41151) urb=7e3783c4 > > > > > > Use > > > > > > echo 0 > tracing_on > > > > > > to disable the recording. > > > > > > > > > I've found a commit that claims to fix an issue with similar wording: > > > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_commit_27d2dcb1b95c8c39da417839c11eda2e665cd389=DwIFaQ=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc=uorjfjHlvtqEyqWp5b8YlexJq-ujpFniwt87GU9IrXI=ZPgwadRZcYtXJwzoIM6ibrGC8jNcJkKloZFNKDK6PBE=pC6Xy7iT5Yf6nx7dSNiDOCvvCs7HENylTiquLeJ_0Og= > > > > > > Could you please try to boot with "amd_iommu=off" kernel command line > > > parameter and see what happens? > > > Is it also possible for you to check the latest kernel version on your > > > setup? > > > > > > > > > чт, 19 нояб. 2020 г. в 18:49, Thimo Emmerich : > > > > > > > > >> Hi, > > >> > > >> not sure about the application behaviour. > > >> I observed this green screen in MS Teams. For easy verification I used > > >> "cheese" which shows a live stream. Same behaviour. > > >> The issue is 100% reproducible: loading the stock Ubuntu (HWE) kernel > > >> module --> green screen, unloading and loading patched module --> > > >> picture. > > >> For this procedure it is even not necessary to disconnect the cam itself. > > >> > > >> I did now also another test excluding possible application magic: > > >> dd if=/dev/video0 of=/tmp/vid.raw bs=460800 count=20 > > >> If I interpret this data as a video (640x720), with the patched module, > > >> I see a running sequence whereas with the stock module there are just > > >> zeros. Of course, the zeros interpreted as YCbCr stream lead to a still > > >> green image. > > >> > > >> I am using the stock Ubuntu HWE kernel (linux-image-5.4.0-53-generic), > > >> config is attached. > > >> I have to admit that I did not use the Ubuntu-patched kernel sources but > > >> the v5.4 tagged sources from git.kernel.org since I guess there are no > > >> patches in this special section. I also verified building the vanilla > > >> module has the same behaviour as the Ubuntu kernel module (green screen). > > >> > > >> I am not using the virtualization feature (currently). Nevertheless, the > > >> IOMMU is enabled (by default) in the BIOS. > > >> Unfortunately, I do not have another webcam at hand (the other ones are > > >> notebook build-ins). > > >> I checked now with another PC (ancient Intel Xeon system) - the stock > > >> driver works without trouble. > > >> > > >> Do you use/have access to a Ryzen CPU? Maybe there is some > > >> issue/specialty with its IOMMU? > > >> > > >> Best regards, > > >>Thimo Emmerich > > >> > > >> On 19.11.20 14:26, Matwey V. Kornilov wrote: > > >>> Hi, > > >>> > > >>> I've just checked my PWC900 camera at kernel 5.9.8 and unfortunately > > >>> It works perfectly, so there issue is probably deeper. > > >>> > > >>> You say that you see the green image. Could you ensure that the green > > >>> image is what you actually get from the kernel driver, but not an > > >>> application behaviour when there are no frames. > > >>> Could you also provide your kernel build config? I see that you are > > >>> running some kind of hardware assisted virtualization, right? May I > > >>> also ask you to look for some other USB webcam
Re: Regression due to commit 1161db6776bd1c35257e1e362e91bcb1b3fe4347 (media: usb: pwc: Don't use coherent DMA buffers for ISO transfer)
пн, 23 нояб. 2020 г. в 14:19, Thimo Emmerich : > > Hi, > > thanks for your prompt answer. Unfortunately I was out of office on Friday. > > Here the updates: > - Tracing showed the lines you mentioned. > - Updating the kernel to the latest stable (v5.9.10) did not change > anything. > - Adding "amd_iommu=off" fixes the issue (of course including > dysfunctional IOMMU) > > Best regards, > Thimo > > On 20.11.20 08:55, Matwey V. Kornilov wrote: > > Hi, > > > > As far as I understand, the driver itself works correctly. It > > communicates with the device and receives the data. Only difference is > > that the data is zeroed. If so, it means that the memory is allocated > > without errors and the DMA is also mapped without errors. Otherwise, > > you would get an error when trying to open the device/start stream. To > > finally prove this, could you please use tracing points to see what is > > going on while the video stream is on? > > > > mount -ttracefs tracefs /sys/kernel/tracing > > cd /sys/kernel/tracing > > echo 1 > events/pwc/enable > > echo 1 > tracing_on > > cat trace > > > > And then you should see lots of lines like the following when video > > stream is running: > > > > -0 [002] ..s. 34645.185239: pwc_handler_enter: dev=Philips webcam > > 1-1.4:1.0 (fbuf=303a809e filled=31581) urb=7e3783c4 > > (status=0 actual_length=9570) > > -0 [002] ..s. 34645.185244: pwc_handler_exit: dev=Philips webcam > > 1-1.4:1.0 (fbuf=303a809e filled=41151) urb=7e3783c4 > > > > Use > > > > echo 0 > tracing_on > > > > to disable the recording. > > > > > > I've found a commit that claims to fix an issue with similar wording: > > https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_torvalds_linux_commit_27d2dcb1b95c8c39da417839c11eda2e665cd389=DwIFaQ=fP4tf--1dS0biCFlB0saz0I0kjO5v7-GLPtvShAo4cc=uorjfjHlvtqEyqWp5b8YlexJq-ujpFniwt87GU9IrXI=ZPgwadRZcYtXJwzoIM6ibrGC8jNcJkKloZFNKDK6PBE=pC6Xy7iT5Yf6nx7dSNiDOCvvCs7HENylTiquLeJ_0Og= > > > > Could you please try to boot with "amd_iommu=off" kernel command line > > parameter and see what happens? > > Is it also possible for you to check the latest kernel version on your > > setup? > > > > > > чт, 19 нояб. 2020 г. в 18:49, Thimo Emmerich : > > > > > >> Hi, > >> > >> not sure about the application behaviour. > >> I observed this green screen in MS Teams. For easy verification I used > >> "cheese" which shows a live stream. Same behaviour. > >> The issue is 100% reproducible: loading the stock Ubuntu (HWE) kernel > >> module --> green screen, unloading and loading patched module --> picture. > >> For this procedure it is even not necessary to disconnect the cam itself. > >> > >> I did now also another test excluding possible application magic: > >> dd if=/dev/video0 of=/tmp/vid.raw bs=460800 count=20 > >> If I interpret this data as a video (640x720), with the patched module, > >> I see a running sequence whereas with the stock module there are just > >> zeros. Of course, the zeros interpreted as YCbCr stream lead to a still > >> green image. > >> > >> I am using the stock Ubuntu HWE kernel (linux-image-5.4.0-53-generic), > >> config is attached. > >> I have to admit that I did not use the Ubuntu-patched kernel sources but > >> the v5.4 tagged sources from git.kernel.org since I guess there are no > >> patches in this special section. I also verified building the vanilla > >> module has the same behaviour as the Ubuntu kernel module (green screen). > >> > >> I am not using the virtualization feature (currently). Nevertheless, the > >> IOMMU is enabled (by default) in the BIOS. > >> Unfortunately, I do not have another webcam at hand (the other ones are > >> notebook build-ins). > >> I checked now with another PC (ancient Intel Xeon system) - the stock > >> driver works without trouble. > >> > >> Do you use/have access to a Ryzen CPU? Maybe there is some > >> issue/specialty with its IOMMU? > >> > >> Best regards, > >>Thimo Emmerich > >> > >> On 19.11.20 14:26, Matwey V. Kornilov wrote: > >>> Hi, > >>> > >>> I've just checked my PWC900 camera at kernel 5.9.8 and unfortunately > >>> It works perfectly, so there issue is probably deeper. > >>> > >>> You say that you see the green image. Could you ensure that the green > >>> image is what you actually get from the kernel driver, but not an > >>> application behaviour when there are no frames. > >>> Could you also provide your kernel build config? I see that you are > >>> running some kind of hardware assisted virtualization, right? May I > >>> also ask you to look for some other USB webcam (other model) around > >>> you and check if it works at the same configuration? > >>> > >>> > >>> чт, 19 нояб. 2020 г. в 14:59, Thimo Emmerich: > >>> > Hi Matwey, > > I am using a Philips SPC 900NC USB webcam on my Ubuntu 18.04 system. > Recently I upgraded from the stock kernel (4.15) to the latest HWE > kernel (5.4). > > Unfortunately my camera was not working anymore.
Re: [PATCH v9 1/4] iommu: Move def_domain type check for untrusted device into core
On Mon, Nov 23, 2020 at 09:54:49PM +0800, Lu Baolu wrote:
> Hi Will,
>
> On 2020/11/23 21:03, Will Deacon wrote:
> > Hi Baolu,
> >
> > On Mon, Nov 23, 2020 at 08:55:17PM +0800, Lu Baolu wrote:
> > > On 2020/11/23 20:04, Will Deacon wrote:
> > > > On Sat, Nov 21, 2020 at 09:56:17PM +0800, Lu Baolu wrote:
> > > > > @@ -1645,13 +1655,10 @@ struct __group_domain_type {
> > > > >static int probe_get_default_domain_type(struct device *dev, void
> > > > > *data)
> > > > >{
> > > > > - const struct iommu_ops *ops = dev->bus->iommu_ops;
> > > > > struct __group_domain_type *gtype = data;
> > > > > unsigned int type = 0;
> > > > > - if (ops->def_domain_type)
> > > > > - type = ops->def_domain_type(dev);
> > > > > -
> > > > > + type = iommu_get_mandatory_def_domain_type(dev);
> > > >
> > > > afaict, this code is only called from probe_alloc_default_domain(),
> > > > which
> > > > has:
> > > >
> > > > /* Ask for default domain requirements of all devices in the
> > > > group */
> > > > __iommu_group_for_each_dev(group, ,
> > > > probe_get_default_domain_type);
> > > >
> > > > if (!gtype.type)
> > > > gtype.type = iommu_def_domain_type;
> > > >
> > > > so is there actually a need to introduce the new
> > > > iommu_get_mandatory_def_domain_type() function, given that a type of 0
> > > > always ends up resolving to the default domain type?
> > >
> > > Another consumer of this helper is in the next patch:
> > >
> > > + dev_def_dom = iommu_get_mandatory_def_domain_type(dev);
> > > +
> > > + /* Check if user requested domain is supported by the device or not */
> > > + if (!type) {
> > > + /*
> > > + * If the user hasn't requested any specific type of domain and
> > > + * if the device supports both the domains, then default to the
> > > + * domain the device was booted with
> > > + */
> > > + type = iommu_get_def_domain_type(dev);
> > > + } else if (dev_def_dom && type != dev_def_dom) {
> > > + dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n",
> > > + iommu_domain_type_str(type));
> > > + ret = -EINVAL;
> > > + goto out;
> > > + }
> > >
> > > I also added the untrusted device check in
> > > iommu_get_mandatory_def_domain_type(), so that we don't need to care
> > > about this in multiple places.
> >
> > I see, but isn't this also setting the default domain type in the case that
> > it gets back a type of 0? I think it would be nice to avoid having both
> > iommu_get_mandatory_def_domain_type() and iommu_get_def_domain_type() of we
> > can, as it's really not clear which one to use when and what is meant by
> > "mandatory" imo.
>
> Yes, agreed. I will remove iommu_get_mandatory_def_domain_type() and
> keep iommu_get_def_domain_type() as the only helper in the next version.
Great, thanks!
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 1/4] iommu: Move def_domain type check for untrusted device into core
Hi Will,
On 2020/11/23 21:03, Will Deacon wrote:
Hi Baolu,
On Mon, Nov 23, 2020 at 08:55:17PM +0800, Lu Baolu wrote:
On 2020/11/23 20:04, Will Deacon wrote:
On Sat, Nov 21, 2020 at 09:56:17PM +0800, Lu Baolu wrote:
@@ -1645,13 +1655,10 @@ struct __group_domain_type {
static int probe_get_default_domain_type(struct device *dev, void *data)
{
- const struct iommu_ops *ops = dev->bus->iommu_ops;
struct __group_domain_type *gtype = data;
unsigned int type = 0;
- if (ops->def_domain_type)
- type = ops->def_domain_type(dev);
-
+ type = iommu_get_mandatory_def_domain_type(dev);
afaict, this code is only called from probe_alloc_default_domain(), which
has:
/* Ask for default domain requirements of all devices in the group */
__iommu_group_for_each_dev(group, ,
probe_get_default_domain_type);
if (!gtype.type)
gtype.type = iommu_def_domain_type;
so is there actually a need to introduce the new
iommu_get_mandatory_def_domain_type() function, given that a type of 0
always ends up resolving to the default domain type?
Another consumer of this helper is in the next patch:
+ dev_def_dom = iommu_get_mandatory_def_domain_type(dev);
+
+ /* Check if user requested domain is supported by the device or not */
+ if (!type) {
+ /*
+* If the user hasn't requested any specific type of domain and
+* if the device supports both the domains, then default to the
+* domain the device was booted with
+*/
+ type = iommu_get_def_domain_type(dev);
+ } else if (dev_def_dom && type != dev_def_dom) {
+ dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n",
+ iommu_domain_type_str(type));
+ ret = -EINVAL;
+ goto out;
+ }
I also added the untrusted device check in
iommu_get_mandatory_def_domain_type(), so that we don't need to care
about this in multiple places.
I see, but isn't this also setting the default domain type in the case that
it gets back a type of 0? I think it would be nice to avoid having both
iommu_get_mandatory_def_domain_type() and iommu_get_def_domain_type() of we
can, as it's really not clear which one to use when and what is meant by
"mandatory" imo.
Yes, agreed. I will remove iommu_get_mandatory_def_domain_type() and
keep iommu_get_def_domain_type() as the only helper in the next version.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] PCI: Mark AMD Raven iGPU ATS as broken
Edgar Merger reports that the AMD Raven GPU does not work reliably on his system when the IOMMU is enabled: | [drm:amdgpu_job_timedout [amdgpu]] *ERROR* ring gfx timeout, signaled seq=1, emitted seq=3 | [...] | amdgpu :0b:00.0: GPU reset begin! | AMD-Vi: Completion-Wait loop timed out | iommu ivhd0: AMD-Vi: Event logged [IOTLB_INV_TIMEOUT device=0b:00.0 address=0x38edc0970] This is indicative of a hardware/platform configuration issue so, since disabling ATS has been shown to resolve the problem, add a quirk to match this particular device while Edgar follows-up with AMD for more information. Cc: Bjorn Helgaas Cc: Alex Deucher Reported-by: Edgar Merger Suggested-by: Joerg Roedel Link: https://lore.kernel.org/linux-iommu/mwhpr10mb1310f042a30661d4158520b589...@mwhpr10mb1310.namprd10.prod.outlook.com Signed-off-by: Will Deacon --- Hi all, Since Joerg is away at the moment, I'm posting this to try to make some progress with the thread in the Link: tag. Cheers, Will drivers/pci/quirks.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c index f70692ac79c5..3911b0ec57ba 100644 --- a/drivers/pci/quirks.c +++ b/drivers/pci/quirks.c @@ -5176,6 +5176,8 @@ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x6900, quirk_amd_harvest_no_ats); DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7312, quirk_amd_harvest_no_ats); /* AMD Navi14 dGPU */ DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x7340, quirk_amd_harvest_no_ats); +/* AMD Raven platform iGPU */ +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ATI, 0x15d8, quirk_amd_harvest_no_ats); #endif /* CONFIG_PCI_ATS */ /* Freescale PCIe doesn't support MSI in RC mode */ -- 2.29.2.454.gaff20da3a2-goog ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 3/3] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module
On Mon, Nov 23, 2020 at 01:35:14PM +0100, Linus Walleij wrote: > On Mon, Nov 16, 2020 at 5:36 PM Will Deacon wrote: > > > Linus -- please can you drop this one (patch 3/3) for now, given that it's > > causing problems? > > Reverted now, sorry for missing to do this earlier. Cheers, Linus! Will ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 1/4] iommu: Move def_domain type check for untrusted device into core
Hi Baolu,
On Mon, Nov 23, 2020 at 08:55:17PM +0800, Lu Baolu wrote:
> On 2020/11/23 20:04, Will Deacon wrote:
> > On Sat, Nov 21, 2020 at 09:56:17PM +0800, Lu Baolu wrote:
> > > @@ -1645,13 +1655,10 @@ struct __group_domain_type {
> > > static int probe_get_default_domain_type(struct device *dev, void *data)
> > > {
> > > - const struct iommu_ops *ops = dev->bus->iommu_ops;
> > > struct __group_domain_type *gtype = data;
> > > unsigned int type = 0;
> > > - if (ops->def_domain_type)
> > > - type = ops->def_domain_type(dev);
> > > -
> > > + type = iommu_get_mandatory_def_domain_type(dev);
> >
> > afaict, this code is only called from probe_alloc_default_domain(), which
> > has:
> >
> > /* Ask for default domain requirements of all devices in the group
> > */
> > __iommu_group_for_each_dev(group, ,
> > probe_get_default_domain_type);
> >
> > if (!gtype.type)
> > gtype.type = iommu_def_domain_type;
> >
> > so is there actually a need to introduce the new
> > iommu_get_mandatory_def_domain_type() function, given that a type of 0
> > always ends up resolving to the default domain type?
>
> Another consumer of this helper is in the next patch:
>
> + dev_def_dom = iommu_get_mandatory_def_domain_type(dev);
> +
> + /* Check if user requested domain is supported by the device or not */
> + if (!type) {
> + /*
> + * If the user hasn't requested any specific type of domain and
> + * if the device supports both the domains, then default to the
> + * domain the device was booted with
> + */
> + type = iommu_get_def_domain_type(dev);
> + } else if (dev_def_dom && type != dev_def_dom) {
> + dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n",
> + iommu_domain_type_str(type));
> + ret = -EINVAL;
> + goto out;
> + }
>
> I also added the untrusted device check in
> iommu_get_mandatory_def_domain_type(), so that we don't need to care
> about this in multiple places.
I see, but isn't this also setting the default domain type in the case that
it gets back a type of 0? I think it would be nice to avoid having both
iommu_get_mandatory_def_domain_type() and iommu_get_def_domain_type() of we
can, as it's really not clear which one to use when and what is meant by
"mandatory" imo.
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 1/4] iommu: Move def_domain type check for untrusted device into core
Hi Will,
On 2020/11/23 20:04, Will Deacon wrote:
On Sat, Nov 21, 2020 at 09:56:17PM +0800, Lu Baolu wrote:
So that the vendor iommu drivers are no more required to provide the
def_domain_type callback to always isolate the untrusted devices.
Link:
https://lore.kernel.org/linux-iommu/243ce89c33fe4b9da4c56ba35aceb...@huawei.com/
Cc: Shameerali Kolothum Thodi
Signed-off-by: Lu Baolu
---
drivers/iommu/intel/iommu.c | 7 ---
drivers/iommu/iommu.c | 21 ++---
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index af3abd285214..6711f78141a4 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -2916,13 +2916,6 @@ static int device_def_domain_type(struct device *dev)
if (dev_is_pci(dev)) {
struct pci_dev *pdev = to_pci_dev(dev);
- /*
-* Prevent any device marked as untrusted from getting
-* placed into the statically identity mapping domain.
-*/
- if (pdev->untrusted)
- return IOMMU_DOMAIN_DMA;
-
if ((iommu_identity_mapping & IDENTMAP_AZALIA) &&
IS_AZALIA(pdev))
return IOMMU_DOMAIN_IDENTITY;
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 88b0c9192d8c..3256784c0358 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1457,13 +1457,23 @@ struct iommu_group *fsl_mc_device_group(struct device
*dev)
}
EXPORT_SYMBOL_GPL(fsl_mc_device_group);
-static int iommu_get_def_domain_type(struct device *dev)
+/* Get the mandatary def_domain type for device. Otherwise, return 0. */
+static int iommu_get_mandatory_def_domain_type(struct device *dev)
{
const struct iommu_ops *ops = dev->bus->iommu_ops;
- unsigned int type = 0;
+
+ if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
+ return IOMMU_DOMAIN_DMA;
if (ops->def_domain_type)
- type = ops->def_domain_type(dev);
+ return ops->def_domain_type(dev);
+
+ return 0;
+}
+
+static int iommu_get_def_domain_type(struct device *dev)
+{
+ int type = iommu_get_mandatory_def_domain_type(dev);
return (type == 0) ? iommu_def_domain_type : type;
}
@@ -1645,13 +1655,10 @@ struct __group_domain_type {
static int probe_get_default_domain_type(struct device *dev, void *data)
{
- const struct iommu_ops *ops = dev->bus->iommu_ops;
struct __group_domain_type *gtype = data;
unsigned int type = 0;
- if (ops->def_domain_type)
- type = ops->def_domain_type(dev);
-
+ type = iommu_get_mandatory_def_domain_type(dev);
afaict, this code is only called from probe_alloc_default_domain(), which
has:
/* Ask for default domain requirements of all devices in the group */
__iommu_group_for_each_dev(group, ,
probe_get_default_domain_type);
if (!gtype.type)
gtype.type = iommu_def_domain_type;
so is there actually a need to introduce the new
iommu_get_mandatory_def_domain_type() function, given that a type of 0
always ends up resolving to the default domain type?
Another consumer of this helper is in the next patch:
+ dev_def_dom = iommu_get_mandatory_def_domain_type(dev);
+
+ /* Check if user requested domain is supported by the device or not */
+ if (!type) {
+ /*
+* If the user hasn't requested any specific type of domain and
+* if the device supports both the domains, then default to the
+* domain the device was booted with
+*/
+ type = iommu_get_def_domain_type(dev);
+ } else if (dev_def_dom && type != dev_def_dom) {
+ dev_err_ratelimited(prev_dev, "Device cannot be in %s domain\n",
+ iommu_domain_type_str(type));
+ ret = -EINVAL;
+ goto out;
+ }
I also added the untrusted device check in
iommu_get_mandatory_def_domain_type(), so that we don't need to care
about this in multiple places.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v6 3/3] firmware: QCOM_SCM: Allow qcom_scm driver to be loadable as a permenent module
On Mon, Nov 16, 2020 at 5:36 PM Will Deacon wrote: > Linus -- please can you drop this one (patch 3/3) for now, given that it's > causing problems? Reverted now, sorry for missing to do this earlier. Yours, Linus Walleij ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] iommu: Improve the performance for direct_mapping
On Fri, Nov 20, 2020 at 05:06:28PM +0800, Yong Wu wrote:
> Currently direct_mapping always use the smallest pgsize which is SZ_4K
> normally to mapping. This is unnecessary. we could gather the size, and
> call iommu_map then, iommu_map could decide how to map better with the
> just right pgsize.
>
> From the original comment, we should take care overlap, otherwise,
> iommu_map may return -EEXIST. In this overlap case, we should map the
> previous region before overlap firstly. then map the left part.
>
> Each a iommu device will call this direct_mapping when its iommu
> initialize, This patch is effective to improve the boot/initialization
> time especially while it only needs level 1 mapping.
>
> Signed-off-by: Anan Sun
> Signed-off-by: Yong Wu
> ---
> drivers/iommu/iommu.c | 20 ++--
> 1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index df87c8e825f7..854a8fcb928d 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -737,6 +737,7 @@ static int iommu_create_device_direct_mappings(struct
> iommu_group *group,
> /* We need to consider overlapping regions for different devices */
> list_for_each_entry(entry, , list) {
> dma_addr_t start, end, addr;
> + size_t unmapped_sz = 0;
I think "unmapped" is the wrong word here, as this variable actually
represents the amount we want to map! I suggest "map_size" instead.
> if (domain->ops->apply_resv_region)
> domain->ops->apply_resv_region(dev, domain, entry);
> @@ -752,10 +753,25 @@ static int iommu_create_device_direct_mappings(struct
> iommu_group *group,
> phys_addr_t phys_addr;
>
> phys_addr = iommu_iova_to_phys(domain, addr);
> - if (phys_addr)
> + if (phys_addr == 0) {
> + unmapped_sz += pg_size; /* Gather the size. */
> continue;
> + }
>
> - ret = iommu_map(domain, addr, addr, pg_size,
> entry->prot);
> + if (unmapped_sz) {
> + /* Map the region before the overlap. */
> + ret = iommu_map(domain, start, start,
> + unmapped_sz, entry->prot);
> + if (ret)
> + goto out;
> + start += unmapped_sz;
I think it's a bit confusing to update start like this. Can we call
iommu_map(domain, addr - map_size, addr - map_size, map_size, entry->prot)
instead?
> + unmapped_sz = 0;
> + }
> + start += pg_size;
> + }
> + if (unmapped_sz) {
> + ret = iommu_map(domain, start, start, unmapped_sz,
> + entry->prot);
Can you avoid this hunk by changing your loop check to something like:
if (!phys_addr) {
map_size += pg_size;
if (addr + pg_size < end)
continue;
}
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v9 1/4] iommu: Move def_domain type check for untrusted device into core
On Sat, Nov 21, 2020 at 09:56:17PM +0800, Lu Baolu wrote:
> So that the vendor iommu drivers are no more required to provide the
> def_domain_type callback to always isolate the untrusted devices.
>
> Link:
> https://lore.kernel.org/linux-iommu/243ce89c33fe4b9da4c56ba35aceb...@huawei.com/
> Cc: Shameerali Kolothum Thodi
> Signed-off-by: Lu Baolu
> ---
> drivers/iommu/intel/iommu.c | 7 ---
> drivers/iommu/iommu.c | 21 ++---
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index af3abd285214..6711f78141a4 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -2916,13 +2916,6 @@ static int device_def_domain_type(struct device *dev)
> if (dev_is_pci(dev)) {
> struct pci_dev *pdev = to_pci_dev(dev);
>
> - /*
> - * Prevent any device marked as untrusted from getting
> - * placed into the statically identity mapping domain.
> - */
> - if (pdev->untrusted)
> - return IOMMU_DOMAIN_DMA;
> -
> if ((iommu_identity_mapping & IDENTMAP_AZALIA) &&
> IS_AZALIA(pdev))
> return IOMMU_DOMAIN_IDENTITY;
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 88b0c9192d8c..3256784c0358 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1457,13 +1457,23 @@ struct iommu_group *fsl_mc_device_group(struct device
> *dev)
> }
> EXPORT_SYMBOL_GPL(fsl_mc_device_group);
>
> -static int iommu_get_def_domain_type(struct device *dev)
> +/* Get the mandatary def_domain type for device. Otherwise, return 0. */
> +static int iommu_get_mandatory_def_domain_type(struct device *dev)
> {
> const struct iommu_ops *ops = dev->bus->iommu_ops;
> - unsigned int type = 0;
> +
> + if (dev_is_pci(dev) && to_pci_dev(dev)->untrusted)
> + return IOMMU_DOMAIN_DMA;
>
> if (ops->def_domain_type)
> - type = ops->def_domain_type(dev);
> + return ops->def_domain_type(dev);
> +
> + return 0;
> +}
> +
> +static int iommu_get_def_domain_type(struct device *dev)
> +{
> + int type = iommu_get_mandatory_def_domain_type(dev);
>
> return (type == 0) ? iommu_def_domain_type : type;
> }
> @@ -1645,13 +1655,10 @@ struct __group_domain_type {
>
> static int probe_get_default_domain_type(struct device *dev, void *data)
> {
> - const struct iommu_ops *ops = dev->bus->iommu_ops;
> struct __group_domain_type *gtype = data;
> unsigned int type = 0;
>
> - if (ops->def_domain_type)
> - type = ops->def_domain_type(dev);
> -
> + type = iommu_get_mandatory_def_domain_type(dev);
afaict, this code is only called from probe_alloc_default_domain(), which
has:
/* Ask for default domain requirements of all devices in the group */
__iommu_group_for_each_dev(group, ,
probe_get_default_domain_type);
if (!gtype.type)
gtype.type = iommu_def_domain_type;
so is there actually a need to introduce the new
iommu_get_mandatory_def_domain_type() function, given that a type of 0
always ends up resolving to the default domain type?
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/7] iommu: Allow the dma-iommu api to use bounce buffers
Hi Will,
On 2020/11/23 19:47, Will Deacon wrote:
On Mon, Nov 23, 2020 at 07:40:57PM +0800, Lu Baolu wrote:
On 2020/11/23 18:08, Christoph Hellwig wrote:
+ /*
+* If both the physical buffer start address and size are
+* page aligned, we don't need to use a bounce page.
+*/
+ if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
+ iova_offset(iovad, phys | org_size)) {
+ aligned_size = iova_align(iovad, org_size);
+ phys = swiotlb_tbl_map_single(dev,
+ phys_to_dma(dev, io_tlb_start),
+ phys, org_size, aligned_size, dir, attrs);
swiotlb_tbl_map_single takes one less argument in 5.10-rc now.
Yes. But Will's iommu/next branch is based on 5.10-rc3. I synced with
him, we agreed to keep it 5.10-rc3 and resolve this conflict when
merging it.
That's right, although I failed to appreciate the conflict was due to a
change in function prototype rather than just a context collision. So
I've updated the vt-d branch to contain the stuff fron Konrad:
https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/iommu/vt-d
Sorry for messing you around!
It's okay. I will re-base the patch series later.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/7] iommu: Allow the dma-iommu api to use bounce buffers
On Mon, Nov 23, 2020 at 07:40:57PM +0800, Lu Baolu wrote:
> On 2020/11/23 18:08, Christoph Hellwig wrote:
> > > + /*
> > > + * If both the physical buffer start address and size are
> > > + * page aligned, we don't need to use a bounce page.
> > > + */
> > > + if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
> > > + iova_offset(iovad, phys | org_size)) {
> > > + aligned_size = iova_align(iovad, org_size);
> > > + phys = swiotlb_tbl_map_single(dev,
> > > + phys_to_dma(dev, io_tlb_start),
> > > + phys, org_size, aligned_size, dir, attrs);
> >
> > swiotlb_tbl_map_single takes one less argument in 5.10-rc now.
> >
>
> Yes. But Will's iommu/next branch is based on 5.10-rc3. I synced with
> him, we agreed to keep it 5.10-rc3 and resolve this conflict when
> merging it.
That's right, although I failed to appreciate the conflict was due to a
change in function prototype rather than just a context collision. So
I've updated the vt-d branch to contain the stuff fron Konrad:
https://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git/log/?h=for-next/iommu/vt-d
Sorry for messing you around!
Will
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/7] iommu: Allow the dma-iommu api to use bounce buffers
Hi Christoph,
On 2020/11/23 18:08, Christoph Hellwig wrote:
+ /*
+* If both the physical buffer start address and size are
+* page aligned, we don't need to use a bounce page.
+*/
+ if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
+ iova_offset(iovad, phys | org_size)) {
+ aligned_size = iova_align(iovad, org_size);
+ phys = swiotlb_tbl_map_single(dev,
+ phys_to_dma(dev, io_tlb_start),
+ phys, org_size, aligned_size, dir, attrs);
swiotlb_tbl_map_single takes one less argument in 5.10-rc now.
Yes. But Will's iommu/next branch is based on 5.10-rc3. I synced with
him, we agreed to keep it 5.10-rc3 and resolve this conflict when
merging it.
Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 3/7] iommu: Allow the dma-iommu api to use bounce buffers
> + /*
> + * If both the physical buffer start address and size are
> + * page aligned, we don't need to use a bounce page.
> + */
> + if (IS_ENABLED(CONFIG_SWIOTLB) && dev_is_untrusted(dev) &&
> + iova_offset(iovad, phys | org_size)) {
> + aligned_size = iova_align(iovad, org_size);
> + phys = swiotlb_tbl_map_single(dev,
> + phys_to_dma(dev, io_tlb_start),
> + phys, org_size, aligned_size, dir, attrs);
swiotlb_tbl_map_single takes one less argument in 5.10-rc now.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
