From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thierry Reding Date: Thu, 19 Feb 2015 11:34:01 +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: <54E2F755.4020500@siemens.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> Message-ID: <20150219103400.GI5086@ulmo.nvidia.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 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. Thierry -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 819 bytes Desc: not available URL: