From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Date: Thu, 19 Feb 2015 12:17:05 +0100 Subject: [U-Boot] [PATCH v2 06/12] virt-dt: Allow reservation of the secure region when it is in a RAM carveout. In-Reply-To: <20150219103400.GI5086@ulmo.nvidia.com> References: <20150216134239.GC8994@leverpostej> <54E1F5E9.8020702@siemens.com> <20150216142540.GF8994@leverpostej> <54E1FF39.5070107@siemens.com> <20150216145643.GG8994@leverpostej> <54E20ED9.4000708@siemens.com> <54E2F755.4020500@siemens.com> <20150219103400.GI5086@ulmo.nvidia.com> Message-ID: <54E5C631.7080106@siemens.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >>>>>>>>> >>>>>>>>> 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