[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

Hi Edgar,

On 23/05/2016 16:42, Edgar E. Iglesias wrote:
On Mon, May 23, 2016 at 04:13:53PM +0100, Julien Grall wrote:
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:

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):

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.

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

I have talked about the memory attribute around me and the consensus is we should use the most relaxed mode that does not have any security implication or undefined behavior for a given device.

For SRAM it would be normal memory uncached (?) when the property "no-memory-wc" is not present, else TBD.

I suspect we would have to relax more MMIOs in the future. Rather than providing a function to map, the code is very similar except the memory attribute, I suggest to provide a list of compatible with the memory attribute to use.

All the children node would inherit the memory attribute of the parent.

What do you think?


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.