Skip to content

feat(armv8r): handle different timer init cases #265

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(armv8r): handle different timer init cases #265

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions src/arch/armv8/armv8-r/vmm.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,38 @@ static uint32_t timer_freq = 0;
void vmm_arch_profile_init()
{
if (cpu_is_master()) {
/**
* Since there is no firmware in cortex-r platforms, we need to initialize the system
* counter.
*/
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 = timer_ctl->CNTDIF0;

mem_unmap(&cpu()->as, (vaddr_t)timer_ctl, sizeof(struct generic_timer_cntctrl), false);
unsigned long cur_cntfrq = sysreg_cntfrq_el0_read();
if (cur_cntfrq != 0UL) {
timer_freq = (uint32_t)cur_cntfrq;
} else {
#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
Comment on lines +22 to +41
Copy link
Member

@DavidMCerdeira DavidMCerdeira Nov 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 {
...
}

Copy link
Member

@DavidMCerdeira DavidMCerdeira Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As you mentioned before we could also extend the platform description. Specifically, adding a fixed_freq field and checking if it is set. In this case this should replace the PLAT_GENERIC_TIMER_FREQ_HZ macro IMO.

...
} else if (platform.arch.generic_timer.fixed_freq != 0) {
...

}
}

cpu_sync_barrier(&cpu_glb_sync);

/* Program CNTFRQ_EL0 and verify. */
sysreg_cntfrq_el0_write(timer_freq);
if (sysreg_cntfrq_el0_read() != (unsigned long)timer_freq) {
ERROR("failed to program CNTFRQ_EL0 to %u Hz", timer_freq);
}
}
Loading