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

[Julien Grall <julien.grall@arm.com>]

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@xilinx.com>
>>>>>
>>>>> 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@lists.xen.org
http://lists.xen.org/xen-devel