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

[Julien Grall <julien.grall@xxxxxxx>]

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.
> > > 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?
Regards,

--
Julien Grall

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