Re: A problem of Intel IOMMU hardware ？
Hi Nadav, On 3/27/21 12:36 PM, Nadav Amit wrote: On Mar 26, 2021, at 7:31 PM, Lu Baolu wrote: Hi Nadav, On 3/19/21 12:46 AM, Nadav Amit wrote: So here is my guess: Intel probably used as a basis for the IOTLB an implementation of some other (regular) TLB design. Intel SDM says regarding TLBs (4.10.4.2 “Recommended Invalidation”): "Software wishing to prevent this uncertainty should not write to a paging-structure entry in a way that would change, for any linear address, both the page size and either the page frame, access rights, or other attributes.” Now the aforementioned uncertainty is a bit different (multiple *valid* translations of a single address). Yet, perhaps this is yet another thing that might happen. From a brief look on the handling of MMU (not IOMMU) hugepages in Linux, indeed the PMD is first cleared and flushed before a new valid PMD is set. This is possible for MMUs since they allow the software to handle spurious page-faults gracefully. This is not the case for the IOMMU though (without PRI). Not sure this explains everything though. If that is the problem, then during a mapping that changes page-sizes, a TLB flush is needed, similarly to the one Longpeng did manually. I have been working with Longpeng on this issue these days. It turned out that your guess is right. The PMD is first cleared but not flushed before a new valid one is set. The previous entry might be cached in the paging structure caches hence leads to disaster. In __domain_mapping(): 2352 /* 2353 * Ensure that old small page tables are 2354 * removed to make room for superpage(s). 2355 * We're adding new large pages, so make sure 2356 * we don't remove their parent tables. 2357 */ 2358 dma_pte_free_pagetable(domain, iov_pfn, end_pfn, 2359 largepage_lvl + 1); I guess adding a cache flush operation after PMD switching should solve the problem. I am still not clear about this comment: " This is possible for MMUs since they allow the software to handle spurious page-faults gracefully. This is not the case for the IOMMU though (without PRI). " Can you please shed more light on this? I was looking at the code in more detail, and apparently my concern is incorrect. I was under the assumption that the IOMMU map/unmap can merge/split (specifically split) huge-pages. For instance, if you map 2MB and then unmap 4KB out of the 2MB, then you would split the hugepage and keep the rest of the mappings alive. This is the way MMU is usually managed. To my defense, I also saw such partial unmappings in Longpeng’s first scenario. If this was possible, then you would have a case in which out of 2MB (for instance), 4KB were unmapped, and you need to split the 2MB hugepage into 4KB pages. If you try to clear the PMD, flush, and then set the PMD to point to table with live 4KB PTES, you can have an interim state in which the PMD is not present. DMAs that arrive at this stage might fault, and without PRI (and device support) you do not have a way of restarting the DMA after the hugepage split is completed. Get you and thanks a lot for sharing. For current IOMMU usage, I can't see any case to split a huge page into 4KB pages, but in the near future, we do have a need of splitting huge pages. For example, when we want to use the A/D bit to track the DMA dirty pages during VM migration, it's an optimization if we could split a huge page into 4K ones. So far, the solution I have considered is: 1) Prepare the split subtables in advance; [it's identical to the existing one only use 4k pages instead of huge page.] 2) Switch the super (huge) page's leaf entry; [at this point, hardware could use both subtables. I am not sure whether the hardware allows a dynamic switch of page table entry from on valid entry to another valid one.] 3) Flush the cache. [hardware will use the new subtable] As long as we can make sure that the old subtable won't be used by hardware, we can safely release the old table. Anyhow, this concern is apparently not relevant. I guess I was too naive to assume the IOMMU management is similar to the MMU. I now see that there is a comment in intel_iommu_unmap() saying: /* Cope with horrid API which requires us to unmap more than the size argument if it happens to be a large-page mapping. */ Regards, Nadav Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: A problem of Intel IOMMU hardware ？
> On Mar 26, 2021, at 7:31 PM, Lu Baolu wrote: > > Hi Nadav, > > On 3/19/21 12:46 AM, Nadav Amit wrote: >> So here is my guess: >> Intel probably used as a basis for the IOTLB an implementation of >> some other (regular) TLB design. >> Intel SDM says regarding TLBs (4.10.4.2 “Recommended Invalidation”): >> "Software wishing to prevent this uncertainty should not write to >> a paging-structure entry in a way that would change, for any linear >> address, both the page size and either the page frame, access rights, >> or other attributes.” >> Now the aforementioned uncertainty is a bit different (multiple >> *valid* translations of a single address). Yet, perhaps this is >> yet another thing that might happen. >> From a brief look on the handling of MMU (not IOMMU) hugepages >> in Linux, indeed the PMD is first cleared and flushed before a >> new valid PMD is set. This is possible for MMUs since they >> allow the software to handle spurious page-faults gracefully. >> This is not the case for the IOMMU though (without PRI). >> Not sure this explains everything though. If that is the problem, >> then during a mapping that changes page-sizes, a TLB flush is >> needed, similarly to the one Longpeng did manually. > > I have been working with Longpeng on this issue these days. It turned > out that your guess is right. The PMD is first cleared but not flushed > before a new valid one is set. The previous entry might be cached in the > paging structure caches hence leads to disaster. > > In __domain_mapping(): > > 2352 /* > 2353 * Ensure that old small page tables are > 2354 * removed to make room for superpage(s). > 2355 * We're adding new large pages, so make > sure > 2356 * we don't remove their parent tables. > 2357 */ > 2358 dma_pte_free_pagetable(domain, iov_pfn, > end_pfn, > 2359 largepage_lvl + 1); > > I guess adding a cache flush operation after PMD switching should solve > the problem. > > I am still not clear about this comment: > > " > This is possible for MMUs since they allow the software to handle > spurious page-faults gracefully. This is not the case for the IOMMU > though (without PRI). > " > > Can you please shed more light on this? I was looking at the code in more detail, and apparently my concern is incorrect. I was under the assumption that the IOMMU map/unmap can merge/split (specifically split) huge-pages. For instance, if you map 2MB and then unmap 4KB out of the 2MB, then you would split the hugepage and keep the rest of the mappings alive. This is the way MMU is usually managed. To my defense, I also saw such partial unmappings in Longpeng’s first scenario. If this was possible, then you would have a case in which out of 2MB (for instance), 4KB were unmapped, and you need to split the 2MB hugepage into 4KB pages. If you try to clear the PMD, flush, and then set the PMD to point to table with live 4KB PTES, you can have an interim state in which the PMD is not present. DMAs that arrive at this stage might fault, and without PRI (and device support) you do not have a way of restarting the DMA after the hugepage split is completed. Anyhow, this concern is apparently not relevant. I guess I was too naive to assume the IOMMU management is similar to the MMU. I now see that there is a comment in intel_iommu_unmap() saying: /* Cope with horrid API which requires us to unmap more than the size argument if it happens to be a large-page mapping. */ Regards, Nadav signature.asc Description: Message signed with OpenPGP ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: A problem of Intel IOMMU hardware ？
Hi Nadav, On 3/19/21 12:46 AM, Nadav Amit wrote: So here is my guess: Intel probably used as a basis for the IOTLB an implementation of some other (regular) TLB design. Intel SDM says regarding TLBs (4.10.4.2 “Recommended Invalidation”): "Software wishing to prevent this uncertainty should not write to a paging-structure entry in a way that would change, for any linear address, both the page size and either the page frame, access rights, or other attributes.” Now the aforementioned uncertainty is a bit different (multiple *valid* translations of a single address). Yet, perhaps this is yet another thing that might happen. From a brief look on the handling of MMU (not IOMMU) hugepages in Linux, indeed the PMD is first cleared and flushed before a new valid PMD is set. This is possible for MMUs since they allow the software to handle spurious page-faults gracefully. This is not the case for the IOMMU though (without PRI). Not sure this explains everything though. If that is the problem, then during a mapping that changes page-sizes, a TLB flush is needed, similarly to the one Longpeng did manually. I have been working with Longpeng on this issue these days. It turned out that your guess is right. The PMD is first cleared but not flushed before a new valid one is set. The previous entry might be cached in the paging structure caches hence leads to disaster. In __domain_mapping(): 2352 /* 2353 * Ensure that old small page tables are 2354 * removed to make room for superpage(s). 2355 * We're adding new large pages, so make sure 2356 * we don't remove their parent tables. 2357 */ 2358 dma_pte_free_pagetable(domain, iov_pfn, end_pfn, 2359 largepage_lvl + 1); I guess adding a cache flush operation after PMD switching should solve the problem. I am still not clear about this comment: " This is possible for MMUs since they allow the software to handle spurious page-faults gracefully. This is not the case for the IOMMU though (without PRI). " Can you please shed more light on this? Best regards, baolu ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/9] arm64: tegra: Prevent early SMMU faults
26.03.2021 19:55, Dmitry Osipenko пишет: > 26.03.2021 19:35, Thierry Reding пишет: >> On Fri, Mar 26, 2021 at 06:29:28PM +0300, Dmitry Osipenko wrote: >>> 25.03.2021 16:03, Thierry Reding пишет: From: Thierry Reding Hi, this is a set of patches that is the result of earlier discussions regarding early identity mappings that are needed to avoid SMMU faults during early boot. The goal here is to avoid early identity mappings altogether and instead postpone the need for the identity mappings to when devices are attached to the SMMU. This works by making the SMMU driver coordinate with the memory controller driver on when to start enforcing SMMU translations. This makes Tegra behave in a more standard way and pushes the code to deal with the Tegra-specific programming into the NVIDIA SMMU implementation. >>> >>> It is an interesting idea which inspired me to try to apply a somewhat >>> similar thing to Tegra SMMU driver by holding the SMMU ASID enable-bit >>> until display driver allows to toggle it. This means that we will need an >>> extra small tegra-specific SMMU API function, but it should be okay. >>> >>> I typed a patch and seems it's working good, I'll prepare a proper patch if >>> you like it. >> >> That would actually be working around the problem that this patch was >> supposed to prepare for. The reason for this current patch series is to >> make sure SMMU translation isn't enabled until a device has actually >> been attached to the SMMU. Once it has been attached, the assumption is >> that any identity mappings will have been created. >> >> One Tegra SMMU that shouldn't be a problem because translations aren't >> enabled until device attach time. So in other words this patch set is to >> get Tegra186 and later to parity with earlier chips from this point of >> view. >> >> I think the problem that you're trying to work around is better solved >> by establishing these identity mappings. I do have patches to implement >> this for Tegra210 and earlier, though they may require additional work >> if you have bootloaders that don't use standard DT bindings for passing >> information about the framebuffer to the kernel. > > I'm not sure what else reasonable could be done without upgrading to a > very specific version of firmware, which definitely isn't a variant for > older devices which have a wild variety of bootloaders, customized > use-cases and etc. > > We could add a kludge that I'm suggesting as a universal fallback > solution, it should work well for all cases that I care about. > > So we could have the variant with identity mappings, and if mapping > isn't provided, then fall back to the kludge. > I tried a slightly different variant of the kludge by holding the ASID's enable till the first mapping is created for the display clients and IOMMU_DOMAIN_DMA now works properly (no EMEM errors on boot and etc) and without a need to change the DC driver. I also tried to remove the arm_iommu_detach_device() from the VDE driver and we now have 3 implicit domains in use (DRM, HX, VDE[wasted]) + 1 explicit (VDE) on T30, which works okay for today. So technically we could support the IOMMU_DOMAIN_DMA with a couple small changes right now or at least revert the hacks that were needed for Nyan. But in order to enable IOMMU_DOMAIN_DMA properly, we will need to do something about the DMA mappings first in the DRM driver and I also found that implicit IOMMU somehow doesn't work for host1x driver at all, so this needs to be fixed too. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
> From: Arnd Bergmann
> Date: Fri, 26 Mar 2021 20:59:58 +0100
>
> On Fri, Mar 26, 2021 at 6:51 PM Sven Peter wrote:
> > On Fri, Mar 26, 2021, at 18:34, Robin Murphy wrote:
> > > On 2021-03-26 17:26, Mark Kettenis wrote:
> > > >
> > > > Anyway, from my viewpoint having the information about the IOVA
> > > > address space sit on the devices makes little sense. This information
> > > > is needed by the DART driver, and there is no direct cnnection from
> > > > the DART to the individual devices in the devicetree. The "iommus"
> > > > property makes a connection in the opposite direction.
> > >
> > > What still seems unclear is whether these addressing limitations are a
> > > property of the DART input interface, the device output interface, or
> > > the interconnect between them. Although the observable end result
> > > appears more or less the same either way, they are conceptually
> > > different things which we have different abstractions to deal with.
> >
> > I'm not really sure if there is any way for us to figure out where these
> > limitation comes from though.
>
> My first guess was that this is done to partition the available address
> address space in a way that allows one physical IOTLB to handle
> multiple devices that each have their own page table for a subset
> of the address space, as was done on old PowerPC IOMMUs.
> However, the ranges you list don't really support that model.
>
> > I've done some more experiments and looked at all DART nodes in Apple's
> > Device
> > Tree though. It seems that most (if not all) masters only connect 32 address
> > lines even though the iommu can handle a much larger address space. I'll
> > therefore
> > remove the code to handle the full space for v2 since it's essentially dead
> > code that can't be tested anyway.
> >
> >
> > There are some exceptions though:
> >
> > There are the PCIe DARTs which have a different limitation which could be
> > encoded as 'dma-ranges' in the pci bus node:
> >
> >name base size
> > dart-apcie1: 0010 3fe0
> > dart-apcie2: 0010 3fe0
> > dart-apcie0: 0010 3fe0
> > dart-apciec0: 4000 7fffc000
> > dart-apciec1: 8000 7fffc000
>
> This looks like they are reserving some address space in the beginning
> and/or the end, and for apciec0, the address space is partitioned into
> two equal-sized regions.
>
> > Then there are also these display controller DARTs. If we wanted to use
> > dma-ranges
> > we could just put them in a single sub bus:
> >
> > name base size
> > dart-disp0: fc00
> > dart-dcp: fc00
> >dart-dispext0: fc00
> > dart-dcpext: fc00
> >
> >
> > And finally we have these strange ones which might eventually each require
> > another awkward sub-bus if we want to stick to the dma-ranges property.
> >
> > name base size
> > dart-aop: 0003 ("always-on processor")
> > dart-pmp: bff0 (no idea yet)
>
> Here I also see a "pio-vm-size" property:
>
> dart-pmp {
> pio-vm-base = <0xc000>;
> pio-vm-size = <0x4000>;
> vm-size = <0xbff0>;
> ...
>};
>
> Which seems to give 3GB of address space to the normal iotlb,
> plus the last 1GB to something else. The vm-base property is also
> missing rather than zero, but that could just be part of their syntax
> instead of a real difference.
Yes. It seems like "vm-base" is omitted when it is 0, and "vm-size"
is omitted when the end of the window is at 0x.
>
> Could it be that there are
>
> > dart-sio: 0021c000 fbde4000 (at least their Secure Enclave/TPM
> > co-processor)
>
> Same here:
> dart-sio {
>vm-base = <0x0>;
>vm-size = <0xfc00>;
>pio-vm-base = <0xfd00>;
> pio-vm-size = <0x200>;
> pio-granularity = <0x100>;
>}
>
> There are clearly two distinct ranges that split up the 4GB space again,
> with a small hole of 16MB (==pio-granularity) at the end of each range.
>
> The "pio" name might indicate that this is a range of addresses that
> can be programmed to point at I/O registers in another device, rather
> than pointing to RAM.
>
>Arnd
>
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
> From: Arnd Bergmann > Date: Fri, 26 Mar 2021 21:03:32 +0100 > > On Fri, Mar 26, 2021 at 6:28 PM Mark Kettenis wrote: > > > I haven't figured out how the bypass stuff really works. Corellium > > added support for it in their codebase when they added support for > > Thunderbolt, and some of the DARTs that seem to be related to > > Thunderbolt do indeed have a "bypass" property. But it is unclear to > > me how the different puzzle pieces fit together for Thunderbolt. > > As a general observation, bypass mode for Thunderbolt is what enabled > the http://thunderclap.io/ attack. This is extremely useful for debugging > a running kernel from another machine, but it's also something that > should never be done in a production kernel. No kidding! I was surprised to see the bypass support on the Thunderbolt-related nodes. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH] dma-mapping: add unlikely hint to error path in dma_mapping_error
Zillions of drivers use the unlikely() hint when checking the result of
dma_mapping_error(). This is an inline function anyway, so we can move
the hint into the function and remove it from drivers over time.
Signed-off-by: Heiner Kallweit
---
This is a resend of a patch from Dec 2020 when I tried to do it
tree-wide. Now start with the actual change, drivers can be changed
afterwards, maybe per subsystem.
---
include/linux/dma-mapping.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index e9d19b974..183e7103a 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -95,7 +95,7 @@ static inline int dma_mapping_error(struct device *dev,
dma_addr_t dma_addr)
{
debug_dma_mapping_error(dev, dma_addr);
- if (dma_addr == DMA_MAPPING_ERROR)
+ if (unlikely(dma_addr == DMA_MAPPING_ERROR))
return -ENOMEM;
return 0;
}
--
2.31.0
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
On Fri, Mar 26, 2021 at 6:28 PM Mark Kettenis wrote: > I haven't figured out how the bypass stuff really works. Corellium > added support for it in their codebase when they added support for > Thunderbolt, and some of the DARTs that seem to be related to > Thunderbolt do indeed have a "bypass" property. But it is unclear to > me how the different puzzle pieces fit together for Thunderbolt. As a general observation, bypass mode for Thunderbolt is what enabled the http://thunderclap.io/ attack. This is extremely useful for debugging a running kernel from another machine, but it's also something that should never be done in a production kernel. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
On Fri, Mar 26, 2021 at 6:51 PM Sven Peter wrote:
> On Fri, Mar 26, 2021, at 18:34, Robin Murphy wrote:
> > On 2021-03-26 17:26, Mark Kettenis wrote:
> > >
> > > Anyway, from my viewpoint having the information about the IOVA
> > > address space sit on the devices makes little sense. This information
> > > is needed by the DART driver, and there is no direct cnnection from
> > > the DART to the individual devices in the devicetree. The "iommus"
> > > property makes a connection in the opposite direction.
> >
> > What still seems unclear is whether these addressing limitations are a
> > property of the DART input interface, the device output interface, or
> > the interconnect between them. Although the observable end result
> > appears more or less the same either way, they are conceptually
> > different things which we have different abstractions to deal with.
>
> I'm not really sure if there is any way for us to figure out where these
> limitation comes from though.
My first guess was that this is done to partition the available address
address space in a way that allows one physical IOTLB to handle
multiple devices that each have their own page table for a subset
of the address space, as was done on old PowerPC IOMMUs.
However, the ranges you list don't really support that model.
> I've done some more experiments and looked at all DART nodes in Apple's Device
> Tree though. It seems that most (if not all) masters only connect 32 address
> lines even though the iommu can handle a much larger address space. I'll
> therefore
> remove the code to handle the full space for v2 since it's essentially dead
> code that can't be tested anyway.
>
>
> There are some exceptions though:
>
> There are the PCIe DARTs which have a different limitation which could be
> encoded as 'dma-ranges' in the pci bus node:
>
>name base size
> dart-apcie1: 0010 3fe0
> dart-apcie2: 0010 3fe0
> dart-apcie0: 0010 3fe0
> dart-apciec0: 4000 7fffc000
> dart-apciec1: 8000 7fffc000
This looks like they are reserving some address space in the beginning
and/or the end, and for apciec0, the address space is partitioned into
two equal-sized regions.
> Then there are also these display controller DARTs. If we wanted to use
> dma-ranges
> we could just put them in a single sub bus:
>
> name base size
> dart-disp0: fc00
> dart-dcp: fc00
>dart-dispext0: fc00
> dart-dcpext: fc00
>
>
> And finally we have these strange ones which might eventually each require
> another awkward sub-bus if we want to stick to the dma-ranges property.
>
> name base size
> dart-aop: 0003 ("always-on processor")
> dart-pmp: bff0 (no idea yet)
Here I also see a "pio-vm-size" property:
dart-pmp {
pio-vm-base = <0xc000>;
pio-vm-size = <0x4000>;
vm-size = <0xbff0>;
...
};
Which seems to give 3GB of address space to the normal iotlb,
plus the last 1GB to something else. The vm-base property is also
missing rather than zero, but that could just be part of their syntax
instead of a real difference.
Could it be that there are
> dart-sio: 0021c000 fbde4000 (at least their Secure Enclave/TPM co-processor)
Same here:
dart-sio {
vm-base = <0x0>;
vm-size = <0xfc00>;
pio-vm-base = <0xfd00>;
pio-vm-size = <0x200>;
pio-granularity = <0x100>;
}
There are clearly two distinct ranges that split up the 4GB space again,
with a small hole of 16MB (==pio-granularity) at the end of each range.
The "pio" name might indicate that this is a range of addresses that
can be programmed to point at I/O registers in another device, rather
than pointing to RAM.
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
On Fri, Mar 26, 2021, at 18:34, Robin Murphy wrote:
> On 2021-03-26 17:26, Mark Kettenis wrote:
> >
> > Anyway, from my viewpoint having the information about the IOVA
> > address space sit on the devices makes little sense. This information
> > is needed by the DART driver, and there is no direct cnnection from
> > the DART to the individual devices in the devicetree. The "iommus"
> > property makes a connection in the opposite direction.
>
> What still seems unclear is whether these addressing limitations are a
> property of the DART input interface, the device output interface, or
> the interconnect between them. Although the observable end result
> appears more or less the same either way, they are conceptually
> different things which we have different abstractions to deal with.
>
> Robin.
>
I'm not really sure if there is any way for us to figure out where these
limitation comes from though.
I've done some more experiments and looked at all DART nodes in Apple's Device
Tree though. It seems that most (if not all) masters only connect 32 address
lines even though the iommu can handle a much larger address space. I'll
therefore
remove the code to handle the full space for v2 since it's essentially dead
code that can't be tested anyway.
There are some exceptions though:
There are the PCIe DARTs which have a different limitation which could be
encoded as 'dma-ranges' in the pci bus node:
name base size
dart-apcie1: 0010 3fe0
dart-apcie2: 0010 3fe0
dart-apcie0: 0010 3fe0
dart-apciec0: 4000 7fffc000
dart-apciec1: 8000 7fffc000
Then there are also these display controller DARTs. If we wanted to use
dma-ranges
we could just put them in a single sub bus:
name base size
dart-disp0: fc00
dart-dcp: fc00
dart-dispext0: fc00
dart-dcpext: fc00
And finally we have these strange ones which might eventually each require
another awkward sub-bus if we want to stick to the dma-ranges property.
name base size
dart-aop: 0003 ("always-on processor")
dart-pmp: bff0 (no idea yet)
dart-sio: 0021c000 fbde4000 (at least their Secure Enclave/TPM co-processor)
dart-ane: e000 ("Neural Engine", their ML accelerator)
For all we know these limitations could even arise for different reasons.
(the secure enclave one looks like it might be imposed by the code running
on there).
Not really sure to proceed from here. I'll give the dma-ranges options a try
for v2 and see how that one works out but that's not going to help us understand
*why* these limitations exist.
At least I won't have to change much code if we agree on a different
abstraction :)
The important ones for now are probably the USB and the PCIe ones. We'll need
the
display ones after that and can probably ignore the strange ones for quite a
while.
Best,
Sven
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
On 2021-03-26 17:26, Mark Kettenis wrote: From: Arnd Bergmann Date: Fri, 26 Mar 2021 17:38:24 +0100 On Fri, Mar 26, 2021 at 5:10 PM Sven Peter wrote: On Fri, Mar 26, 2021, at 16:59, Mark Kettenis wrote: Some of the DARTs provide a bypass facility. That code make using the standard "dma-ranges" property tricky. That property would need to contain the bypass address range. But that would mean that if the DART driver needs to look at that property to figure out the address range that supports translation it will need to be able to distinguish between the translatable address range and the bypass address range. Do we understand if and why we even need to bypass certain streams? My guess is that this is a performance optimization. There are generally three reasons to want an iommu in the first place: - Pass a device down to a guest or user process without giving access to all of memory - Avoid problems with limitations in the device, typically when it only supports 32-bit bus addressing, but the installed memory is larger than 4GB - Protect kernel memory from broken drivers If you care about none of the above, but you do care about data transfer speed, you are better off just leaving the IOMMU in bypass mode. I don't think we have to support it if the IOMMU works reliably, but it's something that users might want. Another reason might be that a device needs access to large amounts of physical memory at the same time and the 32-bit address space that the DART provides is too tight. In U-Boot I might want to use bypass where it works since there is no proper IOMMU support in U-Boot. Generally speaking, the goal is to keep the code size down in U-Boot. In OpenBSD I'll probably avoid bypass mode if I can. I haven't figured out how the bypass stuff really works. Corellium added support for it in their codebase when they added support for Thunderbolt, and some of the DARTs that seem to be related to Thunderbolt do indeed have a "bypass" property. But it is unclear to me how the different puzzle pieces fit together for Thunderbolt. Anyway, from my viewpoint having the information about the IOVA address space sit on the devices makes little sense. This information is needed by the DART driver, and there is no direct cnnection from the DART to the individual devices in the devicetree. The "iommus" property makes a connection in the opposite direction. What still seems unclear is whether these addressing limitations are a property of the DART input interface, the device output interface, or the interconnect between them. Although the observable end result appears more or less the same either way, they are conceptually different things which we have different abstractions to deal with. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
> From: Arnd Bergmann > Date: Fri, 26 Mar 2021 17:38:24 +0100 > > On Fri, Mar 26, 2021 at 5:10 PM Sven Peter wrote: > > On Fri, Mar 26, 2021, at 16:59, Mark Kettenis wrote: > > > Some of the DARTs provide a bypass facility. That code make using the > > > standard "dma-ranges" property tricky. That property would need to > > > contain the bypass address range. But that would mean that if the > > > DART driver needs to look at that property to figure out the address > > > range that supports translation it will need to be able to distinguish > > > between the translatable address range and the bypass address range. > > > > Do we understand if and why we even need to bypass certain streams? > > My guess is that this is a performance optimization. > > There are generally three reasons to want an iommu in the first place: > - Pass a device down to a guest or user process without giving >access to all of memory > - Avoid problems with limitations in the device, typically when it > only supports >32-bit bus addressing, but the installed memory is larger than 4GB > - Protect kernel memory from broken drivers > > If you care about none of the above, but you do care about data transfer > speed, you are better off just leaving the IOMMU in bypass mode. > I don't think we have to support it if the IOMMU works reliably, but it's > something that users might want. Another reason might be that a device needs access to large amounts of physical memory at the same time and the 32-bit address space that the DART provides is too tight. In U-Boot I might want to use bypass where it works since there is no proper IOMMU support in U-Boot. Generally speaking, the goal is to keep the code size down in U-Boot. In OpenBSD I'll probably avoid bypass mode if I can. I haven't figured out how the bypass stuff really works. Corellium added support for it in their codebase when they added support for Thunderbolt, and some of the DARTs that seem to be related to Thunderbolt do indeed have a "bypass" property. But it is unclear to me how the different puzzle pieces fit together for Thunderbolt. Anyway, from my viewpoint having the information about the IOVA address space sit on the devices makes little sense. This information is needed by the DART driver, and there is no direct cnnection from the DART to the individual devices in the devicetree. The "iommus" property makes a connection in the opposite direction. Cheers, Mark ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
On Fri, Mar 26, 2021, at 17:38, Arnd Bergmann wrote: > On Fri, Mar 26, 2021 at 5:10 PM Sven Peter wrote: > > On Fri, Mar 26, 2021, at 16:59, Mark Kettenis wrote: > > > Some of the DARTs provide a bypass facility. That code make using the > > > standard "dma-ranges" property tricky. That property would need to > > > contain the bypass address range. But that would mean that if the > > > DART driver needs to look at that property to figure out the address > > > range that supports translation it will need to be able to distinguish > > > between the translatable address range and the bypass address range. > > > > Do we understand if and why we even need to bypass certain streams? > > My guess is that this is a performance optimization. Makes sense. > > There are generally three reasons to want an iommu in the first place: > - Pass a device down to a guest or user process without giving >access to all of memory > - Avoid problems with limitations in the device, typically when it > only supports >32-bit bus addressing, but the installed memory is larger than 4GB > - Protect kernel memory from broken drivers > > If you care about none of the above, but you do care about data transfer > speed, you are better off just leaving the IOMMU in bypass mode. > I don't think we have to support it if the IOMMU works reliably, but it's > something that users might want. Right now the IOMMU works very reliably while bypass mode seems to be tricky at best. I think I partly know how to enable it but it looks like either not every DART or DART/master combination even supports it or that there is some additional configuration required to make it work reliably. I had it working with the USB DART at one point but I needed to enable it in all 16 streams of the IOMMU even though the pagetables only need to be setup in one stream as indicated by the ADT. I couldn't get it to work at all for the framebuffer IOMMU. I think it's fine to skip it for now until it's either actually required due to some hardware quirk or once we have users requesting support. Apple uses almost all IOMMUs without bypass mode if that ADT is to be believed though. Best, Sven ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/9] arm64: tegra: Prevent early SMMU faults
26.03.2021 19:35, Thierry Reding пишет: > On Fri, Mar 26, 2021 at 06:29:28PM +0300, Dmitry Osipenko wrote: >> 25.03.2021 16:03, Thierry Reding пишет: >>> From: Thierry Reding >>> >>> Hi, >>> >>> this is a set of patches that is the result of earlier discussions >>> regarding early identity mappings that are needed to avoid SMMU faults >>> during early boot. >>> >>> The goal here is to avoid early identity mappings altogether and instead >>> postpone the need for the identity mappings to when devices are attached >>> to the SMMU. This works by making the SMMU driver coordinate with the >>> memory controller driver on when to start enforcing SMMU translations. >>> This makes Tegra behave in a more standard way and pushes the code to >>> deal with the Tegra-specific programming into the NVIDIA SMMU >>> implementation. >> >> It is an interesting idea which inspired me to try to apply a somewhat >> similar thing to Tegra SMMU driver by holding the SMMU ASID enable-bit until >> display driver allows to toggle it. This means that we will need an extra >> small tegra-specific SMMU API function, but it should be okay. >> >> I typed a patch and seems it's working good, I'll prepare a proper patch if >> you like it. > > That would actually be working around the problem that this patch was > supposed to prepare for. The reason for this current patch series is to > make sure SMMU translation isn't enabled until a device has actually > been attached to the SMMU. Once it has been attached, the assumption is > that any identity mappings will have been created. > > One Tegra SMMU that shouldn't be a problem because translations aren't > enabled until device attach time. So in other words this patch set is to > get Tegra186 and later to parity with earlier chips from this point of > view. > > I think the problem that you're trying to work around is better solved > by establishing these identity mappings. I do have patches to implement > this for Tegra210 and earlier, though they may require additional work > if you have bootloaders that don't use standard DT bindings for passing > information about the framebuffer to the kernel. I'm not sure what else reasonable could be done without upgrading to a very specific version of firmware, which definitely isn't a variant for older devices which have a wild variety of bootloaders, customized use-cases and etc. We could add a kludge that I'm suggesting as a universal fallback solution, it should work well for all cases that I care about. So we could have the variant with identity mappings, and if mapping isn't provided, then fall back to the kludge. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
On Fri, Mar 26, 2021 at 5:10 PM Sven Peter wrote: > On Fri, Mar 26, 2021, at 16:59, Mark Kettenis wrote: > > Some of the DARTs provide a bypass facility. That code make using the > > standard "dma-ranges" property tricky. That property would need to > > contain the bypass address range. But that would mean that if the > > DART driver needs to look at that property to figure out the address > > range that supports translation it will need to be able to distinguish > > between the translatable address range and the bypass address range. > > Do we understand if and why we even need to bypass certain streams? My guess is that this is a performance optimization. There are generally three reasons to want an iommu in the first place: - Pass a device down to a guest or user process without giving access to all of memory - Avoid problems with limitations in the device, typically when it only supports 32-bit bus addressing, but the installed memory is larger than 4GB - Protect kernel memory from broken drivers If you care about none of the above, but you do care about data transfer speed, you are better off just leaving the IOMMU in bypass mode. I don't think we have to support it if the IOMMU works reliably, but it's something that users might want. Arnd ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/9] arm64: tegra: Prevent early SMMU faults
On Fri, Mar 26, 2021 at 06:29:28PM +0300, Dmitry Osipenko wrote: > 25.03.2021 16:03, Thierry Reding пишет: > > From: Thierry Reding > > > > Hi, > > > > this is a set of patches that is the result of earlier discussions > > regarding early identity mappings that are needed to avoid SMMU faults > > during early boot. > > > > The goal here is to avoid early identity mappings altogether and instead > > postpone the need for the identity mappings to when devices are attached > > to the SMMU. This works by making the SMMU driver coordinate with the > > memory controller driver on when to start enforcing SMMU translations. > > This makes Tegra behave in a more standard way and pushes the code to > > deal with the Tegra-specific programming into the NVIDIA SMMU > > implementation. > > It is an interesting idea which inspired me to try to apply a somewhat > similar thing to Tegra SMMU driver by holding the SMMU ASID enable-bit until > display driver allows to toggle it. This means that we will need an extra > small tegra-specific SMMU API function, but it should be okay. > > I typed a patch and seems it's working good, I'll prepare a proper patch if > you like it. That would actually be working around the problem that this patch was supposed to prepare for. The reason for this current patch series is to make sure SMMU translation isn't enabled until a device has actually been attached to the SMMU. Once it has been attached, the assumption is that any identity mappings will have been created. One Tegra SMMU that shouldn't be a problem because translations aren't enabled until device attach time. So in other words this patch set is to get Tegra186 and later to parity with earlier chips from this point of view. I think the problem that you're trying to work around is better solved by establishing these identity mappings. I do have patches to implement this for Tegra210 and earlier, though they may require additional work if you have bootloaders that don't use standard DT bindings for passing information about the framebuffer to the kernel. Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
On Fri, Mar 26, 2021, at 16:59, Mark Kettenis wrote: > Some of the DARTs provide a bypass facility. That code make using the > standard "dma-ranges" property tricky. That property would need to > contain the bypass address range. But that would mean that if the > DART driver needs to look at that property to figure out the address > range that supports translation it will need to be able to distinguish > between the translatable address range and the bypass address range. Do we understand if and why we even need to bypass certain streams? And do you have an example for a node in the ADT that contains this bypass range? I've only seen nodes with "bypass" and "bypass-adress" but that could just be some software abstraction Apple uses which doesn't map well to Linux or other OSes and might not even be required here. Sven ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
On Fri, Mar 26, 2021 at 4:59 PM Mark Kettenis wrote:
>
> > From: Arnd Bergmann
> > Date: Thu, 25 Mar 2021 22:41:09 +0100
> >
> > On Thu, Mar 25, 2021 at 8:53 AM Sven Peter wrote:
> > > On Tue, Mar 23, 2021, at 21:53, Rob Herring wrote:
> > >
> > > I'm probably just confused or maybe the documentation is outdated but I
> > > don't
> > > see how I could specify "this device can only use DMA addresses from
> > > 0x0010...0x3ff0 but can map these via the iommu to any physical
> > > address" using 'dma-ranges'.
> >
> > It sounds like this is a holdover from the original powerpc iommu,
> > which also had a limited set of virtual addresses in the iommu.
> >
> > I would think it's sufficient to describe it in the iommu itself,
> > since the limitation is more "addresses coming into the iommu must
> > be this range" than "this device must use that address range for
> > talking to the iommu".
> >
> > If the addresses are allocated by the iommu driver, and each iommu
> > only has one DMA master attached to it, having a simple range
> > property in the iommu node should do the trick here. If there might
> > be multiple devices on the same iommu but with different address
> > ranges (which I don't think is the case), then it could be part of
> > the reference to the iommu.
>
> The ADT has properties on the iommu node that describe the adresses it
> accepts for translation ("vm-base" and "vm-size"). So I think we can
> safely assume that the same limits apply to all DMA masters that are
> attached to it. We don't know if the range limit is baked into the
> silicon or whether it is related to how the firmware sets things up.
> Having the properties on the iommu node makes it easy for m1n1 to
> update the properties with the right values if necessary.
Ok.
> Some of the DARTs provide a bypass facility. That code make using the
> standard "dma-ranges" property tricky. That property would need to
> contain the bypass address range. But that would mean that if the
> DART driver needs to look at that property to figure out the address
> range that supports translation it will need to be able to distinguish
> between the translatable address range and the bypass address range.
I'm not following here. Do you mean the bus address used for bypass is
different from the upstream address that gets programmed into the iommu
when it is active?
Arnd
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/3] Apple M1 DART IOMMU driver
> From: Arnd Bergmann
> Date: Thu, 25 Mar 2021 22:41:09 +0100
>
> On Thu, Mar 25, 2021 at 8:53 AM Sven Peter wrote:
> > On Tue, Mar 23, 2021, at 21:53, Rob Herring wrote:
> >
> > I'm probably just confused or maybe the documentation is outdated but I
> > don't
> > see how I could specify "this device can only use DMA addresses from
> > 0x0010...0x3ff0 but can map these via the iommu to any physical
> > address" using 'dma-ranges'.
>
> It sounds like this is a holdover from the original powerpc iommu,
> which also had a limited set of virtual addresses in the iommu.
>
> I would think it's sufficient to describe it in the iommu itself,
> since the limitation is more "addresses coming into the iommu must
> be this range" than "this device must use that address range for
> talking to the iommu".
>
> If the addresses are allocated by the iommu driver, and each iommu
> only has one DMA master attached to it, having a simple range
> property in the iommu node should do the trick here. If there might
> be multiple devices on the same iommu but with different address
> ranges (which I don't think is the case), then it could be part of
> the reference to the iommu.
The ADT has properties on the iommu node that describe the adresses it
accepts for translation ("vm-base" and "vm-size"). So I think we can
safely assume that the same limits apply to all DMA masters that are
attached to it. We don't know if the range limit is baked into the
silicon or whether it is related to how the firmware sets things up.
Having the properties on the iommu node makes it easy for m1n1 to
update the properties with the right values if necessary.
Some of the DARTs provide a bypass facility. That code make using the
standard "dma-ranges" property tricky. That property would need to
contain the bypass address range. But that would mean that if the
DART driver needs to look at that property to figure out the address
range that supports translation it will need to be able to distinguish
between the translatable address range and the bypass address range.
Cheers,
Mark
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 0/9] arm64: tegra: Prevent early SMMU faults
25.03.2021 16:03, Thierry Reding пишет:
> From: Thierry Reding
>
> Hi,
>
> this is a set of patches that is the result of earlier discussions
> regarding early identity mappings that are needed to avoid SMMU faults
> during early boot.
>
> The goal here is to avoid early identity mappings altogether and instead
> postpone the need for the identity mappings to when devices are attached
> to the SMMU. This works by making the SMMU driver coordinate with the
> memory controller driver on when to start enforcing SMMU translations.
> This makes Tegra behave in a more standard way and pushes the code to
> deal with the Tegra-specific programming into the NVIDIA SMMU
> implementation.
It is an interesting idea which inspired me to try to apply a somewhat similar
thing to Tegra SMMU driver by holding the SMMU ASID enable-bit until display
driver allows to toggle it. This means that we will need an extra small
tegra-specific SMMU API function, but it should be okay.
I typed a patch and seems it's working good, I'll prepare a proper patch if you
like it.
What do you think about this:
diff --git a/drivers/gpu/drm/grate/dc.c b/drivers/gpu/drm/grate/dc.c
index 45a41586f153..8874cfba40a1 100644
--- a/drivers/gpu/drm/grate/dc.c
+++ b/drivers/gpu/drm/grate/dc.c
@@ -17,6 +17,7 @@
#include
#include
+#include
#include
#include
@@ -2640,6 +2641,11 @@ static int tegra_dc_init(struct host1x_client *client)
return err;
}
+ if (dc->soc->sync_smmu) {
+ struct iommu_domain *domain = iommu_get_domain_for_dev(dc->dev);
+ tegra_smmu_sync_domain(domain, dc->dev);
+ }
+
if (dc->soc->wgrps)
primary = tegra_dc_add_shared_planes(drm, dc);
else
@@ -2824,6 +2830,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info
= {
.has_win_b_vfilter_mem_client = true,
.has_win_c_without_vert_filter = true,
.plane_tiled_memory_bandwidth_x2 = false,
+ .sync_smmu = false,
};
static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
@@ -2846,6 +2853,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info
= {
.has_win_b_vfilter_mem_client = true,
.has_win_c_without_vert_filter = false,
.plane_tiled_memory_bandwidth_x2 = true,
+ .sync_smmu = true,
};
static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
@@ -2868,6 +2876,7 @@ static const struct tegra_dc_soc_info
tegra114_dc_soc_info = {
.has_win_b_vfilter_mem_client = false,
.has_win_c_without_vert_filter = false,
.plane_tiled_memory_bandwidth_x2 = true,
+ .sync_smmu = true,
};
static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
@@ -2890,6 +2899,7 @@ static const struct tegra_dc_soc_info
tegra124_dc_soc_info = {
.has_win_b_vfilter_mem_client = false,
.has_win_c_without_vert_filter = false,
.plane_tiled_memory_bandwidth_x2 = false,
+ .sync_smmu = true,
};
static const struct tegra_dc_soc_info tegra210_dc_soc_info = {
@@ -2912,6 +2922,7 @@ static const struct tegra_dc_soc_info
tegra210_dc_soc_info = {
.has_win_b_vfilter_mem_client = false,
.has_win_c_without_vert_filter = false,
.plane_tiled_memory_bandwidth_x2 = false,
+ .sync_smmu = true,
};
static const struct tegra_windowgroup_soc tegra186_dc_wgrps[] = {
@@ -2961,6 +2972,7 @@ static const struct tegra_dc_soc_info
tegra186_dc_soc_info = {
.wgrps = tegra186_dc_wgrps,
.num_wgrps = ARRAY_SIZE(tegra186_dc_wgrps),
.plane_tiled_memory_bandwidth_x2 = false,
+ .sync_smmu = false,
};
static const struct tegra_windowgroup_soc tegra194_dc_wgrps[] = {
@@ -3010,6 +3022,7 @@ static const struct tegra_dc_soc_info
tegra194_dc_soc_info = {
.wgrps = tegra194_dc_wgrps,
.num_wgrps = ARRAY_SIZE(tegra194_dc_wgrps),
.plane_tiled_memory_bandwidth_x2 = false,
+ .sync_smmu = false,
};
static const struct of_device_id tegra_dc_of_match[] = {
diff --git a/drivers/gpu/drm/grate/dc.h b/drivers/gpu/drm/grate/dc.h
index 316a56131cf1..e0057bf7be99 100644
--- a/drivers/gpu/drm/grate/dc.h
+++ b/drivers/gpu/drm/grate/dc.h
@@ -91,6 +91,7 @@ struct tegra_dc_soc_info {
bool has_win_b_vfilter_mem_client;
bool has_win_c_without_vert_filter;
bool plane_tiled_memory_bandwidth_x2;
+ bool sync_smmu;
};
struct tegra_dc {
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 602aab98c079..e750b1844a88 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -47,6 +47,9 @@ struct tegra_smmu {
struct dentry *debugfs;
struct iommu_device iommu; /* IOMMU Core code handle */
+
+ bool display_synced[2];
+ bool display_enabled[2];
};
struct tegra_smmu_as {
@@ -78,6 +81,10 @@ static inline u32 smmu_readl(struct tegra_smmu *smmu,
unsigned long offset)
return readl(smmu->regs + offset);
}
Re: [Patch V2 08/13] genirq: Set auxiliary data for an interrupt
On Fri, Mar 26 2021 at 10:32, Marc Zyngier wrote:
> On Thu, 25 Mar 2021 18:59:48 +,
> Thomas Gleixner wrote:
>> Though that leaves the question of the data type for 'val'. While u64 is
>> probably good enough for most stuff, anything which needs more than that
>> is left out (again). union as arguments are horrible especially if you
>> need the counterpart irq_get_auxdata() for which you need a pointer and
>> then you can't do a forward declaration. Something like this might work
>> though and avoid to make the pointer business unconditional:
>>
>> struct irq_auxdata {
>>union {
>> u64val;
>> struct foo *foo;
>>};
>> };
>
> I guess that at some point, irq_get_auxdata() will become a
> requirement so we might as well bite the bullet now, and the above
> looks like a good start to me.
And because it's some time until rustification, we can come up with
something nasty like the below:
#define IRQ_AUXTYPE(t, m) IRQ_AUXTYPE_ ## t
enum irq_auxdata_type {
IRQ_AUXTYPE(U64,val64),
IRQ_AUXTYPE(IRQSTATE, istate),
IRQ_AUXTYPE(VCPU_AFFINITY, vaff),
};
struct irq_auxdata {
union {
u64 val64;
u8 istate;
struct vcpu_aff *vaff;
};
};
This does not protect you from accessing the wrong union member, but it
allows an analyzer to map IRQ_AUXTYPE_U64 to irq_auxdata::val64 and then
check both the call site and the implementation whether they fiddle with
some other member of irq_auxdata or do weird casts.
Maybe it's just nuts and has no value, but I had the urge to write some
nasty macros.
Thanks,
tglx
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/9] memory: tegra: Move internal data structures into separate header
25.03.2021 19:11, Dmitry Osipenko пишет:
> 25.03.2021 18:52, Thierry Reding пишет:
>> On Thu, Mar 25, 2021 at 06:12:51PM +0300, Dmitry Osipenko wrote:
>>> 25.03.2021 16:03, Thierry Reding пишет:
From: Thierry Reding
From Tegra20 through Tegra210, either the GART or SMMU drivers need
access to the internals of the memory controller driver because they are
tightly coupled (in fact, the GART and SMMU are part of the memory
controller). On later chips, a separate hardware block implements the
SMMU functionality, so this is no longer needed. However, we still want
to reuse some of the existing infrastructure on later chips, so split
the memory controller internals into a separate header file to avoid
conflicts with the implementation on newer chips.
Signed-off-by: Thierry Reding
---
drivers/iommu/tegra-gart.c | 2 +-
drivers/iommu/tegra-smmu.c | 2 +-
drivers/memory/tegra/mc.h | 2 +-
drivers/memory/tegra/tegra186.c | 12 ---
include/soc/tegra/mc-internal.h | 62 +
include/soc/tegra/mc.h | 50 --
6 files changed, 72 insertions(+), 58 deletions(-)
create mode 100644 include/soc/tegra/mc-internal.h
>>>
>>> What about to make T186 to re-use the existing tegra_mc struct? Seems
>>> there is nothing special in that struct which doesn't fit for the newer
>>> SoCs. Please notice that both SMMU and GART are already optional and all
>>> the SoC differences are specified within the tegra_mc_soc. It looks to
>>> me that this could be a much nicer and cleaner variant.
>>
>> The problem is that much of the interesting bits in tegra_mc_soc are
>> basically incompatible between the two. For instance the tegra_mc_client
>> and tegra186_mc_client structures, while they have the same purpose,
>> have completely different content. I didn't see a way to unify that
>> without overly complicating things by making half of the fields
>> basically optional on one or the other SoC generation.
>
> The additional fields aren't problem for T20, which doesn't need most of
> the fields. I'd try to go with the additional fields for now and see how
> it will look like, if it will be bothering too much, then we may
> consider to refactor the drivers more thoroughly (later on, in a
> separate series), with a better/nicer separation and taking into account
> a potential modularization support by the MC drivers.
>
> Using a union for the exclusive fields also could work, although always
> need to be extra careful with the unions.
>
>> Maybe one option would be to split tegra_mc into a tegra_mc_common and
>> then derive tegra_mc and tegra186_mc from that. That way we could share
>> the common bits while still letting the chip-specific differences be
>> handled separately.
>
> But isn't tegra_mc already a superset of tegra186_mc? I think the
> tegra186_mc_client is the main difference here.
>
Another thing we could do is to optimize the size of tegra_mc_client, but not
sure whether it's worthwhile to care about extra ~3KB of data.
This slims down tegra_mc_client by two times:
diff --git a/drivers/memory/tegra/mc.c b/drivers/memory/tegra/mc.c
index edea9b2b406e..1d652bfc6b44 100644
--- a/drivers/memory/tegra/mc.c
+++ b/drivers/memory/tegra/mc.c
@@ -317,11 +317,11 @@ static int tegra_mc_setup_latency_allowance(struct
tegra_mc *mc)
/* write latency allowance defaults */
for (i = 0; i < mc->soc->num_clients; i++) {
const struct tegra_mc_la *la = >soc->clients[i].la;
- u32 value;
+ u32 value, la_mask = la->mask, la_def = la->def;
value = mc_readl(mc, la->reg);
- value &= ~(la->mask << la->shift);
- value |= (la->def & la->mask) << la->shift;
+ value &= ~(la_mask << la->shift);
+ value |= (la_def & la_mask) << la->shift;
mc_writel(mc, value, la->reg);
}
diff --git a/drivers/memory/tegra/tegra30.c b/drivers/memory/tegra/tegra30.c
index 46332fa82d10..ecf05484d656 100644
--- a/drivers/memory/tegra/tegra30.c
+++ b/drivers/memory/tegra/tegra30.c
@@ -1157,7 +1157,7 @@ static void tegra30_mc_tune_client_latency(struct
tegra_mc *mc,
u32 arb_tolerance_compensation_nsec, arb_tolerance_compensation_div;
const struct tegra_mc_la *la = >la;
unsigned int fifo_size = client->fifo_size;
- u32 arb_nsec, la_ticks, value;
+ u32 arb_nsec, la_ticks, value, la_mask;
/* see 18.4.1 Client Configuration in Tegra3 TRM v03p */
if (bandwidth_mbytes_sec)
@@ -1214,11 +1214,12 @@ static void tegra30_mc_tune_client_latency(struct
tegra_mc *mc,
* client may wait in the EMEM arbiter before it becomes a high-priority
* request.
*/
+ la_mask = la->mask;
la_ticks = arb_nsec / mc->tick;
- la_ticks = min(la_ticks, la->mask);
+
Re: [Patch V2 13/13] genirq/msi: Provide helpers to return Linux IRQ/dev_msi hw IRQ number
On Fri, 26 Mar 2021 01:02:43 +,
"Dey, Megha" wrote:
>
> Hi Marc,
>
> On 3/25/2021 10:53 AM, Marc Zyngier wrote:
> > On Fri, 26 Feb 2021 20:11:17 +,
> > Megha Dey wrote:
> >> From: Dave Jiang
> >>
> >> Add new helpers to get the Linux IRQ number and device specific index
> >> for given device-relative vector so that the drivers don't need to
> >> allocate their own arrays to keep track of the vectors and hwirq for
> >> the multi vector device MSI case.
> >>
> >> Reviewed-by: Tony Luck
> >> Signed-off-by: Dave Jiang
> >> Signed-off-by: Megha Dey
> >> ---
> >> include/linux/msi.h | 2 ++
> >> kernel/irq/msi.c| 44
> >> 2 files changed, 46 insertions(+)
> >>
> >> diff --git a/include/linux/msi.h b/include/linux/msi.h
> >> index 24abec0..d60a6ba 100644
> >> --- a/include/linux/msi.h
> >> +++ b/include/linux/msi.h
> >> @@ -451,6 +451,8 @@ struct irq_domain
> >> *platform_msi_create_irq_domain(struct fwnode_handle *fwnode,
> >> int platform_msi_domain_alloc_irqs(struct device *dev, unsigned int nvec,
> >> irq_write_msi_msg_t write_msi_msg);
> >> void platform_msi_domain_free_irqs(struct device *dev);
> >> +int msi_irq_vector(struct device *dev, unsigned int nr);
> >> +int dev_msi_hwirq(struct device *dev, unsigned int nr);
> >> /* When an MSI domain is used as an intermediate domain */
> >> int msi_domain_prepare_irqs(struct irq_domain *domain, struct device
> >> *dev,
> >> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> >> index 047b59d..f2a8f55 100644
> >> --- a/kernel/irq/msi.c
> >> +++ b/kernel/irq/msi.c
> >> @@ -581,4 +581,48 @@ struct msi_domain_info *msi_get_domain_info(struct
> >> irq_domain *domain)
> >>return (struct msi_domain_info *)domain->host_data;
> >> }
> >> +/**
> >> + * msi_irq_vector - Get the Linux IRQ number of a device vector
> >> + * @dev: device to operate on
> >> + * @nr: device-relative interrupt vector index (0-based).
> >> + *
> >> + * Returns the Linux IRQ number of a device vector.
> >> + */
> >> +int msi_irq_vector(struct device *dev, unsigned int nr)
> >> +{
> >> + struct msi_desc *entry;
> >> + int i = 0;
> >> +
> >> + for_each_msi_entry(entry, dev) {
> >> + if (i == nr)
> >> + return entry->irq;
> >> + i++;
> > This obviously doesn't work with Multi-MSI, does it?
>
> This API is only for devices that support device MSI interrupts. They
> follow MSI-x format and don't support multi MSI (part of MSI).
>
> Not sure if I am missing something here, can you please let me know?
Nothing in the prototype of the function indicates this limitation,
nor does the documentation. And I'm not sure why you should exclude
part of the MSI functionality here. It can't be for performance
reason, so you might as well make sure this works for all the MSI
variants:
int msi_irq_vector(struct device *dev, unsigned int nr)
{
struct msi_desc *entry;
int irq, index = 0;
for_each_msi_vector(entry, irq, dev) {
if (index == nr}
return irq;
index++;
}
return WARN_ON_ONCE(-EINVAL);
}
>
> >
> >> + }
> >> + WARN_ON_ONCE(1);
> >> + return -EINVAL;
> >> +}
> >> +EXPORT_SYMBOL_GPL(msi_irq_vector);
> >> +
> >> +/**
> >> + * dev_msi_hwirq - Get the device MSI hw IRQ number of a device vector
> >> + * @dev: device to operate on
> >> + * @nr: device-relative interrupt vector index (0-based).
> >> + *
> >> + * Return the dev_msi hw IRQ number of a device vector.
> >> + */
> >> +int dev_msi_hwirq(struct device *dev, unsigned int nr)
> >> +{
> >> + struct msi_desc *entry;
> >> + int i = 0;
> >> +
> >> + for_each_msi_entry(entry, dev) {
> >> + if (i == nr)
> >> + return entry->device_msi.hwirq;
> >> + i++;
> >> + }
> >> + WARN_ON_ONCE(1);
> >> + return -EINVAL;
> >> +}
And this helper would be more generally useful if it returned the n-th
msi_desc entry rather than some obscure field in a substructure.
struct msi_desc *msi_get_nth_desc(struct device *dev, unsigned int nth)
{
struct msi_desc *entry = NULL;
unsigned int i = 0;
for_each_msi_entry(entry, dev) {
if (i == nth)
return entry;
i++;
}
WARN_ON_ONCE(!entry);
return entry;
}
You can always wrap it for your particular use case.
> >> +EXPORT_SYMBOL_GPL(dev_msi_hwirq);
> >> +
> >> #endif /* CONFIG_GENERIC_MSI_IRQ_DOMAIN */
> > And what uses these helpers?]
> These helpers are to be used by a driver series(Intel's IDXD driver)
> which is currently stuck due to VFIO refactoring.
Then I's suggest you keep the helpers together with the actual user,
unless this can generally be useful to existing users (exported
symbols without in-tree users is always a bit odd).
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
Re: [Patch V2 08/13] genirq: Set auxiliary data for an interrupt
On Thu, 25 Mar 2021 18:59:48 +,
Thomas Gleixner wrote:
>
> On Thu, Mar 25 2021 at 17:23, Marc Zyngier wrote:
> >> +{
> >> + struct irq_desc *desc;
> >> + struct irq_data *data;
> >> + unsigned long flags;
> >> + int res = -ENODEV;
> >> +
> >> + desc = irq_get_desc_buslock(irq, , 0);
> >> + if (!desc)
> >> + return -EINVAL;
> >> +
> >> + for (data = >irq_data; data; data = irqd_get_parent_data(data)) {
> >> + if (data->chip->irq_set_auxdata) {
> >> + res = data->chip->irq_set_auxdata(data, which, val);
> >
> > And this is where things can break: because you don't define what
> > 'which' is, you can end-up with two stacked layers clashing in their
> > interpretation of 'which', potentially doing the wrong thing.
> >
> > Short of having a global, cross architecture definition of all the
> > possible states, this is frankly dodgy.
>
> My bad, I suggested this in the first place.
>
> So what you suggest is to make 'which' an enum and have that in
> include/linux/whatever.h so we end up with unique identifiers accross
> architectures, irqdomains and whatever, right?
Exactly. As long as 'which' is unique and unambiguous, we can solve
the stacking issue (which is oddly starting to smell like the ghost of
the SVR3 STREAMS... /me runs ;-).
Once we have that, I can start killing the irq_set_vcpu_affinity()
abuse I have been guilty of over the past years. Even more, we could
kill irq_set_vcpu_affinity() altogether, because we have a generic way
of passing side-band information from a driver down to the IRQ stack.
> That makes a lot of sense.
>
> Though that leaves the question of the data type for 'val'. While u64 is
> probably good enough for most stuff, anything which needs more than that
> is left out (again). union as arguments are horrible especially if you
> need the counterpart irq_get_auxdata() for which you need a pointer and
> then you can't do a forward declaration. Something like this might work
> though and avoid to make the pointer business unconditional:
>
> struct irq_auxdata {
>union {
>u64val;
> struct foo *foo;
>};
> };
I guess that at some point, irq_get_auxdata() will become a
requirement so we might as well bite the bullet now, and the above
looks like a good start to me.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [Patch V2 07/13] irqdomain/msi: Provide msi_alloc/free_store() callbacks
On Thu, 25 Mar 2021 18:44:48 +,
Thomas Gleixner wrote:
>
> On Thu, Mar 25 2021 at 17:08, Marc Zyngier wrote:
> > Megha Dey wrote:
> >> @@ -434,6 +434,12 @@ int __msi_domain_alloc_irqs(struct irq_domain
> >> *domain, struct device *dev,
> >>if (ret)
> >>return ret;
> >>
> >> + if (ops->msi_alloc_store) {
> >> + ret = ops->msi_alloc_store(domain, dev, nvec);
> >
> > What is supposed to happen if we get aliasing devices (similar to what
> > we have with devices behind a PCI bridge)?
> >
> > The ITS code goes through all kind of hoops to try and detect this
> > case when sizing the translation tables (in the .prepare callback),
> > and I have the feeling that sizing the message store is analogous.
>
> No. The message store itself is sized upfront by the underlying 'master'
> device. Each 'master' device has it's own irqdomain.
>
> This is the allocation for the subdevice and this is not part of PCI and
> therefore not subject to PCI aliasing.
Fair enough. If we are guaranteed that there is no aliasing, then this
point is moot.
M.
--
Without deviation from the norm, progress is not possible.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v13 10/10] iommu/arm-smmu-v3: Add stall support for platform devices
Hi Jean,
On 3/2/21 10:26 AM, Jean-Philippe Brucker wrote:
> The SMMU provides a Stall model for handling page faults in platform
> devices. It is similar to PCIe PRI, but doesn't require devices to have
> their own translation cache. Instead, faulting transactions are parked
> and the OS is given a chance to fix the page tables and retry the
> transaction.
>
> Enable stall for devices that support it (opt-in by firmware). When an
> event corresponds to a translation error, call the IOMMU fault handler.
> If the fault is recoverable, it will call us back to terminate or
> continue the stall.
>
> To use stall device drivers need to enable IOMMU_DEV_FEAT_IOPF, which
> initializes the fault queue for the device.
>
> Tested-by: Zhangfei Gao
> Reviewed-by: Jonathan Cameron
> Signed-off-by: Jean-Philippe Brucker
Reviewed-by: Eric Auger
Thanks
Eric
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 43
> .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 59 +-
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 196 +-
> 3 files changed, 283 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 7b15b7580c6e..59af0bbd2f7b 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -354,6 +354,13 @@
> #define CMDQ_PRI_1_GRPID GENMASK_ULL(8, 0)
> #define CMDQ_PRI_1_RESP GENMASK_ULL(13, 12)
>
> +#define CMDQ_RESUME_0_RESP_TERM 0UL
> +#define CMDQ_RESUME_0_RESP_RETRY 1UL
> +#define CMDQ_RESUME_0_RESP_ABORT 2UL
> +#define CMDQ_RESUME_0_RESP GENMASK_ULL(13, 12)
> +#define CMDQ_RESUME_0_SIDGENMASK_ULL(63, 32)
> +#define CMDQ_RESUME_1_STAG GENMASK_ULL(15, 0)
> +
> #define CMDQ_SYNC_0_CS GENMASK_ULL(13, 12)
> #define CMDQ_SYNC_0_CS_NONE 0
> #define CMDQ_SYNC_0_CS_IRQ 1
> @@ -370,6 +377,25 @@
>
> #define EVTQ_0_IDGENMASK_ULL(7, 0)
>
> +#define EVT_ID_TRANSLATION_FAULT 0x10
> +#define EVT_ID_ADDR_SIZE_FAULT 0x11
> +#define EVT_ID_ACCESS_FAULT 0x12
> +#define EVT_ID_PERMISSION_FAULT 0x13
> +
> +#define EVTQ_0_SSV (1UL << 11)
> +#define EVTQ_0_SSID GENMASK_ULL(31, 12)
> +#define EVTQ_0_SID GENMASK_ULL(63, 32)
> +#define EVTQ_1_STAG GENMASK_ULL(15, 0)
> +#define EVTQ_1_STALL (1UL << 31)
> +#define EVTQ_1_PnU (1UL << 33)
> +#define EVTQ_1_InD (1UL << 34)
> +#define EVTQ_1_RnW (1UL << 35)
> +#define EVTQ_1_S2(1UL << 39)
> +#define EVTQ_1_CLASS GENMASK_ULL(41, 40)
> +#define EVTQ_1_TT_READ (1UL << 44)
> +#define EVTQ_2_ADDR GENMASK_ULL(63, 0)
> +#define EVTQ_3_IPA GENMASK_ULL(51, 12)
> +
> /* PRI queue */
> #define PRIQ_ENT_SZ_SHIFT4
> #define PRIQ_ENT_DWORDS ((1 << PRIQ_ENT_SZ_SHIFT) >> 3)
> @@ -464,6 +490,13 @@ struct arm_smmu_cmdq_ent {
> enum pri_resp resp;
> } pri;
>
> + #define CMDQ_OP_RESUME 0x44
> + struct {
> + u32 sid;
> + u16 stag;
> + u8 resp;
> + } resume;
> +
> #define CMDQ_OP_CMD_SYNC0x46
> struct {
> u64 msiaddr;
> @@ -522,6 +555,7 @@ struct arm_smmu_cmdq_batch {
>
> struct arm_smmu_evtq {
> struct arm_smmu_queue q;
> + struct iopf_queue *iopf;
> u32 max_stalls;
> };
>
> @@ -659,7 +693,9 @@ struct arm_smmu_master {
> struct arm_smmu_stream *streams;
> unsigned intnum_streams;
> boolats_enabled;
> + boolstall_enabled;
> boolsva_enabled;
> + booliopf_enabled;
> struct list_headbonds;
> unsigned intssid_bits;
> };
> @@ -678,6 +714,7 @@ struct arm_smmu_domain {
>
> struct io_pgtable_ops *pgtbl_ops;
> boolnon_strict;
> + boolstall_enabled;
> atomic_tnr_ats_masters;
>
> enum arm_smmu_domain_stage stage;
> @@ -719,6 +756,7 @@ bool arm_smmu_master_sva_supported(struct arm_smmu_master
> *master);
> bool arm_smmu_master_sva_enabled(struct arm_smmu_master *master);
> int arm_smmu_master_enable_sva(struct arm_smmu_master *master);
> int arm_smmu_master_disable_sva(struct
Re: [PATCH v13 07/10] iommu/arm-smmu-v3: Maintain a SID->device structure
On Thu, Mar 25, 2021 at 05:48:07PM +, Will Deacon wrote:
> > +/* smmu->streams_mutex must be held */
>
> Can you add a lockdep assertion for that?
Sure
> > +__maybe_unused
> > +static struct arm_smmu_master *
> > +arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
> > +{
> > + struct rb_node *node;
> > + struct arm_smmu_stream *stream;
> > +
> > + node = smmu->streams.rb_node;
> > + while (node) {
> > + stream = rb_entry(node, struct arm_smmu_stream, node);
> > + if (stream->id < sid)
> > + node = node->rb_right;
> > + else if (stream->id > sid)
> > + node = node->rb_left;
> > + else
> > + return stream->master;
> > + }
> > +
> > + return NULL;
> > +}
>
> [...]
>
> > +static int arm_smmu_insert_master(struct arm_smmu_device *smmu,
> > + struct arm_smmu_master *master)
> > +{
> > + int i;
> > + int ret = 0;
> > + struct arm_smmu_stream *new_stream, *cur_stream;
> > + struct rb_node **new_node, *parent_node = NULL;
> > + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(master->dev);
> > +
> > + master->streams = kcalloc(fwspec->num_ids, sizeof(*master->streams),
> > + GFP_KERNEL);
> > + if (!master->streams)
> > + return -ENOMEM;
> > + master->num_streams = fwspec->num_ids;
> > +
> > + mutex_lock(>streams_mutex);
> > + for (i = 0; i < fwspec->num_ids; i++) {
> > + u32 sid = fwspec->ids[i];
> > +
> > + new_stream = >streams[i];
> > + new_stream->id = sid;
> > + new_stream->master = master;
> > +
> > + /*
> > +* Check the SIDs are in range of the SMMU and our stream table
> > +*/
> > + if (!arm_smmu_sid_in_range(smmu, sid)) {
> > + ret = -ERANGE;
> > + break;
> > + }
> > +
> > + /* Ensure l2 strtab is initialised */
> > + if (smmu->features & ARM_SMMU_FEAT_2_LVL_STRTAB) {
> > + ret = arm_smmu_init_l2_strtab(smmu, sid);
> > + if (ret)
> > + break;
> > + }
> > +
> > + /* Insert into SID tree */
> > + new_node = &(smmu->streams.rb_node);
> > + while (*new_node) {
> > + cur_stream = rb_entry(*new_node, struct arm_smmu_stream,
> > + node);
> > + parent_node = *new_node;
> > + if (cur_stream->id > new_stream->id) {
> > + new_node = &((*new_node)->rb_left);
> > + } else if (cur_stream->id < new_stream->id) {
> > + new_node = &((*new_node)->rb_right);
> > + } else {
> > + dev_warn(master->dev,
> > +"stream %u already in tree\n",
> > +cur_stream->id);
> > + ret = -EINVAL;
> > + break;
> > + }
> > + }
> > + if (ret)
> > + break;
> > +
> > + rb_link_node(_stream->node, parent_node, new_node);
> > + rb_insert_color(_stream->node, >streams);
> > + }
> > +
> > + if (ret) {
> > + for (i--; i >= 0; i--)
>
> Is 'i--' really what you want for the initial value? Doesn't that correspond
> to the ID you *didn't* add to the tree?
In case of error we break out of the loop, with i corresponding to the
stream that caused a fault but wasn't yet added to the tree. So i-- is
the last stream that was successfully added, or -1 in which case we don't
enter this for loop.
> > + rb_erase(>streams[i].node, >streams);
> > + kfree(master->streams);
>
> Do you need to NULLify master->streams and/or reset master->num_streams
> after this? Seems like they're left dangling.
master is freed by arm_smmu_probe_device() when we return an error. Since
this function is unlikely to ever have another caller I didn't bother
cleaning up here
Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/3] drivers: iommu/arm - coding style fix
Fixed following checkpatch error:
- spaces required around '='
- space required before the open parenthesis '('
- "foo * bar" should be "foo *bar"
Signed-off-by: Zhiqi Song
---
drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +-
drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 +++---
drivers/iommu/arm/arm-smmu/qcom_iommu.c | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 8ca7415..53e24f0 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -2479,7 +2479,7 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
}
break;
case IOMMU_DOMAIN_DMA:
- switch(attr) {
+ switch (attr) {
case DOMAIN_ATTR_DMA_USE_FLUSH_QUEUE:
smmu_domain->non_strict = *(int *)data;
break;
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index d8c6bfd..1496033 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -1261,7 +1261,7 @@ static phys_addr_t arm_smmu_iova_to_phys_hard(struct
iommu_domain *domain,
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
struct arm_smmu_device *smmu = smmu_domain->smmu;
struct arm_smmu_cfg *cfg = _domain->cfg;
- struct io_pgtable_ops *ops= smmu_domain->pgtbl_ops;
+ struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
struct device *dev = smmu->dev;
void __iomem *reg;
u32 tmp;
@@ -1486,7 +1486,7 @@ static int arm_smmu_domain_get_attr(struct iommu_domain
*domain,
{
struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
- switch(domain->type) {
+ switch (domain->type) {
case IOMMU_DOMAIN_UNMANAGED:
switch (attr) {
case DOMAIN_ATTR_NESTING:
@@ -1527,7 +1527,7 @@ static int arm_smmu_domain_set_attr(struct iommu_domain
*domain,
mutex_lock(_domain->init_mutex);
- switch(domain->type) {
+ switch (domain->type) {
case IOMMU_DOMAIN_UNMANAGED:
switch (attr) {
case DOMAIN_ATTR_NESTING:
diff --git a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
index 7f280c8..f3985f3 100644
--- a/drivers/iommu/arm/arm-smmu/qcom_iommu.c
+++ b/drivers/iommu/arm/arm-smmu/qcom_iommu.c
@@ -81,7 +81,7 @@ static struct qcom_iommu_domain *to_qcom_iommu_domain(struct
iommu_domain *dom)
static const struct iommu_ops qcom_iommu_ops;
-static struct qcom_iommu_dev * to_iommu(struct device *dev)
+static struct qcom_iommu_dev *to_iommu(struct device *dev)
{
struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
@@ -91,7 +91,7 @@ static struct qcom_iommu_dev * to_iommu(struct device *dev)
return dev_iommu_priv_get(dev);
}
-static struct qcom_iommu_ctx * to_ctx(struct qcom_iommu_domain *d, unsigned
asid)
+static struct qcom_iommu_ctx *to_ctx(struct qcom_iommu_domain *d, unsigned
asid)
{
struct qcom_iommu_dev *qcom_iommu = d->iommu;
if (!qcom_iommu)
--
2.7.4
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/3] drivers: iommu coding style fix
Fix the checkpatch errors in iommu module. Zhiqi Song (3): drivers:iommu - coding style fix drivers:iommu/amd - coding style fix drivers:iommu/arm - coding style fix drivers/iommu/amd/init.c| 4 ++-- drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 2 +- drivers/iommu/arm/arm-smmu/arm-smmu.c | 6 +++--- drivers/iommu/arm/arm-smmu/qcom_iommu.c | 4 ++-- drivers/iommu/io-pgtable-arm.c | 16 5 files changed, 16 insertions(+), 16 deletions(-) -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3] drivers: iommu - coding style fix
Fixed following checkpatch error: - space required after ',' Signed-off-by: Zhiqi Song --- drivers/iommu/io-pgtable-arm.c | 16 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c index 87def58..3bf880f 100644 --- a/drivers/iommu/io-pgtable-arm.c +++ b/drivers/iommu/io-pgtable-arm.c @@ -37,7 +37,7 @@ * Calculate the right shift amount to get to the portion describing level l * in a virtual address mapped by the pagetable in d. */ -#define ARM_LPAE_LVL_SHIFT(l,d) \ +#define ARM_LPAE_LVL_SHIFT(l, d) \ (((ARM_LPAE_MAX_LEVELS - (l)) * (d)->bits_per_level) + \ ilog2(sizeof(arm_lpae_iopte))) @@ -50,15 +50,15 @@ * Calculate the index at level l used to map virtual address a using the * pagetable in d. */ -#define ARM_LPAE_PGD_IDX(l,d) \ +#define ARM_LPAE_PGD_IDX(l, d) \ ((l) == (d)->start_level ? (d)->pgd_bits - (d)->bits_per_level : 0) -#define ARM_LPAE_LVL_IDX(a,l,d) \ - (((u64)(a) >> ARM_LPAE_LVL_SHIFT(l,d)) &\ -((1 << ((d)->bits_per_level + ARM_LPAE_PGD_IDX(l,d))) - 1)) +#define ARM_LPAE_LVL_IDX(a, l, d) \ + (((u64)(a) >> ARM_LPAE_LVL_SHIFT(l, d)) & \ +((1 << ((d)->bits_per_level + ARM_LPAE_PGD_IDX(l, d))) - 1)) /* Calculate the block/page mapping size at level l for pagetable in d. */ -#define ARM_LPAE_BLOCK_SIZE(l,d) (1ULL << ARM_LPAE_LVL_SHIFT(l,d)) +#define ARM_LPAE_BLOCK_SIZE(l, d) (1ULL << ARM_LPAE_LVL_SHIFT(l, d)) /* Page table bits */ #define ARM_LPAE_PTE_TYPE_SHIFT0 @@ -68,7 +68,7 @@ #define ARM_LPAE_PTE_TYPE_TABLE3 #define ARM_LPAE_PTE_TYPE_PAGE 3 -#define ARM_LPAE_PTE_ADDR_MASK GENMASK_ULL(47,12) +#define ARM_LPAE_PTE_ADDR_MASK GENMASK_ULL(47, 12) #define ARM_LPAE_PTE_NSTABLE (((arm_lpae_iopte)1) << 63) #define ARM_LPAE_PTE_XN(((arm_lpae_iopte)3) << 53) @@ -128,7 +128,7 @@ #define ARM_MALI_LPAE_MEMATTR_WRITE_ALLOC 0x8DULL /* IOPTE accessors */ -#define iopte_deref(pte,d) __va(iopte_to_paddr(pte, d)) +#define iopte_deref(pte, d) __va(iopte_to_paddr(pte, d)) #define iopte_type(pte)\ (((pte) >> ARM_LPAE_PTE_TYPE_SHIFT) & ARM_LPAE_PTE_TYPE_MASK) -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] drivers: iommu/amd - coding style fix
Fixed following checkpatch errors: - code indent should use tabs where possible - space prohibited before ',' Signed-off-by: Zhiqi Song --- drivers/iommu/amd/init.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c index 6a1f704..958fa17 100644 --- a/drivers/iommu/amd/init.c +++ b/drivers/iommu/amd/init.c @@ -997,7 +997,7 @@ static bool copy_device_table(void) return false; } - old_dev_tbl_cpy[devid].data[2] = old_devtb[devid].data[2]; + old_dev_tbl_cpy[devid].data[2] = old_devtb[devid].data[2]; } } memunmap(old_devtb); @@ -1248,7 +1248,7 @@ static int __init init_iommu_from_acpi(struct amd_iommu *iommu, devid = e->devid; devid_to = e->ext >> 8; - set_dev_entry_from_acpi(iommu, devid , e->flags, 0); + set_dev_entry_from_acpi(iommu, devid, e->flags, 0); set_dev_entry_from_acpi(iommu, devid_to, e->flags, 0); amd_iommu_alias_table[devid] = devid_to; break; -- 2.7.4 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V4 05/18] iommu/ioasid: Redefine IOASID set and allocation APIs
On Thu, Mar 25, 2021 at 02:16:45PM -0300, Jason Gunthorpe wrote:
> On Thu, Mar 25, 2021 at 10:02:36AM -0700, Jacob Pan wrote:
> > Hi Jean-Philippe,
> >
> > On Thu, 25 Mar 2021 11:21:40 +0100, Jean-Philippe Brucker
> > wrote:
> >
> > > On Wed, Mar 24, 2021 at 03:12:30PM -0700, Jacob Pan wrote:
> > > > Hi Jason,
> > > >
> > > > On Wed, 24 Mar 2021 14:03:38 -0300, Jason Gunthorpe
> > > > wrote:
> > > > > On Wed, Mar 24, 2021 at 10:02:46AM -0700, Jacob Pan wrote:
> > > > > > > Also wondering about device driver allocating auxiliary domains
> > > > > > > for their private use, to do iommu_map/unmap on private PASIDs (a
> > > > > > > clean replacement to super SVA, for example). Would that go
> > > > > > > through the same path as /dev/ioasid and use the cgroup of
> > > > > > > current task?
> > > > > >
> > > > > > For the in-kernel private use, I don't think we should restrict
> > > > > > based on cgroup, since there is no affinity to user processes. I
> > > > > > also think the PASID allocation should just use kernel API instead
> > > > > > of /dev/ioasid. Why would user space need to know the actual PASID
> > > > > > # for device private domains? Maybe I missed your idea?
> > > > >
> > > > > There is not much in the kernel that isn't triggered by a process, I
> > > > > would be careful about the idea that there is a class of users that
> > > > > can consume a cgroup controlled resource without being inside the
> > > > > cgroup.
> > > > >
> > > > > We've got into trouble before overlooking this and with something
> > > > > greenfield like PASID it would be best built in to the API to prevent
> > > > > a mistake. eg accepting a cgroup or process input to the allocator.
> > > > >
> > > > Make sense. But I think we only allow charging the current cgroup, how
> > > > about I add the following to ioasid_alloc():
> > > >
> > > > misc_cg = get_current_misc_cg();
> > > > ret = misc_cg_try_charge(MISC_CG_RES_IOASID, misc_cg, 1);
> > > > if (ret) {
> > > > put_misc_cg(misc_cg);
> > > > return ret;
> > > > }
> > >
> > > Does that allow PASID allocation during driver probe, in kernel_init or
> > > modprobe context?
> > >
> > Good point. Yes, you can get cgroup subsystem state in kernel_init for
> > charging/uncharging. I would think module_init should work also since it is
> > after kernel_init. I have tried the following:
> > static int __ref kernel_init(void *unused)
> > {
> > int ret;
> > + struct cgroup_subsys_state *css;
> > + css = task_get_css(current, pids_cgrp_id);
> >
> > But that would imply:
> > 1. IOASID has to be built-in, not as module
If IOASID is a module, the device driver will probe once the IOMMU module
is available, which I think always happens in probe deferral kworker.
> > 2. IOASIDs charged on PID1/init would not subject to cgroup limit since it
> > will be in the root cgroup and we don't support migration nor will migrate.
> >
> > Then it comes back to the question of why do we try to limit in-kernel
> > users per cgroup if we can't enforce these cases.
It may be better to explicitly pass a cgroup during allocation as Jason
suggested. That way anyone using the API will have to be aware of this and
pass the root cgroup if that's what they want.
> Are these real use cases? Why would a driver binding to a device
> create a single kernel pasid at bind time? Why wouldn't it use
> untagged DMA?
It's not inconceivable to have a control queue doing DMA tagged with
PASID. The devices I know either use untagged DMA, or have a choice to use
a PASID. We're not outright forbidding PASID allocation at boot (I don't
think we can or should) and we won't be able to check every use of the
API, so I'm trying to figure out whether it will always default to root
cgroup, or crash in some corner case.
Thanks,
Jean
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 7/8] iommu/arm-smmu: Fix spelling mistake "initally" -> "initially"
There is a spelling mistake in a comment, fix it. Signed-off-by: Zhen Lei --- drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c index d8c6bfde6a61587..8e4e8fea106b612 100644 --- a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1358,7 +1358,7 @@ static struct iommu_device *arm_smmu_probe_device(struct device *dev) ret = arm_smmu_register_legacy_master(dev, ); /* -* If dev->iommu_fwspec is initally NULL, arm_smmu_register_legacy_master() +* If dev->iommu_fwspec is initially NULL, arm_smmu_register_legacy_master() * will allocate/initialise a new one. Thus we need to update fwspec for * later use. */ -- 1.8.3 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 8/8] iommu/vt-d: fix a couple of spelling mistakes
There are several spelling mistakes, as follows:
guarentees ==> guarantees
resgister ==> register
insufficent ==> insufficient
creats ==> creates
tabke ==> take
Signed-off-by: Zhen Lei
---
drivers/iommu/intel/dmar.c | 6 +++---
drivers/iommu/intel/iommu.c | 2 +-
drivers/iommu/intel/irq_remapping.c | 2 +-
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index d5c51b5c20aff4b..bb6f0880f6f4db0 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -45,7 +45,7 @@ struct dmar_res_callback {
/*
* Assumptions:
- * 1) The hotplug framework guarentees that DMAR unit will be hot-added
+ * 1) The hotplug framework guarantees that DMAR unit will be hot-added
*before IO devices managed by that unit.
* 2) The hotplug framework guarantees that DMAR unit will be hot-removed
*after IO devices managed by that unit.
@@ -960,10 +960,10 @@ static void unmap_iommu(struct intel_iommu *iommu)
/**
* map_iommu: map the iommu's registers
* @iommu: the iommu to map
- * @phys_addr: the physical address of the base resgister
+ * @phys_addr: the physical address of the base register
*
* Memory map the iommu's registers. Start w/ a single page, and
- * possibly expand if that turns out to be insufficent.
+ * possibly expand if that turns out to be insufficient.
*/
static int map_iommu(struct intel_iommu *iommu, u64 phys_addr)
{
diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index ee0932307d646bb..f9a2277fba99f9f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -288,7 +288,7 @@ static inline void context_clear_entry(struct context_entry
*context)
/*
* This domain is a statically identity mapping domain.
- * 1. This domain creats a static 1:1 mapping to all usable memory.
+ * 1. This domain creates a static 1:1 mapping to all usable memory.
* 2. It maps to each iommu if successful.
* 3. Each iommu mapps to this domain if successful.
*/
diff --git a/drivers/iommu/intel/irq_remapping.c
b/drivers/iommu/intel/irq_remapping.c
index 611ef5243cb63b9..12e9f2cf84e5101 100644
--- a/drivers/iommu/intel/irq_remapping.c
+++ b/drivers/iommu/intel/irq_remapping.c
@@ -74,7 +74,7 @@ struct intel_ir_data {
* ->iommu->register_lock
* Note:
* intel_irq_remap_ops.{supported,prepare,enable,disable,reenable} are called
- * in single-threaded environment with interrupt disabled, so no need to tabke
+ * in single-threaded environment with interrupt disabled, so no need to take
* the dmar_global_lock.
*/
DEFINE_RAW_SPINLOCK(irq_2_ir_lock);
--
1.8.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 6/8] iommu/amd: fix a couple of spelling mistakes
There are several spelling mistakes, as follows:
alignement ==> alignment
programing ==> programming
implemtation ==> implementation
assignement ==> assignment
By the way, both "programing" and "programming" are acceptable, but the
latter seems more formal.
Signed-off-by: Zhen Lei
---
drivers/iommu/amd/amd_iommu_types.h | 2 +-
drivers/iommu/amd/init.c| 4 ++--
drivers/iommu/amd/iommu.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/iommu/amd/amd_iommu_types.h
b/drivers/iommu/amd/amd_iommu_types.h
index 6937e3674a16e26..dc1814c355cff77 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -446,7 +446,7 @@ struct irq_remap_table {
/* Interrupt remapping feature used? */
extern bool amd_iommu_irq_remap;
-/* kmem_cache to get tables with 128 byte alignement */
+/* kmem_cache to get tables with 128 byte alignment */
extern struct kmem_cache *amd_iommu_irq_cache;
/*
diff --git a/drivers/iommu/amd/init.c b/drivers/iommu/amd/init.c
index 321f5906e6ed3a5..48799002b3571d1 100644
--- a/drivers/iommu/amd/init.c
+++ b/drivers/iommu/amd/init.c
@@ -1734,7 +1734,7 @@ static void __init init_iommu_perf_ctr(struct amd_iommu
*iommu)
goto pc_false;
/*
-* Disable power gating by programing the performance counter
+* Disable power gating by programming the performance counter
* source to 20 (i.e. counts the reads and writes from/to IOMMU
* Reserved Register [MMIO Offset 1FF8h] that are ignored.),
* which never get incremented during this init phase.
@@ -2088,7 +2088,7 @@ static int intcapxt_irqdomain_activate(struct irq_domain
*domain,
xt.destid_24_31 = cfg->dest_apicid >> 24;
/**
-* Current IOMMU implemtation uses the same IRQ for all
+* Current IOMMU implementation uses the same IRQ for all
* 3 IOMMU interrupts.
*/
writeq(xt.capxt, iommu->mmio_base + MMIO_INTCAPXT_EVT_OFFSET);
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index a69a8b573e40d00..d14e4698f507b89 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1865,7 +1865,7 @@ int __init amd_iommu_init_dma_ops(void)
* The following functions belong to the exported interface of AMD IOMMU
*
* This interface allows access to lower level functions of the IOMMU
- * like protection domain handling and assignement of devices to domains
+ * like protection domain handling and assignment of devices to domains
* which is not possible with the dma_ops interface.
*
*/
--
1.8.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/8] iommu/omap: Fix spelling mistake "alignement" -> "alignment"
There is a spelling mistake in a comment, fix it.
Signed-off-by: Zhen Lei
---
drivers/iommu/omap-iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/omap-iommu.c b/drivers/iommu/omap-iommu.c
index 71f29c0927fc710..b2a6ab700ec43d1 100644
--- a/drivers/iommu/omap-iommu.c
+++ b/drivers/iommu/omap-iommu.c
@@ -1754,7 +1754,7 @@ static int __init omap_iommu_init(void)
{
struct kmem_cache *p;
const slab_flags_t flags = SLAB_HWCACHE_ALIGN;
- size_t align = 1 << 10; /* L2 pagetable alignement */
+ size_t align = 1 << 10; /* L2 pagetable alignment */
struct device_node *np;
int ret;
--
1.8.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 3/8] iommu/mediatek: Fix spelling mistake "phyiscal" -> "physical"
There is a spelling mistake in a comment, fix it.
Signed-off-by: Zhen Lei
---
drivers/iommu/mtk_iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/mtk_iommu.c b/drivers/iommu/mtk_iommu.c
index 6ecc007f07cd52e..c8c9bf1d70b29dc 100644
--- a/drivers/iommu/mtk_iommu.c
+++ b/drivers/iommu/mtk_iommu.c
@@ -160,7 +160,7 @@ struct mtk_iommu_domain {
* The Region 'A'(I/O) can NOT be mapped by M4U; For Region 'B'/'C'/'D', the
* bit32 of the CPU physical address always is needed to set, and for Region
* 'E', the CPU physical address keep as is.
- * Additionally, The iommu consumers always use the CPU phyiscal address.
+ * Additionally, The iommu consumers always use the CPU physical address.
*/
#define MTK_IOMMU_4GB_MODE_REMAP_BASE 0x14000UL
--
1.8.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 4/8] iommu/sun50i: Fix spelling mistake "consits" -> "consists"
There is a spelling mistake in a comment, fix it. Signed-off-by: Zhen Lei --- drivers/iommu/sun50i-iommu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/iommu/sun50i-iommu.c b/drivers/iommu/sun50i-iommu.c index ea6db1341916524..7685b96b2d445a7 100644 --- a/drivers/iommu/sun50i-iommu.c +++ b/drivers/iommu/sun50i-iommu.c @@ -149,7 +149,7 @@ static void iommu_write(struct sun50i_iommu *iommu, u32 offset, u32 value) * 4096 4-bytes Directory Table Entries (DTE), each pointing to a Page * Table (PT). * - * Each PT consits of 256 4-bytes Page Table Entries (PTE), each + * Each PT consists of 256 4-bytes Page Table Entries (PTE), each * pointing to a 4kB page of physical memory. * * The IOMMU supports a single DT, pointed by the IOMMU_TTB_REG -- 1.8.3 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 5/8] iommu: fix a couple of spelling mistakes
There are several spelling mistakes, as follows: funcions ==> functions distiguish ==> distinguish detroyed ==> destroyed Signed-off-by: Zhen Lei --- drivers/iommu/iommu.c | 4 ++-- drivers/iommu/iova.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index d0b0a15dba8413c..0f4e9a6122ee58f 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -1453,7 +1453,7 @@ struct iommu_group *pci_device_group(struct device *dev) /* * Look for existing groups on non-isolated functions on the same -* slot and aliases of those funcions, if any. No need to clear +* slot and aliases of those functions, if any. No need to clear * the search bitmap, the tested devfns are still valid. */ group = get_pci_function_alias_group(pdev, (unsigned long *)devfns); @@ -2267,7 +2267,7 @@ struct iommu_domain *iommu_get_dma_domain(struct device *dev) * iterating over the devices in a group. Ideally we'd have a single * device which represents the requestor ID of the group, but we also * allow IOMMU drivers to create policy defined minimum sets, where - * the physical hardware may be able to distiguish members, but we + * the physical hardware may be able to distinguish members, but we * wish to group them at a higher level (ex. untrusted multi-function * PCI devices). Thus we attach each device. */ diff --git a/drivers/iommu/iova.c b/drivers/iommu/iova.c index e6e2fa85271c3f8..bf710b0a3713e21 100644 --- a/drivers/iommu/iova.c +++ b/drivers/iommu/iova.c @@ -524,7 +524,7 @@ static void fq_destroy_all_entries(struct iova_domain *iovad) int cpu; /* -* This code runs when the iova_domain is being detroyed, so don't +* This code runs when the iova_domain is being destroyed, so don't * bother to free iovas, just call the entry_dtor on all remaining * entries. */ -- 1.8.3 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/8] iommu/pamu: fix a couple of spelling mistakes
There are several spelling mistakes, as follows:
Returs ==> Returns
defaul ==> default
assocaited ==> associated
Signed-off-by: Zhen Lei
---
drivers/iommu/fsl_pamu.c| 2 +-
drivers/iommu/fsl_pamu_domain.c | 2 +-
drivers/iommu/fsl_pamu_domain.h | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
index b9a974d9783113d..48ebbf0daa21cf9 100644
--- a/drivers/iommu/fsl_pamu.c
+++ b/drivers/iommu/fsl_pamu.c
@@ -503,7 +503,7 @@ void get_ome_index(u32 *omi_index, struct device *dev)
* @stash_dest_hint: L1, L2 or L3
* @vcpu: vpcu target for a particular cache type.
*
- * Returs stash on success or ~(u32)0 on failure.
+ * Returns stash on success or ~(u32)0 on failure.
*
*/
u32 get_stash_id(u32 stash_dest_hint, u32 vcpu)
diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
index b2110767caf49c8..be664cd18c51970 100644
--- a/drivers/iommu/fsl_pamu_domain.c
+++ b/drivers/iommu/fsl_pamu_domain.c
@@ -418,7 +418,7 @@ static struct iommu_domain *fsl_pamu_domain_alloc(unsigned
type)
pr_debug("dma_domain allocation failed\n");
return NULL;
}
- /* defaul geometry 64 GB i.e. maximum system address */
+ /* default geometry 64 GB i.e. maximum system address */
dma_domain->iommu_domain. geometry.aperture_start = 0;
dma_domain->iommu_domain.geometry.aperture_end = (1ULL << 36) - 1;
dma_domain->iommu_domain.geometry.force_aperture = true;
diff --git a/drivers/iommu/fsl_pamu_domain.h b/drivers/iommu/fsl_pamu_domain.h
index 2865d42782e8021..4f508fa041080e3 100644
--- a/drivers/iommu/fsl_pamu_domain.h
+++ b/drivers/iommu/fsl_pamu_domain.h
@@ -24,7 +24,7 @@ struct fsl_dma_domain {
*/
dma_addr_t geom_size;
/*
-* Number of windows assocaited with this domain.
+* Number of windows associated with this domain.
* During domain initialization, it is set to the
* the maximum number of subwindows allowed for a LIODN.
* Minimum value for this is 1 indicating a single PAMU
--
1.8.3
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/8] iommu: fix a couple of spelling mistakes detected by codespell tool
This detection and correction covers the entire driver/iommu directory. Zhen Lei (8): iommu/pamu: fix a couple of spelling mistakes iommu/omap: Fix spelling mistake "alignement" -> "alignment" iommu/mediatek: Fix spelling mistake "phyiscal" -> "physical" iommu/sun50i: Fix spelling mistake "consits" -> "consists" iommu: fix a couple of spelling mistakes iommu/amd: fix a couple of spelling mistakes iommu/arm-smmu: Fix spelling mistake "initally" -> "initially" iommu/vt-d: fix a couple of spelling mistakes drivers/iommu/amd/amd_iommu_types.h | 2 +- drivers/iommu/amd/init.c | 4 ++-- drivers/iommu/amd/iommu.c | 2 +- drivers/iommu/arm/arm-smmu/arm-smmu.c | 2 +- drivers/iommu/fsl_pamu.c | 2 +- drivers/iommu/fsl_pamu_domain.c | 2 +- drivers/iommu/fsl_pamu_domain.h | 2 +- drivers/iommu/intel/dmar.c| 6 +++--- drivers/iommu/intel/iommu.c | 2 +- drivers/iommu/intel/irq_remapping.c | 2 +- drivers/iommu/iommu.c | 4 ++-- drivers/iommu/iova.c | 2 +- drivers/iommu/mtk_iommu.c | 2 +- drivers/iommu/omap-iommu.c| 2 +- drivers/iommu/sun50i-iommu.c | 2 +- 15 files changed, 19 insertions(+), 19 deletions(-) -- 1.8.3 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
