Re: [PATCH 1/1] drm/amdkfd: fix build error with AMD_IOMMU_V2=m
Am 2021-03-09 um 3:58 a.m. schrieb Arnd Bergmann: > On Tue, Mar 9, 2021 at 4:23 AM Felix Kuehling wrote: >> Using 'imply AMD_IOMMU_V2' does not guarantee that the driver can link >> against the exported functions. If the GPU driver is built-in but the >> IOMMU driver is a loadable module, the kfd_iommu.c file is indeed >> built but does not work: >> >> x86_64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_iommu.o: in function >> `kfd_iommu_bind_process_to_device': >> kfd_iommu.c:(.text+0x516): undefined reference to `amd_iommu_bind_pasid' >> x86_64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_iommu.o: in function >> `kfd_iommu_unbind_process': >> kfd_iommu.c:(.text+0x691): undefined reference to `amd_iommu_unbind_pasid' >> x86_64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_iommu.o: in function >> `kfd_iommu_suspend': >> kfd_iommu.c:(.text+0x966): undefined reference to >> `amd_iommu_set_invalidate_ctx_cb' >> x86_64-linux-ld: kfd_iommu.c:(.text+0x97f): undefined reference to >> `amd_iommu_set_invalid_ppr_cb' >> x86_64-linux-ld: kfd_iommu.c:(.text+0x9a4): undefined reference to >> `amd_iommu_free_device' >> x86_64-linux-ld: drivers/gpu/drm/amd/amdkfd/kfd_iommu.o: in function >> `kfd_iommu_resume': >> kfd_iommu.c:(.text+0xa9a): undefined reference to `amd_iommu_init_device' >> x86_64-linux-ld: kfd_iommu.c:(.text+0xadc): undefined reference to >> `amd_iommu_set_invalidate_ctx_cb' >> x86_64-linux-ld: kfd_iommu.c:(.text+0xaff): undefined reference to >> `amd_iommu_set_invalid_ppr_cb' >> x86_64-linux-ld: kfd_iommu.c:(.text+0xc72): undefined reference to >> `amd_iommu_bind_pasid' >> x86_64-linux-ld: kfd_iommu.c:(.text+0xe08): undefined reference to >> `amd_iommu_set_invalidate_ctx_cb' >> x86_64-linux-ld: kfd_iommu.c:(.text+0xe26): undefined reference to >> `amd_iommu_set_invalid_ppr_cb' >> x86_64-linux-ld: kfd_iommu.c:(.text+0xe42): undefined reference to >> `amd_iommu_free_device' >> >> Use IS_REACHABLE to only build IOMMU-V2 support if the amd_iommu symbols >> are reachable by the amdkfd driver. Output a warning if they are not, >> because that may not be what the user was expecting. > This would fix the compile-time failure, but it still fails my CI > because you introduce > a compile-time warning. Can you change it into a runtime warning instead? Sure. > > Neither type of warning is likely to actually reach the person trying > to debug the > problem, so you still end up with machines that don't do what they should, > but I can live with the runtime warning as long as the build doesn't break. OK. > > I think the proper fix would be to not rely on custom hooks into a particular > IOMMU driver, but to instead ensure that the amdgpu driver can do everything > it needs through the regular linux/iommu.h interfaces. I realize this > is more work, > but I wonder if you've tried that, and why it didn't work out. As far as I know this hasn't been tried. I see that intel-iommu has its own SVM thing, which seems to be similar to what our IOMMUv2 does. I guess we'd have to abstract that into a common API. Regards, Felix > >Arnd ___ 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 0/2] iommu/amd: Fix IOMMUv2 devices when SME is active
Am 2020-09-06 um 12:08 p.m. schrieb Deucher, Alexander: > [AMD Official Use Only - Internal Distribution Only] > >> -Original Message- >> From: Joerg Roedel >> Sent: Friday, September 4, 2020 6:06 AM >> To: Deucher, Alexander >> Cc: jroe...@suse.de; Kuehling, Felix ; >> iommu@lists.linux-foundation.org; Huang, Ray ; >> Koenig, Christian ; Lendacky, Thomas >> ; Suthikulpanit, Suravee >> ; linux-ker...@vger.kernel.org >> Subject: Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is >> active >> >> On Fri, Aug 28, 2020 at 03:47:07PM +, Deucher, Alexander wrote: >>> Ah, right, So CZ and ST are not an issue. Raven is paired with Zen based >> CPUs. >> >> Okay, so for the Raven case, can you add code to the amdgpu driver which >> makes it fail to initialize on Raven when SME is active? There is a global >> checking function for that, so that shouldn't be hard to do. >> > Sure. How about the attached patch? The patch is Acked-by: Felix Kuehling Thanks, Felix > > Alex > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is active
Am 2020-08-28 um 9:46 a.m. schrieb jroe...@suse.de: > On Wed, Aug 26, 2020 at 03:25:58PM +, Deucher, Alexander wrote: >>> Alex, do you know if anyone has tested amdgpu on an APU with SME >>> enabled? Is this considered something we support? >> It's not something we've tested. I'm not even sure the GPU portion of >> APUs will work properly without an identity mapping. SME should work >> properly with dGPUs however, so this is a proper fix for them. We >> don't use the IOMMUv2 path on dGPUs at all. > Is it possible to make the IOMMUv2 paths optional on iGPUs as well when > SME is active (or better, when the GPU is not identity mapped)? Yes, we're working on this. IOMMUv2 is only needed for KFD. It's not needed for graphics. And we're making it optional for KFD as well. The question Alex and I raised here is more general. We may have some assumptions in the amdgpu driver that are broken when the framebuffer is not identity mapped. This would break the iGPU in a more general sense, regardless of KFD and IOMMUv2. In that case, we don't really need to worry about breaking KFD because we have a much bigger problem. Regards, Felix > > Regards, > > Joerg ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is active
[+Ray] Thanks for the heads up. Currently KFD won't work on APUs when IOMMUv2 is disabled. But Ray is working on fallbacks that will allow KFD to work on APUs even without IOMMUv2, similar to our dGPUs. Along with changes in ROCm user mode, those fallbacks are necessary for making ROCm on APUs generally useful. How common is SME on typical PCs or laptops that would use AMD APUs? Alex, do you know if anyone has tested amdgpu on an APU with SME enabled? Is this considered something we support? Thanks, Felix Am 2020-08-26 um 10:14 a.m. schrieb Deucher, Alexander: > > [AMD Official Use Only - Internal Distribution Only] > > > + Felix > > *From:* Joerg Roedel > *Sent:* Monday, August 24, 2020 6:54 AM > *To:* iommu@lists.linux-foundation.org > *Cc:* Joerg Roedel ; jroe...@suse.de > ; Lendacky, Thomas ; > Suthikulpanit, Suravee ; Deucher, > Alexander ; linux-ker...@vger.kernel.org > > *Subject:* [PATCH 0/2] iommu/amd: Fix IOMMUv2 devices when SME is active > > From: Joerg Roedel > > Hi, > > Some IOMMUv2 capable devices do not work correctly when SME is > active, because their DMA mask does not include the encryption bit, so > that they can not DMA to encrypted memory directly. > > The IOMMU can jump in here, but the AMD IOMMU driver puts IOMMUv2 > capable devices into an identity mapped domain. Fix that by not > forcing an identity mapped domain on devices when SME is active and > forbid using their IOMMUv2 functionality. > > Please review. > > Thanks, > > Joerg > > Joerg Roedel (2): > iommu/amd: Do not force direct mapping when SME is active > iommu/amd: Do not use IOMMUv2 functionality when SME is active > > drivers/iommu/amd/iommu.c | 7 ++- > drivers/iommu/amd/iommu_v2.c | 7 +++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > -- > 2.28.0 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 01/12] iommu: Change type of pasid to u32
Am 2020-07-02 um 3:10 p.m. schrieb Fenghua Yu: > Hi, Felix, Thomas, Joerg and maintainers, > > On Tue, Jun 30, 2020 at 10:12:38PM -0400, Felix Kuehling wrote: >> Am 2020-06-30 um 7:44 p.m. schrieb Fenghua Yu: >> You didn't change the return types of amdgpu_pasid_alloc and >> kfd_pasid_alloc. amdgpu_pasid_alloc returns int, because it can return >> negative error codes. But kfd_pasid_alloc could be updated, because it >> returns 0 for errors. > I fixed return type as "u32" for kfd_pasid_alloc(). Thank you. The patch is Acked-by: Felix Kuehling > > The fix is minor and only limited in patch 1. So instead of sending the > whole series, I only send the updated patch 1 here. If you want me to > send the whole series with the fix, I can do that too. > > Thanks. > > -Fenghua > > From 4ff6c14bb0761dd97d803350d31f87edc4336345 Mon Sep 17 00:00:00 2001 > From: Fenghua Yu > Date: Mon, 4 May 2020 18:00:55 + > Subject: [PATCH v5.1 01/12] iommu: Change type of pasid to u32 > > PASID is defined as a few different types in iommu including "int", > "u32", and "unsigned int". To be consistent and to match with uapi > definitions, define PASID and its variations (e.g. max PASID) as "u32". > "u32" is also shorter and a little more explicit than "unsigned int". > > No PASID type change in uapi although it defines PASID as __u64 in > some places. > > Suggested-by: Thomas Gleixner > Signed-off-by: Fenghua Yu > Reviewed-by: Tony Luck > Reviewed-by: Lu Baolu > --- > v5.1: > - Change return type to u32 for kfd_pasid_alloc() (Felix) > > v5: > - Reviewed by Lu Baolu > > v4: > - Change PASID type from "unsigned int" to "u32" (Christoph) > > v2: > - Create this new patch to define PASID as "unsigned int" consistently in > iommu (Thomas) > > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 4 +-- > .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c| 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h | 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +-- > drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 6 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h | 4 +-- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 8 ++--- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h| 8 ++--- > .../gpu/drm/amd/amdkfd/cik_event_interrupt.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_dbgmgr.h | 2 +- > .../drm/amd/amdkfd/kfd_device_queue_manager.c | 7 ++--- > drivers/gpu/drm/amd/amdkfd/kfd_events.c | 8 ++--- > drivers/gpu/drm/amd/amdkfd/kfd_events.h | 4 +-- > drivers/gpu/drm/amd/amdkfd/kfd_iommu.c| 6 ++-- > drivers/gpu/drm/amd/amdkfd/kfd_pasid.c| 4 +-- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 20 ++-- > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +- > .../gpu/drm/amd/include/kgd_kfd_interface.h | 2 +- > drivers/iommu/amd/amd_iommu.h | 10 +++--- > drivers/iommu/amd/iommu.c | 31 ++- > drivers/iommu/amd/iommu_v2.c | 20 ++-- > drivers/iommu/intel/dmar.c| 7 +++-- > drivers/iommu/intel/intel-pasid.h | 24 +++--- > drivers/iommu/intel/iommu.c | 4 +-- > drivers/iommu/intel/pasid.c | 31 +-- > drivers/iommu/intel/svm.c | 12 +++ > drivers/iommu/iommu.c | 2 +- > drivers/misc/uacce/uacce.c| 2 +- > include/linux/amd-iommu.h | 8 ++--- > include/linux/intel-iommu.h | 12 +++ > include/linux/intel-svm.h | 2 +- > include/linux/iommu.h | 10 +++--- > include/linux/uacce.h | 2 +- > 38 files changed, 141 insertions(+), 141 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > index ffe149aafc39..dfef5a7e0f5a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > @@ -207,11 +207,11 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct > kgd_dev *dst, struct kgd_dev *s > }) > > /* GPUVM API */ > -int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int > pasid, >
Re: [PATCH v5 01/12] iommu: Change type of pasid to u32
Am 2020-06-30 um 7:44 p.m. schrieb Fenghua Yu: > PASID is defined as a few different types in iommu including "int", > "u32", and "unsigned int". To be consistent and to match with uapi > definitions, define PASID and its variations (e.g. max PASID) as "u32". > "u32" is also shorter and a little more explicit than "unsigned int". You didn't change the return types of amdgpu_pasid_alloc and kfd_pasid_alloc. amdgpu_pasid_alloc returns int, because it can return negative error codes. But kfd_pasid_alloc could be updated, because it returns 0 for errors. Regards, Felix > > No PASID type change in uapi although it defines PASID as __u64 in > some places. > > Suggested-by: Thomas Gleixner > Signed-off-by: Fenghua Yu > Reviewed-by: Tony Luck > Reviewed-by: Lu Baolu > --- > v5: > - Reviewed by Lu Baolu > > v4: > - Change PASID type from "unsigned int" to "u32" (Christoph) > > v2: > - Create this new patch to define PASID as "unsigned int" consistently in > iommu (Thomas) > > drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h| 4 +-- > .../drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c| 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.c | 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v9.h | 2 +- > .../gpu/drm/amd/amdgpu/amdgpu_amdkfd_gpuvm.c | 4 +-- > drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 6 ++-- > drivers/gpu/drm/amd/amdgpu/amdgpu_ids.h | 4 +-- > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 8 ++--- > drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h| 8 ++--- > .../gpu/drm/amd/amdkfd/cik_event_interrupt.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_dbgdev.c | 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_dbgmgr.h | 2 +- > .../drm/amd/amdkfd/kfd_device_queue_manager.c | 7 ++--- > drivers/gpu/drm/amd/amdkfd/kfd_events.c | 8 ++--- > drivers/gpu/drm/amd/amdkfd/kfd_events.h | 4 +-- > drivers/gpu/drm/amd/amdkfd/kfd_iommu.c| 6 ++-- > drivers/gpu/drm/amd/amdkfd/kfd_pasid.c| 2 +- > drivers/gpu/drm/amd/amdkfd/kfd_priv.h | 18 +-- > drivers/gpu/drm/amd/amdkfd/kfd_process.c | 2 +- > .../gpu/drm/amd/include/kgd_kfd_interface.h | 2 +- > drivers/iommu/amd/amd_iommu.h | 10 +++--- > drivers/iommu/amd/iommu.c | 31 ++- > drivers/iommu/amd/iommu_v2.c | 20 ++-- > drivers/iommu/intel/dmar.c| 7 +++-- > drivers/iommu/intel/intel-pasid.h | 24 +++--- > drivers/iommu/intel/iommu.c | 4 +-- > drivers/iommu/intel/pasid.c | 31 +-- > drivers/iommu/intel/svm.c | 12 +++ > drivers/iommu/iommu.c | 2 +- > drivers/misc/uacce/uacce.c| 2 +- > include/linux/amd-iommu.h | 8 ++--- > include/linux/intel-iommu.h | 12 +++ > include/linux/intel-svm.h | 2 +- > include/linux/iommu.h | 10 +++--- > include/linux/uacce.h | 2 +- > 38 files changed, 139 insertions(+), 139 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > index ffe149aafc39..dfef5a7e0f5a 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd.h > @@ -207,11 +207,11 @@ uint8_t amdgpu_amdkfd_get_xgmi_hops_count(struct > kgd_dev *dst, struct kgd_dev *s > }) > > /* GPUVM API */ > -int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, unsigned int > pasid, > +int amdgpu_amdkfd_gpuvm_create_process_vm(struct kgd_dev *kgd, u32 pasid, > void **vm, void **process_info, > struct dma_fence **ef); > int amdgpu_amdkfd_gpuvm_acquire_process_vm(struct kgd_dev *kgd, > - struct file *filp, unsigned int pasid, > + struct file *filp, u32 pasid, > void **vm, void **process_info, > struct dma_fence **ef); > void amdgpu_amdkfd_gpuvm_destroy_cb(struct amdgpu_device *adev, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c > index bf927f432506..ee531c3988d1 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v10.c > @@ -105,7 +105,7 @@ static void kgd_program_sh_mem_settings(struct kgd_dev > *kgd, uint32_t vmid, > unlock_srbm(kgd); > } > > -static int kgd_set_pasid_vmid_mapping(struct kgd_dev *kgd, unsigned int > pasid, > +static int kgd_set_pasid_vmid_mapping(struct kgd_dev
Re: [PATCH v5 02/25] iommu/sva: Manage process address spaces
Am 2020-04-20 um 1:44 p.m. schrieb Jacob Pan: >> The bottom line is, when you allocate a PASID for a context, you want >> to know how small it needs to be for all the devices that want to use >> it. If you make it too big, some device will not be able to use it. >> If you make it too small, you waste precious PASIDs that could be >> used for other contexts that need them. >> > So for AMD, system-wide PASID allocation works with the > restriction/optimization above? > Yes for KFD. On multi-GPU systems we allocate one PASID for the whole process and use it on all GPUs. For AMDGPU graphics contexts, we allocate one PASID for each per-GPU context. But they're allocated from a single global PASID namespace managed by the AMDGPU driver and shared with KFD. So we're wasting PASIDs here, but we are compatible with a single system-wide PASID namespace. Regards, Felix ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v5 02/25] iommu/sva: Manage process address spaces
Am 2020-04-20 um 8:40 a.m. schrieb Christian König: > Am 20.04.20 um 13:55 schrieb Christoph Hellwig: >> On Mon, Apr 20, 2020 at 01:44:56PM +0200, Christian König wrote: >>> Am 20.04.20 um 10:10 schrieb Christoph Hellwig: On Mon, Apr 20, 2020 at 09:42:13AM +0200, Jean-Philippe Brucker wrote: > Right, I can see the appeal. I still like having a single mmu > notifier per > mm because it ensures we allocate a single PASID per mm (as > required by > x86). I suppose one alternative is to maintain a hashtable of > mm->pasid, > to avoid iterating over all bonds during allocation. Given that the PASID is a pretty generic and important concept can we just add it directly to the mm_struct and allocate it lazily once we first need it? >>> Well the problem is that the PASID might as well be device specific. >>> E.g. >>> some devices use 16bit PASIDs, some 15bit, some other only 12bit. >>> >>> So what could (at least in theory) happen is that you need to allocate >>> different PASIDs for the same process because different devices need >>> one. >> This directly contradicts the statement from Jean-Philippe above that >> x86 requires a single PASID per mm_struct. If we may need different >> PASIDs for different devices and can actually support this just >> allocating one per [device, mm_struct] would make most sense of me, as >> it doesn't couple otherwise disjoint state. > > Well I'm not an expert on this topic. Felix can probably tell you a > bit more about that. > > Maybe it is sufficient to keep the allocated PASIDs as small as > possible and return an appropriate error if a device can't deal with > the allocated number. > > If a device can only deal with 12bit PASIDs and more than 2^12 try to > use it there isn't much else we can do than returning an error anyway. I'm probably missing some context. But let me try giving a useful reply. The hardware allows you to have different PASIDs for each device referring to the same address space. But I think it's OK for software to choose not to do that. If Linux wants to manage one PASID namespace for all devices, that's a reasonable choice IMO. Different devices have different limits for the size of PASID they can support. For example AMD GPUs support 16-bits but the IOMMU supports less. So on APUs we use small PASIDs for contexts that want to use IOMMUv2 to access memory, but bigger PASIDs for contexts that do not. I choose the word "context" deliberately, because the amdgpu driver uses PASIDs even when we're not using IOMMUv2, and we're using them to identify GPU virtual address spaces. There can be more than one per process. In practice you can have two, one for graphics (not SVM, doesn't use IOMMUv2) and one for KFD compute (SVM, can use IOMMUv2 on APUs). Because the IOMMUv2 supports only smaller PASIDs, we want to avoid exhausting that space with PASID allocations that don't use the IOMMUv2. So our PASID allocation function has a "size" parameter, and we try to allocated a PASID as big as possible in order to leave more precious smaller PASIDs for contexts that need them. The bottom line is, when you allocate a PASID for a context, you want to know how small it needs to be for all the devices that want to use it. If you make it too big, some device will not be able to use it. If you make it too small, you waste precious PASIDs that could be used for other contexts that need them. Regards, Felix ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
