* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback [not found] ` <1465183229-24147-5-git-send-email-wmills@ti.com> @ 2016-06-06 8:56 ` Mark Rutland 2016-06-06 9:09 ` Arnd Bergmann 2016-06-06 11:43 ` Russell King - ARM Linux 0 siblings, 2 replies; 14+ messages in thread From: Mark Rutland @ 2016-06-06 8:56 UTC (permalink / raw) To: Bill Mills Cc: rmk+kernel, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree [adding devicetree] On Sun, Jun 05, 2016 at 11:20:29PM -0400, Bill Mills wrote: > Keystone2 can do DMA coherency but only if: > 1) DDR3A DMA buffers are in high physical addresses (0x8_0000_0000) > (DDR3B does not have this constraint) > 2) Memory is marked outer shared > 3) DMA Master marks transactions as outer shared > (This is taken care of in bootloader) > > Use outer shared instead of inner shared. > This choice is done at early init time and uses the attr_mod facility > > If the kernel is not configured for LPAE and using high PA, or if the > switch to outer shared fails, then we fail to meet this criteria. > Under any of these conditions we veto any dma-coherent attributes in > the DTB. I very much do not like this. As I previously mentioned [1], dma-coherent has de-facto semantics today. This series deliberately changes that, and inverts the relationship between DT and kernel (as the describption in the DT would now depend on the configuration of the kernel). I would prefer that we have a separate property (e.g. "dma-outer-coherent") to describe when a device can be coherent with Normal, Outer Shareable, Inner Write-Back, Outer Write-Back memory. Then the kernel can figure out whether or not device can be used coherently, depending on how it is configured. Thanks, Mark. [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-April/421470.html > > Signed-off-by: Bill Mills <wmills@ti.com> > --- > arch/arm/mach-keystone/keystone.c | 37 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 37 insertions(+) > > diff --git a/arch/arm/mach-keystone/keystone.c b/arch/arm/mach-keystone/keystone.c > index a33a296..d10adaf 100644 > --- a/arch/arm/mach-keystone/keystone.c > +++ b/arch/arm/mach-keystone/keystone.c > @@ -28,6 +28,7 @@ > #include "keystone.h" > > static unsigned long keystone_dma_pfn_offset __read_mostly; > +static bool keystone_dma_coherent; > > static int keystone_platform_notifier(struct notifier_block *nb, > unsigned long event, void *data) > @@ -52,21 +53,53 @@ static struct notifier_block platform_nb = { > .notifier_call = keystone_platform_notifier, > }; > > +void veto_dma_coherent(void) > +{ > + struct device_node *node, *start_node; > + struct property *prop; > + > + for (start_node = NULL; > + (node = of_find_node_with_property(start_node, "dma-coherent")); > + start_node = node) { > + prop = of_find_property(node, "dma-coherent", NULL); > + if (prop) > + of_remove_property(node, prop); > + } > +} > + > static void __init keystone_init(void) > { > + /* If we are running from the high physical addresses then adjust > + * addresses we give to the device's DMA. They will be seeing this > + * memory through the MSMC address translation which makes the first 2GB > + * of high memory appear in the low 4GB space. > + * (DMA masters on keystone2 have 32 bit address buses) > + */ > if (PHYS_OFFSET >= KEYSTONE_HIGH_PHYS_START) { > keystone_dma_pfn_offset = PFN_DOWN(KEYSTONE_HIGH_PHYS_START - > KEYSTONE_LOW_PHYS_START); > bus_register_notifier(&platform_bus_type, &platform_nb); > } > + > + /* if the kernel has not been configured to meet the keystone > + * platform requirements to achieve DMA coherency, then ignore any > + * device tree configuration for this > + */ > + if (!keystone_dma_coherent) > + veto_dma_coherent(); > + > keystone_pm_runtime_init(); > of_platform_populate(NULL, of_default_bus_match_table, NULL, NULL); > } > > static long long __init keystone_pv_fixup(void) > { > +#ifdef CONFIG_ARM_LPAE > long long offset; > phys_addr_t mem_start, mem_end; > + bool dma_ok; > + > + dma_ok = use_outer_shared(); > > mem_start = memblock_start_of_DRAM(); > mem_end = memblock_end_of_DRAM(); > @@ -84,11 +117,15 @@ static long long __init keystone_pv_fixup(void) > } > > offset = KEYSTONE_HIGH_PHYS_START - KEYSTONE_LOW_PHYS_START; > + keystone_dma_coherent = dma_ok; > > /* Populate the arch idmap hook */ > arch_phys_to_idmap_offset = -offset; > > return offset; > +#else > + return 0; > +#endif > } > > static const char *const keystone_match[] __initconst = { > -- > 1.9.1 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 8:56 ` [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback Mark Rutland @ 2016-06-06 9:09 ` Arnd Bergmann 2016-06-06 11:42 ` Mark Rutland 2016-06-06 11:43 ` Russell King - ARM Linux 1 sibling, 1 reply; 14+ messages in thread From: Arnd Bergmann @ 2016-06-06 9:09 UTC (permalink / raw) To: Mark Rutland Cc: Bill Mills, rmk+kernel, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On Monday, June 6, 2016 9:56:27 AM CEST Mark Rutland wrote: > [adding devicetree] > > On Sun, Jun 05, 2016 at 11:20:29PM -0400, Bill Mills wrote: > > Keystone2 can do DMA coherency but only if: > > 1) DDR3A DMA buffers are in high physical addresses (0x8_0000_0000) > > (DDR3B does not have this constraint) > > 2) Memory is marked outer shared > > 3) DMA Master marks transactions as outer shared > > (This is taken care of in bootloader) > > > > Use outer shared instead of inner shared. > > This choice is done at early init time and uses the attr_mod facility > > > > If the kernel is not configured for LPAE and using high PA, or if the > > switch to outer shared fails, then we fail to meet this criteria. > > Under any of these conditions we veto any dma-coherent attributes in > > the DTB. > > I very much do not like this. As I previously mentioned [1], > dma-coherent has de-facto semantics today. This series deliberately > changes that, and inverts the relationship between DT and kernel (as the > describption in the DT would now depend on the configuration of the > kernel). > > I would prefer that we have a separate property (e.g. > "dma-outer-coherent") to describe when a device can be coherent with > Normal, Outer Shareable, Inner Write-Back, Outer Write-Back memory. > Then the kernel can figure out whether or not device can be used > coherently, depending on how it is configured. I share your concern, but I don't think the dma-outer-coherent attribute would be a good solution either. The problem really is that keystone is a platform that is sometimes coherent, depending purely on what kernel we run, and not at all on anything we can describe in devicetree, and I don't see any good way to capture the behavior of the hardware in generic DT bindings. So far, the assumption has been: - when running a non-LPAE kernel, keystone is not coherent, and we must ignore both the dma-coherent properties in devices and the dma-ranges properties in bus nodes. - when running an LPAE kernel, keystone is coherent, and we must respect both of those. My interpretation of Bill's description above is that we now have an additional requirement that at least I was not aware of before, regarding the outer-sharable attribute. I don't think there is much value in making this a boot-time option, since everyone would want to run this platform in a cache-coherent way if at all possible. We already have special hacks to detect the case of keystone running in LPAE mode, in order to do the special rewrite-all-page-tables hack at boot time for relocating the physical address, and we could use the same hack to change the page table attributes. The question is how to communicate the requirement for outer-sharable for a platform. If we think it's a safe assumption that there will not be future 32-bit platforms with this requirement (or maybe one or two more at most), we could leave it in the special keystone hack. Alternatively, a DT property in an appropriate node could indicate that a particular platform requires it. Arnd ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 9:09 ` Arnd Bergmann @ 2016-06-06 11:42 ` Mark Rutland 2016-06-06 12:37 ` Arnd Bergmann 2016-06-06 12:50 ` William Mills 0 siblings, 2 replies; 14+ messages in thread From: Mark Rutland @ 2016-06-06 11:42 UTC (permalink / raw) To: Arnd Bergmann Cc: Bill Mills, rmk+kernel, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On Mon, Jun 06, 2016 at 11:09:07AM +0200, Arnd Bergmann wrote: > On Monday, June 6, 2016 9:56:27 AM CEST Mark Rutland wrote: > > [adding devicetree] > > > > On Sun, Jun 05, 2016 at 11:20:29PM -0400, Bill Mills wrote: > > > Keystone2 can do DMA coherency but only if: > > > 1) DDR3A DMA buffers are in high physical addresses (0x8_0000_0000) > > > (DDR3B does not have this constraint) > > > 2) Memory is marked outer shared > > > 3) DMA Master marks transactions as outer shared > > > (This is taken care of in bootloader) > > > > > > Use outer shared instead of inner shared. > > > This choice is done at early init time and uses the attr_mod facility > > > > > > If the kernel is not configured for LPAE and using high PA, or if the > > > switch to outer shared fails, then we fail to meet this criteria. > > > Under any of these conditions we veto any dma-coherent attributes in > > > the DTB. > > > > I very much do not like this. As I previously mentioned [1], > > dma-coherent has de-facto semantics today. This series deliberately > > changes that, and inverts the relationship between DT and kernel (as the > > describption in the DT would now depend on the configuration of the > > kernel). > > > > I would prefer that we have a separate property (e.g. > > "dma-outer-coherent") to describe when a device can be coherent with > > Normal, Outer Shareable, Inner Write-Back, Outer Write-Back memory. > > Then the kernel can figure out whether or not device can be used > > coherently, depending on how it is configured. > > I share your concern, but I don't think the dma-outer-coherent attribute > would be a good solution either. > > The problem really is that keystone is a platform that is sometimes > coherent, depending purely on what kernel we run, and not at all on > anything we can describe in devicetree, and I don't see any good way > to capture the behavior of the hardware in generic DT bindings. I think that above doesn't quite capture the situation: Some DMA masters can be cache-coherent (only) with Outer Shareable transactions. That is a property we could capture inthe DT (e.g. dma-outer-coherent), and is independent of the kernel configuration. Whether or not the devices are coherent with the kernel's chosen memory attributes certainly depends on the kernel configuration, but that is not what we capture in the DT. > So far, the assumption has been: > > - when running a non-LPAE kernel, keystone is not coherent, and we > must ignore both the dma-coherent properties in devices and the > dma-ranges properties in bus nodes. I wasn't able to spot if/where that was enforced. Is it possible to boot Keystone UP, !LPAE? > - when running an LPAE kernel, keystone is coherent, and we must > respect both of those. Similarly this has not been enforced either way. Currently I cannot see how devices with dma-coherent could possibly work with Keystone. I think we also need to be clear as to what we mean by "keystone is coherent". With LPAE, CPUs can be coherent with each other, so long as the appropriate physical addresses are used (with the magic to handle that). Devices being cache-coherent with CPUs is already something we manage per-device. > My interpretation of Bill's description above is that we now have > an additional requirement that at least I was not aware of before, > regarding the outer-sharable attribute. I don't think there is > much value in making this a boot-time option, since everyone would > want to run this platform in a cache-coherent way if at all possible. > > We already have special hacks to detect the case of keystone running > in LPAE mode, in order to do the special rewrite-all-page-tables > hack at boot time for relocating the physical address, and we could > use the same hack to change the page table attributes. > > The question is how to communicate the requirement for outer-sharable > for a platform. If we think it's a safe assumption that there will > not be future 32-bit platforms with this requirement (or maybe one > or two more at most), we could leave it in the special keystone hack. > Alternatively, a DT property in an appropriate node could indicate > that a particular platform requires it. As above, I think this must be specified per-device, following the usual manner in which we describe Inner Shareable coherency using dma-coherent. Thanks, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 11:42 ` Mark Rutland @ 2016-06-06 12:37 ` Arnd Bergmann 2016-06-06 12:50 ` William Mills 1 sibling, 0 replies; 14+ messages in thread From: Arnd Bergmann @ 2016-06-06 12:37 UTC (permalink / raw) To: Mark Rutland Cc: Bill Mills, rmk+kernel, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On Monday, June 6, 2016 12:42:56 PM CEST Mark Rutland wrote: > On Mon, Jun 06, 2016 at 11:09:07AM +0200, Arnd Bergmann wrote: > > On Monday, June 6, 2016 9:56:27 AM CEST Mark Rutland wrote: > > > > So far, the assumption has been: > > > > - when running a non-LPAE kernel, keystone is not coherent, and we > > must ignore both the dma-coherent properties in devices and the > > dma-ranges properties in bus nodes. > > I wasn't able to spot if/where that was enforced. Is it possible to boot > Keystone UP, !LPAE? With !LPAE, no devices are coherent, so that should work with both SMP and and UP. Not sure about LPAE with coherent devices on UP, IIRC we had a bug in that configuration on Armada XP, as the memory was not marked as sharable at all there, and ended up not being coherent with DMA masters. My first guess is that uniprocessor mode on keystone has not been tested at all (TI's QA seems to test only a very limited number of configurations), so it may or may not work. Arnd ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 11:42 ` Mark Rutland 2016-06-06 12:37 ` Arnd Bergmann @ 2016-06-06 12:50 ` William Mills 2016-06-06 16:18 ` Santosh Shilimkar 1 sibling, 1 reply; 14+ messages in thread From: William Mills @ 2016-06-06 12:50 UTC (permalink / raw) To: Mark Rutland, Arnd Bergmann Cc: rmk+kernel, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On 06/06/2016 07:42 AM, Mark Rutland wrote: > On Mon, Jun 06, 2016 at 11:09:07AM +0200, Arnd Bergmann wrote: >> On Monday, June 6, 2016 9:56:27 AM CEST Mark Rutland wrote: >>> [adding devicetree] >>> >>> On Sun, Jun 05, 2016 at 11:20:29PM -0400, Bill Mills wrote: >>>> Keystone2 can do DMA coherency but only if: >>>> 1) DDR3A DMA buffers are in high physical addresses (0x8_0000_0000) >>>> (DDR3B does not have this constraint) >>>> 2) Memory is marked outer shared >>>> 3) DMA Master marks transactions as outer shared >>>> (This is taken care of in bootloader) >>>> >>>> Use outer shared instead of inner shared. >>>> This choice is done at early init time and uses the attr_mod facility >>>> >>>> If the kernel is not configured for LPAE and using high PA, or if the >>>> switch to outer shared fails, then we fail to meet this criteria. >>>> Under any of these conditions we veto any dma-coherent attributes in >>>> the DTB. >>> >>> I very much do not like this. As I previously mentioned [1], >>> dma-coherent has de-facto semantics today. This series deliberately >>> changes that, and inverts the relationship between DT and kernel (as the >>> describption in the DT would now depend on the configuration of the >>> kernel). >>> >>> I would prefer that we have a separate property (e.g. >>> "dma-outer-coherent") to describe when a device can be coherent with >>> Normal, Outer Shareable, Inner Write-Back, Outer Write-Back memory. >>> Then the kernel can figure out whether or not device can be used >>> coherently, depending on how it is configured. >> >> I share your concern, but I don't think the dma-outer-coherent attribute >> would be a good solution either. >> >> The problem really is that keystone is a platform that is sometimes >> coherent, depending purely on what kernel we run, and not at all on >> anything we can describe in devicetree, and I don't see any good way >> to capture the behavior of the hardware in generic DT bindings. > > I think that above doesn't quite capture the situation: > > Some DMA masters can be cache-coherent (only) with Outer Shareable > transactions. That is a property we could capture inthe DT (e.g. > dma-outer-coherent), and is independent of the kernel configuration. > > Whether or not the devices are coherent with the kernel's chosen memory > attributes certainly depends on the kernel configuration, but that is > not what we capture in the DT. > >> So far, the assumption has been: >> >> - when running a non-LPAE kernel, keystone is not coherent, and we >> must ignore both the dma-coherent properties in devices and the >> dma-ranges properties in bus nodes. > > I wasn't able to spot if/where that was enforced. Is it possible to boot > Keystone UP, !LPAE? > Yes ... with the right combination of DTB, u-boot, u-boot vars, and kernel config. Mismatches either fail hard or use dma-coherent ops without actually providing coherency. I am attempting to make this less fragile. Mis-configured coherency can be dead-wrong and still only fail 1 transaction in 1,000,000. I have seen customers run for weeks or months w/o detecting the issue. Thats why I wanted the veto logic. There are 3 cases to cover: LPAE w/ high PA: this is the normal mode for KS2. Uses coherent dma-ops. !LPAE: obviously uses low PA and must use non-coherent dma-ops. LPAE w/ low PA: This happens with an LPAE kernel but the user has passed a low PA memory DTB and u-boot has not fixed it up. This case must also use non-coherent dma-ops Upstream DTS has keystone memory at the low PA. I agree with that. U-boot and kernel opt-in to the use of high PA. If you give high PA to a non-LPAE kernel I believe it will fail hard and fast. I can check. Thanks, Bill ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 12:50 ` William Mills @ 2016-06-06 16:18 ` Santosh Shilimkar 0 siblings, 0 replies; 14+ messages in thread From: Santosh Shilimkar @ 2016-06-06 16:18 UTC (permalink / raw) To: William Mills, Mark Rutland, Arnd Bergmann Cc: rmk+kernel, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On 6/6/2016 5:50 AM, William Mills wrote: > > I saw only v2 but seems like it already generated discussion(s) > On 06/06/2016 07:42 AM, Mark Rutland wrote: >> On Mon, Jun 06, 2016 at 11:09:07AM +0200, Arnd Bergmann wrote: >>> On Monday, June 6, 2016 9:56:27 AM CEST Mark Rutland wrote: >>>> [adding devicetree] >>>> >>>> On Sun, Jun 05, 2016 at 11:20:29PM -0400, Bill Mills wrote: >>>>> Keystone2 can do DMA coherency but only if: >>>>> 1) DDR3A DMA buffers are in high physical addresses (0x8_0000_0000) >>>>> (DDR3B does not have this constraint) >>>>> 2) Memory is marked outer shared >>>>> 3) DMA Master marks transactions as outer shared >>>>> (This is taken care of in bootloader) >>>>> >>>>> Use outer shared instead of inner shared. >>>>> This choice is done at early init time and uses the attr_mod facility >>>>> >>>>> If the kernel is not configured for LPAE and using high PA, or if the >>>>> switch to outer shared fails, then we fail to meet this criteria. >>>>> Under any of these conditions we veto any dma-coherent attributes in >>>>> the DTB. >>>> >>>> I very much do not like this. As I previously mentioned [1], >>>> dma-coherent has de-facto semantics today. This series deliberately >>>> changes that, and inverts the relationship between DT and kernel (as the >>>> describption in the DT would now depend on the configuration of the >>>> kernel). >>>> >>>> I would prefer that we have a separate property (e.g. >>>> "dma-outer-coherent") to describe when a device can be coherent with >>>> Normal, Outer Shareable, Inner Write-Back, Outer Write-Back memory. >>>> Then the kernel can figure out whether or not device can be used >>>> coherently, depending on how it is configured. >>> >>> I share your concern, but I don't think the dma-outer-coherent attribute >>> would be a good solution either. >>> >>> The problem really is that keystone is a platform that is sometimes >>> coherent, depending purely on what kernel we run, and not at all on >>> anything we can describe in devicetree, and I don't see any good way >>> to capture the behavior of the hardware in generic DT bindings. >> >> I think that above doesn't quite capture the situation: >> >> Some DMA masters can be cache-coherent (only) with Outer Shareable >> transactions. That is a property we could capture inthe DT (e.g. >> dma-outer-coherent), and is independent of the kernel configuration. >> >> Whether or not the devices are coherent with the kernel's chosen memory >> attributes certainly depends on the kernel configuration, but that is >> not what we capture in the DT. >> >>> So far, the assumption has been: >>> >>> - when running a non-LPAE kernel, keystone is not coherent, and we >>> must ignore both the dma-coherent properties in devices and the >>> dma-ranges properties in bus nodes. Correct. >> >> I wasn't able to spot if/where that was enforced. Is it possible to boot >> Keystone UP, !LPAE? >> > > Yes ... with the right combination of DTB, u-boot, u-boot vars, and > kernel config. Mismatches either fail hard or use dma-coherent ops > without actually providing coherency. I am attempting to make this less > fragile. > > Mis-configured coherency can be dead-wrong and still only fail 1 > transaction in 1,000,000. I have seen customers run for weeks or months > w/o detecting the issue. Thats why I wanted the veto logic. > > There are 3 cases to cover: > LPAE w/ high PA: > this is the normal mode for KS2. Uses coherent dma-ops. > !LPAE: > obviously uses low PA and must use non-coherent dma-ops. > LPAE w/ low PA: > This happens with an LPAE kernel but the user has passed a low > PA memory DTB and u-boot has not fixed it up. > This case must also use non-coherent dma-ops > > Upstream DTS has keystone memory at the low PA. I agree with that. > U-boot and kernel opt-in to the use of high PA. > > If you give high PA to a non-LPAE kernel I believe it will fail hard and > fast. I can check. > UP will mostly boot from boot view the memory. The keystone_pv_fixup() will bail out for higher PA. Let me know if you see otherwise. Regards, Santosh ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 8:56 ` [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback Mark Rutland 2016-06-06 9:09 ` Arnd Bergmann @ 2016-06-06 11:43 ` Russell King - ARM Linux 2016-06-06 11:59 ` Mark Rutland 1 sibling, 1 reply; 14+ messages in thread From: Russell King - ARM Linux @ 2016-06-06 11:43 UTC (permalink / raw) To: Mark Rutland Cc: Bill Mills, t-kristo-l0cyMroinI0, ssantosh-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, r-woodruff2-l0cyMroinI0, devicetree-u79uwXL29TY76Z2rM5mHXA On Mon, Jun 06, 2016 at 09:56:27AM +0100, Mark Rutland wrote: > I very much do not like this. As I previously mentioned [1], > dma-coherent has de-facto semantics today. This series deliberately > changes that, and inverts the relationship between DT and kernel (as the > describption in the DT would now depend on the configuration of the > kernel). dma-coherent's semantics are not very well defined - just grep for it in Documention/devicetree/ and you'll find several different wordings for what this property means. Anyway, my point here is that all of these merely say that the hardware is coherent in _some regard_ - it doesn't specify under what conditions DMA coherency is guaranteed by the hardware. It happens that on ARM, most platforms give that guarantee when using inner shared mappings. If we were to use some other sharing, or disable sharing altogether (eg, by disabling SMP support) then all these platforms would immediately break. In other words, DMA coherence today already depends on the kernel's setup of the page tables corresponding to the requirements of the hardware. Keystone II is just slightly different - and as I understand it, TI followed one of the early specifications that ARM Ltd produced. That specification may have contained errors, but unfortunately, we now have a situation where there is hardware out there which followed in good faith. So, it seems to me to be entirely reasonable that Keystone II should mark devices with the "dma-coherent" property - just the same way as every other platform does. It also seems to be entirely appropriate for a platform to remove this property if it determines that the conditions for DMA coherency are not met - in order to save the users data from corruption. TI Keystone II is not the only platform with issues here: there are Marvell Armada platforms out there which have DMA coherence, but are uniprocessor, we don't set the shared bit (which they require for DMA coherence) and so we omit the dma-coherent property from the device tree at the moment. And they're inner-shared coherent. We just don't set the page tables up so that they can work. So, I think to require a whole new property is absurd. The existing property means "if the rest of the system is appropriately configured, this device can be dma-coherent". So, what I think we need is a way to communicate whether the rest of the system has been appropriately configured, so the property can be attached to devices which meet the criteria, but the arch/platform level can signal whether the conditions for device DMA coherence have been met. That's not a DT property, that's a matter of how the kernel has setup the system. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 11:43 ` Russell King - ARM Linux @ 2016-06-06 11:59 ` Mark Rutland 2016-06-06 12:19 ` William Mills 2016-06-06 12:32 ` Russell King - ARM Linux 0 siblings, 2 replies; 14+ messages in thread From: Mark Rutland @ 2016-06-06 11:59 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Bill Mills, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On Mon, Jun 06, 2016 at 12:43:21PM +0100, Russell King - ARM Linux wrote: > On Mon, Jun 06, 2016 at 09:56:27AM +0100, Mark Rutland wrote: > > I very much do not like this. As I previously mentioned [1], > > dma-coherent has de-facto semantics today. This series deliberately > > changes that, and inverts the relationship between DT and kernel (as the > > describption in the DT would now depend on the configuration of the > > kernel). > > dma-coherent's semantics are not very well defined - just grep for it > in Documention/devicetree/ and you'll find several different wordings > for what this property means. Indeed. This is the tip of the iceberg w.r.t. under-specification of memory attribute usage. > Anyway, my point here is that all of these merely say that the hardware > is coherent in _some regard_ - it doesn't specify under what conditions > DMA coherency is guaranteed by the hardware. It happens that on ARM, > most platforms give that guarantee when using inner shared mappings. If > we were to use some other sharing, or disable sharing altogether (eg, by > disabling SMP support) then all these platforms would immediately break. > > In other words, DMA coherence today already depends on the kernel's setup > of the page tables corresponding to the requirements of the hardware. I agree that whether or not devices are coherent in practice depends on the kernel's configuration. The flip side, as you point out, is that devices are coherent when a specific set of attributes are used. i.e. that if you read dma-coherent as meaning "coherent iff Normal, Inner Shareable, Inner WB Cacheable, Outer WB Cacheable", then dma-coherent consistently describes the same thing, rather than depending on the configuration of the OS. DT is a datastructure provided to the kernel, potentially without deep internal knowledge of that kernel configuration. Having a consistent rule that is independent of the kernel configuration seems worth aiming for. A dma-outer-coherent property would allow us to accurately describe the keystone case in the same way, independent of kernel configuration. > Keystone II is just slightly different - and as I understand it, TI > followed one of the early specifications that ARM Ltd produced. That > specification may have contained errors, but unfortunately, we now have > a situation where there is hardware out there which followed in good > faith. To be clear, I am not arguing against supporting keystone. I just wish to avoid muddying the waters further w.r.t. the semantics of dma-coherent, which I believe can be salvaged and made consistent. Clearly, those semantics are the point of contention here. Thanks, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 11:59 ` Mark Rutland @ 2016-06-06 12:19 ` William Mills 2016-06-06 12:32 ` Russell King - ARM Linux 1 sibling, 0 replies; 14+ messages in thread From: William Mills @ 2016-06-06 12:19 UTC (permalink / raw) To: Mark Rutland, Russell King - ARM Linux Cc: t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On 06/06/2016 07:59 AM, Mark Rutland wrote: > On Mon, Jun 06, 2016 at 12:43:21PM +0100, Russell King - ARM Linux wrote: >> On Mon, Jun 06, 2016 at 09:56:27AM +0100, Mark Rutland wrote: >>> I very much do not like this. As I previously mentioned [1], >>> dma-coherent has de-facto semantics today. This series deliberately >>> changes that, and inverts the relationship between DT and kernel (as the >>> describption in the DT would now depend on the configuration of the >>> kernel). >> >> dma-coherent's semantics are not very well defined - just grep for it >> in Documention/devicetree/ and you'll find several different wordings >> for what this property means. > > Indeed. This is the tip of the iceberg w.r.t. under-specification of > memory attribute usage. > >> Anyway, my point here is that all of these merely say that the hardware >> is coherent in _some regard_ - it doesn't specify under what conditions >> DMA coherency is guaranteed by the hardware. It happens that on ARM, >> most platforms give that guarantee when using inner shared mappings. If >> we were to use some other sharing, or disable sharing altogether (eg, by >> disabling SMP support) then all these platforms would immediately break. >> >> In other words, DMA coherence today already depends on the kernel's setup >> of the page tables corresponding to the requirements of the hardware. > > I agree that whether or not devices are coherent in practice depends on > the kernel's configuration. The flip side, as you point out, is that > devices are coherent when a specific set of attributes are used. > > i.e. that if you read dma-coherent as meaning "coherent iff Normal, > Inner Shareable, Inner WB Cacheable, Outer WB Cacheable", then > dma-coherent consistently describes the same thing, rather than > depending on the configuration of the OS. > Even w/o inner / outer it seems to me it is under specified. What about a system where main DDR memory is DMA coherent but an on chip SRAM needs manual flush/invalidate? Right now dma coherency is all or nothing. In the above case you would need to ensure that the device never tried to use that SRAM for dma transactions even though the device is perfectly capable with the right hand-holding. Alternatively you could use the SRAM but penalize the normal case of DDR. > DT is a datastructure provided to the kernel, potentially without deep > internal knowledge of that kernel configuration. Having a consistent > rule that is independent of the kernel configuration seems worth aiming > for. > > A dma-outer-coherent property would allow us to accurately describe the > keystone case in the same way, independent of kernel configuration. > >> Keystone II is just slightly different - and as I understand it, TI >> followed one of the early specifications that ARM Ltd produced. That >> specification may have contained errors, but unfortunately, we now have >> a situation where there is hardware out there which followed in good >> faith. > > To be clear, I am not arguing against supporting keystone. I just wish > to avoid muddying the waters further w.r.t. the semantics of > dma-coherent, which I believe can be salvaged and made consistent. > > Clearly, those semantics are the point of contention here. > To me this seems like a choice between embracing outer-shared or treating it like a quirk of some early armv7 devices. With ARM's current recommendations to use inner-shared for everything in many contexts (like ARM servers etc), I was assuming you would want the latter. Thanks for looking. This is not the patch that I expected to generate the most discussion. :) -- Bill ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 11:59 ` Mark Rutland 2016-06-06 12:19 ` William Mills @ 2016-06-06 12:32 ` Russell King - ARM Linux 2016-06-06 16:28 ` Santosh Shilimkar [not found] ` <20160606123210.GL1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> 1 sibling, 2 replies; 14+ messages in thread From: Russell King - ARM Linux @ 2016-06-06 12:32 UTC (permalink / raw) To: Mark Rutland Cc: Bill Mills, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On Mon, Jun 06, 2016 at 12:59:18PM +0100, Mark Rutland wrote: > I agree that whether or not devices are coherent in practice depends on > the kernel's configuration. The flip side, as you point out, is that > devices are coherent when a specific set of attributes are used. > > i.e. that if you read dma-coherent as meaning "coherent iff Normal, > Inner Shareable, Inner WB Cacheable, Outer WB Cacheable", then > dma-coherent consistently describes the same thing, rather than > depending on the configuration of the OS. > > DT is a datastructure provided to the kernel, potentially without deep > internal knowledge of that kernel configuration. Having a consistent > rule that is independent of the kernel configuration seems worth aiming > for. I think you've missed the point. dma-coherent is _already_ dependent on the kernel configuration. "Having a consistent rule that is independent of the kernel configuration" is already an impossibility, as I illustrated in my previous message concerning Marvell Armada SoCs, and you also said in your preceding paragraph! For example, if you clear the shared bit in the page tables on non-LPAE SoCs, devices are no longer coherent. DMA coherence on ARM _is_ already tightly linked with the kernel configuration. You already can't get away from that, so I think you should give up trying to argue that point. :) Whether devices are DMA coherent is a combination of two things: * is the device connected to a coherent bus. * is the system setup to allow coherency on that bus to work. We capture the first through the dma-coherent property, which is clearly a per-device property. We ignore the second because we assume everyone is going to configure the CPU side correctly. That's untrue today, and it's untrue not only because of Keystone II, but also because of other SoCs as well which pre-date Keystone II. We currently miss out on considering that, because if we ignore it, we get something that works for most platforms. I don't see that adding a dma-outer-coherent property helps this - it's muddying the waters somewhat - and it's also forcing additional complexity into places where we shouldn't have it. We would need to parse two properties in the DMA API code, and then combine it with knowledge as to how the system page tables have been setup. If they've been setup as inner sharable, then dma-coherent identifies whether the device is coherent. If they've been setup as outer sharable, then dma-outer-coherent specifies that and dma-coherent is meaningless. Sounds like a recipe for confusion. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-06 12:32 ` Russell King - ARM Linux @ 2016-06-06 16:28 ` Santosh Shilimkar [not found] ` <20160606123210.GL1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> 1 sibling, 0 replies; 14+ messages in thread From: Santosh Shilimkar @ 2016-06-06 16:28 UTC (permalink / raw) To: Russell King - ARM Linux, Mark Rutland, Bill Mills Cc: t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree (Joining discussion late since only this thread showed up in my inbox) On 6/6/2016 5:32 AM, Russell King - ARM Linux wrote: > On Mon, Jun 06, 2016 at 12:59:18PM +0100, Mark Rutland wrote: >> I agree that whether or not devices are coherent in practice depends on >> the kernel's configuration. The flip side, as you point out, is that >> devices are coherent when a specific set of attributes are used. >> >> i.e. that if you read dma-coherent as meaning "coherent iff Normal, >> Inner Shareable, Inner WB Cacheable, Outer WB Cacheable", then >> dma-coherent consistently describes the same thing, rather than >> depending on the configuration of the OS. >> I think there is a bit of miss-understanding with 'dma-coherent' DT property and as RMK pointed out "dma-coherent-outer" isn't right direction either. >> DT is a datastructure provided to the kernel, potentially without deep >> internal knowledge of that kernel configuration. Having a consistent >> rule that is independent of the kernel configuration seems worth aiming >> for. > > I think you've missed the point. dma-coherent is _already_ dependent on > the kernel configuration. "Having a consistent rule that is independent > of the kernel configuration" is already an impossibility, as I illustrated > in my previous message concerning Marvell Armada SoCs, and you also said > in your preceding paragraph! > > For example, if you clear the shared bit in the page tables on non-LPAE > SoCs, devices are no longer coherent. > > DMA coherence on ARM _is_ already tightly linked with the kernel > configuration. You already can't get away from that, so I think you > should give up trying to argue that point. :) > > Whether devices are DMA coherent is a combination of two things: > * is the device connected to a coherent bus. > * is the system setup to allow coherency on that bus to work. > > We capture the first through the dma-coherent property, which is clearly > a per-device property. We ignore the second because we assume everyone > is going to configure the CPU side correctly. That's untrue today, and > it's untrue not only because of Keystone II, but also because of other > SoCs as well which pre-date Keystone II. We currently miss out on > considering that, because if we ignore it, we get something that works > for most platforms. > I agree with Russell. When I added "dma-coherent" per device DT property, the intention was to distinguish certain devices which may not be coherent sitting on coherent fabric for some hardware reasons. > I don't see that adding a dma-outer-coherent property helps this - it's > muddying the waters somewhat - and it's also forcing additional complexity > into places where we shouldn't have it. We would need to parse two > properties in the DMA API code, and then combine it with knowledge as > to how the system page tables have been setup. If they've been setup > as inner sharable, then dma-coherent identifies whether the device is > coherent. If they've been setup as outer sharable, then > dma-outer-coherent specifies that and dma-coherent is meaningless. > > Sounds like a recipe for confusion. > Exactly. We should leave the "dma-coherent" property to mark coherent vs non coherent device(s). The inner vs outer is really page table ARCH setup issue and should be handled exactly the way it was done first place to handle the special memory view(outside 4 GB). Keystone needs outer shared bit set while setting up MMU pages which is best done in MMU off mode while recreating the new page tables. Regards, Santosh ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20160606123210.GL1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>]
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback [not found] ` <20160606123210.GL1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> @ 2016-06-07 10:01 ` Mark Rutland 2016-06-07 12:32 ` Russell King - ARM Linux 0 siblings, 1 reply; 14+ messages in thread From: Mark Rutland @ 2016-06-07 10:01 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Bill Mills, t-kristo-l0cyMroinI0, ssantosh-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, r-woodruff2-l0cyMroinI0, devicetree-u79uwXL29TY76Z2rM5mHXA On Mon, Jun 06, 2016 at 01:32:10PM +0100, Russell King - ARM Linux wrote: > On Mon, Jun 06, 2016 at 12:59:18PM +0100, Mark Rutland wrote: > > I agree that whether or not devices are coherent in practice depends on > > the kernel's configuration. The flip side, as you point out, is that > > devices are coherent when a specific set of attributes are used. > > > > i.e. that if you read dma-coherent as meaning "coherent iff Normal, > > Inner Shareable, Inner WB Cacheable, Outer WB Cacheable", then > > dma-coherent consistently describes the same thing, rather than > > depending on the configuration of the OS. > > > > DT is a datastructure provided to the kernel, potentially without deep > > internal knowledge of that kernel configuration. Having a consistent > > rule that is independent of the kernel configuration seems worth aiming > > for. > > I think you've missed the point. dma-coherent is _already_ dependent on > the kernel configuration. I understood this point. Please, allow me to clarify below, as I've evidently not done a good job so far. > "Having a consistent rule that is independent of the kernel > configuration" is already an impossibility, as I illustrated in my > previous message concerning Marvell Armada SoCs, and you also said in > your preceding paragraph! That's not quite what I said. What I said was that whether or not you end up with coherency depends on the kernel's configuration. That's why I pointed out that in practice, the only cases that work with today's mainline kernels are this for which the devices which are coherent with the kernel's usual memory attributes in an SMP configuration. If you grep for dma-coherent in arch/arm/boot/dts, you'll find that appears in: arch/arm/boot/dts/artpec6.dtsi arch/arm/boot/dts/ecx-common.dtsi arch/arm/boot/dts/ls1021a.dtsi Which are all SMP Cortex-{A7,A9,A15} platforms, and: arch/arm/boot/dts/k2e.dtsi arch/arm/boot/dts/k2e-netcp.dtsi arch/arm/boot/dts/k2hk-netcp.dtsi arch/arm/boot/dts/k2l-netcp.dtsi arch/arm/boot/dts/keystone.dtsi For which, as far as I am aware, the dma-coherent property does not yield coherency with a mainline kernel, due to the requirement of Outer Shareable attributes. So, if we codify the dma-coherent semantics as only matching the working case today, then it becomes consistent and independent of kernel configuration, and we can add properties to cater for the other cases, independent of kernel configuration. > For example, if you clear the shared bit in the page tables on non-LPAE > SoCs, devices are no longer coherent. Yes. This is a problem, but one that we already face. If we clarified the semantics as above, we would know that the device is simply not coherent. > DMA coherence on ARM _is_ already tightly linked with the kernel > configuration. You already can't get away from that, so I think you > should give up trying to argue that point. :) I hope that I've clarified my position w.r.t. coherence vs specification thereof. :) > Whether devices are DMA coherent is a combination of two things: > * is the device connected to a coherent bus. > * is the system setup to allow coherency on that bus to work. > > We capture the first through the dma-coherent property, which is clearly > a per-device property. We ignore the second because we assume everyone > is going to configure the CPU side correctly. That's untrue today, and > it's untrue not only because of Keystone II, but also because of other > SoCs as well which pre-date Keystone II. We currently miss out on > considering that, because if we ignore it, we get something that works > for most platforms. > > I don't see that adding a dma-outer-coherent property helps this - it's > muddying the waters somewhat - and it's also forcing additional complexity > into places where we shouldn't have it. We would need to parse two > properties in the DMA API code, and then combine it with knowledge as > to how the system page tables have been setup. If they've been setup > as inner sharable, then dma-coherent identifies whether the device is > coherent. If they've been setup as outer sharable, then > dma-outer-coherent specifies that and dma-coherent is meaningless. I think that at minimum, the attributes devices require needs to be describe to the kernel, rather than being something we hope just happened to match. > Sounds like a recipe for confusion. Unfortunately, I think everything in this area leads to confusion. :( Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback 2016-06-07 10:01 ` Mark Rutland @ 2016-06-07 12:32 ` Russell King - ARM Linux [not found] ` <20160607123248.GO1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> 0 siblings, 1 reply; 14+ messages in thread From: Russell King - ARM Linux @ 2016-06-07 12:32 UTC (permalink / raw) To: Mark Rutland Cc: Bill Mills, t-kristo, ssantosh, catalin.marinas, linux-arm-kernel, linux-kernel, r-woodruff2, devicetree On Tue, Jun 07, 2016 at 11:01:43AM +0100, Mark Rutland wrote: > So, if we codify the dma-coherent semantics as only matching the working > case today, then it becomes consistent and independent of kernel > configuration, and we can add properties to cater for the other cases, > independent of kernel configuration. That's where our points of view differ. You claim that it becomes independent of the kernel configuration. I'm saying that's total rubbish, because it's dependent on the kernel setting the CPU page tables up as it does today. If we set them up differently, then it doesn't work so well. This is evidenced by Marvell Armada uniprocessor platforms, where they are DMA coherent provided that the S bit is set. However, because they are uniprocessor platforms, the kernel sets the page tables up with the S bit clear. That means that the kernel configures the system in a way which results in it being non-coherent. So here, we have an example of why your position is actually incorrect. dma-coherent does *not* give a "consistent and independent of kernel configuration" property - it's inherently tied to how the kernel has setup the page tables. So, dma-coherent is coherent _provided_ the kernel sets the page tables up as we currently expect it to - the S bit set on non-LPAE systems, on LPAE systems, inner-sharable. If we deviate from that, (eg by clearing the S bit on non-LPAE systems) we end up with a non-coherent system, even if dma-coherent is specified. The Keystone II case is no different - Keystone II is coherent if the correct conditions are met with the CPU page tables. The only difference is that it's a slightly different set of conditions. > > For example, if you clear the shared bit in the page tables on non-LPAE > > SoCs, devices are no longer coherent. > > Yes. This is a problem, but one that we already face. If we clarified > the semantics as above, we would know that the device is simply not > coherent. How? We would need to introduce some flag which is passed from the architecture code into the OF code to disable the effect of dma-coherent, making of_dma_is_coherent() return false if the S bit is clear. > > Whether devices are DMA coherent is a combination of two things: > > * is the device connected to a coherent bus. > > * is the system setup to allow coherency on that bus to work. > > > > We capture the first through the dma-coherent property, which is clearly > > a per-device property. We ignore the second because we assume everyone > > is going to configure the CPU side correctly. That's untrue today, and > > it's untrue not only because of Keystone II, but also because of other > > SoCs as well which pre-date Keystone II. We currently miss out on > > considering that, because if we ignore it, we get something that works > > for most platforms. > > > > I don't see that adding a dma-outer-coherent property helps this - it's > > muddying the waters somewhat - and it's also forcing additional complexity > > into places where we shouldn't have it. We would need to parse two > > properties in the DMA API code, and then combine it with knowledge as > > to how the system page tables have been setup. If they've been setup > > as inner sharable, then dma-coherent identifies whether the device is > > coherent. If they've been setup as outer sharable, then > > dma-outer-coherent specifies that and dma-coherent is meaningless. > > I think that at minimum, the attributes devices require needs to be > describe to the kernel, rather than being something we hope just > happened to match. Yuck. Seriously? What happens when we have two devices which have different required attributes for the CPU mapping? Should architecture code have to parse the entire DT tree to work out what attributes each device needs, and try to then work out how the CPU page tables should be setup? I really don't think that's a good idea. -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up according to speedtest.net. ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <20160607123248.GO1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org>]
* Re: [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback [not found] ` <20160607123248.GO1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> @ 2016-06-07 12:55 ` Mark Rutland 0 siblings, 0 replies; 14+ messages in thread From: Mark Rutland @ 2016-06-07 12:55 UTC (permalink / raw) To: Russell King - ARM Linux Cc: Bill Mills, t-kristo-l0cyMroinI0, ssantosh-DgEjT+Ai2ygdnm+yROfE0A, catalin.marinas-5wv7dgnIgG8, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, linux-kernel-u79uwXL29TY76Z2rM5mHXA, r-woodruff2-l0cyMroinI0, devicetree-u79uwXL29TY76Z2rM5mHXA On Tue, Jun 07, 2016 at 01:32:48PM +0100, Russell King - ARM Linux wrote: > On Tue, Jun 07, 2016 at 11:01:43AM +0100, Mark Rutland wrote: > > So, if we codify the dma-coherent semantics as only matching the working > > case today, then it becomes consistent and independent of kernel > > configuration, and we can add properties to cater for the other cases, > > independent of kernel configuration. > > That's where our points of view differ. You claim that it becomes > independent of the kernel configuration. I'm saying that's total > rubbish, because it's dependent on the kernel setting the CPU page > tables up as it does today. The key point is that *description* of the requirements for coherency becomes independent of kernel configuration. Yes, whether or not a kernel can support those depends on the configuration. > If we set them up differently, then it doesn't work so well. This > is evidenced by Marvell Armada uniprocessor platforms, where they > are DMA coherent provided that the S bit is set. However, because > they are uniprocessor platforms, the kernel sets the page tables up > with the S bit clear. That means that the kernel configures the > system in a way which results in it being non-coherent. > > So here, we have an example of why your position is actually incorrect. > dma-coherent does *not* give a "consistent and independent of kernel > configuration" property - it's inherently tied to how the kernel has > setup the page tables. Sorry, but that is not quite what I said. I said that if you read dma-coherent as specifying *the requirements* for coherency (i.e. "coherent iff Normal, Inner Shareable, Inner WB Cacheable, Outer WB Cacheable"), rather than specifying that there is coherency given some unspecified requirements, then it is possible to use it in a manner which is consistent and independent of kernel configuration. If the kernel uses memory attributes that don't meet those requirements, it can know that the device cannot be used in a coherent manner. If we take that stance, then we can cater for other requirements (e.g. Outer Shareable on Keystone) by having properties to specify those requirements (e.g. dma-outer-coherent). The tricky part is how the kernel decides how best to use that information, but that is a problem regardless. > > > For example, if you clear the shared bit in the page tables on non-LPAE > > > SoCs, devices are no longer coherent. > > > > Yes. This is a problem, but one that we already face. If we clarified > > the semantics as above, we would know that the device is simply not > > coherent. > > How? We would need to introduce some flag which is passed from the > architecture code into the OF code to disable the effect of dma-coherent, > making of_dma_is_coherent() return false if the S bit is clear. Yes, we would need to either alter the OF code, or some code which makes use of this. Surely it's possible to have this logic in an arch callback? > > > Whether devices are DMA coherent is a combination of two things: > > > * is the device connected to a coherent bus. > > > * is the system setup to allow coherency on that bus to work. > > > > > > We capture the first through the dma-coherent property, which is clearly > > > a per-device property. We ignore the second because we assume everyone > > > is going to configure the CPU side correctly. That's untrue today, and > > > it's untrue not only because of Keystone II, but also because of other > > > SoCs as well which pre-date Keystone II. We currently miss out on > > > considering that, because if we ignore it, we get something that works > > > for most platforms. > > > > > > I don't see that adding a dma-outer-coherent property helps this - it's > > > muddying the waters somewhat - and it's also forcing additional complexity > > > into places where we shouldn't have it. We would need to parse two > > > properties in the DMA API code, and then combine it with knowledge as > > > to how the system page tables have been setup. If they've been setup > > > as inner sharable, then dma-coherent identifies whether the device is > > > coherent. If they've been setup as outer sharable, then > > > dma-outer-coherent specifies that and dma-coherent is meaningless. > > > > I think that at minimum, the attributes devices require needs to be > > describe to the kernel, rather than being something we hope just > > happened to match. > > Yuck. Seriously? What happens when we have two devices which have > different required attributes for the CPU mapping? Should architecture > code have to parse the entire DT tree to work out what attributes each > device needs, and try to then work out how the CPU page tables should > be setup? No, we do not necessarily have to try to dynamically handle every possible case, especially as the vastly common case is the one I called out above. For those boards where we're going to have some code special-casing those regardless, automatically deciding to have the kernel use the preferred set of attributes is fine. However, to do this I don't think we should provide board+kernel specific semantics to dma-coherent, and should at least precisely specify the coherency requirements. Thanks, Mark. -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-06-07 12:55 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <1465183229-24147-1-git-send-email-wmills@ti.com> [not found] ` <1465183229-24147-5-git-send-email-wmills@ti.com> 2016-06-06 8:56 ` [RFC v2 4/4] ARM: keystone: dma-coherent with safe fallback Mark Rutland 2016-06-06 9:09 ` Arnd Bergmann 2016-06-06 11:42 ` Mark Rutland 2016-06-06 12:37 ` Arnd Bergmann 2016-06-06 12:50 ` William Mills 2016-06-06 16:18 ` Santosh Shilimkar 2016-06-06 11:43 ` Russell King - ARM Linux 2016-06-06 11:59 ` Mark Rutland 2016-06-06 12:19 ` William Mills 2016-06-06 12:32 ` Russell King - ARM Linux 2016-06-06 16:28 ` Santosh Shilimkar [not found] ` <20160606123210.GL1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> 2016-06-07 10:01 ` Mark Rutland 2016-06-07 12:32 ` Russell King - ARM Linux [not found] ` <20160607123248.GO1041-l+eeeJia6m9URfEZ8mYm6t73F7V6hmMc@public.gmane.org> 2016-06-07 12:55 ` Mark Rutland
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).