Re: [Xen-devel] [RFC for-4.8 0/6] xen/arm: Add support for mapping mmio-sram nodes into dom0

["Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxx>]

On Mon, May 23, 2016 at 04:13:53PM +0100, Julien Grall wrote:
> Hi Edgar,
> 
> On 23/05/16 15:02, Edgar E. Iglesias wrote:
> >On Mon, May 23, 2016 at 02:02:39PM +0100, Julien Grall wrote:
> >>(CC Wei Liu)
> >>
> >>On 23/05/16 12:56, Edgar E. Iglesias wrote:
> >>>On Mon, May 23, 2016 at 11:29:31AM +0100, Julien Grall wrote:
> >>>>On 20/05/16 16:51, Edgar E. Iglesias wrote:
> >>>>>From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
> >>>>>
> >>>>>This series adds support for mapping mmio-sram nodes into dom0
> >>>>>as MEMORY, cached and with RWX perms.
> >>>>
> >>>>Can you explain why you chose to map those nodes as MEMORY, cached and 
> >>>>with
> >>>>RWX perms?
> >>>
> >>>My understanding is that these mmio-sram nodes are allowed to be treated as
> >>>Normal memory by the guest OS.
> >>>Guests could potentially do any kind of memory like operations on them.
> >>>
> >>>In our specific case, dom0 won't execute code from these regions but
> >>>Linux/dom0 ends up using standard memcpy/memset/x functions (not
> >>>memcpy_fromio and friends) on the regions.
> >>
> >>I looked at the generic sram driver in Linux (drivers/misc/sram.c) which
> >>actually use memcpy_{to,from}io. So how you driver differs from the generic
> >>one? What the SRAM will contain?
> >
> >We intend to use that same driver to map the memory but mmio-sram
> >nodes allow you to assign the regions to other device-drivers.
> >
> >Some examples:
> >Documentation/devicetree/bindings/crypto/marvell-cesa.txt
> >arch/arm/boot/dts/orion5x.dtsi
> >drivers/crypto/mv_cesa.c
> >
> >The cover letter for the sram driver had an example aswell allthough
> >the function names have changed since (it's of_gen_pool_get now):
> >https://lwn.net/Articles/537728/
> >
> >Nothing explicitely says that the regions can be assumed to be mapped
> >as Normal memory, but on Linux they do get mapped as Mormal WC mem
> >(unless the no-memory-wc prop is set on the node).
> >The marvell-cesa example also uses plain memset on the sram.
> 
> I am a bit confused with this example. From my understanding of
> mv_cesa_get_ram, cp->sram can point either to a normal memory (?) area (see
> gen_pool_dma_alloc) or a Device_nGnRE area (see devm_ioremap_resource).
> 
> However, memcpy_{from,to}io should be used when dealing with MMIO (the field
> sram has the __iomem attribute). See the commit 0f3304dc from Russel King
> related to marvell/cesa.


Yeah, I'm started to get confused too. Maybe they just forgot the memset
in drivers/crypto/mv_cesa.c.

There are other examples though, that don't do fromio/toio at all.
Documentation/devicetree/bindings/media/coda.txt
drivers/media/platform/coda/coda-common.c

Allthough ofcourse, these could also be wrong. Maybe I've missunderstood how
mmio-sram is supposed to be used.


> >>>We saw issues with memset doing cache operations on DEVICE memory in
> >>>the past (external data aborts). I can't find the text in the ARM ARM
> >>>regarding this at the moment but IIRC, dc ops on device mem are not
> >>>allowed.
> >>
> >>The ARM ARM (D3-1632 in ARM DDI 0487A.i) states "that the effects of the
> >>cache maintenance instructions can apply regardless of:
> >>Whether the address accessed:
> >>    * Is Normal memory or Device memory.
> >>    * Has the Cacheable attribute or the Non-cacheable attribute.
> >>"
> >>
> >>So I am not sure why you would get an external data aborts when executing dc
> >>ops on device memory.
> >
> >
> >OK, I found a reference to the issue we were seeing. If you look at ARM ARM
> >D3.4.9 Data cache zero instruction, you'll see that the DC ZVA insn always
> >generates an alignment fault if used on device memory.
> 
> Thinking a bit more, I find weird to use cache instructions on the SRAM
> given the region will be mapped uncacheable by Linux. Note that SRAM is
> usually very-fast so using the cache may not improve that much the
> performance.
> 
> How bad would the performance be if the processor cannot speculate access to
> the SRAM?

I don't have the setup available right now to try it out but I wouldn't
expect it to be very signifcant for our apps. In our case it had more to
do with the ability to use the remote-proc drivers as they are and really
any linux driver that potentially can take an sram as a memory region for
local use (DMA buffers or whatever), without changing the memory ops to
the _fromio/_toio versions.

Best regards,
Edgar

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel