Re: [PATCH 03/23] ARM: dt: tegra30: iommu: Add nvidia,memory-clients
On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote: Add nvidia,memory-clients to identify which swgroup ID a device belongs to. Why not call the property nvidia,swgid instead? That seems a lot more intuitive. Thierry pgpsrF4R4_pOe.pgp Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 01/23] ARM: tegra: Create a DT header defining swgroups ID
On Wed, Jun 26, 2013 at 12:28:04PM +0300, Hiroshi Doyu wrote: [...] +#define SWGID_AFI 0 +#define SWGID_AVPC 1 +#define SWGID_DC 2 +#define SWGID_DCB 3 +#define SWGID_EPP 4 +#define SWGID_G2 5 +#define SWGID_HC 6 +#define SWGID_HDA 7 +#define SWGID_ISP 8 +#define SWGID_ISP2 SWGID_ISP +/* UNUSED: 9 */ +/* UNUSED: 10 */ +#define SWGID_MPE 11 +#define SWGID_MSENC SWGID_MPE +#define SWGID_NV 12 +#define SWGID_NV2 13 +#define SWGID_PPCS 14 +#define SWGID_SATA2 15 +#define SWGID_SATA 16 +#define SWGID_VDE 17 +#define SWGID_VI 18 +#define SWGID_VIC 19 +#define SWGID_XUSB_HOST 20 +#define SWGID_XUSB_DEV 21 +#define SWGID_A9AVP 22 +#define SWGID_TSEC 23 +#define SWGID_PPCS1 24 +/* UNUSED: 25 */ +/* UNUSED: 26 */ +/* UNUSED: 27 */ +/* UNUSED: 28 */ +/* UNUSED: 29 */ +/* UNUSED: 30 */ +/* UNUSED: 31 */ + +/* Reserved: 32-63 */ + +#define SWGID(x) (1ULL SWGID_##x) I'm not entirely sure where to find these mappings in the TRM. I see that there's a list of the groups in 15.10.11, but where do the numbers come from? And why are some of the names aliased? If it's for readability only maybe we could add some more for SWGID_HC - SWGID_HOST1X and perhaps SWGID_NV - SWGID_GR3D. Thierry pgpA6aXEqUo3Z.pgp Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2] PCI: Reset PCIe devices to stop ongoing DMA
On Fri, Jun 14, 2013 at 10:11 AM, Takao Indoh indou.ta...@jp.fujitsu.comwrote: (2013/06/13 12:41), Bjorn Helgaas wrote: On Wed, Jun 12, 2013 at 8:44 PM, Takao Indoh indou.ta...@jp.fujitsu.com wrote: (2013/06/12 13:45), Bjorn Helgaas wrote: [+cc Vivek, Haren; sorry I didn't think to add you earlier] On Tue, Jun 11, 2013 at 12:08 AM, Takao Indoh indou.ta...@jp.fujitsu.com wrote: (2013/06/11 11:20), Bjorn Helgaas wrote: I'm not sure you need to reset legacy devices (or non-PCI devices) yet, but the current hook isn't anchored anywhere -- it's just an fs_initcall() that doesn't give the reader any clue about the connection between the reset and the problem it's solving. If we do something like this patch, I think it needs to be done at the point where we enable or disable the IOMMU. That way, it's connected to the important event, and there's a clue about how to make corresponding fixes for other IOMMUs. Ok. pci_iommu_init() is appropriate place to add this hook? I looked at various IOMMU init places today, and it's far more complicated and varied than I had hoped. This reset scheme depends on enumerating PCI devices before we initialize the IOMMU used by those devices. x86 works that way today, but not all architectures do (see the sparc pci_fire_pbm_init(), for Sorry, could you tell me which part depends on architecture? Your patch works if PCIe devices are reset before the kdump kernel enables the IOMMU. On x86, this is possible because PCI enumeration happens before the IOMMU initialization. On sparc, the IOMMU is initialized before PCI devices are enumerated, so there would still be a window where ongoing DMA could cause an IOMMU error. Ok, understood, thanks. Hmmm, it seems to be difficult to find out method which is common to all architectures. So, what I can do for now is introducing reset scheme which is only for x86. 1) Change this patch so that it work only on x86 platform. For example call this reset code from x86_init.iommu.iommu_init() instead of fs_initcall. Or another idea is: 2) Enumerate PCI devices in IOMMU layer. That is: PCI layer Just provide interface to reset given strcut pci_dev. Maybe pci_reset_function() looks good for this purpose. IOMMU layer Determine which devices should be reset. On kernel boot, check if IOMMU is already active or not, and if active, check IOMMU page table and reset devices whose entry exists there. Of course, it might be possible to reorganize the sparc code to to the IOMMU init *after* it enumerates PCI devices. But I think that change would be hard to justify. And I think even on x86, it would be better if we did the IOMMU init before PCI enumeration -- the PCI devices depend on the IOMMU, so logically the IOMMU should be initialized first so the PCI devices can be associated with it as they are enumerated. So third idea is: 3) Do reset before PCI enumeration(arch_initcall_sync or somewhere). We need to implement new code to enumerate PCI devices and reset them for this purpose. Idea 2 is not difficult to implement, but one problem is that this method may be dangerous. We need to scan IOMMU page table which is used in previous kernel, but it may be broken. Idea 3 seems to be difficult to implement... example). And I think conceptually, the IOMMU should be enumerated and initialized *before* the devices that use it. So I'm uncomfortable with that aspect of this scheme. It would be at least conceivable to reset the devices in the system kernel, before the kexec. I know we want to do as little as possible in the crashing kernel, but it's at least a possibility, and it might be cleaner. I bet this will be not accepted by kdump maintainer. Everything in panic kernel is unreliable. kdump is inherently unreliable. The kdump kernel doesn't start from an arbitrary machine state. We don't expect it to tolerate all CPUs running, for example. Maybe it should be expected to tolerate PCI devices running, either. What I wanted to say is that any resources of first kernel are unreliable. Under panic situation, struct pci_dev tree may be broken, or pci_lock may be already hold by someone, etc. So, if we do this in first kernel, maybe kdump needs its own code to enumerate PCI devices and reset them. Vivek? Ping Vivek Thanks, Takao Indoh -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Regards Dave ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 18/23] iommu/tegra: smmu: Workaround PCIe IOMMU'able
On Wed, Jun 26, 2013 at 01:09:06PM +0200, Hiroshi Doyu wrote: Thierry Reding thierry.red...@gmail.com wrote @ Wed, 26 Jun 2013 13:06:27 +0200: * PGP Signed by an unknown key On Wed, Jun 26, 2013 at 12:28:21PM +0300, Hiroshi Doyu wrote: Make PCIe work as it is. IOMMU support can be implemented later. Signed-off-by: Hiroshi Doyu hd...@nvidia.com --- drivers/iommu/tegra-smmu.c | 3 +++ 1 file changed, 3 insertions(+) Can you provide more information about what the problem is here? Why is PCIe not working when mapped through the IOMMU? I haven't had a code to register PCI device as IOMMU'able as ops-add_device() does for platform_devices. I'll add this comment. Okay, that should be solved then when we merge the PCIe driver. I hope that can happen soon. Thierry pgpQK0fHHO2lL.pgp Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 19/23] iommu/tegra: smmu: Unfied driver for Tegra SoCs
On Wed, Jun 26, 2013 at 12:28:22PM +0300, Hiroshi Doyu wrote: Support multiple generation of Tegra SoCs with this unified SMMU driver. Necessary info is expected to be passed from DT. Signed-off-by: Hiroshi Doyu hd...@nvidia.com --- drivers/iommu/tegra-smmu.c | 80 ++ 1 file changed, 3 insertions(+), 77 deletions(-) diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c index 6e82df3..ae71721 100644 --- a/drivers/iommu/tegra-smmu.c +++ b/drivers/iommu/tegra-smmu.c @@ -1,5 +1,5 @@ /* - * IOMMU API for SMMU in Tegra30 + * IOMMU API for SMMU in Tegra SoC Maybe Tegra30 and later SoCs given that Tegra20 has no compatible IOMMU? -MODULE_DESCRIPTION(IOMMU API for SMMU in Tegra30); +MODULE_DESCRIPTION(IOMMU API for SMMU in Tegra SoC); Same here. Thierry pgpNuabtdnCd4.pgp Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 11/23] iommu/tegra: smmu: Add of_mach_id for T114
On Wed, Jun 26, 2013 at 12:28:14PM +0300, Hiroshi Doyu wrote: Add of_mach_id for T114 I don't think of_mach_id is the right word. Why not use a subject such as iommu/tegra: smmu: Add Tegra114 support. And instead of duplicating the subject in the commit message, perhaps you can say instead that for Tegra114 the SMMU is the same as for Tegra30 (which I assume here given that no code changes are required). And we really need the nvidia,tegra114-smmu.txt binding documentation. Either that or update the nvidia,tegra30-smmu.txt with Tegra114 specific information. Thierry pgpwLFUy6zrru.pgp Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 18/23] iommu/tegra: smmu: Workaround PCIe IOMMU'able
On Wed, Jun 26, 2013 at 12:28:21PM +0300, Hiroshi Doyu wrote: Make PCIe work as it is. IOMMU support can be implemented later. Signed-off-by: Hiroshi Doyu hd...@nvidia.com --- drivers/iommu/tegra-smmu.c | 3 +++ 1 file changed, 3 insertions(+) Can you provide more information about what the problem is here? Why is PCIe not working when mapped through the IOMMU? Thierry pgpoKtEYKA_Y6.pgp Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 03/23] ARM: dt: tegra30: iommu: Add nvidia,memory-clients
On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
[...]
diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
[...]
@@ -23,3 +24,13 @@ Example:
nvidia,swgroups = 0x 0x000779ff;
nvidia,ahb = ahb;
};
+
+ host1x {
+ compatible = nvidia,tegra30-host1x, simple-bus;
+ nvidia,memory-clients = SWGID_HC;
And this could use the SWGID(HC) to match up with how GPIOs are
referenced in the DTS files. Though I see that the clocks don't use a
parameterized version either, so things are inconsistent anyway. But if
SWGID() isn't used then maybe it shouldn't be provided by the header
file in the first place.
Oh, one other thing: both GPIO and CAR use the TEGRA_ prefix, perhaps
this should use it as well?
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
[...]
index 14ec3f9..3fcee3f 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -1,5 +1,6 @@
#include dt-bindings/clock/tegra30-car.h
#include dt-bindings/gpio/tegra-gpio.h
+#include dt-bindings/iommu/tegra-swgid.h
#include dt-bindings/interrupt-controller/arm-gic.h
Nit: these includes seem to be ordered alphabetically; if so then iommu
should go below interrupt-controller.
@@ -286,6 +300,7 @@
interrupts = GIC_SPI 91 IRQ_TYPE_LEVEL_HIGH;
nvidia,dma-request-selector = apbdma 20;
clocks = tegra_car TEGRA30_CLK_UARTE;
+ nvidia,memory-clients = 14;
SWGID_PPCS?
Thierry
pgpi4eij0DQCo.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 17/23] iommu/tegra: smmu: Use swgroups instead of pdata
On Wed, Jun 26, 2013 at 12:28:20PM +0300, Hiroshi Doyu wrote: Instead of using platfrom data, DT passes nvidia,swgroups. Signed-off-by: Hiroshi Doyu hd...@nvidia.com --- drivers/iommu/tegra-smmu.c | 24 +++- 1 file changed, 7 insertions(+), 17 deletions(-) Could this patch not be squashed into the previous one? Moving from using the swgroups from DT and replacing platform data seems to be the same logical change. Thierry pgpBxuPrlqf7k.pgp Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 03/23] ARM: dt: tegra30: iommu: Add nvidia,memory-clients
On Wed, Jun 26, 2013 at 12:18:17PM +0200, Thierry Reding wrote:
On Wed, Jun 26, 2013 at 12:28:06PM +0300, Hiroshi Doyu wrote:
[...]
diff --git
a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
[...]
@@ -23,3 +24,13 @@ Example:
nvidia,swgroups = 0x 0x000779ff;
nvidia,ahb = ahb;
};
+
+ host1x {
+ compatible = nvidia,tegra30-host1x, simple-bus;
+ nvidia,memory-clients = SWGID_HC;
And this could use the SWGID(HC) to match up with how GPIOs are
referenced in the DTS files.
Scratch that. SWGID() yields a mask for the corresponding swgroup so
it's not the same thing.
Thierry
pgp3NHzNB8807.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 21/23] iommu/tegra: smmu: Get swgroup ID from DT
On Wed, Jun 26, 2013 at 12:28:24PM +0300, Hiroshi Doyu wrote:
Get swgroup ID from DT. nvidia,swgroups indicates which swgroup IDs
a device belongs to.
Signed-off-by: Hiroshi Doyu hd...@nvidia.com
---
arch/arm/boot/dts/tegra30.dtsi | 1 -
drivers/iommu/tegra-smmu.c | 20 +++-
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 7c480f2..a116737 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -1,6 +1,5 @@
#include dt-bindings/clock/tegra30-car.h
#include dt-bindings/gpio/tegra-gpio.h
-#include dt-bindings/iommu/tegra-swgid.h
#include dt-bindings/interrupt-controller/arm-gic.h
#include skeleton.dtsi
diff --git a/drivers/iommu/tegra-smmu.c b/drivers/iommu/tegra-smmu.c
index 50eb843..96dbef3 100644
--- a/drivers/iommu/tegra-smmu.c
+++ b/drivers/iommu/tegra-smmu.c
@@ -314,6 +314,24 @@ static inline void smmu_write(struct smmu_device *smmu,
u32 val, size_t offs)
#define smmu_client_hwgrp(c) (c-as-smmu-swgroups)
+static u64 tegra_smmu_of_get_swgids(struct device *dev)
+{
+ size_t bytes;
+ const char *propname = nvidia,memory-clients;
+ const __be32 *prop;
+ int i;
+ u64 swgids = 0;
+
+ prop = of_get_property(dev-of_node, propname, bytes);
+ if (!prop || !bytes)
+ return 0;
+
+ for (i = 0; i bytes / sizeof(u32); i++, prop++)
+ swgids |= 1ULL be32_to_cpup(prop);
+
+ return swgids;
+}
+
static int __smmu_client_set_hwgrp(struct smmu_client *c,
u64 map, int on)
{
@@ -725,7 +743,7 @@ static int smmu_iommu_attach_dev(struct iommu_domain
*domain,
return -ENOMEM;
client-dev = dev;
client-as = as;
- map = smmu-swgroups;
+ map = tegra_smmu_of_get_swgids(dev);
if (!map)
return -EINVAL;
Shouldn't this be part of an earlier patch ([PATCH 17/23] iommu/tegra:
smmu: Use swgroups instead of pdata)? What's the reason for the split?
Thierry
pgppPXAsbVDak.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 02/23] ARM: dt: tegra30: iommu: Add nvidia,swgroups
On Wed, Jun 26, 2013 at 12:28:05PM +0300, Hiroshi Doyu wrote:
This is a bitmap that indicates which HardWare Accelerators(HWA) are
supported on Tegra30 SoC.
Signed-off-by: Hiroshi Doyu hd...@nvidia.com
---
Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt | 6 +-
arch/arm/boot/dts/tegra30.dtsi | 1 +
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
index 89fb543..6be51f6 100644
--- a/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/nvidia,tegra30-smmu.txt
@@ -8,14 +8,18 @@ Required properties:
- nvidia,#asids : # of ASIDs
- dma-window : IOVA start address and length.
- nvidia,ahb : phandle to the ahb bus connected to SMMU.
+- nvidia,swgroups: A bit map of supported HardWare Accelerators(HWA).
Nit: you use the spelling bitmap in the subject but bit map here.
Both are correct but perhaps we should stick to one.
+ Each bit represents one sgroup. The assignments may be found in header
Nit: swgroup
- smmu {
+ iommu {
This change isn't really related, but given that the tegra30.dtsi
already uses it I guess we can keep it as part of this patch.
compatible = nvidia,tegra30-smmu;
reg = 0x7000f010 0x02c
0x7000f1f0 0x010
0x7000f228 0x05c;
nvidia,#asids = 4;/* # of ASIDs */
dma-window = 0 0x4000;/* IOVA start length */
+ nvidia,swgroups = 0x 0x000779ff;
Perhaps this should be a symbolic name too? Perhaps something like
TEGRA30_SWGID_ALL?
Thierry
pgpiIQXSQ7tV2.pgp
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
