Re: [Xen-devel] [PATCH v1 1/1] xen/arm: Map mmio-sram nodes as cached memory

[Julien Grall <julien.grall@xxxxxxx>]

Hi Edgar,

On 20/12/2016 13:53, Edgar E. Iglesias wrote:
> On Mon, Dec 19, 2016 at 04:01:00PM -0800, Stefano Stabellini wrote:
> > On Mon, 19 Dec 2016, Julien Grall wrote:
> > > Hi Edgar,
> > >
> > > On 16/12/2016 18:04, Edgar E. Iglesias wrote:
> > > > On Fri, Dec 16, 2016 at 04:12:00PM +0000, Julien Grall wrote:
> > > > > On 15/12/16 11:26, Edgar E. Iglesias wrote:
> > > > > > From: "Edgar E. Iglesias" <edgar.iglesias@xxxxxxxxxx>
> > > > > >
> > > > > > Relax the mapping of mmio-sram nodes that do not set the
> > > > > > no-memory-wc property to cached normal memory.
> > > > > >
> > > > > > Rationale:
> > > > > > Allthough on chip memories are relatively fast compared to
> > > > > s/Allthough/Although/
> > > > Fixed for v2.
> > > >
> > > > > > off-chip memories, large OCMs are still significantly slower
> > > > > May I ask what does OCMs mean?
> > > > It means On Chip Memories. I can spell it out in v2.
> > > Yes please. I was not able to find the acronym with a quick search Google.
> > >
> > > [...]
> > >
> > > > > I consider it as the most trusted domain and has other way to mess up the
> > > > > platform. So I am fine with this relaxation for the hardware domain (AKA
> > > > > DOM0).
> > > > >
> > > > > However, I have the feeling that we need this kind of relaxation for many
> > > > > devices. For instance prefetchable memory BARs for PCI would have to be
> > > > > cacheable, same for integrated graphic cards.
> > > > Yes, I agree.
> > > >
> > > > > I am wondering whether for DOM0 only (*not the other guests), we should
> > > > > relax the default stage-2 attribute mapping to p2m_mmio_direct_c.
> > > > >
> > > > > So we would let DOM0 in charge of the final attribute. This may boost the
> > > > > performance of memory access in DOM0.
> > > > >
> > > > > Any opinions?
> > > > I think it makes sense. We had a discussiong about it a while back ago:
> > > > https://lists.xenproject.org/archives/html/xen-devel/2015-05/msg02349.html
> > > >
> > > > The concerns that were raised were wether there could be devices that
> > > > could behave badly and possibly giving dom0 access to trig undefined or
> > > > unpredictable behavior that could be exploited.
> > > I think it would be fine for DOM0. It is already a trusted domain and have
> > > other way to take down the platform.
> > >
> > > > If dom0 accesses devices through a cache, access patterns will change.
> > > > In theory it may trig unexpected behaviour in some device. It's hard
> > > > to say unless talking about specific chips and implementations.
> > > >
> > > > For example, given dom0 access to a DMA capable device may also achieve
> > > > the same. I.e access patterns from DMA units differ from the ones
> > > > originating from a CPU using uncached device memory. It could trig
> > > > similar kind of errors.
> > > Another example is having the device mapped with different in 2 places with
> > > different cacheability attributes. The data accessed would not be coherent.
> > > But I believe that should not lead to a security issue.
> > >
> > > > It would be interesting to see a concrete example of a problematic
> > > > system.
> > > I believe it would depend a lot on how the platform have been designed.
> > >
> > > I think we have two choices to go forward:
> > >         1) Relax the memory attribute on case by case. DOM0 would not be able
> > > to exploit potential undefined behavior. However, this means that code is been
> > > added for any new device (e.g compatible string).
> > >         2) Relax the memory attribute on every case. DOM0 would be able to
> > > exploit potential undefined behavior. On the benefit side, every devices can
> > > be used at its full performance without any change required in Xen.
> > >
> > > In the case of ACPI, we rely on DOM0 to provide the correct mapping attribute
> > > when asking to do the stage-2 mapping (see XENMAPSPACE_dev_mmio). Note that
> > > currently, the MMIO are always mapped with the attribute Device-nGnRE. There
> > > is plan to change that in the future. So ACPI is implementing 2).
> > >
> > > Device Tree is currently implementing 1). But I would like to see the behavior
> > > of Xen the same no matter the firmware tables used. So I would lean towards
> > > 2).
> > That's fine by me.
>
> OK, I've sent a v2 of the mmio-sram patch to avoid delays with that part.
>
> But now that we seem to be in agreement with going for relaxing the default
> S2 mappings, perhaps we should ditch the whitelist/blacklist mechanism?
> Or does someone see a need for a blacklist?
I don't have in mind of any device requiring more tighter memory 
attribute from the hypervisor. AFAICT, it would only be necessary to do 
it if DOM0 was using wrong attribute, but that kernel would also have 
issue whilst running on baremetal.
Cheers,

--
Julien Grall

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