Assign non-sec DDR configuration from DT #1623
Conversation
There was a problem hiding this comment.
I have a minor comments, but overall it is fine
- second patch can be split into number of smaller ones. For example,
get_dt_val_and_advance()can be introduced in separate patch. - empty lines are missing in indicated locates
Anyways:
Reviewed-by: Volodymyr Babchuk <vlad.babchuk@gmail.com>
core/arch/arm/kernel/generic_boot.c
Outdated
|
|
||
| DMSG("No SHM configured"); | ||
| return -1; | ||
|
|
There was a problem hiding this comment.
minor: empty line is not needed.
core/arch/arm/kernel/generic_boot.c
Outdated
| offs = fdt_subnode_offset(fdt, 0, "memory"); | ||
| if (offs < 0) | ||
| return NULL; | ||
| prop = fdt_getprop(fdt, offs, "reg", &addr_size); |
There was a problem hiding this comment.
minor: add empty line before prop =
core/arch/arm/kernel/generic_boot.c
Outdated
| prop = fdt_getprop(fdt, offs, "reg", &addr_size); | ||
| if (!prop) | ||
| return NULL; | ||
| prop_len = addr_size; |
There was a problem hiding this comment.
minor: minor: add empty line before prop_len =
core/arch/arm/kernel/generic_boot.c
Outdated
| addr_size = fdt_address_cells(fdt, offs); | ||
| if (addr_size < 0) | ||
| return NULL; | ||
| len_size = fdt_size_cells(fdt, offs); |
There was a problem hiding this comment.
minor: minor: add empty line before len_size =
|
Added/removed newlines as commented. |
core/arch/arm/mm/core_mmu.c
Outdated
| * non-secure memory is used for NSEC_SHM. | ||
| */ | ||
| carve_out_phys_mem(&m, &num_elems, map->pa, map->size); | ||
| } |
There was a problem hiding this comment.
I think this loop could also check that the discovered SHM does not intersect any of the 'secure' memory areas.
core/arch/arm/mm/core_mmu.c
Outdated
|
|
||
| while (!core_mmap_is_end_of_table(map)) { | ||
| if (map->type == MEM_AREA_NSEC_SHM || | ||
| map->type == MEM_AREA_SDP_MEM) { |
There was a problem hiding this comment.
SDP memory is listed in the static_memory_mapping array: actually these areas are not default (statically) mapped in the core. You must parse the array __start/end_phys_nsec_ddr_section.
core/arch/arm/mm/core_mmu.c
Outdated
|
|
||
| while (true) { | ||
| if (n >= *nelems) { | ||
| DMSG("Can't carve out %#" PRIxPA " size %#zx", |
There was a problem hiding this comment.
Minor: this sound like an error trace. Suggest "No need to carve out ..."
| return; | ||
| } | ||
| if (core_is_buffer_inside(pa, size, m[n].addr, m[n].size)) | ||
| break; |
There was a problem hiding this comment.
Should add if (core_is_buffer_intersect(...)) panic();
There was a problem hiding this comment.
Or perhaps even better, only carve out the part that intersects?
Edit: Hmm, that turned out to be a bit tricky and shouldn't happen anyway so I guess a panic() is OK.
There was a problem hiding this comment.
yes, could be fine. But drawback is that linux world may/will later try to register a SHM in the reject area and will get an late error at runtime whereas optee could warn at init of this non conformance.
|
Update |
1 similar comment
|
Update |
|
|
jenswi-linaro
force-pushed
the
dt_nsec_ddr
branch
from
58d3a2e
to
1d15b2a
Compare
|
Squashed, rebased, tags applied. |
|
The checkpatch warnings can be fixed easily, I think (at least the first one). |
|
Checkpatch fixes |
|
checkpatch fixed indeed |
Plat-vexpress uses register_nsec_ddr() to define the non-secure DDR memory. Reviewed-by: Volodymyr Babchuk <vlad.babchuk@gmail.com> Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Assigns non-secure DDR configuration from device tree if CFG_DT=y. Already present DDR configuration from register_nsec_ddr() is overridden. Reviewed-by: Volodymyr Babchuk <vlad.babchuk@gmail.com> Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org> Tested-by: Jens Wiklander <jens.wiklander@linaro.org> (QEMU) Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
jenswi-linaro
force-pushed
the
dt_nsec_ddr
branch
from
4ce386f
to
3a702c4
Compare
|
Rebased, checkpatch patch applied. |
|
Thanks |
This should take care of the problem with unexpected non-sec addresses from normal world in for instance #1232 by updating the non-sec DDR config from DT instead of using hard coded value on QEMU.
@lorc please confirm