On 14.11.2018 00:03, Heinrich Schuchardt wrote:
On 11/13/18 10:47 PM, Simon Goldschmidt wrote:
Am Di., 13. Nov. 2018, 22:37 hat Heinrich Schuchardt <xypron.g...@gmx.de
<mailto:xypron.g...@gmx.de>> geschrieben:
On 11/13/18 9:01 PM, Simon Goldschmidt wrote:
> On 13.11.2018 20:42, Heinrich Schuchardt wrote:
>> On 11/13/18 6:47 AM, Simon Goldschmidt wrote:
>>> On Tue, Nov 13, 2018 at 3:23 AM Fabio Estevam
<feste...@gmail.com <mailto:feste...@gmail.com>>
>>> wrote:
>>>> Hi Simon,
>>>>
>>>> On Mon, Nov 12, 2018 at 7:25 PM Simon Goldschmidt
>>>> <simon.k.r.goldschm...@gmail.com
<mailto:simon.k.r.goldschm...@gmail.com>> wrote:
>>>>
>>>>> diff --git a/fs/fs.c b/fs/fs.c
>>>>> index adae98d021..4baf6b1c39 100644
>>>>> --- a/fs/fs.c
>>>>> +++ b/fs/fs.c
>>>>> @@ -428,13 +428,57 @@ int fs_size(const char *filename, loff_t
*size)
>>>>> return ret;
>>>>> }
>>>>>
>>>>> -int fs_read(const char *filename, ulong addr, loff_t offset,
>>>>> loff_t len,
>>>>> - loff_t *actread)
>>>>> +#ifdef CONFIG_LMB
>>>> Unrelated to your series, but I was wondering if we could get
rid of
>>>> the CONFIG_LMB option.
>>>>
>>>> As far as I can see all the architectures define it, the only
>>>> exception being arch/sh.
>>>>
>>>> If you agree I can send a patch after your series gets applied that
>>>> removes CONFIG_LMB.
>>> Sure, that would clean things up.
>>>
>>> Simon
>>>
>> NAK
>>
>> This patch-series does not provide what is needed. With
>> odroid-c2_defconfig I get
>>
>> fdt list /reserved-memory/secmon@10000000
>> reserved-memory {
>> secmon@10000000 {
>> reg = <0x00000000 0x10000000 0x00000000 0x00200000>;
>> no-map;
>> };
>> };
>>
>> => load mmc 0:1 0x10000000 dtb
>> 22925 bytes read in 8 ms (2.7 MiB/s)
>>
>> So now I have successfully overwritten the secure monitor. Urrgh.
>>
>> As you have observed load is still writing into a memory area that is
>> reserved by the device-tree.
>>
>> Please, iterate over the device tree to ensure that nothing is loaded
>> into a reserved memory area. Do not expect board files to do anything
>> but create the reserve-memory entry in the device tree.
>
> First of all, thanks for testing. I must admit I haven't tested this
> case, I just had the impression the existing function
> 'boot_fdt_add_mem_rsv_regions()' (in image-fdt.c) would do that. I'll
> have to dig into why it doesn't.
>
> Or do you have CONFIG_OF_LIBFDT disabled?
`make odroid-c2_defconfig` sets
CONFIG_OF_LIBFDT=y
CONFIG_CMD_FDT depends on it. So without I would not have had the fdt
command used above.
The device-tree I was looking at was the one provided by U-Boot at
${fdtcontroladdr}.
That might be an explanation. I used 'gd->fdt_blob' only in my patch.
For the Odroid C2 the relevant memory reservations are created in
arch/arm/mach-meson/board.c:61:
void meson_gx_init_reserved_memory(void *fdt)
According to /README ${fdtcontroladdr} and gd->fdt_blob should be the same:
lib/fdtdec.c:1255:
gd->fdt_blob = (void *)env_get_ulong("fdtcontroladdr", 16,
(uintptr_t)gd->fdt_blob);
The boards with CONFIG_OF_PRIOR_STAGE=y set fdtcontroladdr in the board
file (board/broadcom/bcmstb/bcmstb.c).
You should use gd->fdt_blob. Just make sure it is already set.
OK, so it seems U-Boot just cannot handle the "reserved-memory" node
with its subnodes but only the "memreserve" thing on top level?
I have the rest of the patches in a state I would submit, but I'll need
some more time to dig into the dts reserved memory reservation...
Simon
Best regards
Heinrich
Are there any more device tree locations to care about?
We should also think about making this testable. I would be happy if we
had a test on a QEMU board.
I tried to build the tests but I only got build errors. Is there any
documentation about what I could be missing? I think my Ubuntu should be
up to date, so maybe I am missing some dependencies...?
Simon
Best regards
Heinrich
>
> In any case, it *was* my intention to include the dts memory
> reservation! I'll make sure I test that for a v2.
>
> Simon
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
