[U-Boot] [PATCH v2 06/12] virt-dt: Allow reservation of the secure region when it is in a RAM carveout.

[Thierry Reding]

On Tue, Feb 17, 2015 at 09:09:57AM +0100, Jan Kiszka wrote:
> On 2015-02-16 16:38, Jan Kiszka wrote:
> > On 2015-02-16 15:56, Mark Rutland wrote:
> >> On Mon, Feb 16, 2015 at 02:31:21PM +0000, Jan Kiszka wrote:
> >>> On 2015-02-16 15:25, Mark Rutland wrote:
> >>>> On Mon, Feb 16, 2015 at 01:51:37PM +0000, Jan Kiszka wrote:
> >>>>> On 2015-02-16 14:42, Mark Rutland wrote:
> >>>>>> On Mon, Feb 16, 2015 at 12:54:43PM +0000, Jan Kiszka wrote:
> >>>>>>> From: Ian Campbell <ijc at hellion.org.uk>
> >>>>>>>
> >>>>>>> In this case the secure code lives in RAM, and hence needs to be reserved, but
> >>>>>>> it has been relocated, so the reservation of __secure_start does not apply.
> >>>>>>>
> >>>>>>> Add support for setting CONFIG_ARMV7_SECURE_RESERVE_SIZE to reserve such a
> >>>>>>> region.
> >>>>>>>
> >>>>>>> This will be used in a subsequent patch for Jetson-TK1
> >>>>>>
> >>>>>> Using a memreserve and allowing the OS to map the memory but not poke it
> >>>>>> can be problematic due to the potential of mismatched attributes between
> >>>>>> the monitor and the OS.
> >>>>>
> >>>>> OK, here my knowledge is not yet sufficient to process this remark. What
> >>>>> kind of problems can arise from what kind of attribute mismatch? And why
> >>>>> should the OS be able to cause problems for the monitor?
> >>>>
> >>>> For example, consider the case of the region being mapped cacheable by
> >>>> the OS but not by the monitor. The monitor communicates between cores
> >>>> expecting to never hit in a cache (because it uses a non-cacheable
> >>>> mapping), but the mapping used by the OS can cause the region to be
> >>>> allocated into caches at any point in time even if it never accesses the
> >>>> region explicitly.
> >>>>
> >>>> The CPU _may_ hit in a cache even if making a non-cacheable access (this
> >>>> is called an "unexepcted data cache hit"), so the cache allocations
> >>>> caused by the OS can mask data other CPUs wrote straight to memory.
> >>>>
> >>>> Other than that case, I believe the rules given in the ARM ARM for
> >>>> mismatched memory attributes may apply for similar reasons.  Thus
> >>>> allowing the OS to map this memory can cause a loss of coherency on the
> >>>> monitor side, if the OS and monitor map the region with different
> >>>> attributes.
> >>>>
> >>>> This is all IMPLEMENTATION DEFINED, so it may be that you're fine on the
> >>>> system you're dealing with. I don't immediately know whether that is the
> >>>> case, however. Never telling the OS about the memory in the first place
> >>>> avoids the possibility in all cases.
> >>>
> >>> But from a security point of view, it must not matter if the OS maps the
> >>> memory or not - the monitor must be robust against that, no? If the
> >>> architecture cannot provide such guarantees, it has to be worked around
> >>> in software in the monitor (I hope you can do so...).
> >>
> >> Well, yes and no.
> >>
> >> In this case it sounds like due to the security controller you should
> >> never encounter the mismatched attributes issue in the first place,
> >> though you may encounter issues w.r.t. speculative accesses triggering
> >> violations arbitrarily. Not telling the OS about the secure memory means
> >> that said violations shouldn't occur in normal operation; only when the
> >> non-secure OS is trying to do something bad.
> >>
> >> If the OS has access to the memory, then you're already trusting it to
> >> not write to there or you can't trust that memory at all (and hence
> >> cannot use it). Given that means you must already assume that the OS is
> >> cooperative, it's simpler to not tell it about the memory than to add
> >> cache maintenance around every memory access within the monitor. You can
> >> never make things secure in this case, but you can at least offer the
> >> abstraction provided by PSCI.
> >>
> >> So as far as I can see in either case it's better to not tell the OS
> >> about the memory you wish to use from the monitor. If you have no HW
> >> protection and can't trust the OS then you've already lost, and if you
> >> do have HW protection you don't want it to trigger
> >> continuously/spuriously as a result of speculation.
> > 
> > OK, that makes sense again.
> > 
> > Now I just need to figure out how to split/adjust the memory node
> > instead of adding a reservation region.
> 
> This is getting invasive:
> 
> If I add carveouts via adjusting memory banks, I need to account for the
> case that an existing bank is split into two halves, creating additional
> banks this way. But then current fdt_fixup_memory_banks will no longer
> work due to its limitation to the number of physical banks. I could
> always add one spare bank to that service, ok, but then the next use
> case for carveouts will hit the wall again. So I better double that
> limit, or so.

fdt_fixup_memory_banks() will shout if it doesn't have enough banks, so
I suppose we could put that problem off to the configuration files. For
example we could add something like:

	#ifdef CONFIG_ARMV7_PSCI
	#  define CONFIG_NR_DRAM_BANKS 2
	#else
	#  define CONFIG_NR_DRAM_BANKS 1
	#endif

to tegra-common.h and make sure to define CONFIG_ARMV7_PSCI before that
is included. That could easily be extended using something like the
following if you're concerned about there being many carveout use-cases:

	#ifdef CONFIG_ARMV7_PSCI
	#  define PSCI_EXTRA_DRAM_BANKS 1
	#else
	#  define PSCI_EXTRA_DRAM_BANKS 0
	#endif

	#ifdef CONFIG_FOO
	#  define FOO_EXTRA_DRAM_BANKS 1
	#else
	#  define FOO_EXTRA_DRAM_BANKS 0
	#endif

	#define CONFIG_NR_DRAM_BANKS (1 +
				      PSCI_EXTRA_DRAM_BANKS +
				      FOO_EXTRA_DRAM_BANKS)

But I think it'd be good enough for now to go with the first snippet.

Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150219/4070be9a/attachment.sig>