feat(armv8r): handle different timer init cases #265
Conversation
Signed-off-by: Jose Martins <[email protected]>
|
The code seems to be reliant on platform defines. Shouldn't we try to use the infrastructure from #243 ? |
What do you mean? The only thing I can't think it might make sense is having the frequency define part of the platform description instead of only being a macro (possibly generate the macro using the scripts). |
Hmm. I thought it would be better to have a platform function or something. I just don't really like having these kind of ifdefs. But maybe I'm not understanding the code correctly and it doesn't justify that level of indirections. |
| #ifdef PLAT_GENERIC_TIMER_FREQ_HZ | ||
| timer_freq = (uint32_t)PLAT_GENERIC_TIMER_FREQ_HZ; | ||
| #else | ||
| if (platform.arch.generic_timer.base_addr == 0) { | ||
| ERROR("generic timer base_addr undefined; cannot init system counter"); | ||
| } else { | ||
| volatile struct generic_timer_cntctrl* timer_ctl; | ||
| timer_ctl = (struct generic_timer_cntctrl*)mem_alloc_map_dev(&cpu()->as, | ||
| SEC_HYP_PRIVATE, platform.arch.generic_timer.base_addr, | ||
| platform.arch.generic_timer.base_addr, sizeof(struct generic_timer_cntctrl)); | ||
|
|
||
| timer_ctl->CNTCR |= GENERIC_TIMER_CNTCTL_CNTCR_EN; | ||
| fence_ord_write(); | ||
|
|
||
| timer_freq = (uint32_t)timer_ctl->CNTDIF0; | ||
|
|
||
| mem_unmap(&cpu()->as, (vaddr_t)timer_ctl, sizeof(struct generic_timer_cntctrl), | ||
| false); | ||
| } | ||
| #endif |
There was a problem hiding this comment.
This code segment would benefit from an if(DEFINED(<>)). This would remove need for ifdef'ed code, which we have been avoid successfully in the rest of the hypervisor code, and make it easier to read IMO.
if (cur_cntfrq != 0UL) {
...
} else if (DEFINED(PLAT_GENERIC_TIMER_FREQ_HZ)) {
...
} else if (platform.arch.generic_timer.base_addr == 0) {
...
} else {
...
}
On armv8r there is no firmware and the system timer must be initialized by the hypevisor. There are multiple cases from where to get the frequecy: (i) it might be fixed already in the hardware, or (ii) it can be retrieved from the plaform level timer if such timer is defined, or (iii) the hypervisor must know by default which is it (which must be defined by platform descrption from the
PLAT_GENERIC_TIMER_FREQ_HZmacro). This PR handles all this cases.