rcar_gen5: Rebase X5H support on top of 4.10 - #14
Open
TigranGabrielyan wants to merge 37 commits into
Open
Conversation
Detail: - Create baseline for rcar gen5 platform - Config the MMU table - Config the number cores/clusters and threads 32 cores totally, 8 clusters and 4 core/cluster Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com> (cherry picked from commit a83266e)
Add HSCIF feature to support showing the secure log through the UART-PHY terminal Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 2af91cb)
This patch supports logging function with 2 modes: - Mode 1: output log direct to serial console via HSCIF driver. - Mode 2: output log via logging function which print out log by normal world console. Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 197b66c)
Enable GIC v3 following the change of ARM Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 06ce7e8)
When turning on the secondary cores, it's necessary to add mask processing of interrupt. Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com> (cherry picked from commit 33c7582)
- Following the upstream: (commit: 9e93523: core: gic: support to configure PPI interrupts ...) when having the CPU ON event from Normal world, OPTEE will make some setting on each core, as well as configure Sec SGIs and PPIs to Secure Group1 ensure that it's synchronized with primary CPU in the same way. - On VDK simulation, updating of gic_data structure is incorrect when handling CPU_ON event, Which makes above process failed at PPIs configuration. => We need to update gd->per_cpu_group_status and gd->per_cpu_group_modifier of gic_data again by reading values of GICR_IGROUPR0 and GICR_IGRPMODR0 again. Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 5e57cbd)
Added CFG_NS_VIRTUALIZATION configuration to enable Dynamic Shared Memory. If "CFG_NS_VIRTUALIZATION=y" is specified, the shared memory is set as follows: - Dynamic Shared Memory: Enabled - Reserved Shared Memory: Disabled Change fast SMC to OPTEE_SMC_ENABLE_SHM_CACHE to determine the completion of Normal World initialization. Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit e875302)
Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 3a67856)
Update interrupt id following affinity routing mechanism of GICv4 architecture Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 7123109)
Below are necessary functions need to change: - gic_it_add() - gic_it_set_prio() - gic_it_enable() - gic_it_disable() - probe_max_it() Resolve conflict in gic_it_configure() function caused by changes related to introduction of is_espi flag and renaming CFG_ARM_GICV3 to _CFG_ARM_V3_OR_V4 (on 4.10). Signed-off-by: Tu Tran <tu.tran.wz@renesas.com> Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com> (cherry picked from commit bb78be1)
Update the base address of GICR following the hardware specification Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 514ed93)
During the CPU OFF handler, the cpu mask calculation for CPU31 is incorrect, which resulted in an infinite loop forerver. Include distinct functions to support mask CPU of CPU_ON and CPU_OFF, which prevents shifting the CPU mask with the incorrect value. Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 0d3173b)
Update the GIC view following the latest setting of IPL, BL31 and LinuxBSP Signed-off-by: namnguyen10 <nam.nguyen.ub@bp.renesas.com> Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 20ed24e)
Implement the SWDT driver for gen5 Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 9566227)
Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 378c053)
There are some areas that registers access is performed by physical address We need to map virtual address to physical address one by one Resolve conflict in assign_mem_va_dir() function caused by introducing of core_mmu_va_is_valid() function in 4.10. commit 26685a9 ("core: mm: factor out virtual address range validation to arch code") Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com> (cherry picked from commit 40ee54e)
Implement the COMM driver with polling method for CA and CM33 cores communication, which supports some features need to call to RSIP-M LLD Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 571d7bb)
Change to use the register_phys_mem_pgdir() in stead of register_phys_mem() to avoid hanging when mapping direct areas (virtual address = physical address) Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 1ef3cb7)
Implement the TRNG function that is using the RSIPM API to request the random number from hardware Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit f183475)
Update the memory map for communication between CA and CM core Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 24d10d2)
Signed-off-by: Tu Tran <tu.tran.wz@renesas.com> (cherry picked from commit d119548)
Implement MFIS mutex lock/unlock to prevent conflict logs that display through serial HSCIF0 Resolve conflict in trace_ext_puts() function caused by introducing release_trace_contention() function in 4.10. commit 3ad46cd ("core: kernel: allow not masking interrupts on output traces") Signed-off-by: namnguyen10 <nam.nguyen.ub@bp.renesas.com> Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com> (cherry picked from commit 91658a7)
Signed-off-by: Dien Pham <dien.pham.ry@renesas.com> (cherry picked from commit ff5f407)
Add the condition for MFIS mutex initialization through the SCIF configuration. When user want to use the SCIF feature, the MFIS mutex control must be enabled to prevent to conflict logs by HSCIF0. ===================== If CFG_SCIF = y -> CFG_MFIS_DRV = y ===================== Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 69a54b0)
There are some change points: 1/ Update the start address of TZDRAM to ensure alignment between tee.dmp and tee.srec 2/ Update the memory map for gen5 to ensure that the MMU table is cleared. Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 1de43b4)
…tication There are some change points: - Simplify get_cert_size() implementation - Improve argument naming in check_object_addr() - Refine TA_CONTENT_CERT_ADDR macro - Add error logging for Secure Boot API failures Signed-off-by: Tu Tran <tu.tran.wz@renesas.com> (cherry picked from commit 037f344)
Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 5382260)
- Implement transfer of the R-Car software revision to the Normal World for default display of optee_os information. - Display the RCar revision in initialization process when enable the config CFG_SCIF=y. Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com> (cherry picked from commit 03f3158)
Adjust the MFIS Mutex control for HSCIF to disable write protection on the target register, Which mechanism ensures avoiding conflicts when updating register values across components. Signed-off-by: Nhi Nguyen <nhi.nguyen.kx@renesas.com> (cherry picked from commit 134a538)
OP-TEE on gen5 advertised no dynamic SHM, so the Xen OP-TEE mediator
rejected EXCHANGE_CAPABILITIES and the Linux optee driver failed probe
in dom0 and DomD ("optee: capabilities mismatch" / -22)
Three reasons, all of them are fixed here:
1) plat-rcar_gen5 never called register_ddr() (CFG_DT=n), so
phys_ddr_overall was empty, core_mmu_nsec_ddr_is_defined() was false and
tee_entry_exchange_capabilities() did not set
OPTEE_SMC_SEC_CAP_DYNAMIC_SHM. Register the board non-secure DDR.
core_mmu_set_discovered_nsec_ddr() panics if a range overlaps a
non-secure_only OP-TEE map, and the low bank (0x40000000-0xC0000000) is
full of them (TZDRAM/TEE RAM @0x8C400000, SoC reg block, ...) plus
firmware-reserved areas, so only the clean high banks (>=0x10_80000000,
per r8a78000-ironhide-common.dtsi) are registered. DomD (8 GiB) does not
fit in the low bank anyway and is placed by Xen in these high banks, so
guest SHM buffers are covered.
2) CFG_CORE_ARM64_PA_BITS is forced to 36 (64GB), so OP-TEE could neither
address nor register the high banks (core_mmu_check_end_pa() panic).
Enable CFG_AUTO_MAX_PA_BITS=y to use the runtime HW PARange (>=40 bits
here), like plat-vexpress does for virtualization.
3) Under CFG_NS_VIRTUALIZATION the per-guest MMU partition base translation
table must fit one 4KB page; with 32 cores at 36-bit VA it is 32KB and
trips a COMPILE_TIME_ASSERT in core_alloc_mmu_prtn(). Narrow the
virtualization build to a 33-bit address space (still covers every
static map; the high banks are reached via PA width and the on-demand
SHM window, not VA). Non-virtualization builds keep the 36-bit address
space.
Signed-off-by: Andrii Shtompel <andrii_shtompel@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
Reviewed-by: Leonid Komarianskyi <leonid_komarianskyi@epam.com>
(cherry picked from commit 7f25ff9)
In mk/aosp_optee.mk the signed TA (.ta) is registered as an EXECUTABLES prebuilt, so AOSP runs check_elf_file on it. A signed TA is an ELF binary wrapped in a signed header and does not start with the ELF magic. On Android 17 the build no longer passes --skip-bad-elf-magic to check_elf_file, so this is now fatal: error: File "<uuid>.ta" must have a valid ELF magic word. Set LOCAL_CHECK_ELF_FILES := false for the .ta module in mk/aosp_optee.mk to skip this check. Signed-off-by: ming-jen.chang <ming-jen.chang@mediatek.com> Signed-off-by: Leo Chen <shf.chen@mediatek.com> Acked-by: Jens Wiklander <jenswi@kernel.org> Acked-by: Tadd Kao <tadd.kao@mediatek.com> (cherry picked from commit 887ccb3)
Remove workaround related to decreasing CFG_LPAE_ADDR_SPACE_BITS when virtualization is enabled. In 4.10 in core_mmu_get_total_pages_size() ROUNDUP() is used, and COMPILE_TIME_ASSERT is removed. After removing this workaround with CFG_DYN_CONFIG=n base_xlation_table = 32 KiB (2 tables × 32 cores × 512 B), while in case of 33 bits it was 4KiB. Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com>
CFG_DYN_CONFIG, new in 4.10, makes thread_init_threads() allocate threads[] from the guest heap instead of .bss. With 32 threads that is 32 * 1792 = 57344 B of the 65536 B pool, leaving 9552 B, while crypto_rng_init() needs 33 * sizeof(struct ltc_hash_ctx) = 14256 B for the Fortuna pools -- hence TEE_ERROR_OUT_OF_MEMORY. Eight threads bring that to 14336 B and leave the heap with 40224 B. They also shrink the PGT cache from 256 KiB to 64 KiB and the thread stacks from 384 KiB to 96 KiB, both per guest, so VCORE_UNPG_RW drops from 388 KiB to 196 KiB. Eight simultaneous yielding calls per guest is above the observed load, and the Xen OP-TEE mediator already enforces the same per-domain limit, returning OPTEE_SMC_RETURN_ETHREAD_LIMIT which the Linux driver retries. Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com>
ROUNDDOWN should not be used here, since MAP_DEVICE_REG_RCAR_SIZE is not power of 2. With previous implementation of ROUNDDOWN the value of the MEMORY3_BASE was unintentionally correctly calculated due to MAP_DEVICE_REG_RCAR_BASE and MAP_DEVICE_REG_RCAR_SIZE values. However, in new implementation of ROUNDDOWN the value is calculated incorrectly (with 28MiB shift from base). All necessary alignments are performed inside register_phys_mem_pgdir(). Signed-off-by: Tigran Gabrielyan <tigran_gabrielyan@epam.com>
Chain each TA dev kit onto the one declared before it, and make tee.bin depend on them all, so that a build enters optee_os once at a time instead of several times at once. Every rule here runs make over the same output directory with its own CFG_USER_TA_TARGETS: the core rule builds the default goal, which covers the dev kits of $(OPTEE_TA_TARGETS) as well, and one dev kit rule is emitted per TA target in use. They all rewrite the configuration of that output directory, and everything built there is compiled against it, so two of them running at once write the same files at the same time. The Android build system does run them at once, as it turns them into independent ninja edges. The dev kits come first because the TAs are built against them, and an early TA setup feeds a built TA back into the core build. Signed-off-by: Andrii Shtompel <andrii_shtompel@epam.com> Acked-by: Jens Wiklander <jens.wiklander@oss.qualcomm.com> Reviewed-by: Tadd Kao <tadd.kao@mediatek.com> (cherry picked from commit cd9f542)
Add ta_dev_kit_conf, which exports conf.h, conf.mk and conf.cmake. They are written by plain shell commands, so this target compiles nothing and completes in about a second, unlike ta_dev_kit, which builds the libraries too. It is meant for dev kit consumers that need the configuration before they can build anything: optee_test picks its sources from the CFG_* flags it reads from the dev kit conf.mk, and some build systems have to know their sources before any rule can run. Signed-off-by: Andrii Shtompel <andrii_shtompel@epam.com> Reviewed-by: Jerome Forissier <jerome.forissier@arm.com> Acked-by: Tadd Kao <tadd.kao@mediatek.com> (cherry picked from commit 61f0064)
Run ta_dev_kit_conf while the makefiles are parsed, so that the configuration is on disk before any Android.mk is read; building the dev kit itself is left to the graph as before. The MD5 of the generated conf.mk is kept in a variable so that ckati parses again when the configuration changes, even though it comes from files ckati never reads itself. All optee_os invocations now share one command line, so the configuration read while parsing cannot drift from the one the dev kit is built with. This is for the Android.mk files which select what to build from the CFG_ flags of the dev kit, as optee_test does for its GP socket tests. They read them with -include $(TA_DEV_KIT_DIR)/host_include/conf.mk, which finds nothing on a clean build: ckati parses the makefiles to emit the ninja graph, and the dev kit is built by that very graph. The flags then come out empty and the sources they select are dropped with no diagnostic, so what ends up in the binary depends on whether the output directory is warm. No build dependency can fix this, as the parsing is what produces the rules. Signed-off-by: Andrii Shtompel <andrii_shtompel@epam.com> Reviewed-by: Jerome Forissier <jerome.forissier@arm.com> Acked-by: Tadd Kao <tadd.kao@mediatek.com> (cherry picked from commit 591fa9e)
|
Looks good to me. Commits order is ok (they are shown by date and not in order). |
lorc
reviewed
Sep 2, 2026
lorc
left a comment
lorc
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for your submission. I understand that you (mostly) just rebased the patches made by Renesas.
Problem is that these patches are well known to have security issues. I highlighted some of these in my comments below. So, I am against of using these in any sort of production. The good news is that we actually don't need all this stuff and simple port (like this: OP-TEE#7974) should work good enough.
Anyways, do not perceive this review as critic of your job. It is mostly call for attention for @klogg and @arminn . Guys, we really should not use this Renesas stuff. Especially if you want to get approval for Widevine.
This diff is just the cherry on top:
-CFG_INSECURE ?= y
+CFG_INSECURE ?= n
| endif | ||
| endif # CFG_WARN_INSECURE defined | ||
| CFG_INSECURE ?= y | ||
| CFG_INSECURE ?= n |
Collaborator
There was a problem hiding this comment.
This is very fishy. Please remove this.
| TEE_Result res = TEE_SUCCESS; | ||
| TEE_Time t = { }; | ||
|
|
||
| res = crypto_rng_init(&t, sizeof(t)); |
Collaborator
There was a problem hiding this comment.
This is not secure at all. You always initialize RNG with the same state.
| static uint8_t remain; | ||
| bool mmu_enabled = cpu_mmu_enabled(); | ||
|
|
||
| if (mmu_enabled) { |
Collaborator
There was a problem hiding this comment.
We have chip_to_base for this....
| struct serial_ops { | ||
| /* Mandatory handler */ | ||
| void (*putc)(struct serial_chip *chip, int ch); | ||
| void (*putc)(struct serial_chip *chip, uint8_t ch); |
Collaborator
There was a problem hiding this comment.
I don't think that it is a good idea to alter global API
| static int8_t *log_nonsec_ptr __nex_bss; | ||
| uint32_t log_spin_lock __nex_bss; | ||
| uint32_t cpu_id_run_log __nex_bss; | ||
| int32_t is_normal_world_initialized __nex_bss; |
Collaborator
There was a problem hiding this comment.
I'm curious how this is gonna work, taking into account in one VM normal word can be initialized, but in another one - not.
Also, to which VM will go log_debug_send() output? To all of them?
|
|
||
| void r_comm_trigger_secure(const void *p_sevice_param, size_t param_size) | ||
| { | ||
| while (p_shared_flag->flag != REQ_STATE_NONE) |
Collaborator
There was a problem hiding this comment.
What is the common wait time for this?
| void *p_data; /* A pointer where the actual data is stored */ | ||
| } r_lsc_comm_payload_t; | ||
|
|
||
| extern volatile shared_flag_t *p_shared_flag; |
Collaborator
There was a problem hiding this comment.
How do you serialize access to this shared buffer? I don't see spinlocks or other synchronisation primitives
| p_shared_flag->flag = REQ_STATE_NONE; | ||
|
|
||
| /* Set payload */ | ||
| p_payload->service_id = (uint8_t)SERVICE_ID_TRNG; |
Collaborator
There was a problem hiding this comment.
Again, there is not synchronization while accessing the global resource
| else | ||
| lock_reg = mfis_mfislckr_table[target]; | ||
|
|
||
| /* Wait until acquired (with timeout to avoid hang) */ |
Collaborator
There was a problem hiding this comment.
So, if timeout is occured, this function will return like normal, without taking the lock. This makes no sense.
| else | ||
| $(call force,CFG_LPAE_ADDR_SPACE_BITS,36) | ||
| endif | ||
|
|
Collaborator
There was a problem hiding this comment.
Well, you can squash this change into commit that introduced it. No need to add and then remove code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bring X5H support made on 4.4 to 4.10.
In 4.10 thread management was changed, so when CFG_DYN_CONFIG=y, threads are created in heap (with 32 threads that is
32 * 1792 = 57344 B of the 65536 B pool), while before they were created in .bss. Remaining heap was not sufficient for normal operation. Taking into account overall load on optee-os and some limitations in Xen OP-TEE mediator layer, the decision was made to decrease number of threads to 8 (commit 914e9f4 (gen5: reduce CFG_NUM_THREADS to 8)).
However, solutions below can be considered as well: