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 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.


> >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.

I must have remembered incorrectly about the external abort, it's an
alignment fault.


> >We could also see alignment problems, as the alignment checks for
> >ARMv8 differ somewhat between normal memory and device memory.
> >
> >A third reason is performance. The rules for speculative accesses
> >and caching are different between device and normal memory.
> >
> >So I opted for the most permissive attributes thinking that dom0 can
> >apply further restrictions if it needs to do so.
> >
> >
> >>>Dom0 can then choose to further restrict these mappings if needed.
> >>>We only look at the outer mmio-sram region. The sub-area nodes that
> >>>describe allocations within the mmio-sram region are only meaningful
> >>>to the guest AFAICT.
> >>>
> >>>In an attempt to avoid growing the already fairly large domain_build.c
> >>>file, I've tried to implement a distributed way to deal with these kind
> >>>of special/custom mapping needs. These can live outside of domain_build.c
> >>>and are registerd by means of a .map method in the device_spec.
> >>>
> >>>If this approach is not good, I'm happy to bin it and try something else.
> >>
> >>We will have a similar problem when using ACPI for DOM0 or mapping a such
> >>MMIO to the guest. The hypercalls XENMAPSPACE_dev_mmio and
> >>XEN_DOMCTL_memory_mapping do not provide enough information to know the
> >>attribute to be used for mapping.
> >>
> >>MMIO are always mapped in Stage-2 with Device_nGnRE, which is quite
> >>restrictive. This would also impact any MMIO regions, such as the video RAM
> >>buffer, that could be mapped write-combine.
> >>
> >>After reading the ARM ARM (B2.8.2 ARM DII 0486.i), I think we could relax
> >>the stage-2 mapping by using Device_GRE for all the device MMIOs but the
> >>GIC.
> >>
> >>We have to keep the GIC MMIO with the most restrictive memory attribute to
> >>avoid potential side-effect when Xen is switching between multiple vCPUs.
> >
> >I see, that makes sense.
> >
> >
> >>All the other devices will be exclusive to a specific guest, so the guest
> >>can handle the device the way it wants. This may require some extra-care
> >>when reassigning the device to another domain.
> >>
> >>Edgar, would Device_GRE be fine for you?
> >
> >Sorry, but I don't think so. We can live with performance impacts but it's
> >harder with the external data aborts and potential alignment checks.
> >
> >
> >>Note, that XENMAPSPACE_dev_mmio has been introduced in Xen 4.7 (which is due
> >>in a couple of weeks) and part of the stable ABI. So if it is not possible
> >>to relax the memory attribute, it might be worth to think fixing/reverting
> >>the hypercall for 4.7. Otherwise we would have to introduce a new one in the
> >>next release.
> >
> >Yes, maybe we could add something along the lines of the pgprot arg
> >that Linux/arm64 has in it's __ioremap call. Even if we only support
> >PROT_DEVICE_nGnRE (or GRE) in 4.7 at the least we can reuse the ABI
> >to add more modes in 4.8?
> 
> I will bring it on a separate subject with the REST of the maintainers when
> we find out what are the possible memory attributes.
> 
> Depending on the result, this might be considered as a blocker as I do not
> think we should avoid to  introduce a new hypercall (XENMAPSPACE_dev_mmio)
> which is known to not fit for every case.

Thanks!

Best regards,
Edgar

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