Conversation
f92535a to
57284c8
Compare
9ad59e5 to
474545c
Compare
b9e1caf to
7da4f29
Compare
829c219 to
61b37aa
Compare
0e5ea75 to
bf61453
Compare
bf61453 to
20a40db
Compare
674ffd5 to
3d1e91a
Compare
20a40db to
fd1cd5d
Compare
3d1e91a to
53e6ea9
Compare
44a2cbb to
3d6619f
Compare
04f6748 to
1a50d5c
Compare
3d6619f to
1e889fb
Compare
bffccac to
661df87
Compare
ba20ef7 to
f2194f5
Compare
f2194f5 to
f245bf9
Compare
85e295e to
319dace
Compare
There was a problem hiding this comment.
This is just a quick review.
For now the my main comments are regarding stale code comments and magic numbers, I don't think my comments address them all, please check them more carefully than I did.
There's also some instances of using platformed defined values that need to be made more general to handle multiple platforms.
| #define IPI_CPU_MSG (0x1460 / 4) /* TODO this is the first GPSR in TC49 */ | ||
| #define GSPR_SRC_BASE (0x1460 / 4) /* TODO this is the first GPSR in TC49 */ |
There was a problem hiding this comment.
Move to a platform defined
There was a problem hiding this comment.
IPI_CPU_MSG needs to stay in this file due to other core files. The other is fixed in 62b4555
There was a problem hiding this comment.
What's the problem with including platform.h for the correct IPI_CPU_MSG?
Otherwise this assumes all TC4 platforms have IPI_CPU_MSG in the same place
There was a problem hiding this comment.
IR_MAX_INTERRUPTS and GSPR_SRC_BASE were moved to the wrong file
There was a problem hiding this comment.
It was a distraction. Moved them to a wrong file. Should be fixed in 8620639
There was a problem hiding this comment.
GSPR_SRC_BASE should be in src/platform/tc4dx/inc/plat/platform.h
0878a8c to
c3db84e
Compare
c3db84e to
abd09d6
Compare
83500c2 to
c5745c5
Compare
c5745c5 to
860b5b8
Compare
860b5b8 to
1bc98b7
Compare
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
1bc98b7 to
5cf813f
Compare
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
e2a03e4 to
62b4555
Compare
Optimizations were replacing calls with jump-links, which in turn were using more stack instead of CSAs. Increasing the stack size solved this problem. Furthermore, refactored the unlock function to include memory in the clobber list. Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
81871e4 to
8a2cbcb
Compare
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
5ae13a6 to
c2c9fcc
Compare
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
Signed-off-by: Miguel Silva <miguelafsilva5@gmail.com>
DavidMCerdeira
left a comment
There was a problem hiding this comment.
I've finished my second pass
In comments were I suggest to consider something, I believe it's worth doing but open to discuss why we shouldn't
| accessen->WRA |= ACCESSEN_CPU0_MASK << (cpuid * 2); | ||
| accessen->RDA |= ACCESSEN_CPU0_MASK << (cpuid * 2); |
There was a problem hiding this comment.
It's better to create a macro that takes cpuid as an argument and outputs the correct mask.
Something like ACCESSEN_CPU_MASK(c)
| { | ||
| unsigned long temp = PROT_ODEF_MASK | PROT_OWEN_MASK; | ||
| temp |= (vm << PROT_VM_POS); | ||
| temp |= (cpu * 2 << PROT_TAGID_POS); |
There was a problem hiding this comment.
Again I suggest a macro that takes cpuid as an argument and outputs the correct bits
| bitmap_clear_consecutive(cpu()->arch.mpu.data_locked, 0, MPU_DATA_MAX_NUM_ENTRIES); | ||
|
|
||
| for (mpid_t mpid = 0; mpid < MPU_CODE_MAX_NUM_ENTRIES; mpid++) { | ||
| cpu()->arch.mpu.code_entries[mpid].mpid = mpid; |
There was a problem hiding this comment.
If mpid indexes the entry, why do we need to store mpid?
|
|
||
| void mpu_enable(void) | ||
| { | ||
| csfr_corecon_write(2); |
There was a problem hiding this comment.
magic number, should also it should only set the relevant bit
| } | ||
|
|
||
| csfr_cpu_pc_write(coreid, load_addr); | ||
| csfr_cpu_bootcon_write(coreid, 0); |
| const struct plat_device dev_array[] = { | ||
| { |
There was a problem hiding this comment.
Consider moving to seperate c module for the sake of readability
| #include <platform.h> | ||
|
|
||
| #ifdef GENERATING_DEFS | ||
| uint32_t plat_ints[] = { 0, 1, 2, 3, 4, 5, 6, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, |
There was a problem hiding this comment.
consider moving to its own module for the sake of readability
| .int_addr = 0xF4430000, | ||
| .src_addr = 0xF4432000, | ||
| .GPSR_offset = 0x700, | ||
| .GPSR_size = (0xB00 + (8*4)) - 0x700, |
| #ifdef MEM_NON_UNIFIED | ||
| { | ||
| .base = 0xA0000000, | ||
| .size = 0x400000, | ||
| .perms = MEM_RX, | ||
| }, | ||
| #endif |
There was a problem hiding this comment.
Why add one this region if memory is non-unified?
There was a problem hiding this comment.
Currently, you can change the .mk file to have bao be either unified or non-unified. I can remove that, if it seems better to you.
| #ifndef MEM_NON_UNIFIED | ||
| { | ||
| // DLMU1 | ||
| .base = 0x90080000, | ||
| .size = 0x80000, | ||
| .perms = MEM_RWX, | ||
| }, | ||
| #endif |
There was a problem hiding this comment.
Why remove this region if memory is unified?
There was a problem hiding this comment.
Currently, you can change the .mk file to have bao be either unified or non-unified. I can remove that, if it seems better to you.
PR Description
This PR introduces the initial support for Infineon Tricore architectures.
This PR requires #218 to support boot from flash with a non-unified memory model.
The current state of this port supports baremetal and freeRTOS guests in single or multicore setups. At the moment, each guest is given an unique ID, and each core only executes one guest.
The following list shows what is missing: