On 21/04/2021 08:36, Michal Orzel wrote:
Hi Julien,
Hi Michal,
On 20.04.2021 15:12, Julien Grall wrote:
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 registers: actlr, cntkctl to register_t.
ACTLR is implementation defined, so in theory there might already bits already
defined in the range [32:63]. So I would consider to split it from the patch so
we can backport it.
You mean not to touch it at all in this series or to create a seperate patch
only for ACTLR?
I meant creating a separate patch for ACTLR as part of this series.
Signed-off-by: Michal Orzel <[email protected]>
---
xen/arch/arm/domain.c | 20 ++++++++++----------
xen/include/asm-arm/domain.h | 4 ++--
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index bdd3d3e5b5..c021a03c61 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -113,13 +113,13 @@ static void ctxt_switch_from(struct vcpu *p)
p->arch.tpidr_el1 = READ_SYSREG(TPIDR_EL1);
/* Arch timer */
- p->arch.cntkctl = READ_SYSREG32(CNTKCTL_EL1);
+ p->arch.cntkctl = READ_SYSREG(CNTKCTL_EL1);
virt_timer_save(p);
if ( is_32bit_domain(p->domain) && cpu_has_thumbee )
{
- p->arch.teecr = READ_SYSREG32(TEECR32_EL1);
- p->arch.teehbr = READ_SYSREG32(TEEHBR32_EL1);
+ p->arch.teecr = READ_SYSREG(TEECR32_EL1);
+ p->arch.teehbr = READ_SYSREG(TEEHBR32_EL1);
It feels strange you converted cntkctl and actlr to use register_t but not
teecr and teehbr. Can you explain why?
Because thumbee and its registers do not appear in any of ARMv8 ARM version.
This was defined on the very first version of the ARMv8-A spec (issue
a.a) but was dropped on subsequent versinos. Hence why the compiler
doesn't shout at you when finding TEECR32_EL1 and TEEHBR32_EL1.
This means that this code could be protected even with #ifdef CONFIG_ARM_32 as
AArch64 do not have them.
+1. The feature was completely dropped for Armv8 (even if it appeared in
earlier version of the spec). I have checked AMD Seattle, QEMU and the
Foundation model to confirm the feature is not exposed.
Cheers,
--
Julien Grall
