On 2015-02-19 11:34, Thierry Reding wrote: > 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 <[email protected]> >>>>>>>>> >>>>>>>>> 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.
Well, I rather hope we can get away with v3 from the time being, i.e. enforced beginning/end of bank. Jan -- Siemens AG, Corporate Technology, CT RTC ITP SES-DE Corporate Competence Center Embedded Linux _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
