Re: remove the ->mapping_error method from dma_map_ops V2
On Wed, Nov 28, 2018 at 12:48 PM Russell King - ARM Linux wrote: > > On Wed, Nov 28, 2018 at 11:27:17AM -0800, David Miller wrote: > > From: Linus Torvalds > > Date: Wed, 28 Nov 2018 10:00:06 -0800 > > > > > Not all memory is accessible even to the kernel. If you have memory > > > that shows up in the last page of phys_addr_t, you just mark it > > > reserved at boot-time. > > > > It's not the physical memory at the end that needs to be reserved. > > > > It's the IOMMU mapping arena. > > True, if and only if you have an IOMMU. > > Where there isn't an IOMMU, then we'd have to reserve every page that > that translates to a bus address in the top 4K of dma_addr_t on any > bus in the system - that means knowing early in the kernel > initialisation about all buses in the system so we can detect and > reserve these pages. > The arch and platform differences/confusion as to "what is DMA error value" is the reason why dma_mapping_error is part dma ops. I understand that iy would make sense to reduce the overhead of an additional call, however, I am not sure if would be trivial change to address this. I was down this path of trying to address the missing mapping error checks a few years ago and introduced dma_mapping_error checks in the DMA DEBUG API. As you might already know that there is no common definition for the mapping error. Quick look at the defines shows: #define CALGARY_MAPPING_ERROR 0 #define S390_MAPPING_ERROR (~(dma_addr_t) 0x0) #define SPARC_MAPPING_ERROR (~(dma_addr_t)0x0) This is the reason why there is a arch or in some cases bus specific mapping_error ops is needed. We could unify this and fix all these first. I haven't looked at the patch set closely, maybe you are already doing this. thanks, -- Shuah ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/9] iommu: ipmmu-vmsa: make it explicitly non-modular
[Re: [PATCH 5/9] iommu: ipmmu-vmsa: make it explicitly non-modular] On 28/11/2018 (Wed 19:22) Laurent Pinchart wrote: > Hi Paul, > > On Wednesday, 28 November 2018 17:32:05 EET Paul Gortmaker wrote: > > [Re: [PATCH 5/9] iommu: ipmmu-vmsa: make it explicitly non-modular] On > 28/11/2018 (Wed 12:50) Robin Murphy wrote: > > > On 26/11/2018 22:31, Paul Gortmaker wrote: > > [...] > > > > >> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c > > >> index 9e2655f1c1bf..de39ef992d8a 100644 > > >> --- a/drivers/iommu/ipmmu-vmsa.c > > >> +++ b/drivers/iommu/ipmmu-vmsa.c > > >> @@ -1,6 +1,8 @@ > > >> > > >> // SPDX-License-Identifier: GPL-2.0 > > >> /* > > >> * IPMMU VMSA > > > > > > Nit: this line doesn't convey any information that the module description > > > below doesn't also say far more clearly, so you may as well just replace > > > it entirely. > > > > No problem. Will do in v2. > > With that change, the blank line after MODULE_DEVICE_TABLE() removed, and > moved to keep alphabetical order sorting of the includes, > please add > > Reviewed-by: Laurent Pinchart Updated and review tag added for v2. Paul. -- > > to v2. Thank you for the patch. > > > > >+ * IOMMU API for Renesas VMSA-compatible IPMMU > > > >+ * Author: Laurent Pinchart > > > > [...] > > -- > Regards, > > Laurent Pinchart > > > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use generic DMA mapping code in powerpc V4
On Wed, 28 Nov 2018 16:55:30 +0100 Christian Zigotzky wrote: > On 28 November 2018 at 12:05PM, Michael Ellerman wrote: > > Nothing specific yet. > > > > I'm a bit worried it might break one of the many old obscure platforms > > we have that aren't well tested. > > > Please don't apply the new DMA mapping code if you don't be sure if it > works on all supported PowerPC machines. Is the new DMA mapping code > really necessary? It's not really nice, to rewrote code if the old code > works perfect. We must not forget, that we work for the end users. Does > the end user have advantages with this new code? Is it faster? The old > code works without any problems. There is another service provided to the users as well: new code that is cleaner and simpler which allows easier bug fixes and new features. Without being familiar with the DMA mapping code I cannot really say if that's the case here. > I am also worried about this code. How > can I test this new DMA mapping code? I suppose if your machine works it works for you. Thanks Michal ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] PCI / ACPI: Identify untrusted PCI devices
On Tue, Nov 27, 2018 at 10:54:26AM +0200, Mika Westerberg wrote: > On Mon, Nov 26, 2018 at 06:17:11PM -0600, Bjorn Helgaas wrote: > > Hi Mika, > > Hi, > > > On Mon, Nov 26, 2018 at 02:15:23PM +0300, Mika Westerberg wrote: > > > Recent systems with Thunderbolt ports may support IOMMU natively. > > > > This sentence doesn't make sense to me. There's no logical connection > > between having an IOMMU and having a Thunderbolt port. > > > > > This means that the platform utilizes IOMMU to prevent DMA attacks > > > over externally exposed PCIe root ports (typically Thunderbolt > > > ports) > > > > Nor this one. The platform only uses the IOMMU to prevent DMA attacks > > if the OS chooses to do that. I think by "platform" you're referring to the system firmware; I was only thinking of the hardware, so the IOMMU wouldn't be used unless someone (the OS) enabled it. But your cover letter talks about the BIOS enabling some IOMMU functionality. > I guess I'm trying to say here that the recent changes add such support > to the platform BIOS that allows the OS to enable IOMMU without being > compromised by a malicious device that is already connected. The BIOS > sets the new ACPI DMAR bit in that case. Ah, there's useful info to this effect in your [0/4] cover letter. That info and the URL should be in the changelog of one of the patches so it doesn't get lost. > > > The system BIOS marks these PCIe root ports as being externally facing > > > ports by implementing following ACPI _DSD [1] under the root port in > > > question: > > > > There's no standard that requires this, so the best we can say is that > > a system BIOS *may* mark externally facing ports with this mechanism. > > There is no standard but I'm quite sure this is something that will be > required to be implemented properly by the OEM by Microsoft hardware > compatibility suite. Sure. Your statement suggests that all external ports will be marked with the _DSD. I'm just pointing out that the OS can't assume that because there are probably systems in the field that predate the _DSD. Bjorn ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use generic DMA mapping code in powerpc V4
I will compile and test the kernel from the following Git on my PowerPC machines. http://git.infradead.org/users/hch/misc.git On 28 November 2018 at 12:05PM, Michael Ellerman wrote: Nothing specific yet. I'm a bit worried it might break one of the many old obscure platforms we have that aren't well tested. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
On Wed, Nov 28, 2018 at 11:27:17AM -0800, David Miller wrote: > From: Linus Torvalds > Date: Wed, 28 Nov 2018 10:00:06 -0800 > > > Not all memory is accessible even to the kernel. If you have memory > > that shows up in the last page of phys_addr_t, you just mark it > > reserved at boot-time. > > It's not the physical memory at the end that needs to be reserved. > > It's the IOMMU mapping arena. True, if and only if you have an IOMMU. Where there isn't an IOMMU, then we'd have to reserve every page that that translates to a bus address in the top 4K of dma_addr_t on any bus in the system - that means knowing early in the kernel initialisation about all buses in the system so we can detect and reserve these pages. I don't think that's trivial to do. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
On Wed, Nov 28, 2018 at 11:19:15AM -0800, Linus Torvalds wrote: > On Wed, Nov 28, 2018, 10:08 Russell King - ARM Linux wrote: > > > > > > > You already cannot do that kmalloc(), exactly because of ERR_PTR(). > > > > I'm very sorry, but I think you are confused. > > > > kmalloc() returns a _virtual_ address, which quite rightly must not be > > in the top 4K of 4GB, exactly due to ERR_PTR(). That is fine. > > > > However, that is a completely different kettle of fish from a physical > > or DMA address - neither of which are virtual addresses. > > > > You cut away the part that showed that I'm not in the least confused. > > Let me just paste it back in here: > > "Which is what we ALREADY do for these exact reasons. If the DMA > mappings means that you'd need to add one more page to that list of > reserved pages, then so be it." We're not talking about just one more page. We're talking about _every_ page that _may_ map to a bus address in the range of 4GB - 4K (a point I've already made earlier in this discussion.) It's not just about physical addresses, it's about bus addresses, and there are buses out there that have a translation between physical address and bus address without IOMMUs and the like. Can we know ahead of time about all these translations? It means moving that information out of various bus drivers into core architecture code (yuck, when you have multiple different platforms) or into DT (which means effectively breaking every damn platform that's using older DT files) - something that we don't have to do today. I remain 100% opposed to your idea. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
From: Linus Torvalds Date: Wed, 28 Nov 2018 10:00:06 -0800 > Not all memory is accessible even to the kernel. If you have memory > that shows up in the last page of phys_addr_t, you just mark it > reserved at boot-time. It's not the physical memory at the end that needs to be reserved. It's the IOMMU mapping arena. That basically requires changes to every IOMMU implementation in the tree. Not saying it's hard or impossible, but it is real meticulous work that needs to be done in order to realize this. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
On Wed, Nov 28, 2018 at 06:08:41PM +, Russell King - ARM Linux wrote: > On Wed, Nov 28, 2018 at 10:00:06AM -0800, Linus Torvalds wrote: > > On Wed, Nov 28, 2018 at 9:45 AM Russell King - ARM Linux > > wrote: > > > > > > > I don't think this is a huge deal, but ERR_PTR() has been hugely > > > > successful elsewhere. And I'm not hugely convinced about all these > > > > "any address can be valid" arguments. How the hell do you generate a > > > > random dma address in the last page that isn't even page-aligned? > > > > > > kmalloc() a 64-byte buffer, dma_map_single() that buffer. > > > > No. > > > > You already cannot do that kmalloc(), exactly because of ERR_PTR(). > > I'm very sorry, but I think you are confused. > > kmalloc() returns a _virtual_ address, which quite rightly must not be > in the top 4K of 4GB, exactly due to ERR_PTR(). That is fine. > > However, that is a completely different kettle of fish from a physical > or DMA address - neither of which are virtual addresses. > > Now, say we have 1GB of RAM which starts at 0xc000 _physical_. > The kernel is configured with a 2GB/2GB user/kernel split, which means > all 1GB of RAM is mapped as lowmem from 0x8000 to 0xbfff > inclusive. This means kmalloc() can return any address in that range. > > ERR_PTR() will work correctly on any of those pointers, meaning that > none of them will be seen as an error. > > However, map any virtual address in the range of 0xb000 to > 0xbfff into DMA space, and the resulting DMA address could well > be in the range of 0xf000 to 0x - precisely the range > of addresses that you are advocating to be used for error codes. > > > The whole argument of "every possible piece of memory is DMA'able" is > > just wrong. > > I'm very sorry, but I do not buy your argument - you are conflating > virtual addresses which ERR_PTR() deals in with physical and bus > addresses - and if you persist down this route, you will cause > regressions. Here's another case: i.MX6 with 4GB of RAM. Devices are mapped to 0-0x0fff physical, RAM is mapped to 0x1000-0x physical. The last 256MB of RAM is not accessible as this is a 32-bit device. DMA addresses are the same as physical addresses. While the final physical page will be highmem in a normal kernel, and thus will not be available for kmalloc(), that doesn't mean it can't happen. A crashdump kernel loaded high in physical memory (eg, last 512MB and given the last 512MB to play around in) would have the top 512MB as lowmem, and therefore available for kmalloc(). If a page is available in lowmem, it's available for kmalloc(), and we can't say that we will never allocate memory from such a page for DMA - if we do and we're using an IS_ERR_VALUE() scheme, it _will_ break if that happens as memory will end up being mapped by the DMA API but dma_mapping_error() will see it as a failure. It won't be an obvious breakage, because it depends on the right conditions happening - a kmalloc() from the top page of physical RAM and that being passed to dma_map_single(). IOW, it's not something that a quick boot test would find, it's something that is likely to cause failures after a system has been running for a period of time. There are other situations where there are possibilities - such as: dma_map_page(dev, page, offset, size, direction) If 'page' is a highmem page which happens to be the top page in the 4GB space, and offset is non-zero, and there's a 1:1 mapping between physical address and DMA address, the returned value will be 0xf000 + offset - within the "last 4095 values are errors" range. Networking uses this for fragments - the packet fragment list is a list of pages, offsets and sizes - we have sendpage() that may end up finding that last page, and TCP-sized packets may be generated from it which would certianly result in non-zero offsets being passed to dma_map_page(). So, whatever way _I_ look at it, I find your proposal to be unsafe and potentially regression causing, and I *completely* and strongly oppose it in its current form. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
On Wed, Nov 28, 2018, 10:08 Russell King - ARM Linux > > > You already cannot do that kmalloc(), exactly because of ERR_PTR(). > > I'm very sorry, but I think you are confused. > > kmalloc() returns a _virtual_ address, which quite rightly must not be > in the top 4K of 4GB, exactly due to ERR_PTR(). That is fine. > > However, that is a completely different kettle of fish from a physical > or DMA address - neither of which are virtual addresses. > You cut away the part that showed that I'm not in the least confused. Let me just paste it back in here: "Which is what we ALREADY do for these exact reasons. If the DMA mappings means that you'd need to add one more page to that list of reserved pages, then so be it." So no, I'm not at all confused. Let me re-iterate: the argument that all addresses have to be dma'able is garbage. *Exactly* as with kmalloc and limited virtual addresses, we can limit physical addresses. Linus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
On Wed, Nov 28, 2018 at 10:00:06AM -0800, Linus Torvalds wrote: > On Wed, Nov 28, 2018 at 9:45 AM Russell King - ARM Linux > wrote: > > > > > I don't think this is a huge deal, but ERR_PTR() has been hugely > > > successful elsewhere. And I'm not hugely convinced about all these > > > "any address can be valid" arguments. How the hell do you generate a > > > random dma address in the last page that isn't even page-aligned? > > > > kmalloc() a 64-byte buffer, dma_map_single() that buffer. > > No. > > You already cannot do that kmalloc(), exactly because of ERR_PTR(). I'm very sorry, but I think you are confused. kmalloc() returns a _virtual_ address, which quite rightly must not be in the top 4K of 4GB, exactly due to ERR_PTR(). That is fine. However, that is a completely different kettle of fish from a physical or DMA address - neither of which are virtual addresses. Now, say we have 1GB of RAM which starts at 0xc000 _physical_. The kernel is configured with a 2GB/2GB user/kernel split, which means all 1GB of RAM is mapped as lowmem from 0x8000 to 0xbfff inclusive. This means kmalloc() can return any address in that range. ERR_PTR() will work correctly on any of those pointers, meaning that none of them will be seen as an error. However, map any virtual address in the range of 0xb000 to 0xbfff into DMA space, and the resulting DMA address could well be in the range of 0xf000 to 0x - precisely the range of addresses that you are advocating to be used for error codes. > The whole argument of "every possible piece of memory is DMA'able" is > just wrong. I'm very sorry, but I do not buy your argument - you are conflating virtual addresses which ERR_PTR() deals in with physical and bus addresses - and if you persist down this route, you will cause regressions. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
On Wed, Nov 28, 2018 at 9:45 AM Russell King - ARM Linux wrote: > > > I don't think this is a huge deal, but ERR_PTR() has been hugely > > successful elsewhere. And I'm not hugely convinced about all these > > "any address can be valid" arguments. How the hell do you generate a > > random dma address in the last page that isn't even page-aligned? > > kmalloc() a 64-byte buffer, dma_map_single() that buffer. No. You already cannot do that kmalloc(), exactly because of ERR_PTR(). Not all memory is accessible even to the kernel. If you have memory that shows up in the last page of phys_addr_t, you just mark it reserved at boot-time. Which is what we ALREADY do for these exact reasons. If the DMA mappings means that you'd need to add one more page to that list of reserved pages, then so be it. The whole argument of "every possible piece of memory is DMA'able" is just wrong. Linus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
On Wed, Nov 28, 2018 at 08:47:05AM -0800, Linus Torvalds wrote: > On Tue, Nov 27, 2018 at 11:41 PM Christoph Hellwig wrote: > > > > On Fri, Nov 23, 2018 at 07:55:11AM +0100, Christoph Hellwig wrote: > > > Well, I can tweak the last patch to return -EINVAL from dma_mapping_error > > > instead of the old 1 is as bool true. The callers should all be fine, > > > although I'd have to audit them. Still wouldn't help with being able to > > > return different errors. > > > > Any opinions? I'd really like to make some forward progress on this > > series. > > So I do think that yes, dma_mapping_error() should return an error > code, not 0/1. > > But I was really hoping that the individual drivers themselves could > return error codes. Right now the patch-series has code like this: > > ret = needs_bounce(dev, dma_addr, size); > if (ret < 0) > - return ARM_MAPPING_ERROR; > + return DMA_MAPPING_ERROR; > > which while it all makes sense in the context of this patch-series, I > *really* think it would have been so much nicer to return the error > code 'ret' instead (which in this case is -E2BIG). > > I don't think this is a huge deal, but ERR_PTR() has been hugely > successful elsewhere. And I'm not hugely convinced about all these > "any address can be valid" arguments. How the hell do you generate a > random dma address in the last page that isn't even page-aligned? kmalloc() a 64-byte buffer, dma_map_single() that buffer. If you have RAM that maps to a _bus_ address in the top page of 4GB of a 32-bit bus address, then you lose. Simples. Subsystems like I2C, SPI, USB etc all deal with small kmalloc'd buffers and their drivers make use of DMA. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 8/9] iommu: arm-smmu: make it explicitly non-modular
On 28/11/2018 15:24, Paul Gortmaker wrote: [Re: [PATCH 8/9] iommu: arm-smmu: make it explicitly non-modular] On 28/11/2018 (Wed 12:42) Robin Murphy wrote: Hi Paul, On 26/11/2018 22:31, Paul Gortmaker wrote: [...] We add a moduleparam.h include since the file does actually declare some module parameters, and leaving them as such is the easiest way currently to remain backwards compatible with existing use cases. [...] Is it worth introducing builtin_param() and friends for this sort of thing, to echo the *_platform_driver() helpers? It seems like that could be justifiable under the motivation described in the cover letter. I've definitely gone back and looked at this a few times when coming across the few corner cases like these, to remind myself why I didn't do it already. We'd not want to replicate all the module_param stuff as an instance of builtin_param() because we already have setup() and setup_param() in init.h -- however they don't do the file name in the param - hence the reason it isn't a direct swap in replacement. So, it would become some more complex refactoring of moduleparam.h into say bootparam.h - to reduce code/macro duplication, while at the same time being aware of existing setup_param stuff and making something like a new setup_param_named() that is consistent with existing setup fcns. And based on past experience, there will be reviewers who don't see the value in the distinction and simply reply with two words "why bother?". Not impossible, but not as simple as the builtin_platform_driver and similar wrappers that I've already added to mainline. You've made we want to go have another look at it again, but in the meantime we can do what I've done here, and circle around later to update the few instances of moduleparam in non-modules once/if the refactoring I describe above works out and is accepted in mainline. Sure, I definitely agree with doing a first pass like this to sweep up all the cruft and audit the module_param users at the same time, then considering a robust refactoring once we've got a clear idea of how many users actually need it. TBH, at this point I was thinking along the lines of a simple: #ifndef MODULE #define builtin_param(name, type, perm) \ module_param(name, type, perm) #define builtin_param_named(name, name, type, perrm)\ module_param_named(name, name, type, perm) #endif still in moduleparam.h, purely so that the intent can be made really clear in driver code and it's more searchable than just comments. But yeah, even that would probably be objectionable to many. Otherwise, the changes look reasonable. I still hold out hope that one day we'll be able to make IOMMU drivers modular (it can work with minimal hacks today, but it's far from robust in general), but for now I agree this makes sense (and it'll be easy enough to revert for playing with further hacks). I totally agree - I've had similar discussions with the DMA maintainers, and if being modular can be made to work and has a use case - great! But it should be a conscious decision, since nobody writes a new driver from scratch; they copy one that is "close" as a template and then go from there. Which leads to a good percentage of drivers having hints of modular stuff when there is no intent of them ever being modular. With the title fixed up as Joerg asked, Acked-by: Robin Murphy Thanks for the feedback/review. Will re-send with updated titles before the week is finished and a good chance for additional feedback has elapsed. Great! There's probably some more subtleties that could be tidied up when the driver is truly non-removable, but I can take a look at that myself once this patch has gone in. Cheers, Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/9] iommu: ipmmu-vmsa: make it explicitly non-modular
Hi Paul, On Wednesday, 28 November 2018 17:32:05 EET Paul Gortmaker wrote: > [Re: [PATCH 5/9] iommu: ipmmu-vmsa: make it explicitly non-modular] On 28/11/2018 (Wed 12:50) Robin Murphy wrote: > > On 26/11/2018 22:31, Paul Gortmaker wrote: > [...] > > >> diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c > >> index 9e2655f1c1bf..de39ef992d8a 100644 > >> --- a/drivers/iommu/ipmmu-vmsa.c > >> +++ b/drivers/iommu/ipmmu-vmsa.c > >> @@ -1,6 +1,8 @@ > >> > >> // SPDX-License-Identifier: GPL-2.0 > >> /* > >> * IPMMU VMSA > > > > Nit: this line doesn't convey any information that the module description > > below doesn't also say far more clearly, so you may as well just replace > > it entirely. > > No problem. Will do in v2. With that change, the blank line after MODULE_DEVICE_TABLE() removed, and moved to keep alphabetical order sorting of the includes, please add Reviewed-by: Laurent Pinchart to v2. Thank you for the patch. > > >+ * IOMMU API for Renesas VMSA-compatible IPMMU > > >+ * Author: Laurent Pinchart > > [...] -- Regards, Laurent Pinchart ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: remove the ->mapping_error method from dma_map_ops V2
On Tue, Nov 27, 2018 at 11:41 PM Christoph Hellwig wrote: > > On Fri, Nov 23, 2018 at 07:55:11AM +0100, Christoph Hellwig wrote: > > Well, I can tweak the last patch to return -EINVAL from dma_mapping_error > > instead of the old 1 is as bool true. The callers should all be fine, > > although I'd have to audit them. Still wouldn't help with being able to > > return different errors. > > Any opinions? I'd really like to make some forward progress on this > series. So I do think that yes, dma_mapping_error() should return an error code, not 0/1. But I was really hoping that the individual drivers themselves could return error codes. Right now the patch-series has code like this: ret = needs_bounce(dev, dma_addr, size); if (ret < 0) - return ARM_MAPPING_ERROR; + return DMA_MAPPING_ERROR; which while it all makes sense in the context of this patch-series, I *really* think it would have been so much nicer to return the error code 'ret' instead (which in this case is -E2BIG). I don't think this is a huge deal, but ERR_PTR() has been hugely successful elsewhere. And I'm not hugely convinced about all these "any address can be valid" arguments. How the hell do you generate a random dma address in the last page that isn't even page-aligned? Linus ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v18 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops
On 28/11/2018 16:24, Stephen Boyd wrote:
Quoting Vivek Gautam (2018-11-27 02:11:41)
@@ -1966,6 +1970,23 @@ static const struct of_device_id arm_smmu_of_match[] = {
};
MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
+static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
+ const char * const *clks)
+{
+ int i;
+
+ if (smmu->num_clks < 1)
+ return;
+
+ smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
+ sizeof(*smmu->clks), GFP_KERNEL);
+ if (!smmu->clks)
+ return;
+
+ for (i = 0; i < smmu->num_clks; i++)
+ smmu->clks[i].id = clks[i];
Is this clk_bulk_get_all()?
Ooh, did that finally get merged while we weren't looking? Great!
Much as I don't want to drag this series out to a v19, it *would* be
neat if we no longer need to open-code that bit...
Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v18 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops
Quoting Vivek Gautam (2018-11-27 02:11:41)
> @@ -1966,6 +1970,23 @@ static const struct of_device_id arm_smmu_of_match[] =
> {
> };
> MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
>
> +static void arm_smmu_fill_clk_data(struct arm_smmu_device *smmu,
> + const char * const *clks)
> +{
> + int i;
> +
> + if (smmu->num_clks < 1)
> + return;
> +
> + smmu->clks = devm_kcalloc(smmu->dev, smmu->num_clks,
> + sizeof(*smmu->clks), GFP_KERNEL);
> + if (!smmu->clks)
> + return;
> +
> + for (i = 0; i < smmu->num_clks; i++)
> + smmu->clks[i].id = clks[i];
Is this clk_bulk_get_all()?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use generic DMA mapping code in powerpc V4
On 28 November 2018 at 12:05PM, Michael Ellerman wrote: Nothing specific yet. I'm a bit worried it might break one of the many old obscure platforms we have that aren't well tested. Please don't apply the new DMA mapping code if you don't be sure if it works on all supported PowerPC machines. Is the new DMA mapping code really necessary? It's not really nice, to rewrote code if the old code works perfect. We must not forget, that we work for the end users. Does the end user have advantages with this new code? Is it faster? The old code works without any problems. I am also worried about this code. How can I test this new DMA mapping code? Thanks ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/9] iommu: ipmmu-vmsa: make it explicitly non-modular
[Re: [PATCH 5/9] iommu: ipmmu-vmsa: make it explicitly non-modular] On 28/11/2018 (Wed 12:50) Robin Murphy wrote: > On 26/11/2018 22:31, Paul Gortmaker wrote: [...] > >diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c > >index 9e2655f1c1bf..de39ef992d8a 100644 > >--- a/drivers/iommu/ipmmu-vmsa.c > >+++ b/drivers/iommu/ipmmu-vmsa.c > >@@ -1,6 +1,8 @@ > > // SPDX-License-Identifier: GPL-2.0 > > /* > > * IPMMU VMSA > > Nit: this line doesn't convey any information that the module description > below doesn't also say far more clearly, so you may as well just replace it > entirely. No problem. Will do in v2. P. -- > > Robin. > > >+ * IOMMU API for Renesas VMSA-compatible IPMMU > >+ * Author: Laurent Pinchart [...] ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 8/9] iommu: arm-smmu: make it explicitly non-modular
[Re: [PATCH 8/9] iommu: arm-smmu: make it explicitly non-modular] On 28/11/2018 (Wed 12:42) Robin Murphy wrote: > Hi Paul, > > On 26/11/2018 22:31, Paul Gortmaker wrote: [...] > >We add a moduleparam.h include since the file does actually declare > >some module parameters, and leaving them as such is the easiest way > >currently to remain backwards compatible with existing use cases. [...] > Is it worth introducing builtin_param() and friends for this sort of thing, > to echo the *_platform_driver() helpers? It seems like that could be > justifiable under the motivation described in the cover letter. I've definitely gone back and looked at this a few times when coming across the few corner cases like these, to remind myself why I didn't do it already. We'd not want to replicate all the module_param stuff as an instance of builtin_param() because we already have setup() and setup_param() in init.h -- however they don't do the file name in the param - hence the reason it isn't a direct swap in replacement. So, it would become some more complex refactoring of moduleparam.h into say bootparam.h - to reduce code/macro duplication, while at the same time being aware of existing setup_param stuff and making something like a new setup_param_named() that is consistent with existing setup fcns. And based on past experience, there will be reviewers who don't see the value in the distinction and simply reply with two words "why bother?". Not impossible, but not as simple as the builtin_platform_driver and similar wrappers that I've already added to mainline. You've made we want to go have another look at it again, but in the meantime we can do what I've done here, and circle around later to update the few instances of moduleparam in non-modules once/if the refactoring I describe above works out and is accepted in mainline. > > Otherwise, the changes look reasonable. I still hold out hope that one day > we'll be able to make IOMMU drivers modular (it can work with minimal hacks > today, but it's far from robust in general), but for now I agree this makes > sense (and it'll be easy enough to revert for playing with further hacks). I totally agree - I've had similar discussions with the DMA maintainers, and if being modular can be made to work and has a use case - great! But it should be a conscious decision, since nobody writes a new driver from scratch; they copy one that is "close" as a template and then go from there. Which leads to a good percentage of drivers having hints of modular stuff when there is no intent of them ever being modular. > With the title fixed up as Joerg asked, > > Acked-by: Robin Murphy Thanks for the feedback/review. Will re-send with updated titles before the week is finished and a good chance for additional feedback has elapsed. Paul. -- > > > module_param(force_stage, int, S_IRUGO); > > MODULE_PARM_DESC(force_stage, > > "Force SMMU mappings to be installed at a particular stage of > > translation. A value of '1' or '2' forces the corresponding stage. All > > other values are ignored (i.e. no stage is forced). Note that selecting a > > specific stage will disable support for nested translation."); ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: PCI passthrough with multiple devices in same iommu group
On Wed, 28 Nov 2018 20:21:02 +0530 gokul cg wrote: > Hi Folks, > > Please excuse me , I just writing to you as i could see you had made > changes regarding iommu and I thought you could give help me here. > > We are testing visualization with QEMU-2.7.0 + Linux-4.8+, we are facing > issue while configuring pass through PCI devices in QEMU to pass it to > guest OS. > We are using following QEMU argument to configure PCI device as passthrough > device to guest, which was working with Linux 3.10 distro (hypervisor: QEMU > 1.7.2, Linux: 3.10+). > > /usr/bin/qemu-system-x86_64 -name guest_1 -S -machine > pc-i440fx-1.7,accel=kvm,usb=off -m 49152\ > -device kvm-pci-assign,host=:00:1f.3 -device > kvm-pci-assign,host=:09:0e.0 .. Legacy KVM device assignment (aka kvm-pci-assign) has been deprecated for some time now, you'll find it doesn't even exist in newer kernels and QEMU. > Here is the error message that we will get when we try to configure PCI > devices as passthrough using kvm pci assignment method in Linux 4.8+ > (hypervisor: QEMU 2.7.0, Linux: 4.8+). > > which is shown below. > > ---log--- > > From QEMU: > qemu-system-x86_64: -device kvm-pci-assign,host=:00:1f.3: Failed to > assign device "(null)": Invalid argument > > From dmesg: > pci-stub :00:1f.3: kvm assign device failed ret -22 > > ---end > > Info about devices (CPU: Intel(R) Xeon(R) CPU E5-2608L): > > root@shining-node:~# lspci -k -s 00:1f.3 > 00:1f.3 SMBus: Intel Corporation C610/X99 series chipset SMBus Controller > (rev 05) > Subsystem: Intel Corporation C610/X99 series chipset SMBus > Controller > Kernel driver in use: pci-stub > Kernel modules: i2c_i801 Why are you trying to assign an SMBus controller to a VM? > root@shining-node:~# > root@shining-node:~# lspci -k -s 09:0e.0 > 09:0e.0 Network controller: Broadcom Limited Device b680 (rev 12) > root@shining-node:~# > > From the web search i could see that it is because there are multiple > devices in same iommu_group that the passthrough device belongs to. > When i check iommu_group , i have multiple devices in same group but all > those are intel devices under Wellsburg PCH. Nope, kvm-pci-assign doesn't make use of IOMMU groups, more likely just some state of disrepair as it's deprecated and replaced by vfio-pci, which does use iommu groups. So iommu groups will be a problem, but not in the configuration you're attempting to use above. > root@shining-node:~# ls > /sys/bus/pci/devices/\:00\:1f.3/iommu_group/devices/ > :00:1f.0 :00:1f.2 :00:1f.3 :00:1f.6 > root@shining-node:~# > root@shining-node:~# lspci -v -s :00:1f > 00:1f.0 ISA bridge: Intel Corporation C610/X99 series chipset LPC > Controller (rev 05) > Subsystem: Intel Corporation C610/X99 series chipset LPC Controller > Flags: bus master, medium devsel, latency 0, NUMA node 0 > Capabilities: [e0] Vendor Specific Information: Len=0c > Kernel driver in use: lpc_ich > Kernel modules: lpc_ich > > 00:1f.2 SATA controller: Intel Corporation C610/X99 series chipset 6-Port > SATA Controller [AHCI mode] (rev 05) (prog-if 01 [AHCI 1.0]) > Subsystem: Intel Corporation C610/X99 series chipset 6-Port SATA Controller > [AHCI mode] > Flags: bus master, 66MHz, medium devsel, latency 0, IRQ 35, NUMA node 0 > I/O ports at 3068 [size=8] > I/O ports at 3074 [size=4] > I/O ports at 3060 [size=8] > I/O ports at 3070 [size=4] > I/O ports at 3020 [size=32] > Memory at 91d2 (32-bit, non-prefetchable) [size=2K] > Capabilities: [80] MSI: Enable+ Count=1/1 Maskable- 64bit- > Capabilities: [70] Power Management version 3 > Capabilities: [a8] SATA HBA v1.0 > Kernel driver in use: ahci > > 00:1f.3 SMBus: Intel Corporation C610/X99 series chipset SMBus Controller > (rev 05) > Subsystem: Intel Corporation C610/X99 series chipset SMBus Controller > Flags: medium devsel, IRQ 18, NUMA node 0 > Memory at 1860 (64-bit, non-prefetchable) [size=4K] > I/O ports at 3000 [size=32] > Kernel driver in use: pci-stub > Kernel modules: i2c_i801 > > 00:1f.6 Signal processing controller: Intel Corporation C610/X99 series > chipset Thermal Subsystem (rev 05) > Subsystem: Intel Corporation C610/X99 series chipset Thermal Subsystem > Flags: bus master, fast devsel, latency 0, IRQ 255, NUMA node 0 > Memory at 183fc001 (64-bit, non-prefetchable) [size=4K] > Capabilities: [50] Power Management version 3 > Capabilities: [80] MSI: Enable- Count=1/1 Maskable- 64bit- Again I have to ask, why are these things you'd want to pass to a VM? The SATA controller is the only reasonable thing to assign here, but can easily be replaced by a plugin card with proper isolation or the performance can be nearly matched with virtio devices. These devices do not provide isolation between them therefore the assignment group is the full set of devices within the iommu group. > root@shining-node:~# > > > I have tried ACS override patch to rearrange these devices to different > iommu_group and it
Re: [PATCH 1/9] mm: Introduce new vm_insert_range API
Am Donnerstag, 15. November 2018, 16:45:30 CET schrieb Souptick Joarder: > Previouly drivers have their own way of mapping range of > kernel pages/memory into user vma and this was done by > invoking vm_insert_page() within a loop. > > As this pattern is common across different drivers, it can > be generalized by creating a new function and use it across > the drivers. > > vm_insert_range is the new API which will be used to map a > range of kernel memory/pages to user vma. > > Signed-off-by: Souptick Joarder > Reviewed-by: Matthew Wilcox Except the missing EXPORT_SYMBOL for module builds this new API is supposed to run also within the Rockchip drm driver, so on rk3188, rk3288, rk3328 and rk3399 with graphics Tested-by: Heiko Stuebner ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
PCI passthrough with multiple devices in same iommu group
Hi Folks, Please excuse me , I just writing to you as i could see you had made changes regarding iommu and I thought you could give help me here. We are testing visualization with QEMU-2.7.0 + Linux-4.8+, we are facing issue while configuring pass through PCI devices in QEMU to pass it to guest OS. We are using following QEMU argument to configure PCI device as passthrough device to guest, which was working with Linux 3.10 distro (hypervisor: QEMU 1.7.2, Linux: 3.10+). /usr/bin/qemu-system-x86_64 -name guest_1 -S -machine pc-i440fx-1.7,accel=kvm,usb=off -m 49152\ -device kvm-pci-assign,host=:00:1f.3 -device kvm-pci-assign,host=:09:0e.0 .. Here is the error message that we will get when we try to configure PCI devices as passthrough using kvm pci assignment method in Linux 4.8+ (hypervisor: QEMU 2.7.0, Linux: 4.8+). which is shown below. ---log--- >From QEMU: qemu-system-x86_64: -device kvm-pci-assign,host=:00:1f.3: Failed to assign device "(null)": Invalid argument >From dmesg: pci-stub :00:1f.3: kvm assign device failed ret -22 ---end Info about devices (CPU: Intel(R) Xeon(R) CPU E5-2608L): root@shining-node:~# lspci -k -s 00:1f.3 00:1f.3 SMBus: Intel Corporation C610/X99 series chipset SMBus Controller (rev 05) Subsystem: Intel Corporation C610/X99 series chipset SMBus Controller Kernel driver in use: pci-stub Kernel modules: i2c_i801 root@shining-node:~# root@shining-node:~# lspci -k -s 09:0e.0 09:0e.0 Network controller: Broadcom Limited Device b680 (rev 12) root@shining-node:~# >From the web search i could see that it is because there are multiple devices in same iommu_group that the passthrough device belongs to. When i check iommu_group , i have multiple devices in same group but all those are intel devices under Wellsburg PCH. root@shining-node:~# ls /sys/bus/pci/devices/\:00\:1f.3/iommu_group/devices/ :00:1f.0 :00:1f.2 :00:1f.3 :00:1f.6 root@shining-node:~# root@shining-node:~# lspci -v -s :00:1f 00:1f.0 ISA bridge: Intel Corporation C610/X99 series chipset LPC Controller (rev 05) Subsystem: Intel Corporation C610/X99 series chipset LPC Controller Flags: bus master, medium devsel, latency 0, NUMA node 0 Capabilities: [e0] Vendor Specific Information: Len=0c Kernel driver in use: lpc_ich Kernel modules: lpc_ich 00:1f.2 SATA controller: Intel Corporation C610/X99 series chipset 6-Port SATA Controller [AHCI mode] (rev 05) (prog-if 01 [AHCI 1.0]) Subsystem: Intel Corporation C610/X99 series chipset 6-Port SATA Controller [AHCI mode] Flags: bus master, 66MHz, medium devsel, latency 0, IRQ 35, NUMA node 0 I/O ports at 3068 [size=8] I/O ports at 3074 [size=4] I/O ports at 3060 [size=8] I/O ports at 3070 [size=4] I/O ports at 3020 [size=32] Memory at 91d2 (32-bit, non-prefetchable) [size=2K] Capabilities: [80] MSI: Enable+ Count=1/1 Maskable- 64bit- Capabilities: [70] Power Management version 3 Capabilities: [a8] SATA HBA v1.0 Kernel driver in use: ahci 00:1f.3 SMBus: Intel Corporation C610/X99 series chipset SMBus Controller (rev 05) Subsystem: Intel Corporation C610/X99 series chipset SMBus Controller Flags: medium devsel, IRQ 18, NUMA node 0 Memory at 1860 (64-bit, non-prefetchable) [size=4K] I/O ports at 3000 [size=32] Kernel driver in use: pci-stub Kernel modules: i2c_i801 00:1f.6 Signal processing controller: Intel Corporation C610/X99 series chipset Thermal Subsystem (rev 05) Subsystem: Intel Corporation C610/X99 series chipset Thermal Subsystem Flags: bus master, fast devsel, latency 0, IRQ 255, NUMA node 0 Memory at 183fc001 (64-bit, non-prefetchable) [size=4K] Capabilities: [50] Power Management version 3 Capabilities: [80] MSI: Enable- Count=1/1 Maskable- 64bit- root@shining-node:~# I have tried ACS override patch to rearrange these devices to different iommu_group and its not working for these devices.I have tried to passthrough vfio method, vfio complains that it needs all driver to be vfio compatible. How can we achieve pci passthrough for Intel SMBus Controller [00:1f.3 here] with latest kernel(We use 4.8+ now) Regards, Gokul ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v18 1/5] iommu/arm-smmu: Add pm_runtime/sleep ops
On 11/27/18 4:11 AM, Vivek Gautam wrote: From: Sricharan R The smmu needs to be functional only when the respective master's using it are active. The device_link feature helps to track such functional dependencies, so that the iommu gets powered when the master device enables itself using pm_runtime. So by adapting the smmu driver for runtime pm, above said dependency can be addressed. This patch adds the pm runtime/sleep callbacks to the driver and also the functions to parse the smmu clocks from DT and enable them in resume/suspend. We pull all the information about clocks from device tree. Also, while we enable the runtime pm add a pm sleep suspend callback that pushes devices to low power state by turning the clocks off in a system sleep. Also add corresponding clock enable path in resume callback. Signed-off-by: Sricharan R Signed-off-by: Archit Taneja [Thor: Rework to get clocks from device tree] Signed-off-by: Thor Thayer [vivek: rework for clock and pm ops] Signed-off-by: Vivek Gautam Reviewed-by: Tomasz Figa Tested-by: Srinivas Kandagatla Reviewed-by: Robin Murphy --- drivers/iommu/arm-smmu.c | 100 +-- 1 file changed, 97 insertions(+), 3 deletions(-) diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c index 5a28ae892504..e47c840fc6a8 100644 --- a/drivers/iommu/arm-smmu.c +++ b/drivers/iommu/arm-smmu.c @@ -44,10 +44,12 @@ #include #include #include +#include #include #include #include #include +#include #include #include Thanks! Tested the device tree clock portions on Intel SOCFPGA Stratix10 DevKit. Tested-by: Thor Thayer ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 5/9] iommu: ipmmu-vmsa: make it explicitly non-modular
On 26/11/2018 22:31, Paul Gortmaker wrote:
The Kconfig currently controlling compilation of this code is:
drivers/iommu/Kconfig:config IPMMU_VMSA
drivers/iommu/Kconfig:bool "Renesas VMSA-compatible IPMMU"
...meaning that it currently is not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
Since module_init was not even used by this driver, the init ordering
remains unchanged with this commit.
We also delete the MODULE_LICENSE tag etc. since all that information
was (or is now) contained at the top of the file in the comments.
Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
Cc: Joerg Roedel
Cc: Laurent Pinchart
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Paul Gortmaker
---
drivers/iommu/ipmmu-vmsa.c | 16 +++-
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 9e2655f1c1bf..de39ef992d8a 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -1,6 +1,8 @@
// SPDX-License-Identifier: GPL-2.0
/*
* IPMMU VMSA
Nit: this line doesn't convey any information that the module
description below doesn't also say far more clearly, so you may as well
just replace it entirely.
Robin.
+ * IOMMU API for Renesas VMSA-compatible IPMMU
+ * Author: Laurent Pinchart
*
* Copyright (C) 2014 Renesas Electronics Corporation
*/
@@ -14,7 +16,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -968,7 +970,6 @@ static const struct of_device_id ipmmu_of_ids[] = {
},
};
-MODULE_DEVICE_TABLE(of, ipmmu_of_ids);
static int ipmmu_probe(struct platform_device *pdev)
{
@@ -1140,15 +1141,4 @@ static int __init ipmmu_init(void)
setup_done = true;
return 0;
}
-
-static void __exit ipmmu_exit(void)
-{
- return platform_driver_unregister(&ipmmu_driver);
-}
-
subsys_initcall(ipmmu_init);
-module_exit(ipmmu_exit);
-
-MODULE_DESCRIPTION("IOMMU API for Renesas VMSA-compatible IPMMU");
-MODULE_AUTHOR("Laurent Pinchart ");
-MODULE_LICENSE("GPL v2");
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 9/9] iommu: arm-smmu-v3: make it explicitly non-modular
On 26/11/2018 22:31, Paul Gortmaker wrote:
The Kconfig currently controlling compilation of this code is:
drivers/iommu/Kconfig:config ARM_SMMU_V3
drivers/iommu/Kconfig: bool "ARM Ltd. System MMU Version 3 (SMMUv3) Support"
...meaning that it currently is not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
Since module_platform_driver() uses the same init level priority as
builtin_platform_driver() the init ordering remains unchanged with
this commit.
We explicitly disallow a driver unbind, since that doesn't have a
sensible use case anyway, but unlike most drivers, we can't delete the
function tied to the ".remove" field. This is because as of commit
7aa8619a66ae ("iommu/arm-smmu-v3: Implement shutdown method") the
.remove function was given a one line wrapper and re-used to provide a
.shutdown service. So we delete the wrapper and re-name the function
from remove to shutdown.
We add a moduleparam.h include since the file does actually declare
some module parameters, and leaving them as such is the easiest way
currently to remain backwards compatible with existing use cases.
Also note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.
With the title fixed up,
Acked-by: Robin Murphy
Cc: Will Deacon
Cc: Joerg Roedel
Cc: Robin Murphy
Cc: Nate Watterson
Cc: linux-arm-ker...@lists.infradead.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Paul Gortmaker
---
drivers/iommu/arm-smmu-v3.c | 25 +
1 file changed, 9 insertions(+), 16 deletions(-)
diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
index 6947ccf26512..1189c06079d4 100644
--- a/drivers/iommu/arm-smmu-v3.c
+++ b/drivers/iommu/arm-smmu-v3.c
@@ -20,7 +20,8 @@
#include
#include
#include
-#include
+#include
+#include
#include
#include
#include
@@ -356,6 +357,10 @@
#define MSI_IOVA_BASE 0x800
#define MSI_IOVA_LENGTH 0x10
+/*
+ * not really modular, but the easiest way to keep compat with existing
+ * bootargs behaviour is to continue using module_param_named here.
+ */
static bool disable_bypass = 1;
module_param_named(disable_bypass, disable_bypass, bool, S_IRUGO);
MODULE_PARM_DESC(disable_bypass,
@@ -2928,37 +2933,25 @@ static int arm_smmu_device_probe(struct platform_device
*pdev)
return 0;
}
-static int arm_smmu_device_remove(struct platform_device *pdev)
+static void arm_smmu_device_shutdown(struct platform_device *pdev)
{
struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
arm_smmu_device_disable(smmu);
-
- return 0;
-}
-
-static void arm_smmu_device_shutdown(struct platform_device *pdev)
-{
- arm_smmu_device_remove(pdev);
}
static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "arm,smmu-v3", },
{ },
};
-MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
static struct platform_driver arm_smmu_driver = {
.driver = {
.name = "arm-smmu-v3",
.of_match_table = of_match_ptr(arm_smmu_of_match),
+ .suppress_bind_attrs = true,
},
.probe = arm_smmu_device_probe,
- .remove = arm_smmu_device_remove,
.shutdown = arm_smmu_device_shutdown,
};
-module_platform_driver(arm_smmu_driver);
-
-MODULE_DESCRIPTION("IOMMU API for ARM architected SMMUv3 implementations");
-MODULE_AUTHOR("Will Deacon ");
-MODULE_LICENSE("GPL v2");
+builtin_platform_driver(arm_smmu_driver);
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 8/9] iommu: arm-smmu: make it explicitly non-modular
Hi Paul,
On 26/11/2018 22:31, Paul Gortmaker wrote:
The Kconfig currently controlling compilation of this code is:
drivers/iommu/Kconfig:config ARM_SMMU
drivers/iommu/Kconfig: bool "ARM Ltd. System MMU (SMMU) Support"
...meaning that it currently is not being built as a module by anyone.
Lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
Since module_platform_driver() uses the same init level priority as
builtin_platform_driver() the init ordering remains unchanged with
this commit.
We explicitly disallow a driver unbind, since that doesn't have a
sensible use case anyway, but unlike most drivers, we can't delete the
function tied to the ".remove" field. This is because as of commit
7aa8619a66ae ("iommu/arm-smmu-v3: Implement shutdown method") the
.remove function was given a one line wrapper and re-used to provide a
.shutdown service. So we delete the wrapper and re-name the function
from remove to shutdown.
We add a moduleparam.h include since the file does actually declare
some module parameters, and leaving them as such is the easiest way
currently to remain backwards compatible with existing use cases.
We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.
Cc: Will Deacon
Cc: Joerg Roedel
Cc: Robin Murphy
Cc: Nate Watterson
Cc: linux-arm-ker...@lists.infradead.org
Cc: iommu@lists.linux-foundation.org
Signed-off-by: Paul Gortmaker
---
drivers/iommu/arm-smmu.c | 32 +---
1 file changed, 13 insertions(+), 19 deletions(-)
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 5a28ae892504..4a2e143fdf52 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -41,7 +41,8 @@
#include
#include
#include
-#include
+#include
+#include
#include
#include
#include
@@ -101,6 +102,10 @@
#define MSI_IOVA_LENGTH 0x10
static int force_stage;
+/*
+ * not really modular, but the easiest way to keep compat with existing
+ * bootargs behaviour is to continue using module_param() here.
+ */
Is it worth introducing builtin_param() and friends for this sort of
thing, to echo the *_platform_driver() helpers? It seems like that could
be justifiable under the motivation described in the cover letter.
Otherwise, the changes look reasonable. I still hold out hope that one
day we'll be able to make IOMMU drivers modular (it can work with
minimal hacks today, but it's far from robust in general), but for now I
agree this makes sense (and it'll be easy enough to revert for playing
with further hacks). With the title fixed up as Joerg asked,
Acked-by: Robin Murphy
module_param(force_stage, int, S_IRUGO);
MODULE_PARM_DESC(force_stage,
"Force SMMU mappings to be installed at a particular stage of translation. A
value of '1' or '2' forces the corresponding stage. All other values are ignored (i.e. no
stage is forced). Note that selecting a specific stage will disable support for nested
translation.");
@@ -1964,7 +1969,6 @@ static const struct of_device_id arm_smmu_of_match[] = {
{ .compatible = "cavium,smmu-v2", .data = &cavium_smmuv2 },
{ },
};
-MODULE_DEVICE_TABLE(of, arm_smmu_of_match);
#ifdef CONFIG_ACPI
static int acpi_smmu_get_data(u32 model, struct arm_smmu_device *smmu)
@@ -2224,24 +2228,18 @@ static int arm_smmu_legacy_bus_init(void)
}
device_initcall_sync(arm_smmu_legacy_bus_init);
-static int arm_smmu_device_remove(struct platform_device *pdev)
+static void arm_smmu_device_shutdown(struct platform_device *pdev)
{
struct arm_smmu_device *smmu = platform_get_drvdata(pdev);
if (!smmu)
- return -ENODEV;
+ return;
if (!bitmap_empty(smmu->context_map, ARM_SMMU_MAX_CBS))
dev_err(&pdev->dev, "removing device with active domains!\n");
/* Turn the thing off */
writel(sCR0_CLIENTPD, ARM_SMMU_GR0_NS(smmu) + ARM_SMMU_GR0_sCR0);
- return 0;
-}
-
-static void arm_smmu_device_shutdown(struct platform_device *pdev)
-{
- arm_smmu_device_remove(pdev);
}
static int __maybe_unused arm_smmu_pm_resume(struct device *dev)
@@ -2256,16 +2254,12 @@ static SIMPLE_DEV_PM_OPS(arm_smmu_pm_ops, NULL,
arm_smmu_pm_resume);
static struct platform_driver arm_smmu_driver = {
.driver = {
- .name = "arm-smmu",
- .of_match_table = of_match_ptr(arm_smmu_of_match),
- .pm = &arm_smmu_pm_ops,
+ .name = "arm-smmu",
+ .of_match_table = of_match_ptr(arm_smmu_of_match),
+ .pm = &arm_smmu_pm_ops,
+ .suppress_bind_attrs= true,
},
.probe = arm_smmu_device_probe,
- .remove = arm_smmu_device_remove,
.shutdown = arm_sm
Re: move the arm arch_dma_alloc implementation to common code
On 27/11/2018 07:37, Christoph Hellwig wrote: On Thu, Nov 15, 2018 at 12:58:04PM -0800, Robin Murphy wrote: On 2018-11-15 11:50 am, Will Deacon wrote: On Fri, Nov 09, 2018 at 08:52:38AM +0100, Christoph Hellwig wrote: Can I get a quick review from the arm64 folks? I think it should be fine there as it basically is a code move, but an additional pair or two of eyes always helps to weed out bugs. I reviewed the arm64 parts, but it would be ideal if Robin could have a look as well. Yup, from a quick skim the general shape of the whole series looks pleasing, but I've been holding off going through it in detail until I've figured out what's up with the last thing I thought I'd reviewed exhaustively... Either way I'll make some time for a proper look next week once I'm back. Did you get a chance to look over this? Sorry, it took a little longer than expected to catch up, but I'm looking at it now. Robin. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] PCI / ACPI: Identify untrusted PCI devices
On Wed, Nov 28, 2018 at 12:24:27PM +0100, Rafael J. Wysocki wrote: > I'm not sure if this is worth the extra complexity either, which is > why I have no strong opinion here. :-) > > Maybe you can add a comment, next to the prp_guids[] definition, to > explain that the GUIDs are made equivalent to each other in order to > avoid extra complexity in the properties handling code, with the > caveat that the kernel will accept certain combinations of GUIDs and > properties that are not defined strictly speaking without warning, but > those combinations of GUIDs and properties are not expected to be used > by firmware and they should be caught by firmware validation tools and > reported as errors anyway. Sure, I'll add the comment in the next version of the series. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] PCI / ACPI: Identify untrusted PCI devices
On Wed, Nov 28, 2018 at 11:54 AM Mika Westerberg
wrote:
>
> On Tue, Nov 27, 2018 at 06:14:43PM +0100, Rafael J. Wysocki wrote:
> > > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > > index 8c7c4583b52d..4bdad32f62c8 100644
> > > --- a/drivers/acpi/property.c
> > > +++ b/drivers/acpi/property.c
> > > @@ -31,6 +31,9 @@ static const guid_t prp_guids[] = {
> > > /* Hotplug in D3 GUID: 6211e2c0-58a3-4af3-90e1-927a4e0c55a4 */
> > > GUID_INIT(0x6211e2c0, 0x58a3, 0x4af3,
> > > 0x90, 0xe1, 0x92, 0x7a, 0x4e, 0x0c, 0x55, 0xa4),
> > > + /* External facing port GUID:
> > > efcc06cc-73ac-4bc3-bff0-76143807c389 */
> > > + GUID_INIT(0xefcc06cc, 0x73ac, 0x4bc3,
> > > + 0xbf, 0xf0, 0x76, 0x14, 0x38, 0x07, 0xc3, 0x89),
> > > };
> >
> > One observation here. Note that I really have no strong opinion on
> > that, so this is not an objection, but IMO it is fair to make things
> > clear for everybody.
> >
> > So this causes the External facing port GUID (which already is the
> > case with the Hotplug in D3 GUID for that matter) to be practically
> > equivalent to the ACPI _DSD device properties GUID. This means that
> > Linux will consider a combination of any of these GUIDs with any
> > properties defined for any of them as valid, which need not be the
> > case in Windows.
> >
> > For example, since the ExternalFacingPort property is defined along
> > with the External facing port GUID, Windows will likely ignore it if
> > it is used in a combination with the Hotplug in D3 GUID or the ACPI
> > _DSD device properties GUID. If the firmware combines the Hotplug in
> > D3 GUID or the ACPI _DSD device properties GUID with that property,
> > Windows will not regard it as valid, most likely, but Linux will use
> > it just fine. In the face of ASL bugs, which are inevitable (at least
> > just because ASL is code written by humans), that may become a real
> > problem, as systems successfully tested with Windows may not work with
> > Linux as expected and vice versa because of it.
>
> That's a fair point.
>
> > >From the ecosystem purity perspective this should be avoided, but then
> > I do realize that this allows code to be re-used and avoids quite a
> > bit of complexity. The likelihood of an ASL bug that will expose this
> > issue is arguably small, so maybe it is not a practical concern after
> > all.
>
> One option assuming we want to prevent the potential discrepancy you
> described above is to add ACPI specific property accessors that take
> GUID as parameter. Then it would only look properties inside that
> particular _DSD entry. I'm not sure if it is worth the added complexity,
> though.
I'm not sure if this is worth the extra complexity either, which is
why I have no strong opinion here. :-)
Maybe you can add a comment, next to the prp_guids[] definition, to
explain that the GUIDs are made equivalent to each other in order to
avoid extra complexity in the properties handling code, with the
caveat that the kernel will accept certain combinations of GUIDs and
properties that are not defined strictly speaking without warning, but
those combinations of GUIDs and properties are not expected to be used
by firmware and they should be caught by firmware validation tools and
reported as errors anyway.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: use generic DMA mapping code in powerpc V4
Christoph Hellwig writes: > Any comments? I'd like to at least get the ball moving on the easy > bits. Nothing specific yet. I'm a bit worried it might break one of the many old obscure platforms we have that aren't well tested. There's not much we can do about that, but I'll just try and test it on everything I can find. Is the plan that you take these via the dma-mapping tree or that they go via powerpc? cheers > On Wed, Nov 14, 2018 at 09:22:40AM +0100, Christoph Hellwig wrote: >> Hi all, >> >> this series switches the powerpc port to use the generic swiotlb and >> noncoherent dma ops, and to use more generic code for the coherent >> direct mapping, as well as removing a lot of dead code. >> >> As this series is very large and depends on the dma-mapping tree I've >> also published a git tree: >> >> git://git.infradead.org/users/hch/misc.git powerpc-dma.4 >> >> Gitweb: >> >> >> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/powerpc-dma.4 >> >> Changes since v3: >> - rebase on the powerpc fixes tree >> - add a new patch to actually make the baseline amigaone config >>configure without warnings >> - only use ZONE_DMA for 64-bit embedded CPUs, on pseries an IOMMU is >>always present >> - fix compile in mem.c for one configuration >> - drop the full npu removal for now, will be resent separately >> - a few git bisection fixes >> >> The changes since v1 are to big to list and v2 was not posted in public. >> >> ___ >> iommu mailing list >> iommu@lists.linux-foundation.org >> https://lists.linuxfoundation.org/mailman/listinfo/iommu > ---end quoted text--- ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] PCI / ACPI: Identify untrusted PCI devices
On Tue, Nov 27, 2018 at 06:14:43PM +0100, Rafael J. Wysocki wrote:
> > diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> > index 8c7c4583b52d..4bdad32f62c8 100644
> > --- a/drivers/acpi/property.c
> > +++ b/drivers/acpi/property.c
> > @@ -31,6 +31,9 @@ static const guid_t prp_guids[] = {
> > /* Hotplug in D3 GUID: 6211e2c0-58a3-4af3-90e1-927a4e0c55a4 */
> > GUID_INIT(0x6211e2c0, 0x58a3, 0x4af3,
> > 0x90, 0xe1, 0x92, 0x7a, 0x4e, 0x0c, 0x55, 0xa4),
> > + /* External facing port GUID: efcc06cc-73ac-4bc3-bff0-76143807c389
> > */
> > + GUID_INIT(0xefcc06cc, 0x73ac, 0x4bc3,
> > + 0xbf, 0xf0, 0x76, 0x14, 0x38, 0x07, 0xc3, 0x89),
> > };
>
> One observation here. Note that I really have no strong opinion on
> that, so this is not an objection, but IMO it is fair to make things
> clear for everybody.
>
> So this causes the External facing port GUID (which already is the
> case with the Hotplug in D3 GUID for that matter) to be practically
> equivalent to the ACPI _DSD device properties GUID. This means that
> Linux will consider a combination of any of these GUIDs with any
> properties defined for any of them as valid, which need not be the
> case in Windows.
>
> For example, since the ExternalFacingPort property is defined along
> with the External facing port GUID, Windows will likely ignore it if
> it is used in a combination with the Hotplug in D3 GUID or the ACPI
> _DSD device properties GUID. If the firmware combines the Hotplug in
> D3 GUID or the ACPI _DSD device properties GUID with that property,
> Windows will not regard it as valid, most likely, but Linux will use
> it just fine. In the face of ASL bugs, which are inevitable (at least
> just because ASL is code written by humans), that may become a real
> problem, as systems successfully tested with Windows may not work with
> Linux as expected and vice versa because of it.
That's a fair point.
> >From the ecosystem purity perspective this should be avoided, but then
> I do realize that this allows code to be re-used and avoids quite a
> bit of complexity. The likelihood of an ASL bug that will expose this
> issue is arguably small, so maybe it is not a practical concern after
> all.
One option assuming we want to prevent the potential discrepancy you
described above is to add ACPI specific property accessors that take
GUID as parameter. Then it would only look properties inside that
particular _DSD entry. I'm not sure if it is worth the added complexity,
though.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 2/2] iommu/ipmmu-vmsa: add an array of slave devices whitelist
To avoid adding copy and pasted strcmp codes in the future,
this patch adds an array "rcar_gen3_slave_whitelist" to check
whether the device can work with the IPMMU or not.
Signed-off-by: Yoshihiro Shimoda
Reviewed-by: Geert Uytterhoeven
---
drivers/iommu/ipmmu-vmsa.c | 13 -
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 5f88031..60e3314 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -773,8 +773,13 @@ static int ipmmu_init_platform_device(struct device *dev,
{ /* sentinel */ }
};
+static const char * const rcar_gen3_slave_whitelist[] = {
+};
+
static bool ipmmu_slave_whitelist(struct device *dev)
{
+ unsigned int i;
+
/*
* For R-Car Gen3 use a white list to opt-in slave devices.
* For Other SoCs, this returns true anyway.
@@ -786,7 +791,13 @@ static bool ipmmu_slave_whitelist(struct device *dev)
if (!soc_device_match(soc_rcar_gen3_whitelist))
return false;
- /* By default, do not allow use of IPMMU */
+ /* Check whether this slave device can work with the IPMMU */
+ for (i = 0; i < ARRAY_SIZE(rcar_gen3_slave_whitelist); i++) {
+ if (!strcmp(dev_name(dev), rcar_gen3_slave_whitelist[i]))
+ return true;
+ }
+
+ /* Otherwise, do not allow use of IPMMU */
return false;
}
--
1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH v2 1/2] iommu/ipmmu-vmsa: Modify ipmmu_slave_whitelist() to check SoC revisions
Some R-Car Gen3 SoCs has hardware restrictions on the IPMMU. So,
to check whether this R-Car Gen3 SoC can use the IPMMU correctly,
this patch modifies the ipmmu_slave_whitelist().
Signed-off-by: Yoshihiro Shimoda
Reviewed-by: Geert Uytterhoeven
---
drivers/iommu/ipmmu-vmsa.c | 34 ++
1 file changed, 26 insertions(+), 8 deletions(-)
diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
index 9e2655f..5f88031 100644
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -754,12 +754,6 @@ static int ipmmu_init_platform_device(struct device *dev,
return 0;
}
-static bool ipmmu_slave_whitelist(struct device *dev)
-{
- /* By default, do not allow use of IPMMU */
- return false;
-}
-
static const struct soc_device_attribute soc_rcar_gen3[] = {
{ .soc_id = "r8a774a1", },
{ .soc_id = "r8a7795", },
@@ -771,11 +765,35 @@ static bool ipmmu_slave_whitelist(struct device *dev)
{ /* sentinel */ }
};
+static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
+ { .soc_id = "r8a7795", .revision = "ES3.*" },
+ { .soc_id = "r8a77965", },
+ { .soc_id = "r8a77990", },
+ { .soc_id = "r8a77995", },
+ { /* sentinel */ }
+};
+
+static bool ipmmu_slave_whitelist(struct device *dev)
+{
+ /*
+* For R-Car Gen3 use a white list to opt-in slave devices.
+* For Other SoCs, this returns true anyway.
+*/
+ if (!soc_device_match(soc_rcar_gen3))
+ return true;
+
+ /* Check whether this R-Car Gen3 can use the IPMMU correctly or not */
+ if (!soc_device_match(soc_rcar_gen3_whitelist))
+ return false;
+
+ /* By default, do not allow use of IPMMU */
+ return false;
+}
+
static int ipmmu_of_xlate(struct device *dev,
struct of_phandle_args *spec)
{
- /* For R-Car Gen3 use a white list to opt-in slave devices */
- if (soc_device_match(soc_rcar_gen3) && !ipmmu_slave_whitelist(dev))
+ if (!ipmmu_slave_whitelist(dev))
return -ENODEV;
iommu_fwspec_add_ids(dev, spec->args, 1);
--
1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 0/2] iommu/ipmmu-vmsa: Modify ipmmu_slave_whitelist()
This patch set is based on iommu.git / latest next branch
(commit id = f262283c224537962cba0f41b8823e3be9f7b0ff)
I talked with Geert-san about this topic on below:
https://patchwork.kernel.org/patch/10651375/
Also Simon-san suggests we should keep the whitelist.
So, not to change behavior of R-Car Gen2, this patch set adds
two conditions. After applied this patch set, we can add slave
devices easily like below:
--- a/drivers/iommu/ipmmu-vmsa.c
+++ b/drivers/iommu/ipmmu-vmsa.c
@@ -774,6 +774,8 @@ static int ipmmu_init_platform_device(struct device *dev,
};
static const char * const rcar_gen3_slave_whitelist[] = {
+ "e670.dma-controller",
+ "e730.dma-controller"
};
static bool ipmmu_slave_whitelist(struct device *dev)
Changes from v1:
- Use "ES3.*" instead of "ES3.0" for r8a7795 in patch 1.
- Use "unsigned int" instead of "int" in patch 2.
- Add Geert-san's Reviewed-by.
Yoshihiro Shimoda (2):
iommu/ipmmu-vmsa: Modify ipmmu_slave_whitelist() to check SoC
revisions
iommu/ipmmu-vmsa: add an array of slave devices whitelist
drivers/iommu/ipmmu-vmsa.c | 45 +
1 file changed, 37 insertions(+), 8 deletions(-)
--
1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 2/2] iommu/ipmmu-vmsa: add an array of slave devices whitelist
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Wednesday, November 28, 2018 5:48 PM
>
> Hi Shimoda-san,
>
> On Wed, Nov 28, 2018 at 7:10 AM Yoshihiro Shimoda
> wrote:
> > To avoid adding copy and pasted strcmp codes in the future,
> > this patch adds an array "rcar_gen3_slave_whitelist" to check
> > whether the device can work with the IPMMU or not.
> >
> > Signed-off-by: Yoshihiro Shimoda
>
> Reviewed-by: Geert Uytterhoeven
Thank you for your review!
> One small comment below.
>
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -773,8 +773,13 @@ static int ipmmu_init_platform_device(struct device
> > *dev,
> > { /* sentinel */ }
> > };
> >
> > +static const char * const rcar_gen3_slave_whitelist[] = {
> > +};
> > +
> > static bool ipmmu_slave_whitelist(struct device *dev)
> > {
> > + int i;
>
> unsigned int
I got it. I'll submit v2 patch.
Best regards,
Yoshihiro Shimoda
> > +
> > /*
> > * For R-Car Gen3 use a white list to opt-in slave devices.
> > * For Other SoCs, this returns true anyway.
> > @@ -786,7 +791,13 @@ static bool ipmmu_slave_whitelist(struct device *dev)
> > if (!soc_device_match(soc_rcar_gen3_whitelist))
> > return false;
> >
> > - /* By default, do not allow use of IPMMU */
> > + /* Check whether this slave device can work with the IPMMU */
> > + for (i = 0; i < ARRAY_SIZE(rcar_gen3_slave_whitelist); i++) {
> > + if (!strcmp(dev_name(dev), rcar_gen3_slave_whitelist[i]))
> > + return true;
> > + }
> > +
> > + /* Otherwise, do not allow use of IPMMU */
> > return false;
> > }
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
RE: [PATCH 1/2] iommu/ipmmu-vmsa: Modify ipmmu_slave_whitelist() to check SoC revisions
Hi Geert-san,
> From: Geert Uytterhoeven, Sent: Wednesday, November 28, 2018 5:47 PM
>
> Hi Shimoda-san,
>
> On Wed, Nov 28, 2018 at 7:10 AM Yoshihiro Shimoda
> wrote:
> > Some R-Car Gen3 SoCs has hardware restrictions on the IPMMU. So,
> > to check whether this R-Car Gen3 SoC can use the IPMMU correctly,
> > this patch modifies the ipmmu_slave_whitelist().
> >
> > Signed-off-by: Yoshihiro Shimoda
>
> Reviewed-by: Geert Uytterhoeven
Thank you for the review!
> One question below.
>
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
>
> > @@ -771,11 +765,35 @@ static bool ipmmu_slave_whitelist(struct device *dev)
> > { /* sentinel */ }
> > };
> >
> > +static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
> > + { .soc_id = "r8a7795", .revision = "ES3.0" },
>
> Don't you want "ES3.*"?
Indeed (I want "ES3.*"). So, I'll submit v2 patch to fix it.
Best regards,
Yoshihiro Shimoda
> > + { .soc_id = "r8a77965", },
> > + { .soc_id = "r8a77990", },
> > + { .soc_id = "r8a77995", },
> > + { /* sentinel */ }
> > +};
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> ge...@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like
> that.
> -- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 0/3] iommu/io-pgtable-arm-v7s: Use DMA32 zone for page tables
On Mon, Nov 26, 2018 at 4:02 PM Christoph Hellwig wrote: > > On Fri, Nov 23, 2018 at 01:23:41PM +0100, Vlastimil Babka wrote: > > Is this also true for caches created by kmem_cache_create(), that > > debugging options can result in not respecting the alignment passed to > > kmem_cache_create()? That would be rather bad, IMHO. > > That's what I understood in the discussion. If not it would make > our live simpler, but would need to be well document. >From my experiment, adding `slub_debug` to command line does _not_ break the alignment of kmem_cache_alloc'ed objects. We do see an increase in slab_size (/sys/kernel/slab/io-pgtable_armv7s_l2/slab_size), from 1024 to 3072 (probably because slub needs to allocate space on each side for the red zone/padding, while keeping the alignment?) > Christoph can probably explain the alignment choices in slub. > > > > > > But I do agree with the sentiment of not wanting to spread GFP_DMA32 > > > futher into the slab allocator. > > > > I don't see a problem with GFP_DMA32 for custom caches. Generic > > kmalloc() would be worse, since it would have to create a new array of > > kmalloc caches. But that's already ruled out due to the alignment. > > True, purely slab probably isn't too bad. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 2/2] iommu/ipmmu-vmsa: add an array of slave devices whitelist
Hi Shimoda-san,
On Wed, Nov 28, 2018 at 7:10 AM Yoshihiro Shimoda
wrote:
> To avoid adding copy and pasted strcmp codes in the future,
> this patch adds an array "rcar_gen3_slave_whitelist" to check
> whether the device can work with the IPMMU or not.
>
> Signed-off-by: Yoshihiro Shimoda
Reviewed-by: Geert Uytterhoeven
One small comment below.
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -773,8 +773,13 @@ static int ipmmu_init_platform_device(struct device *dev,
> { /* sentinel */ }
> };
>
> +static const char * const rcar_gen3_slave_whitelist[] = {
> +};
> +
> static bool ipmmu_slave_whitelist(struct device *dev)
> {
> + int i;
unsigned int
> +
> /*
> * For R-Car Gen3 use a white list to opt-in slave devices.
> * For Other SoCs, this returns true anyway.
> @@ -786,7 +791,13 @@ static bool ipmmu_slave_whitelist(struct device *dev)
> if (!soc_device_match(soc_rcar_gen3_whitelist))
> return false;
>
> - /* By default, do not allow use of IPMMU */
> + /* Check whether this slave device can work with the IPMMU */
> + for (i = 0; i < ARRAY_SIZE(rcar_gen3_slave_whitelist); i++) {
> + if (!strcmp(dev_name(dev), rcar_gen3_slave_whitelist[i]))
> + return true;
> + }
> +
> + /* Otherwise, do not allow use of IPMMU */
> return false;
> }
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/2] iommu/ipmmu-vmsa: Modify ipmmu_slave_whitelist() to check SoC revisions
Hi Shimoda-san,
On Wed, Nov 28, 2018 at 7:10 AM Yoshihiro Shimoda
wrote:
> Some R-Car Gen3 SoCs has hardware restrictions on the IPMMU. So,
> to check whether this R-Car Gen3 SoC can use the IPMMU correctly,
> this patch modifies the ipmmu_slave_whitelist().
>
> Signed-off-by: Yoshihiro Shimoda
Reviewed-by: Geert Uytterhoeven
One question below.
> --- a/drivers/iommu/ipmmu-vmsa.c
> +++ b/drivers/iommu/ipmmu-vmsa.c
> @@ -771,11 +765,35 @@ static bool ipmmu_slave_whitelist(struct device *dev)
> { /* sentinel */ }
> };
>
> +static const struct soc_device_attribute soc_rcar_gen3_whitelist[] = {
> + { .soc_id = "r8a7795", .revision = "ES3.0" },
Don't you want "ES3.*"?
> + { .soc_id = "r8a77965", },
> + { .soc_id = "r8a77990", },
> + { .soc_id = "r8a77995", },
> + { /* sentinel */ }
> +};
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/3] iommu/amd: Use pr_fmt()
From: Joerg Roedel
Make use of pr_fmt instead of having the 'AMD-Vi' prefix
added manually at every printk() call.
Signed-off-by: Joerg Roedel
---
drivers/iommu/amd_iommu.c | 38 ++--
drivers/iommu/amd_iommu_init.c | 64 ++
drivers/iommu/amd_iommu_v2.c | 2 ++
3 files changed, 55 insertions(+), 49 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 1167ff0416cf..9b96b424add4 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -17,6 +17,8 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#define pr_fmt(fmt) "AMD-Vi: " fmt
+
#include
#include
#include
@@ -279,7 +281,7 @@ static u16 get_alias(struct device *dev)
return pci_alias;
}
- pr_info("AMD-Vi: Using IVRS reported alias %02x:%02x.%d "
+ pr_info("Using IVRS reported alias %02x:%02x.%d "
"for device %s[%04x:%04x], kernel reported alias "
"%02x:%02x.%d\n", PCI_BUS_NUM(ivrs_alias), PCI_SLOT(ivrs_alias),
PCI_FUNC(ivrs_alias), dev_name(dev), pdev->vendor, pdev->device,
@@ -293,7 +295,7 @@ static u16 get_alias(struct device *dev)
if (pci_alias == devid &&
PCI_BUS_NUM(ivrs_alias) == pdev->bus->number) {
pci_add_dma_alias(pdev, ivrs_alias & 0xff);
- pr_info("AMD-Vi: Added PCI DMA alias %02x.%d for %s\n",
+ pr_info("Added PCI DMA alias %02x.%d for %s\n",
PCI_SLOT(ivrs_alias), PCI_FUNC(ivrs_alias),
dev_name(dev));
}
@@ -513,7 +515,7 @@ static void dump_dte_entry(u16 devid)
int i;
for (i = 0; i < 4; ++i)
- pr_err("AMD-Vi: DTE[%d]: %016llx\n", i,
+ pr_err("DTE[%d]: %016llx\n", i,
amd_iommu_dev_table[devid].data[i]);
}
@@ -523,7 +525,7 @@ static void dump_command(unsigned long phys_addr)
int i;
for (i = 0; i < 4; ++i)
- pr_err("AMD-Vi: CMD[%d]: %08x\n", i, cmd->data[i]);
+ pr_err("CMD[%d]: %08x\n", i, cmd->data[i]);
}
static void amd_iommu_report_page_fault(u16 devid, u16 domain_id,
@@ -541,7 +543,7 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
dev_err(&pdev->dev, "AMD-Vi: Event logged [IO_PAGE_FAULT
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
domain_id, address, flags);
} else if (printk_ratelimit()) {
- pr_err("AMD-Vi: Event logged [IO_PAGE_FAULT device=%02x:%02x.%x
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+ pr_err("Event logged [IO_PAGE_FAULT device=%02x:%02x.%x
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
domain_id, address, flags);
}
@@ -568,7 +570,7 @@ static void iommu_print_event(struct amd_iommu *iommu, void
*__evt)
if (type == 0) {
/* Did we hit the erratum? */
if (++count == LOOP_TIMEOUT) {
- pr_err("AMD-Vi: No event written to event log\n");
+ pr_err("No event written to event log\n");
return;
}
udelay(1);
@@ -654,7 +656,7 @@ static void iommu_handle_ppr_entry(struct amd_iommu *iommu,
u64 *raw)
struct amd_iommu_fault fault;
if (PPR_REQ_TYPE(raw[0]) != PPR_REQ_FAULT) {
- pr_err_ratelimited("AMD-Vi: Unknown PPR request received\n");
+ pr_err_ratelimited("Unknown PPR request received\n");
return;
}
@@ -759,12 +761,12 @@ static void iommu_poll_ga_log(struct amd_iommu *iommu)
if (!iommu_ga_log_notifier)
break;
- pr_debug("AMD-Vi: %s: devid=%#x, ga_tag=%#x\n",
+ pr_debug("%s: devid=%#x, ga_tag=%#x\n",
__func__, GA_DEVID(log_entry),
GA_TAG(log_entry));
if (iommu_ga_log_notifier(GA_TAG(log_entry)) != 0)
- pr_err("AMD-Vi: GA log notifier failed.\n");
+ pr_err("GA log notifier failed.\n");
break;
default:
break;
@@ -789,18 +791,18 @@ irqreturn_t amd_iommu_int_thread(int irq, void *data)
iommu->mmio_base + MMIO_STATUS_OFFSET);
if (status & MMIO_STATUS_EVT_INT_MASK) {
- pr_devel("AMD-Vi: Processing IOMMU Event Log\n");
+ pr_devel("Processing IOMMU Event Log\n");
iommu_poll_events(iommu);
}
if (status & MMIO_STATUS_PPR_INT_MASK) {
- pr_devel("AMD-Vi: Processing IOMMU PP
[PATCH 3/3] iommu/amd: Remove leading 0s in error log messages
From: Joerg Roedel
Remove the leading 0s in the address field of the error log
messages.
Signed-off-by: Joerg Roedel
---
drivers/iommu/amd_iommu.c | 20 ++--
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 7f38eab5aef4..77c457e897cc 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -540,10 +540,10 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
dev_data = get_dev_data(&pdev->dev);
if (dev_data && __ratelimit(&dev_data->rs)) {
- dev_err(&pdev->dev, "Event logged [IO_PAGE_FAULT domain=0x%04x
address=0x%016llx flags=0x%04x]\n",
+ dev_err(&pdev->dev, "Event logged [IO_PAGE_FAULT domain=0x%04x
address=0x%llx flags=0x%04x]\n",
domain_id, address, flags);
} else if (printk_ratelimit()) {
- pr_err("Event logged [IO_PAGE_FAULT device=%02x:%02x.%x
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+ pr_err("Event logged [IO_PAGE_FAULT device=%02x:%02x.%x
domain=0x%04x address=0x%llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
domain_id, address, flags);
}
@@ -584,37 +584,37 @@ static void iommu_print_event(struct amd_iommu *iommu,
void *__evt)
switch (type) {
case EVENT_TYPE_ILL_DEV:
- dev_err(dev, "Event logged [ILLEGAL_DEV_TABLE_ENTRY
device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [ILLEGAL_DEV_TABLE_ENTRY
device=%02x:%02x.%x pasid=0x%05x address=0x%llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
dump_dte_entry(devid);
break;
case EVENT_TYPE_DEV_TAB_ERR:
dev_err(dev, "Event logged [DEV_TAB_HARDWARE_ERROR
device=%02x:%02x.%x "
- "address=0x%016llx flags=0x%04x]\n",
+ "address=0x%llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
address, flags);
break;
case EVENT_TYPE_PAGE_TAB_ERR:
- dev_err(dev, "Event logged [PAGE_TAB_HARDWARE_ERROR
device=%02x:%02x.%x domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [PAGE_TAB_HARDWARE_ERROR
device=%02x:%02x.%x domain=0x%04x address=0x%llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
break;
case EVENT_TYPE_ILL_CMD:
- dev_err(dev, "Event logged [ILLEGAL_COMMAND_ERROR
address=0x%016llx]\n", address);
+ dev_err(dev, "Event logged [ILLEGAL_COMMAND_ERROR
address=0x%llx]\n", address);
dump_command(address);
break;
case EVENT_TYPE_CMD_HARD_ERR:
- dev_err(dev, "Event logged [COMMAND_HARDWARE_ERROR
address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [COMMAND_HARDWARE_ERROR
address=0x%llx flags=0x%04x]\n",
address, flags);
break;
case EVENT_TYPE_IOTLB_INV_TO:
- dev_err(dev, "Event logged [IOTLB_INV_TIMEOUT
device=%02x:%02x.%x address=0x%016llx]\n",
+ dev_err(dev, "Event logged [IOTLB_INV_TIMEOUT
device=%02x:%02x.%x address=0x%llx]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
address);
break;
case EVENT_TYPE_INV_DEV_REQ:
- dev_err(dev, "Event logged [INVALID_DEVICE_REQUEST
device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [INVALID_DEVICE_REQUEST
device=%02x:%02x.%x pasid=0x%05x address=0x%llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
break;
@@ -622,7 +622,7 @@ static void iommu_print_event(struct amd_iommu *iommu, void
*__evt)
pasid = ((event[0] >> 16) & 0x)
| ((event[1] << 6) & 0xF);
tag = event[1] & 0x03FF;
- dev_err(dev, "Event logged [INVALID_PPR_REQUEST
device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [INVALID_PPR_REQUEST
device=%02x:%02x.%x pasid=0x%05x address=0x%llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
break;
--
2.17.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org
[PATCH 0/3] Driver message cleanups
Hi, here are a couple of patches to clean up the message printing in the AMD IOMMU driver a bit. The patches switch the driver to use pr_fmt() instead of inconsistently open coding the 'AMD-Vi' prefix in error messages. Furhter the Event log messages are fixed and contain no newline anymore. The addresses in the event log messages got rid of the leading 0s to make the messages shorter. Regards, Joerg Joerg Roedel (3): iommu/amd: Use pr_fmt() iommu/amd: Fix line-break in error log reporting iommu/amd: Remove leading 0s in error log messages drivers/iommu/amd_iommu.c | 62 drivers/iommu/amd_iommu_init.c | 64 ++ drivers/iommu/amd_iommu_v2.c | 2 ++ 3 files changed, 66 insertions(+), 62 deletions(-) -- 2.17.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 2/3] iommu/amd: Fix line-break in error log reporting
From: Joerg Roedel
With the switch to dev_err for reporting errors from the
iommu log there was an unwanted newline introduced. The
reason was that the reporting was done in multiple dev_err()
calls, and dev_err adds a newline after every call.
Fix it by printing the log messages with only one dev_err()
call.
Signed-off-by: Joerg Roedel
---
drivers/iommu/amd_iommu.c | 22 ++
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 9b96b424add4..7f38eab5aef4 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -540,7 +540,7 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
dev_data = get_dev_data(&pdev->dev);
if (dev_data && __ratelimit(&dev_data->rs)) {
- dev_err(&pdev->dev, "AMD-Vi: Event logged [IO_PAGE_FAULT
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+ dev_err(&pdev->dev, "Event logged [IO_PAGE_FAULT domain=0x%04x
address=0x%016llx flags=0x%04x]\n",
domain_id, address, flags);
} else if (printk_ratelimit()) {
pr_err("Event logged [IO_PAGE_FAULT device=%02x:%02x.%x
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
@@ -580,43 +580,41 @@ static void iommu_print_event(struct amd_iommu *iommu,
void *__evt)
if (type == EVENT_TYPE_IO_FAULT) {
amd_iommu_report_page_fault(devid, pasid, address, flags);
return;
- } else {
- dev_err(dev, "AMD-Vi: Event logged [");
}
switch (type) {
case EVENT_TYPE_ILL_DEV:
- dev_err(dev, "ILLEGAL_DEV_TABLE_ENTRY device=%02x:%02x.%x
pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [ILLEGAL_DEV_TABLE_ENTRY
device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
dump_dte_entry(devid);
break;
case EVENT_TYPE_DEV_TAB_ERR:
- dev_err(dev, "DEV_TAB_HARDWARE_ERROR device=%02x:%02x.%x "
+ dev_err(dev, "Event logged [DEV_TAB_HARDWARE_ERROR
device=%02x:%02x.%x "
"address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
address, flags);
break;
case EVENT_TYPE_PAGE_TAB_ERR:
- dev_err(dev, "PAGE_TAB_HARDWARE_ERROR device=%02x:%02x.%x
domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [PAGE_TAB_HARDWARE_ERROR
device=%02x:%02x.%x domain=0x%04x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
break;
case EVENT_TYPE_ILL_CMD:
- dev_err(dev, "ILLEGAL_COMMAND_ERROR address=0x%016llx]\n",
address);
+ dev_err(dev, "Event logged [ILLEGAL_COMMAND_ERROR
address=0x%016llx]\n", address);
dump_command(address);
break;
case EVENT_TYPE_CMD_HARD_ERR:
- dev_err(dev, "COMMAND_HARDWARE_ERROR address=0x%016llx
flags=0x%04x]\n",
+ dev_err(dev, "Event logged [COMMAND_HARDWARE_ERROR
address=0x%016llx flags=0x%04x]\n",
address, flags);
break;
case EVENT_TYPE_IOTLB_INV_TO:
- dev_err(dev, "IOTLB_INV_TIMEOUT device=%02x:%02x.%x
address=0x%016llx]\n",
+ dev_err(dev, "Event logged [IOTLB_INV_TIMEOUT
device=%02x:%02x.%x address=0x%016llx]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
address);
break;
case EVENT_TYPE_INV_DEV_REQ:
- dev_err(dev, "INVALID_DEVICE_REQUEST device=%02x:%02x.%x
pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [INVALID_DEVICE_REQUEST
device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
break;
@@ -624,12 +622,12 @@ static void iommu_print_event(struct amd_iommu *iommu,
void *__evt)
pasid = ((event[0] >> 16) & 0x)
| ((event[1] << 6) & 0xF);
tag = event[1] & 0x03FF;
- dev_err(dev, "INVALID_PPR_REQUEST device=%02x:%02x.%x
pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "Event logged [INVALID_PPR_REQUEST
device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
break;
