Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
05.11.2020 18:49, Thierry Reding пишет: > On Thu, Sep 24, 2020 at 07:23:34PM +0300, Dmitry Osipenko wrote: >> 24.09.2020 17:01, Thierry Reding пишет: >>> On Thu, Sep 24, 2020 at 04:23:59PM +0300, Dmitry Osipenko wrote: 04.09.2020 15:59, Thierry Reding пишет: > From: Thierry Reding > > Reserved memory regions can be marked as "active" if hardware is > expected to access the regions during boot and before the operating > system can take control. One example where this is useful is for the > operating system to infer whether the region needs to be identity- > mapped through an IOMMU. > > Signed-off-by: Thierry Reding > --- > .../bindings/reserved-memory/reserved-memory.txt | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > index 4dd20de6977f..163d2927e4fc 100644 > --- > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > +++ > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > @@ -63,6 +63,13 @@ reusable (optional) - empty property >able to reclaim it back. Typically that means that the operating >system can use that region to store volatile or cached data that >can be otherwise regenerated or migrated elsewhere. > +active (optional) - empty property > +- If this property is set for a reserved memory region, it indicates > + that some piece of hardware may be actively accessing this region. > + Should the operating system want to enable IOMMU protection for a > + device, all active memory regions must have been identity-mapped > + in order to ensure that non-quiescent hardware during boot can > + continue to access the memory. > > Linux implementation note: > - If a "linux,cma-default" property is present, then Linux will use the > Hi, Could you please explain what devices need this quirk? I see that you're targeting Tegra SMMU driver, which means that it should be some pre-T186 device. >>> >>> Primarily I'm looking at Tegra210 and later, because on earlier devices >>> the bootloader doesn't consistently initialize display. I know that it >>> does on some devices, but not all of them. >> >> AFAIK, all tablet devices starting with Tegra20 that have display panel >> are initializing display at a boot time for showing splash screen. This >> includes all T20/T30/T114 tablets that are already supported by upstream >> kernel. >> >>> This same code should also >>> work on Tegra186 and later (with an ARM SMMU) although the situation is >>> slightly more complicated there because IOMMU translations will fault by >>> default long before these identity mappings can be established. >>> Is this reservation needed for some device that has display hardwired to a very specific IOMMU domain at the boot time? >>> >>> No, this is only used to convey information about the active framebuffer >>> to the kernel. In practice the DMA/IOMMU code will use this information >>> to establish a 1:1 mapping on whatever IOMMU domain that was picked for >>> display. >>> If you're targeting devices that don't have IOMMU enabled by default at the boot time, then this approach won't work for the existing devices which won't ever get an updated bootloader. >>> >>> If the devices don't use an IOMMU, then there should be no problem. The >>> extra reserved-memory nodes would still be necessary to ensure that the >>> kernel doesn't reuse the framebuffer memory for the slab allocator, but >>> if no IOMMU is used, then the display controller accessing the memory >>> isn't going to cause problems other than perhaps scanning out data that >>> is no longer a framebuffer. >>> >>> There should also be no problem for devices with an old bootloader >>> because this code is triggered by the presence of a reserved-memory node >>> referenced via the memory-region property. Devices with an old >>> bootloader should continue to work as they did before. Although I >>> suppose they would start faulting once we enable DMA/IOMMU integration >>> for Tegra SMMU if they have a bootloader that does initialize display to >>> actively scan out during boot. >>> I think Robin Murphy already suggested that we should simply create a dummy "identity" IOMMU domain by default for the DRM/VDE devices and then replace it with an explicitly created domain within the drivers. >>> >>> I don't recall reading about that suggestion. So does this mean that for >>> certain devices we'd want to basically passthrough by default and then >>> at some point during boot take over with a properly managed IOMMU >>> domain? >> >> Yes, my understanding that this is what Robin suggested here: >> >> https://lore.
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Fri, Dec 18, 2020 at 04:15:45PM -0600, Rob Herring wrote: > On Thu, Dec 17, 2020 at 9:00 AM Thierry Reding > wrote: > > > > On Tue, Nov 10, 2020 at 08:33:09PM +0100, Thierry Reding wrote: > > > On Fri, Nov 06, 2020 at 04:25:48PM +0100, Thierry Reding wrote: > > > > On Thu, Nov 05, 2020 at 05:47:21PM +, Robin Murphy wrote: > > > > > On 2020-11-05 16:43, Thierry Reding wrote: > > > > > > On Thu, Sep 24, 2020 at 01:27:25PM +0200, Thierry Reding wrote: > > > > > > > On Tue, Sep 15, 2020 at 02:36:48PM +0200, Thierry Reding wrote: > > > > > > > > On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: > > > > > > > > > On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding > > > > > > > > > wrote: > > > > > > > > > > From: Thierry Reding > > > > > > > > > > > > > > > > > > > > Reserved memory regions can be marked as "active" if > > > > > > > > > > hardware is > > > > > > > > > > expected to access the regions during boot and before the > > > > > > > > > > operating > > > > > > > > > > system can take control. One example where this is useful > > > > > > > > > > is for the > > > > > > > > > > operating system to infer whether the region needs to be > > > > > > > > > > identity- > > > > > > > > > > mapped through an IOMMU. > > > > > > > > > > > > > > > > > > I like simple solutions, but this hardly seems adequate to > > > > > > > > > solve the > > > > > > > > > problem of passing IOMMU setup from bootloader/firmware to > > > > > > > > > the OS. Like > > > > > > > > > what is the IOVA that's supposed to be used if identity > > > > > > > > > mapping is not > > > > > > > > > used? > > > > > > > > > > > > > > > > The assumption here is that if the region is not active there > > > > > > > > is no need > > > > > > > > for the IOVA to be specified because the kernel will allocate > > > > > > > > memory and > > > > > > > > assign any IOVA of its choosing. > > > > > > > > > > > > > > > > Also, note that this is not meant as a way of passing IOMMU > > > > > > > > setup from > > > > > > > > the bootloader or firmware to the OS. The purpose of this is to > > > > > > > > specify > > > > > > > > that some region of memory is actively being accessed during > > > > > > > > boot. The > > > > > > > > particular case that I'm looking at is where the bootloader set > > > > > > > > up a > > > > > > > > splash screen and keeps it on during boot. The bootloader has > > > > > > > > not set up > > > > > > > > an IOMMU mapping and the identity mapping serves as a way of > > > > > > > > keeping the > > > > > > > > accesses by the display hardware working during the > > > > > > > > transitional period > > > > > > > > after the IOMMU translations have been enabled by the kernel > > > > > > > > but before > > > > > > > > the kernel display driver has had a chance to set up its own > > > > > > > > IOMMU > > > > > > > > mappings. > > > > > > > > > > > > > > > > > If you know enough about the regions to assume identity > > > > > > > > > mapping, then > > > > > > > > > can't you know if active or not? > > > > > > > > > > > > > > > > We could alternatively add some property that describes the > > > > > > > > region as > > > > > > > > requiring an identity mapping. But note that we can't make any > > > > > > > > assumptions here about the usage of these regions because the > > > > > > > > IOMMU > > > > > > > > driver simply has no way of knowing what they are being used > > > > > > > > for. > > > > > > > > > > > > > > > > Some additional information is required in device tree for the > > > > > > > > IOMMU > > > > > > > > driver to be able to make that decision. > > > > > > > > > > > > > > Rob, can you provide any hints on exactly how you want to move > > > > > > > this > > > > > > > forward? I don't know in what direction you'd like to proceed. > > > > > > > > > > > > Hi Rob, > > > > > > > > > > > > do you have any suggestions on how to proceed with this? I'd like > > > > > > to get > > > > > > this moving again because it's something that's been nagging me for > > > > > > some > > > > > > months now. It also requires changes across two levels in the > > > > > > bootloader > > > > > > stack as well as Linux and it takes quite a bit of work to make all > > > > > > the > > > > > > changes, so before I go and rewrite everything I'd like to get the > > > > > > DT > > > > > > bindings sorted out first. > > > > > > > > > > > > So just to summarize why I think this simple solution is good > > > > > > enough: it > > > > > > tries to solve a very narrow and simple problem. This is not an > > > > > > attempt > > > > > > at describing the firmware's full IOMMU setup to the kernel. In > > > > > > fact, it > > > > > > is primarily targetted at cases where the firmware hasn't setup an > > > > > > IOMMU > > > > > > at all, and we just want to make sure that when the kernel takes > > > > > > over > > > > > > and does want to enable the IOMMU, that all the regions that are > > > > > > actively being accessed by no
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Thu, Dec 17, 2020 at 9:00 AM Thierry Reding wrote: > > On Tue, Nov 10, 2020 at 08:33:09PM +0100, Thierry Reding wrote: > > On Fri, Nov 06, 2020 at 04:25:48PM +0100, Thierry Reding wrote: > > > On Thu, Nov 05, 2020 at 05:47:21PM +, Robin Murphy wrote: > > > > On 2020-11-05 16:43, Thierry Reding wrote: > > > > > On Thu, Sep 24, 2020 at 01:27:25PM +0200, Thierry Reding wrote: > > > > > > On Tue, Sep 15, 2020 at 02:36:48PM +0200, Thierry Reding wrote: > > > > > > > On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: > > > > > > > > On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: > > > > > > > > > From: Thierry Reding > > > > > > > > > > > > > > > > > > Reserved memory regions can be marked as "active" if hardware > > > > > > > > > is > > > > > > > > > expected to access the regions during boot and before the > > > > > > > > > operating > > > > > > > > > system can take control. One example where this is useful is > > > > > > > > > for the > > > > > > > > > operating system to infer whether the region needs to be > > > > > > > > > identity- > > > > > > > > > mapped through an IOMMU. > > > > > > > > > > > > > > > > I like simple solutions, but this hardly seems adequate to > > > > > > > > solve the > > > > > > > > problem of passing IOMMU setup from bootloader/firmware to the > > > > > > > > OS. Like > > > > > > > > what is the IOVA that's supposed to be used if identity mapping > > > > > > > > is not > > > > > > > > used? > > > > > > > > > > > > > > The assumption here is that if the region is not active there is > > > > > > > no need > > > > > > > for the IOVA to be specified because the kernel will allocate > > > > > > > memory and > > > > > > > assign any IOVA of its choosing. > > > > > > > > > > > > > > Also, note that this is not meant as a way of passing IOMMU setup > > > > > > > from > > > > > > > the bootloader or firmware to the OS. The purpose of this is to > > > > > > > specify > > > > > > > that some region of memory is actively being accessed during > > > > > > > boot. The > > > > > > > particular case that I'm looking at is where the bootloader set > > > > > > > up a > > > > > > > splash screen and keeps it on during boot. The bootloader has not > > > > > > > set up > > > > > > > an IOMMU mapping and the identity mapping serves as a way of > > > > > > > keeping the > > > > > > > accesses by the display hardware working during the transitional > > > > > > > period > > > > > > > after the IOMMU translations have been enabled by the kernel but > > > > > > > before > > > > > > > the kernel display driver has had a chance to set up its own IOMMU > > > > > > > mappings. > > > > > > > > > > > > > > > If you know enough about the regions to assume identity > > > > > > > > mapping, then > > > > > > > > can't you know if active or not? > > > > > > > > > > > > > > We could alternatively add some property that describes the > > > > > > > region as > > > > > > > requiring an identity mapping. But note that we can't make any > > > > > > > assumptions here about the usage of these regions because the > > > > > > > IOMMU > > > > > > > driver simply has no way of knowing what they are being used for. > > > > > > > > > > > > > > Some additional information is required in device tree for the > > > > > > > IOMMU > > > > > > > driver to be able to make that decision. > > > > > > > > > > > > Rob, can you provide any hints on exactly how you want to move this > > > > > > forward? I don't know in what direction you'd like to proceed. > > > > > > > > > > Hi Rob, > > > > > > > > > > do you have any suggestions on how to proceed with this? I'd like to > > > > > get > > > > > this moving again because it's something that's been nagging me for > > > > > some > > > > > months now. It also requires changes across two levels in the > > > > > bootloader > > > > > stack as well as Linux and it takes quite a bit of work to make all > > > > > the > > > > > changes, so before I go and rewrite everything I'd like to get the DT > > > > > bindings sorted out first. > > > > > > > > > > So just to summarize why I think this simple solution is good enough: > > > > > it > > > > > tries to solve a very narrow and simple problem. This is not an > > > > > attempt > > > > > at describing the firmware's full IOMMU setup to the kernel. In fact, > > > > > it > > > > > is primarily targetted at cases where the firmware hasn't setup an > > > > > IOMMU > > > > > at all, and we just want to make sure that when the kernel takes over > > > > > and does want to enable the IOMMU, that all the regions that are > > > > > actively being accessed by non-quiesced hardware (the most typical > > > > > example would be a framebuffer scanning out a splat screen or > > > > > animation, > > > > > but it could equally well be some sort of welcoming tone or music > > > > > being > > > > > played back) are described in device tree. > > > > > > > > > > In other words, and this is perhaps better answering y
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Tue, Nov 10, 2020 at 08:33:09PM +0100, Thierry Reding wrote: > On Fri, Nov 06, 2020 at 04:25:48PM +0100, Thierry Reding wrote: > > On Thu, Nov 05, 2020 at 05:47:21PM +, Robin Murphy wrote: > > > On 2020-11-05 16:43, Thierry Reding wrote: > > > > On Thu, Sep 24, 2020 at 01:27:25PM +0200, Thierry Reding wrote: > > > > > On Tue, Sep 15, 2020 at 02:36:48PM +0200, Thierry Reding wrote: > > > > > > On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: > > > > > > > On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: > > > > > > > > From: Thierry Reding > > > > > > > > > > > > > > > > Reserved memory regions can be marked as "active" if hardware is > > > > > > > > expected to access the regions during boot and before the > > > > > > > > operating > > > > > > > > system can take control. One example where this is useful is > > > > > > > > for the > > > > > > > > operating system to infer whether the region needs to be > > > > > > > > identity- > > > > > > > > mapped through an IOMMU. > > > > > > > > > > > > > > I like simple solutions, but this hardly seems adequate to solve > > > > > > > the > > > > > > > problem of passing IOMMU setup from bootloader/firmware to the > > > > > > > OS. Like > > > > > > > what is the IOVA that's supposed to be used if identity mapping > > > > > > > is not > > > > > > > used? > > > > > > > > > > > > The assumption here is that if the region is not active there is no > > > > > > need > > > > > > for the IOVA to be specified because the kernel will allocate > > > > > > memory and > > > > > > assign any IOVA of its choosing. > > > > > > > > > > > > Also, note that this is not meant as a way of passing IOMMU setup > > > > > > from > > > > > > the bootloader or firmware to the OS. The purpose of this is to > > > > > > specify > > > > > > that some region of memory is actively being accessed during boot. > > > > > > The > > > > > > particular case that I'm looking at is where the bootloader set up a > > > > > > splash screen and keeps it on during boot. The bootloader has not > > > > > > set up > > > > > > an IOMMU mapping and the identity mapping serves as a way of > > > > > > keeping the > > > > > > accesses by the display hardware working during the transitional > > > > > > period > > > > > > after the IOMMU translations have been enabled by the kernel but > > > > > > before > > > > > > the kernel display driver has had a chance to set up its own IOMMU > > > > > > mappings. > > > > > > > > > > > > > If you know enough about the regions to assume identity mapping, > > > > > > > then > > > > > > > can't you know if active or not? > > > > > > > > > > > > We could alternatively add some property that describes the region > > > > > > as > > > > > > requiring an identity mapping. But note that we can't make any > > > > > > assumptions here about the usage of these regions because the IOMMU > > > > > > driver simply has no way of knowing what they are being used for. > > > > > > > > > > > > Some additional information is required in device tree for the IOMMU > > > > > > driver to be able to make that decision. > > > > > > > > > > Rob, can you provide any hints on exactly how you want to move this > > > > > forward? I don't know in what direction you'd like to proceed. > > > > > > > > Hi Rob, > > > > > > > > do you have any suggestions on how to proceed with this? I'd like to get > > > > this moving again because it's something that's been nagging me for some > > > > months now. It also requires changes across two levels in the bootloader > > > > stack as well as Linux and it takes quite a bit of work to make all the > > > > changes, so before I go and rewrite everything I'd like to get the DT > > > > bindings sorted out first. > > > > > > > > So just to summarize why I think this simple solution is good enough: it > > > > tries to solve a very narrow and simple problem. This is not an attempt > > > > at describing the firmware's full IOMMU setup to the kernel. In fact, it > > > > is primarily targetted at cases where the firmware hasn't setup an IOMMU > > > > at all, and we just want to make sure that when the kernel takes over > > > > and does want to enable the IOMMU, that all the regions that are > > > > actively being accessed by non-quiesced hardware (the most typical > > > > example would be a framebuffer scanning out a splat screen or animation, > > > > but it could equally well be some sort of welcoming tone or music being > > > > played back) are described in device tree. > > > > > > > > In other words, and this is perhaps better answering your second > > > > question: in addition to describing reserved memory regions, we want to > > > > add a bit of information here about the usage of these memory regions. > > > > Some memory regions may contain information that the kernel may want to > > > > use (such an external memory frequency scaling tables) and those I would > > > > describe as "inactive" memory because it isn't being acc
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Fri, Nov 06, 2020 at 04:25:48PM +0100, Thierry Reding wrote: > On Thu, Nov 05, 2020 at 05:47:21PM +, Robin Murphy wrote: > > On 2020-11-05 16:43, Thierry Reding wrote: > > > On Thu, Sep 24, 2020 at 01:27:25PM +0200, Thierry Reding wrote: > > > > On Tue, Sep 15, 2020 at 02:36:48PM +0200, Thierry Reding wrote: > > > > > On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: > > > > > > On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: > > > > > > > From: Thierry Reding > > > > > > > > > > > > > > Reserved memory regions can be marked as "active" if hardware is > > > > > > > expected to access the regions during boot and before the > > > > > > > operating > > > > > > > system can take control. One example where this is useful is for > > > > > > > the > > > > > > > operating system to infer whether the region needs to be identity- > > > > > > > mapped through an IOMMU. > > > > > > > > > > > > I like simple solutions, but this hardly seems adequate to solve the > > > > > > problem of passing IOMMU setup from bootloader/firmware to the OS. > > > > > > Like > > > > > > what is the IOVA that's supposed to be used if identity mapping is > > > > > > not > > > > > > used? > > > > > > > > > > The assumption here is that if the region is not active there is no > > > > > need > > > > > for the IOVA to be specified because the kernel will allocate memory > > > > > and > > > > > assign any IOVA of its choosing. > > > > > > > > > > Also, note that this is not meant as a way of passing IOMMU setup from > > > > > the bootloader or firmware to the OS. The purpose of this is to > > > > > specify > > > > > that some region of memory is actively being accessed during boot. The > > > > > particular case that I'm looking at is where the bootloader set up a > > > > > splash screen and keeps it on during boot. The bootloader has not set > > > > > up > > > > > an IOMMU mapping and the identity mapping serves as a way of keeping > > > > > the > > > > > accesses by the display hardware working during the transitional > > > > > period > > > > > after the IOMMU translations have been enabled by the kernel but > > > > > before > > > > > the kernel display driver has had a chance to set up its own IOMMU > > > > > mappings. > > > > > > > > > > > If you know enough about the regions to assume identity mapping, > > > > > > then > > > > > > can't you know if active or not? > > > > > > > > > > We could alternatively add some property that describes the region as > > > > > requiring an identity mapping. But note that we can't make any > > > > > assumptions here about the usage of these regions because the IOMMU > > > > > driver simply has no way of knowing what they are being used for. > > > > > > > > > > Some additional information is required in device tree for the IOMMU > > > > > driver to be able to make that decision. > > > > > > > > Rob, can you provide any hints on exactly how you want to move this > > > > forward? I don't know in what direction you'd like to proceed. > > > > > > Hi Rob, > > > > > > do you have any suggestions on how to proceed with this? I'd like to get > > > this moving again because it's something that's been nagging me for some > > > months now. It also requires changes across two levels in the bootloader > > > stack as well as Linux and it takes quite a bit of work to make all the > > > changes, so before I go and rewrite everything I'd like to get the DT > > > bindings sorted out first. > > > > > > So just to summarize why I think this simple solution is good enough: it > > > tries to solve a very narrow and simple problem. This is not an attempt > > > at describing the firmware's full IOMMU setup to the kernel. In fact, it > > > is primarily targetted at cases where the firmware hasn't setup an IOMMU > > > at all, and we just want to make sure that when the kernel takes over > > > and does want to enable the IOMMU, that all the regions that are > > > actively being accessed by non-quiesced hardware (the most typical > > > example would be a framebuffer scanning out a splat screen or animation, > > > but it could equally well be some sort of welcoming tone or music being > > > played back) are described in device tree. > > > > > > In other words, and this is perhaps better answering your second > > > question: in addition to describing reserved memory regions, we want to > > > add a bit of information here about the usage of these memory regions. > > > Some memory regions may contain information that the kernel may want to > > > use (such an external memory frequency scaling tables) and those I would > > > describe as "inactive" memory because it isn't being accessed by > > > hardware. The framebuffer in this case is the opposite and it is being > > > actively accessed (hence it is marked "active") by hardware while the > > > kernel is busy setting everything up so that it can reconfigure that > > > hardware and take over with its own framebuffer (for the console,
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Thu, Nov 05, 2020 at 05:47:21PM +, Robin Murphy wrote: > On 2020-11-05 16:43, Thierry Reding wrote: > > On Thu, Sep 24, 2020 at 01:27:25PM +0200, Thierry Reding wrote: > > > On Tue, Sep 15, 2020 at 02:36:48PM +0200, Thierry Reding wrote: > > > > On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: > > > > > On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: > > > > > > From: Thierry Reding > > > > > > > > > > > > Reserved memory regions can be marked as "active" if hardware is > > > > > > expected to access the regions during boot and before the operating > > > > > > system can take control. One example where this is useful is for the > > > > > > operating system to infer whether the region needs to be identity- > > > > > > mapped through an IOMMU. > > > > > > > > > > I like simple solutions, but this hardly seems adequate to solve the > > > > > problem of passing IOMMU setup from bootloader/firmware to the OS. > > > > > Like > > > > > what is the IOVA that's supposed to be used if identity mapping is not > > > > > used? > > > > > > > > The assumption here is that if the region is not active there is no need > > > > for the IOVA to be specified because the kernel will allocate memory and > > > > assign any IOVA of its choosing. > > > > > > > > Also, note that this is not meant as a way of passing IOMMU setup from > > > > the bootloader or firmware to the OS. The purpose of this is to specify > > > > that some region of memory is actively being accessed during boot. The > > > > particular case that I'm looking at is where the bootloader set up a > > > > splash screen and keeps it on during boot. The bootloader has not set up > > > > an IOMMU mapping and the identity mapping serves as a way of keeping the > > > > accesses by the display hardware working during the transitional period > > > > after the IOMMU translations have been enabled by the kernel but before > > > > the kernel display driver has had a chance to set up its own IOMMU > > > > mappings. > > > > > > > > > If you know enough about the regions to assume identity mapping, then > > > > > can't you know if active or not? > > > > > > > > We could alternatively add some property that describes the region as > > > > requiring an identity mapping. But note that we can't make any > > > > assumptions here about the usage of these regions because the IOMMU > > > > driver simply has no way of knowing what they are being used for. > > > > > > > > Some additional information is required in device tree for the IOMMU > > > > driver to be able to make that decision. > > > > > > Rob, can you provide any hints on exactly how you want to move this > > > forward? I don't know in what direction you'd like to proceed. > > > > Hi Rob, > > > > do you have any suggestions on how to proceed with this? I'd like to get > > this moving again because it's something that's been nagging me for some > > months now. It also requires changes across two levels in the bootloader > > stack as well as Linux and it takes quite a bit of work to make all the > > changes, so before I go and rewrite everything I'd like to get the DT > > bindings sorted out first. > > > > So just to summarize why I think this simple solution is good enough: it > > tries to solve a very narrow and simple problem. This is not an attempt > > at describing the firmware's full IOMMU setup to the kernel. In fact, it > > is primarily targetted at cases where the firmware hasn't setup an IOMMU > > at all, and we just want to make sure that when the kernel takes over > > and does want to enable the IOMMU, that all the regions that are > > actively being accessed by non-quiesced hardware (the most typical > > example would be a framebuffer scanning out a splat screen or animation, > > but it could equally well be some sort of welcoming tone or music being > > played back) are described in device tree. > > > > In other words, and this is perhaps better answering your second > > question: in addition to describing reserved memory regions, we want to > > add a bit of information here about the usage of these memory regions. > > Some memory regions may contain information that the kernel may want to > > use (such an external memory frequency scaling tables) and those I would > > describe as "inactive" memory because it isn't being accessed by > > hardware. The framebuffer in this case is the opposite and it is being > > actively accessed (hence it is marked "active") by hardware while the > > kernel is busy setting everything up so that it can reconfigure that > > hardware and take over with its own framebuffer (for the console, for > > example). It's also not so much that we know enough about the region to > > assume it needs identity mapping. We don't really care about that from > > the DT point of view. In fact, depending on the rest of the system > > configuration, we may not need identity mapping (i.e. if none of the > > users of the reserved memory region are behind an IO
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On 2020-11-05 16:43, Thierry Reding wrote: On Thu, Sep 24, 2020 at 01:27:25PM +0200, Thierry Reding wrote: On Tue, Sep 15, 2020 at 02:36:48PM +0200, Thierry Reding wrote: On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: From: Thierry Reding Reserved memory regions can be marked as "active" if hardware is expected to access the regions during boot and before the operating system can take control. One example where this is useful is for the operating system to infer whether the region needs to be identity- mapped through an IOMMU. I like simple solutions, but this hardly seems adequate to solve the problem of passing IOMMU setup from bootloader/firmware to the OS. Like what is the IOVA that's supposed to be used if identity mapping is not used? The assumption here is that if the region is not active there is no need for the IOVA to be specified because the kernel will allocate memory and assign any IOVA of its choosing. Also, note that this is not meant as a way of passing IOMMU setup from the bootloader or firmware to the OS. The purpose of this is to specify that some region of memory is actively being accessed during boot. The particular case that I'm looking at is where the bootloader set up a splash screen and keeps it on during boot. The bootloader has not set up an IOMMU mapping and the identity mapping serves as a way of keeping the accesses by the display hardware working during the transitional period after the IOMMU translations have been enabled by the kernel but before the kernel display driver has had a chance to set up its own IOMMU mappings. If you know enough about the regions to assume identity mapping, then can't you know if active or not? We could alternatively add some property that describes the region as requiring an identity mapping. But note that we can't make any assumptions here about the usage of these regions because the IOMMU driver simply has no way of knowing what they are being used for. Some additional information is required in device tree for the IOMMU driver to be able to make that decision. Rob, can you provide any hints on exactly how you want to move this forward? I don't know in what direction you'd like to proceed. Hi Rob, do you have any suggestions on how to proceed with this? I'd like to get this moving again because it's something that's been nagging me for some months now. It also requires changes across two levels in the bootloader stack as well as Linux and it takes quite a bit of work to make all the changes, so before I go and rewrite everything I'd like to get the DT bindings sorted out first. So just to summarize why I think this simple solution is good enough: it tries to solve a very narrow and simple problem. This is not an attempt at describing the firmware's full IOMMU setup to the kernel. In fact, it is primarily targetted at cases where the firmware hasn't setup an IOMMU at all, and we just want to make sure that when the kernel takes over and does want to enable the IOMMU, that all the regions that are actively being accessed by non-quiesced hardware (the most typical example would be a framebuffer scanning out a splat screen or animation, but it could equally well be some sort of welcoming tone or music being played back) are described in device tree. In other words, and this is perhaps better answering your second question: in addition to describing reserved memory regions, we want to add a bit of information here about the usage of these memory regions. Some memory regions may contain information that the kernel may want to use (such an external memory frequency scaling tables) and those I would describe as "inactive" memory because it isn't being accessed by hardware. The framebuffer in this case is the opposite and it is being actively accessed (hence it is marked "active") by hardware while the kernel is busy setting everything up so that it can reconfigure that hardware and take over with its own framebuffer (for the console, for example). It's also not so much that we know enough about the region to assume it needs identity mapping. We don't really care about that from the DT point of view. In fact, depending on the rest of the system configuration, we may not need identity mapping (i.e. if none of the users of the reserved memory region are behind an IOMMU). But the point here is that the IOMMU drivers can use this "active" property to determine that if a device is using an "active" region and it is behind an IOMMU, then it must identity map that region in order for the hardware, which is not under the kernel's control yet, to be able to continue to access that memory through an IOMMU mapping. Hmm, "active" is not a property of the memory itself, though, it's really a property of the device accessing it. If several distinct devices share a carveout region, and for simplicity the bootloader marks it as active because one of those device
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Thu, Sep 24, 2020 at 01:27:25PM +0200, Thierry Reding wrote: > On Tue, Sep 15, 2020 at 02:36:48PM +0200, Thierry Reding wrote: > > On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: > > > On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: > > > > From: Thierry Reding > > > > > > > > Reserved memory regions can be marked as "active" if hardware is > > > > expected to access the regions during boot and before the operating > > > > system can take control. One example where this is useful is for the > > > > operating system to infer whether the region needs to be identity- > > > > mapped through an IOMMU. > > > > > > I like simple solutions, but this hardly seems adequate to solve the > > > problem of passing IOMMU setup from bootloader/firmware to the OS. Like > > > what is the IOVA that's supposed to be used if identity mapping is not > > > used? > > > > The assumption here is that if the region is not active there is no need > > for the IOVA to be specified because the kernel will allocate memory and > > assign any IOVA of its choosing. > > > > Also, note that this is not meant as a way of passing IOMMU setup from > > the bootloader or firmware to the OS. The purpose of this is to specify > > that some region of memory is actively being accessed during boot. The > > particular case that I'm looking at is where the bootloader set up a > > splash screen and keeps it on during boot. The bootloader has not set up > > an IOMMU mapping and the identity mapping serves as a way of keeping the > > accesses by the display hardware working during the transitional period > > after the IOMMU translations have been enabled by the kernel but before > > the kernel display driver has had a chance to set up its own IOMMU > > mappings. > > > > > If you know enough about the regions to assume identity mapping, then > > > can't you know if active or not? > > > > We could alternatively add some property that describes the region as > > requiring an identity mapping. But note that we can't make any > > assumptions here about the usage of these regions because the IOMMU > > driver simply has no way of knowing what they are being used for. > > > > Some additional information is required in device tree for the IOMMU > > driver to be able to make that decision. > > Rob, can you provide any hints on exactly how you want to move this > forward? I don't know in what direction you'd like to proceed. Hi Rob, do you have any suggestions on how to proceed with this? I'd like to get this moving again because it's something that's been nagging me for some months now. It also requires changes across two levels in the bootloader stack as well as Linux and it takes quite a bit of work to make all the changes, so before I go and rewrite everything I'd like to get the DT bindings sorted out first. So just to summarize why I think this simple solution is good enough: it tries to solve a very narrow and simple problem. This is not an attempt at describing the firmware's full IOMMU setup to the kernel. In fact, it is primarily targetted at cases where the firmware hasn't setup an IOMMU at all, and we just want to make sure that when the kernel takes over and does want to enable the IOMMU, that all the regions that are actively being accessed by non-quiesced hardware (the most typical example would be a framebuffer scanning out a splat screen or animation, but it could equally well be some sort of welcoming tone or music being played back) are described in device tree. In other words, and this is perhaps better answering your second question: in addition to describing reserved memory regions, we want to add a bit of information here about the usage of these memory regions. Some memory regions may contain information that the kernel may want to use (such an external memory frequency scaling tables) and those I would describe as "inactive" memory because it isn't being accessed by hardware. The framebuffer in this case is the opposite and it is being actively accessed (hence it is marked "active") by hardware while the kernel is busy setting everything up so that it can reconfigure that hardware and take over with its own framebuffer (for the console, for example). It's also not so much that we know enough about the region to assume it needs identity mapping. We don't really care about that from the DT point of view. In fact, depending on the rest of the system configuration, we may not need identity mapping (i.e. if none of the users of the reserved memory region are behind an IOMMU). But the point here is that the IOMMU drivers can use this "active" property to determine that if a device is using an "active" region and it is behind an IOMMU, then it must identity map that region in order for the hardware, which is not under the kernel's control yet, to be able to continue to access that memory through an IOMMU mapping. Thanks, Thierry signature.asc Description: PGP signature __
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Fri, Sep 25, 2020 at 04:21:17PM +0300, Dmitry Osipenko wrote:
> 25.09.2020 15:39, Robin Murphy пишет:
> ...
> >> IIRC, in the past Robin Murphy was suggesting to read out hardware state
> >> early during kernel boot in order to find what regions are in use by
> >> hardware.
> >
> > I doubt I suggested that in general, because I've always firmly believed
> > it to be a terrible idea. I've debugged too many cases where firmware or
> > kexec has inadvertently left DMA running and corrupted kernel memory, so
> > in general we definitely *don't* want to blindly trust random hardware
> > state. Anything I may have said in relation to Qualcomm's fundamentally
> > broken hypervisor/bootloader setup should not be considered outside that
> > specific context ;)
> >
> > Robin.
> >
> >> I think it should be easy to do for the display controller since we
> >> could check clock and PD states in order to decide whether DC's IO could
> >> be accessed and then read out the FB pointer and size. I guess it should
> >> take about hundred lines of code.
>
> The active DMA is indeed very dangerous, but it's a bit less dangerous
> in a case of read-only DMA.
>
> I got another idea of how we could benefit from the active display
> hardware. Maybe we could do the following:
>
> 1. Check whether display is active
>
> 2. Allocate CMA that matches the FB size
>
> 3. Create identity mapping for the CMA
>
> 4. Switch display framebuffer to our CMA
>
> 5. Create very early simple-framebuffer out of the CMA
>
> 6. Once Tegra DRM driver is loaded, it will kick out the simple-fb, and
> thus, release temporal CMA and identity mapping.
>
> This will provide us with a very early framebuffer output and it will
> work on all devices out-of-the-box!
Well that's already kind of what this is trying to achieve, only
skipping the CMA step because the memory is already there and actively
being scanned out from. The problem with your sequence above is first
that you have to allocate from CMA, which means that this has to wait
until CMA becomes available. That's fairly early, but it's not
immediately there. Until you get to that point, there's always the
potential for the display controller to read out from memory that may
now be used for something else. As you said, read-only active DMA isn't
as dangerous as write DMA, but it's not very nice either.
Furthermore, your point 5. above requires device-specific knowledge and
as I mentioned earlier that requires a small, but not necessarily
trivial, device-specific driver to work, which is very impractical for
multi-platform kernels.
There's nothing preventing these reserved-memory regions from being
reused to implement simple-framebuffer. I could in fact imagine a fairly
simple extension to the existing simple-framebuffer binding that could
look like this for Tegra:
dc@5200 {
compatible = "nvidia,tegra210-display", "simple-framebuffer";
...
memory-region = <&framebuffer>;
width = <1920>;
height = <1080>;
stride = <7680>;
format = "r8g8b8";
...
};
That's not dissimilar to what you're proposing above, except that it
moves everything before step 5. into the bootloader's responsibility and
therefore avoids the need for hardware-specific early display code in
the kernel.
Thierry
signature.asc
Description: PGP signature
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Fri, Sep 25, 2020 at 01:39:07PM +0100, Robin Murphy wrote: > On 2020-09-24 17:23, Dmitry Osipenko wrote: > > 24.09.2020 17:01, Thierry Reding пишет: > > > On Thu, Sep 24, 2020 at 04:23:59PM +0300, Dmitry Osipenko wrote: > > > > 04.09.2020 15:59, Thierry Reding пишет: > > > > > From: Thierry Reding > > > > > > > > > > Reserved memory regions can be marked as "active" if hardware is > > > > > expected to access the regions during boot and before the operating > > > > > system can take control. One example where this is useful is for the > > > > > operating system to infer whether the region needs to be identity- > > > > > mapped through an IOMMU. > > > > > > > > > > Signed-off-by: Thierry Reding > > > > > --- > > > > > .../bindings/reserved-memory/reserved-memory.txt | 7 > > > > > +++ > > > > > 1 file changed, 7 insertions(+) > > > > > > > > > > diff --git > > > > > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > > > > > > > > > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > > > > index 4dd20de6977f..163d2927e4fc 100644 > > > > > --- > > > > > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > > > > +++ > > > > > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > > > > @@ -63,6 +63,13 @@ reusable (optional) - empty property > > > > > able to reclaim it back. Typically that means that the > > > > > operating > > > > > system can use that region to store volatile or cached data > > > > > that > > > > > can be otherwise regenerated or migrated elsewhere. > > > > > +active (optional) - empty property > > > > > +- If this property is set for a reserved memory region, it > > > > > indicates > > > > > + that some piece of hardware may be actively accessing this > > > > > region. > > > > > + Should the operating system want to enable IOMMU protection > > > > > for a > > > > > + device, all active memory regions must have been > > > > > identity-mapped > > > > > + in order to ensure that non-quiescent hardware during boot can > > > > > + continue to access the memory. > > > > > Linux implementation note: > > > > > - If a "linux,cma-default" property is present, then Linux will use > > > > > the > > > > > > > > > > > > > Hi, > > > > > > > > Could you please explain what devices need this quirk? I see that you're > > > > targeting Tegra SMMU driver, which means that it should be some pre-T186 > > > > device. > > > > > > Primarily I'm looking at Tegra210 and later, because on earlier devices > > > the bootloader doesn't consistently initialize display. I know that it > > > does on some devices, but not all of them. > > > > AFAIK, all tablet devices starting with Tegra20 that have display panel > > are initializing display at a boot time for showing splash screen. This > > includes all T20/T30/T114 tablets that are already supported by upstream > > kernel. > > > > > This same code should also > > > work on Tegra186 and later (with an ARM SMMU) although the situation is > > > slightly more complicated there because IOMMU translations will fault by > > > default long before these identity mappings can be established. > > > > > > > Is this reservation needed for some device that has display > > > > hardwired to a very specific IOMMU domain at the boot time? > > > > > > No, this is only used to convey information about the active framebuffer > > > to the kernel. In practice the DMA/IOMMU code will use this information > > > to establish a 1:1 mapping on whatever IOMMU domain that was picked for > > > display. > > > > > > > If you're targeting devices that don't have IOMMU enabled by default at > > > > the boot time, then this approach won't work for the existing devices > > > > which won't ever get an updated bootloader. > > > > > > If the devices don't use an IOMMU, then there should be no problem. The > > > extra reserved-memory nodes would still be necessary to ensure that the > > > kernel doesn't reuse the framebuffer memory for the slab allocator, but > > > if no IOMMU is used, then the display controller accessing the memory > > > isn't going to cause problems other than perhaps scanning out data that > > > is no longer a framebuffer. > > > > > > There should also be no problem for devices with an old bootloader > > > because this code is triggered by the presence of a reserved-memory node > > > referenced via the memory-region property. Devices with an old > > > bootloader should continue to work as they did before. Although I > > > suppose they would start faulting once we enable DMA/IOMMU integration > > > for Tegra SMMU if they have a bootloader that does initialize display to > > > actively scan out during boot. > > > > > > > I think Robin Murphy already suggested that we should simply create > > > > a dummy "identity" IOMMU domain by default for the DRM/VDE devices and > > > > then replace it with an e
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Thu, Sep 24, 2020 at 07:23:34PM +0300, Dmitry Osipenko wrote: > 24.09.2020 17:01, Thierry Reding пишет: > > On Thu, Sep 24, 2020 at 04:23:59PM +0300, Dmitry Osipenko wrote: > >> 04.09.2020 15:59, Thierry Reding пишет: > >>> From: Thierry Reding > >>> > >>> Reserved memory regions can be marked as "active" if hardware is > >>> expected to access the regions during boot and before the operating > >>> system can take control. One example where this is useful is for the > >>> operating system to infer whether the region needs to be identity- > >>> mapped through an IOMMU. > >>> > >>> Signed-off-by: Thierry Reding > >>> --- > >>> .../bindings/reserved-memory/reserved-memory.txt | 7 +++ > >>> 1 file changed, 7 insertions(+) > >>> > >>> diff --git > >>> a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > >>> b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > >>> index 4dd20de6977f..163d2927e4fc 100644 > >>> --- > >>> a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > >>> +++ > >>> b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > >>> @@ -63,6 +63,13 @@ reusable (optional) - empty property > >>>able to reclaim it back. Typically that means that the operating > >>>system can use that region to store volatile or cached data that > >>>can be otherwise regenerated or migrated elsewhere. > >>> +active (optional) - empty property > >>> +- If this property is set for a reserved memory region, it indicates > >>> + that some piece of hardware may be actively accessing this region. > >>> + Should the operating system want to enable IOMMU protection for a > >>> + device, all active memory regions must have been identity-mapped > >>> + in order to ensure that non-quiescent hardware during boot can > >>> + continue to access the memory. > >>> > >>> Linux implementation note: > >>> - If a "linux,cma-default" property is present, then Linux will use the > >>> > >> > >> Hi, > >> > >> Could you please explain what devices need this quirk? I see that you're > >> targeting Tegra SMMU driver, which means that it should be some pre-T186 > >> device. > > > > Primarily I'm looking at Tegra210 and later, because on earlier devices > > the bootloader doesn't consistently initialize display. I know that it > > does on some devices, but not all of them. > > AFAIK, all tablet devices starting with Tegra20 that have display panel > are initializing display at a boot time for showing splash screen. This > includes all T20/T30/T114 tablets that are already supported by upstream > kernel. > > > This same code should also > > work on Tegra186 and later (with an ARM SMMU) although the situation is > > slightly more complicated there because IOMMU translations will fault by > > default long before these identity mappings can be established. > > > >> Is this reservation needed for some device that has display > >> hardwired to a very specific IOMMU domain at the boot time? > > > > No, this is only used to convey information about the active framebuffer > > to the kernel. In practice the DMA/IOMMU code will use this information > > to establish a 1:1 mapping on whatever IOMMU domain that was picked for > > display. > > > >> If you're targeting devices that don't have IOMMU enabled by default at > >> the boot time, then this approach won't work for the existing devices > >> which won't ever get an updated bootloader. > > > > If the devices don't use an IOMMU, then there should be no problem. The > > extra reserved-memory nodes would still be necessary to ensure that the > > kernel doesn't reuse the framebuffer memory for the slab allocator, but > > if no IOMMU is used, then the display controller accessing the memory > > isn't going to cause problems other than perhaps scanning out data that > > is no longer a framebuffer. > > > > There should also be no problem for devices with an old bootloader > > because this code is triggered by the presence of a reserved-memory node > > referenced via the memory-region property. Devices with an old > > bootloader should continue to work as they did before. Although I > > suppose they would start faulting once we enable DMA/IOMMU integration > > for Tegra SMMU if they have a bootloader that does initialize display to > > actively scan out during boot. > > > >> I think Robin Murphy already suggested that we should simply create > >> a dummy "identity" IOMMU domain by default for the DRM/VDE devices and > >> then replace it with an explicitly created domain within the drivers. > > > > I don't recall reading about that suggestion. So does this mean that for > > certain devices we'd want to basically passthrough by default and then > > at some point during boot take over with a properly managed IOMMU > > domain? > > Yes, my understanding that this is what Robin suggested here: > > https://lore.kernel.org/linux-iommu/cb12808b-7316-19db
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
25.09.2020 15:39, Robin Murphy пишет: ... >> Yes, my understanding that this is what Robin suggested here: >> >> https://lore.kernel.org/linux-iommu/cb12808b-7316-19db-7413-b7f852a6f...@arm.com/ >> > > Just to clarify, what I was talking about there is largely orthogonal to > the issue here. That was about systems with limited translation > resources letting translation be specifically opt-in by IOMMU-aware > drivers. It probably *would* happen to obviate the issue of disrupting > live DMA at boot time on these particular Tegra platforms, but we still > need something like Thierry's solution in general, since IOMMU drivers > may have no other way to determine whether devices are active at boot > and they have to take care to avoid breaking anything - e.g. SMMUv3 will > at a bare minimum need to set up *some* form of valid stream table entry > for the relevant device(s) right at the beginning where we first probe > and reset the SMMU itself, regardless of what happens with domains and > addresses later down the line. Yes, I only meant that yours suggestion also should be useful here. Anyways, thank you for the clarification :) I agree that the Thierry's proposal is good! But it needs some more thought yet because it's not very applicable to the current devices. >>> The primary goal here is to move towards using the DMA API rather than >>> the IOMMU API directly, so we don't really have the option of replacing >>> with an explicitly created domain. Unless we have code in the DMA/IOMMU >>> code that does this somehow. >>> >>> But I'm not sure what would be a good way to mark certain devices as >>> needing an identity domain by default. Do we still use the reserved- >>> memory node for that? >> >> The reserved-memory indeed shouldn't be needed for resolving the >> implicit IOMMU problem since we could mark certain devices within the >> kernel IOMMU driver. >> >> I haven't got around to trying to implement the implicit IOMMU support >> yet, but I suppose we could implement the def_domain_type() hook in the >> SMMU driver and then return IOMMU_DOMAIN_IDENTITY for the Display/VDE >> devices. Then the Display/VDE drivers will take over the identity domain >> and replace it with the explicit domain. > > FWIW I've already cooked up identity domain support for tegra-gart; I > was planning on tackling it for tegra-smmu as well for the next version > of my arm default domains series (which will be after the next -rc1 now > since I'm just about to take some long-overdue holiday). Very nice! Maybe we will have some more food for the discussion by the time you'll return. Have a good time! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
25.09.2020 15:39, Robin Murphy пишет: ... >> IIRC, in the past Robin Murphy was suggesting to read out hardware state >> early during kernel boot in order to find what regions are in use by >> hardware. > > I doubt I suggested that in general, because I've always firmly believed > it to be a terrible idea. I've debugged too many cases where firmware or > kexec has inadvertently left DMA running and corrupted kernel memory, so > in general we definitely *don't* want to blindly trust random hardware > state. Anything I may have said in relation to Qualcomm's fundamentally > broken hypervisor/bootloader setup should not be considered outside that > specific context ;) > > Robin. > >> I think it should be easy to do for the display controller since we >> could check clock and PD states in order to decide whether DC's IO could >> be accessed and then read out the FB pointer and size. I guess it should >> take about hundred lines of code. The active DMA is indeed very dangerous, but it's a bit less dangerous in a case of read-only DMA. I got another idea of how we could benefit from the active display hardware. Maybe we could do the following: 1. Check whether display is active 2. Allocate CMA that matches the FB size 3. Create identity mapping for the CMA 4. Switch display framebuffer to our CMA 5. Create very early simple-framebuffer out of the CMA 6. Once Tegra DRM driver is loaded, it will kick out the simple-fb, and thus, release temporal CMA and identity mapping. This will provide us with a very early framebuffer output and it will work on all devices out-of-the-box! ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On 2020-09-24 17:23, Dmitry Osipenko wrote: 24.09.2020 17:01, Thierry Reding пишет: On Thu, Sep 24, 2020 at 04:23:59PM +0300, Dmitry Osipenko wrote: 04.09.2020 15:59, Thierry Reding пишет: From: Thierry Reding Reserved memory regions can be marked as "active" if hardware is expected to access the regions during boot and before the operating system can take control. One example where this is useful is for the operating system to infer whether the region needs to be identity- mapped through an IOMMU. Signed-off-by: Thierry Reding --- .../bindings/reserved-memory/reserved-memory.txt | 7 +++ 1 file changed, 7 insertions(+) diff --git a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt index 4dd20de6977f..163d2927e4fc 100644 --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt @@ -63,6 +63,13 @@ reusable (optional) - empty property able to reclaim it back. Typically that means that the operating system can use that region to store volatile or cached data that can be otherwise regenerated or migrated elsewhere. +active (optional) - empty property +- If this property is set for a reserved memory region, it indicates + that some piece of hardware may be actively accessing this region. + Should the operating system want to enable IOMMU protection for a + device, all active memory regions must have been identity-mapped + in order to ensure that non-quiescent hardware during boot can + continue to access the memory. Linux implementation note: - If a "linux,cma-default" property is present, then Linux will use the Hi, Could you please explain what devices need this quirk? I see that you're targeting Tegra SMMU driver, which means that it should be some pre-T186 device. Primarily I'm looking at Tegra210 and later, because on earlier devices the bootloader doesn't consistently initialize display. I know that it does on some devices, but not all of them. AFAIK, all tablet devices starting with Tegra20 that have display panel are initializing display at a boot time for showing splash screen. This includes all T20/T30/T114 tablets that are already supported by upstream kernel. This same code should also work on Tegra186 and later (with an ARM SMMU) although the situation is slightly more complicated there because IOMMU translations will fault by default long before these identity mappings can be established. Is this reservation needed for some device that has display hardwired to a very specific IOMMU domain at the boot time? No, this is only used to convey information about the active framebuffer to the kernel. In practice the DMA/IOMMU code will use this information to establish a 1:1 mapping on whatever IOMMU domain that was picked for display. If you're targeting devices that don't have IOMMU enabled by default at the boot time, then this approach won't work for the existing devices which won't ever get an updated bootloader. If the devices don't use an IOMMU, then there should be no problem. The extra reserved-memory nodes would still be necessary to ensure that the kernel doesn't reuse the framebuffer memory for the slab allocator, but if no IOMMU is used, then the display controller accessing the memory isn't going to cause problems other than perhaps scanning out data that is no longer a framebuffer. There should also be no problem for devices with an old bootloader because this code is triggered by the presence of a reserved-memory node referenced via the memory-region property. Devices with an old bootloader should continue to work as they did before. Although I suppose they would start faulting once we enable DMA/IOMMU integration for Tegra SMMU if they have a bootloader that does initialize display to actively scan out during boot. I think Robin Murphy already suggested that we should simply create a dummy "identity" IOMMU domain by default for the DRM/VDE devices and then replace it with an explicitly created domain within the drivers. I don't recall reading about that suggestion. So does this mean that for certain devices we'd want to basically passthrough by default and then at some point during boot take over with a properly managed IOMMU domain? Yes, my understanding that this is what Robin suggested here: https://lore.kernel.org/linux-iommu/cb12808b-7316-19db-7413-b7f852a6f...@arm.com/ Just to clarify, what I was talking about there is largely orthogonal to the issue here. That was about systems with limited translation resources letting translation be specifically opt-in by IOMMU-aware drivers. It probably *would* happen to obviate the issue of disrupting live DMA at boot time on these particular Tegra platforms, but we still need something like Thierry's solution in general, since IOMMU drivers
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
24.09.2020 17:01, Thierry Reding пишет: > On Thu, Sep 24, 2020 at 04:23:59PM +0300, Dmitry Osipenko wrote: >> 04.09.2020 15:59, Thierry Reding пишет: >>> From: Thierry Reding >>> >>> Reserved memory regions can be marked as "active" if hardware is >>> expected to access the regions during boot and before the operating >>> system can take control. One example where this is useful is for the >>> operating system to infer whether the region needs to be identity- >>> mapped through an IOMMU. >>> >>> Signed-off-by: Thierry Reding >>> --- >>> .../bindings/reserved-memory/reserved-memory.txt | 7 +++ >>> 1 file changed, 7 insertions(+) >>> >>> diff --git >>> a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt >>> b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt >>> index 4dd20de6977f..163d2927e4fc 100644 >>> --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt >>> +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt >>> @@ -63,6 +63,13 @@ reusable (optional) - empty property >>>able to reclaim it back. Typically that means that the operating >>>system can use that region to store volatile or cached data that >>>can be otherwise regenerated or migrated elsewhere. >>> +active (optional) - empty property >>> +- If this property is set for a reserved memory region, it indicates >>> + that some piece of hardware may be actively accessing this region. >>> + Should the operating system want to enable IOMMU protection for a >>> + device, all active memory regions must have been identity-mapped >>> + in order to ensure that non-quiescent hardware during boot can >>> + continue to access the memory. >>> >>> Linux implementation note: >>> - If a "linux,cma-default" property is present, then Linux will use the >>> >> >> Hi, >> >> Could you please explain what devices need this quirk? I see that you're >> targeting Tegra SMMU driver, which means that it should be some pre-T186 >> device. > > Primarily I'm looking at Tegra210 and later, because on earlier devices > the bootloader doesn't consistently initialize display. I know that it > does on some devices, but not all of them. AFAIK, all tablet devices starting with Tegra20 that have display panel are initializing display at a boot time for showing splash screen. This includes all T20/T30/T114 tablets that are already supported by upstream kernel. > This same code should also > work on Tegra186 and later (with an ARM SMMU) although the situation is > slightly more complicated there because IOMMU translations will fault by > default long before these identity mappings can be established. > >> Is this reservation needed for some device that has display >> hardwired to a very specific IOMMU domain at the boot time? > > No, this is only used to convey information about the active framebuffer > to the kernel. In practice the DMA/IOMMU code will use this information > to establish a 1:1 mapping on whatever IOMMU domain that was picked for > display. > >> If you're targeting devices that don't have IOMMU enabled by default at >> the boot time, then this approach won't work for the existing devices >> which won't ever get an updated bootloader. > > If the devices don't use an IOMMU, then there should be no problem. The > extra reserved-memory nodes would still be necessary to ensure that the > kernel doesn't reuse the framebuffer memory for the slab allocator, but > if no IOMMU is used, then the display controller accessing the memory > isn't going to cause problems other than perhaps scanning out data that > is no longer a framebuffer. > > There should also be no problem for devices with an old bootloader > because this code is triggered by the presence of a reserved-memory node > referenced via the memory-region property. Devices with an old > bootloader should continue to work as they did before. Although I > suppose they would start faulting once we enable DMA/IOMMU integration > for Tegra SMMU if they have a bootloader that does initialize display to > actively scan out during boot. > >> I think Robin Murphy already suggested that we should simply create >> a dummy "identity" IOMMU domain by default for the DRM/VDE devices and >> then replace it with an explicitly created domain within the drivers. > > I don't recall reading about that suggestion. So does this mean that for > certain devices we'd want to basically passthrough by default and then > at some point during boot take over with a properly managed IOMMU > domain? Yes, my understanding that this is what Robin suggested here: https://lore.kernel.org/linux-iommu/cb12808b-7316-19db-7413-b7f852a6f...@arm.com/ > The primary goal here is to move towards using the DMA API rather than > the IOMMU API directly, so we don't really have the option of replacing > with an explicitly created domain. Unless we have code in the DMA/IOMMU > code that does this s
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Thu, Sep 24, 2020 at 04:23:59PM +0300, Dmitry Osipenko wrote: > 04.09.2020 15:59, Thierry Reding пишет: > > From: Thierry Reding > > > > Reserved memory regions can be marked as "active" if hardware is > > expected to access the regions during boot and before the operating > > system can take control. One example where this is useful is for the > > operating system to infer whether the region needs to be identity- > > mapped through an IOMMU. > > > > Signed-off-by: Thierry Reding > > --- > > .../bindings/reserved-memory/reserved-memory.txt | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > index 4dd20de6977f..163d2927e4fc 100644 > > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > @@ -63,6 +63,13 @@ reusable (optional) - empty property > >able to reclaim it back. Typically that means that the operating > >system can use that region to store volatile or cached data that > >can be otherwise regenerated or migrated elsewhere. > > +active (optional) - empty property > > +- If this property is set for a reserved memory region, it indicates > > + that some piece of hardware may be actively accessing this region. > > + Should the operating system want to enable IOMMU protection for a > > + device, all active memory regions must have been identity-mapped > > + in order to ensure that non-quiescent hardware during boot can > > + continue to access the memory. > > > > Linux implementation note: > > - If a "linux,cma-default" property is present, then Linux will use the > > > > Hi, > > Could you please explain what devices need this quirk? I see that you're > targeting Tegra SMMU driver, which means that it should be some pre-T186 > device. Primarily I'm looking at Tegra210 and later, because on earlier devices the bootloader doesn't consistently initialize display. I know that it does on some devices, but not all of them. This same code should also work on Tegra186 and later (with an ARM SMMU) although the situation is slightly more complicated there because IOMMU translations will fault by default long before these identity mappings can be established. > Is this reservation needed for some device that has display > hardwired to a very specific IOMMU domain at the boot time? No, this is only used to convey information about the active framebuffer to the kernel. In practice the DMA/IOMMU code will use this information to establish a 1:1 mapping on whatever IOMMU domain that was picked for display. > If you're targeting devices that don't have IOMMU enabled by default at > the boot time, then this approach won't work for the existing devices > which won't ever get an updated bootloader. If the devices don't use an IOMMU, then there should be no problem. The extra reserved-memory nodes would still be necessary to ensure that the kernel doesn't reuse the framebuffer memory for the slab allocator, but if no IOMMU is used, then the display controller accessing the memory isn't going to cause problems other than perhaps scanning out data that is no longer a framebuffer. There should also be no problem for devices with an old bootloader because this code is triggered by the presence of a reserved-memory node referenced via the memory-region property. Devices with an old bootloader should continue to work as they did before. Although I suppose they would start faulting once we enable DMA/IOMMU integration for Tegra SMMU if they have a bootloader that does initialize display to actively scan out during boot. > I think Robin Murphy already suggested that we should simply create > a dummy "identity" IOMMU domain by default for the DRM/VDE devices and > then replace it with an explicitly created domain within the drivers. I don't recall reading about that suggestion. So does this mean that for certain devices we'd want to basically passthrough by default and then at some point during boot take over with a properly managed IOMMU domain? The primary goal here is to move towards using the DMA API rather than the IOMMU API directly, so we don't really have the option of replacing with an explicitly created domain. Unless we have code in the DMA/IOMMU code that does this somehow. But I'm not sure what would be a good way to mark certain devices as needing an identity domain by default. Do we still use the reserved- memory node for that? That would still require some sort of flag to specify which reserved-memory regions would need this identity mapping because, as was pointed out in earlier review, some devices may have reserved-memory regions that are not meant to be identity mapped. > Secondly, all NVIDIA bootloaders are passing tegra_fbmem=... via > kernel's cmdl
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
04.09.2020 15:59, Thierry Reding пишет: > From: Thierry Reding > > Reserved memory regions can be marked as "active" if hardware is > expected to access the regions during boot and before the operating > system can take control. One example where this is useful is for the > operating system to infer whether the region needs to be identity- > mapped through an IOMMU. > > Signed-off-by: Thierry Reding > --- > .../bindings/reserved-memory/reserved-memory.txt | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > index 4dd20de6977f..163d2927e4fc 100644 > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > @@ -63,6 +63,13 @@ reusable (optional) - empty property >able to reclaim it back. Typically that means that the operating >system can use that region to store volatile or cached data that >can be otherwise regenerated or migrated elsewhere. > +active (optional) - empty property > +- If this property is set for a reserved memory region, it indicates > + that some piece of hardware may be actively accessing this region. > + Should the operating system want to enable IOMMU protection for a > + device, all active memory regions must have been identity-mapped > + in order to ensure that non-quiescent hardware during boot can > + continue to access the memory. > > Linux implementation note: > - If a "linux,cma-default" property is present, then Linux will use the > Hi, Could you please explain what devices need this quirk? I see that you're targeting Tegra SMMU driver, which means that it should be some pre-T186 device. Is this reservation needed for some device that has display hardwired to a very specific IOMMU domain at the boot time? If you're targeting devices that don't have IOMMU enabled by default at the boot time, then this approach won't work for the existing devices which won't ever get an updated bootloader. I think Robin Murphy already suggested that we should simply create a dummy "identity" IOMMU domain by default for the DRM/VDE devices and then replace it with an explicitly created domain within the drivers. Secondly, all NVIDIA bootloaders are passing tegra_fbmem=... via kernel's cmdline with the physical location of the framebuffer in memory. Maybe we could support this option? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Tue, Sep 15, 2020 at 02:36:48PM +0200, Thierry Reding wrote: > On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: > > On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: > > > From: Thierry Reding > > > > > > Reserved memory regions can be marked as "active" if hardware is > > > expected to access the regions during boot and before the operating > > > system can take control. One example where this is useful is for the > > > operating system to infer whether the region needs to be identity- > > > mapped through an IOMMU. > > > > I like simple solutions, but this hardly seems adequate to solve the > > problem of passing IOMMU setup from bootloader/firmware to the OS. Like > > what is the IOVA that's supposed to be used if identity mapping is not > > used? > > The assumption here is that if the region is not active there is no need > for the IOVA to be specified because the kernel will allocate memory and > assign any IOVA of its choosing. > > Also, note that this is not meant as a way of passing IOMMU setup from > the bootloader or firmware to the OS. The purpose of this is to specify > that some region of memory is actively being accessed during boot. The > particular case that I'm looking at is where the bootloader set up a > splash screen and keeps it on during boot. The bootloader has not set up > an IOMMU mapping and the identity mapping serves as a way of keeping the > accesses by the display hardware working during the transitional period > after the IOMMU translations have been enabled by the kernel but before > the kernel display driver has had a chance to set up its own IOMMU > mappings. > > > If you know enough about the regions to assume identity mapping, then > > can't you know if active or not? > > We could alternatively add some property that describes the region as > requiring an identity mapping. But note that we can't make any > assumptions here about the usage of these regions because the IOMMU > driver simply has no way of knowing what they are being used for. > > Some additional information is required in device tree for the IOMMU > driver to be able to make that decision. Rob, can you provide any hints on exactly how you want to move this forward? I don't know in what direction you'd like to proceed. Thierry signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Mon, Sep 14, 2020 at 04:08:29PM -0600, Rob Herring wrote: > On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: > > From: Thierry Reding > > > > Reserved memory regions can be marked as "active" if hardware is > > expected to access the regions during boot and before the operating > > system can take control. One example where this is useful is for the > > operating system to infer whether the region needs to be identity- > > mapped through an IOMMU. > > I like simple solutions, but this hardly seems adequate to solve the > problem of passing IOMMU setup from bootloader/firmware to the OS. Like > what is the IOVA that's supposed to be used if identity mapping is not > used? The assumption here is that if the region is not active there is no need for the IOVA to be specified because the kernel will allocate memory and assign any IOVA of its choosing. Also, note that this is not meant as a way of passing IOMMU setup from the bootloader or firmware to the OS. The purpose of this is to specify that some region of memory is actively being accessed during boot. The particular case that I'm looking at is where the bootloader set up a splash screen and keeps it on during boot. The bootloader has not set up an IOMMU mapping and the identity mapping serves as a way of keeping the accesses by the display hardware working during the transitional period after the IOMMU translations have been enabled by the kernel but before the kernel display driver has had a chance to set up its own IOMMU mappings. > If you know enough about the regions to assume identity mapping, then > can't you know if active or not? We could alternatively add some property that describes the region as requiring an identity mapping. But note that we can't make any assumptions here about the usage of these regions because the IOMMU driver simply has no way of knowing what they are being used for. Some additional information is required in device tree for the IOMMU driver to be able to make that decision. Thierry > > > Signed-off-by: Thierry Reding > > --- > > .../bindings/reserved-memory/reserved-memory.txt | 7 +++ > > 1 file changed, 7 insertions(+) > > > > diff --git > > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > index 4dd20de6977f..163d2927e4fc 100644 > > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > > @@ -63,6 +63,13 @@ reusable (optional) - empty property > >able to reclaim it back. Typically that means that the operating > >system can use that region to store volatile or cached data that > >can be otherwise regenerated or migrated elsewhere. > > +active (optional) - empty property > > +- If this property is set for a reserved memory region, it indicates > > + that some piece of hardware may be actively accessing this region. > > + Should the operating system want to enable IOMMU protection for a > > + device, all active memory regions must have been identity-mapped > > + in order to ensure that non-quiescent hardware during boot can > > + continue to access the memory. > > > > Linux implementation note: > > - If a "linux,cma-default" property is present, then Linux will use the > > -- > > 2.28.0 > > signature.asc Description: PGP signature ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v2 1/4] dt-bindings: reserved-memory: Document "active" property
On Fri, Sep 04, 2020 at 02:59:57PM +0200, Thierry Reding wrote: > From: Thierry Reding > > Reserved memory regions can be marked as "active" if hardware is > expected to access the regions during boot and before the operating > system can take control. One example where this is useful is for the > operating system to infer whether the region needs to be identity- > mapped through an IOMMU. I like simple solutions, but this hardly seems adequate to solve the problem of passing IOMMU setup from bootloader/firmware to the OS. Like what is the IOVA that's supposed to be used if identity mapping is not used? If you know enough about the regions to assume identity mapping, then can't you know if active or not? > Signed-off-by: Thierry Reding > --- > .../bindings/reserved-memory/reserved-memory.txt | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git > a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > index 4dd20de6977f..163d2927e4fc 100644 > --- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > +++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt > @@ -63,6 +63,13 @@ reusable (optional) - empty property >able to reclaim it back. Typically that means that the operating >system can use that region to store volatile or cached data that >can be otherwise regenerated or migrated elsewhere. > +active (optional) - empty property > +- If this property is set for a reserved memory region, it indicates > + that some piece of hardware may be actively accessing this region. > + Should the operating system want to enable IOMMU protection for a > + device, all active memory regions must have been identity-mapped > + in order to ensure that non-quiescent hardware during boot can > + continue to access the memory. > > Linux implementation note: > - If a "linux,cma-default" property is present, then Linux will use the > -- > 2.28.0 > ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
