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

Jan Kiszka
2015-02-19T11:17:05Z

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 <ijc@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.

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