Hi Michal,
On 20/04/2021 08:08, Michal Orzel wrote:
AArch64 system registers are 64bit whereas AArch32 ones
are 32bit or 64bit. MSR/MRS are expecting 64bit values thus
we should get rid of helpers READ/WRITE_SYSREG32
in favour of using READ/WRITE_SYSREG.
We should also use register_t type when reading sysregs
which can correspond to uint64_t or uint32_t.
Even though many AArch64 sysregs have upper 32bit reserved
it does not mean that they can't be widen in the future.
Modify type of vtimer structure's member: ctl to register_t.
Signed-off-by: Michal Orzel <michal.or...@arm.com>
---
xen/arch/arm/time.c | 28 ++++++++++++++--------------
xen/arch/arm/vtimer.c | 10 +++++-----
xen/include/asm-arm/domain.h | 2 +-
3 files changed, 20 insertions(+), 20 deletions(-)
diff --git a/xen/arch/arm/time.c b/xen/arch/arm/time.c
index b0021c2c69..e6c96eb90c 100644
--- a/xen/arch/arm/time.c
+++ b/xen/arch/arm/time.c
@@ -145,7 +145,7 @@ void __init preinit_xen_time(void)
preinit_acpi_xen_time();
if ( !cpu_khz )
- cpu_khz = READ_SYSREG32(CNTFRQ_EL0) / 1000;
+ cpu_khz = (unsigned long)(READ_SYSREG(CNTFRQ_EL0) / 1000);
From the Arm Arm (DDI 0486 F.c), only the lower 32-bit represents the
clock frequency. The rest are RES0 (IOW can have a different meaning in
the future). So I think you want to mask it.
static bool vtimer_cntp_ctl(struct cpu_user_regs *regs, uint32_t *r, bool read)
@@ -347,7 +347,7 @@ bool vtimer_emulate(struct cpu_user_regs *regs, union hsr
hsr)
}
static void vtimer_update_irq(struct vcpu *v, struct vtimer *vtimer,
- uint32_t vtimer_ctl)
+ register_t vtimer_ctl)
{
bool level;
@@ -389,7 +389,7 @@ void vtimer_update_irqs(struct vcpu *v)
* but this requires reworking the arch timer to implement this.
*/
vtimer_update_irq(v, &v->arch.virt_timer,
- READ_SYSREG32(CNTV_CTL_EL0) & ~CNTx_CTL_MASK);
+ READ_SYSREG(CNTV_CTL_EL0) & ~CNTx_CTL_MASK);
AFAICT, CNTx_CTL_MASK is an unsigned int. This will not only mask IMASK
but also the bits 32-63 for Armv8.
So I think you want to switch CTNx_CTL_MASK to return an unsigned long
and explain it in the commit message.
Cheers,
--
Julien Grall