> Subject: Re: [PATCH V2 1/4] arm64: support inmate cell in AArch32 mode
>
> On 14.08.20 15:17, Jan Kiszka wrote:
> > On 15.08.20 21:09, Alice Guo wrote:
> >> An AArch64 hypervisor can host both AArch32 and AArch64 virtual
> >> machines at the same time. If the inmate cell wants to run in AArch32
> >> mode, the assigned cpu must change to AArch32. Because AArch64
> >> hypervisor and
> >> AArch64 root cell are used, when the AArch32 inmate cell is
> >> destroyed, cpu owned by inmate cell will be reassigned to AArch64
> >> root cell, switch the cpu back to AArch64.
> >>
> >> The following is a summary of some of the points when supporting
> >> inmate cell in AArch32 mode:
> >> Define a macro "JAILHOUSE_CELL_AARCH32" to indicate AArch32
> execution
> >> state. Add this macro to flags of struct jailhouse_cell_desc, and you
> >> can use it to indicate whether a cell is AArch32.
> >>
> >> AArch32 and AArch64 virtual machines use different
> ARM_PARKING_CODE.
> >> 0xd503207f and 0x17ffffff are used in AArch64 and 0xe320f003 and
> >> 0xeafffffd are used in AArch32. Add ARM_PARKING_CODE which is used by
> >> AArch32 in arm64/include/asm/processor.h, and then select which one
> >> to use by arm_cpu_reset().
> >>
> >> When an exception occurs, the processor must execute handler code
> >> which corresponds to the exception. When the exception is being taken
> >> at a lower Exception level, the execution state of the next lower
> >> level
> >> (AArch64 or AArch32) will be used. Fill exception handling functions
> >> for Lower EL using AArch32 in hypervisor/arch/arm64/entry.S.
> >>
> >> Changing to AArch32 happens after the command "jailhouse cell start 1"
> >> is executed. Changing to AArch64 happens after the command "jailhouse
> >> cell destroy 1". If a cell is AArch32, SPSR_EL2.M[4] will be set to
> >> 0b1 which means AArch32 execution state, SPSR_EL2.M[3:0] will be set
> >> to 0b0011 which means Supervisor, and HCR_EL2.RW will be set to 0b0
> >> which means lower levels are all AArch32. If a cell is AArch64, make
> >> sure HCR_EL2.RW is 0 and the other registers are configured according
> >> to the previous code.
> >>
> >> After Linux operating system boots up, execute the following commands
> >> to use AArch32 virtual machine on the i.MX8DXL:
> >> ./jailhouse enable imx8dxl.cell
> >> ./jailhouse cell create imx8dxl-gic-demo-aarch32.cell ./jailhouse
> >> cell load 1 gic-demo.bin (32-bit) ./jailhouse cell start 1
> >>
> >> Signed-off-by: Alice Guo <[email protected]>
> >> ---
> >> hypervisor/arch/arm-common/control.c | 5 ++++-
> >> hypervisor/arch/arm64/control.c | 10
> ++++++++++
> >> hypervisor/arch/arm64/entry.S | 8 ++++----
> >> hypervisor/arch/arm64/include/asm/processor.h | 4 +++-
> >> hypervisor/arch/arm64/include/asm/sysregs.h | 4 ++++
> >> include/jailhouse/cell-config.h | 4 ++++
> >> 6 files changed, 29 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/hypervisor/arch/arm-common/control.c
> >> b/hypervisor/arch/arm-common/control.c
> >> index 70793432..bfd9e710 100644
> >> --- a/hypervisor/arch/arm-common/control.c
> >> +++ b/hypervisor/arch/arm-common/control.c
> >> @@ -32,7 +32,10 @@ void arm_cpu_park(void)
> >> enter_cpu_off(cpu_public);
> >> spin_unlock(&cpu_public->control_lock);
> >> - arm_cpu_reset(0);
> >> + if (this_cell()->config->flags & JAILHOUSE_CELL_AARCH32)
> >> + arm_cpu_reset(PARKING_ENTRY_ADDR_AARCH32);
> >> + else
> >> + arm_cpu_reset(PARKING_ENTRY_ADDR_AARCH64);
> >
> > Let's do
> >
> > arm_cpu_reset(this_cell()->config->flags & JAILHOUSE_CELL_AARCH32 ?
> > PARKING_ENTRY_ADDR_AARCH32 :
> PARKING_ENTRY_ADDR_AARCH64);
> >
>
> Just thought about again, and I think there is an even better way: Pass the
> mode as second parameter to arm_cpu_reset
"the mode" you mean "boot aarch64_mode"? or "bool parking"?
I think "bool parking" is reasonable. Because for parking, we could
park in aarch64 mode. For return to aarch32 inmate vm, we need
to configure to run in aarch32 mode.
Thanks,
Peng.
and park in AARCH64 mode
> unconditionally. Obviously, that parameter will be ignored on 32-bit ARM.
>
> Jan
>
> >> arm_paging_vcpu_init(&parking_pt);
> >> }
> >> diff --git a/hypervisor/arch/arm64/control.c
> >> b/hypervisor/arch/arm64/control.c index 6e1ffebf..71615c04 100644
> >> --- a/hypervisor/arch/arm64/control.c
> >> +++ b/hypervisor/arch/arm64/control.c
> >> @@ -20,6 +20,8 @@
> >> void arm_cpu_reset(unsigned long pc)
> >> {
> >> + u64 hcr_el2;
> >> +
> >> /* put the cpu in a reset state */
> >> /* AARCH64_TODO: handle big endian support */
> >> arm_write_sysreg(SPSR_EL2, RESET_PSR);
> >
> > Please pull this into this AARCH64 branch below to avoid needless
> > duplicate writing.
> >
> >> @@ -67,6 +69,14 @@ void arm_cpu_reset(unsigned long pc)
> >> /* AARCH64_TODO: handle PMU registers */
> >> /* AARCH64_TODO: handle debug registers */
> >> /* AARCH64_TODO: handle system registers for AArch32 state */
> >> + arm_read_sysreg(HCR_EL2, hcr_el2);
> >> + if (this_cell()->config->flags & JAILHOUSE_CELL_AARCH32) {
> >> + arm_write_sysreg(SPSR_EL2, RESET_PSR_AARCH32);
> >> + hcr_el2 &= ~HCR_RW_BIT;
> >> + } else {
> >> + hcr_el2 |= HCR_RW_BIT;
> >> + }
> >> + arm_write_sysreg(HCR_EL2, hcr_el2);
> >> arm_write_sysreg(ELR_EL2, pc);
> >> diff --git a/hypervisor/arch/arm64/entry.S
> >> b/hypervisor/arch/arm64/entry.S index 27e148c6..4789e933 100644
> >> --- a/hypervisor/arch/arm64/entry.S
> >> +++ b/hypervisor/arch/arm64/entry.S
> >> @@ -401,8 +401,8 @@ hyp_vectors:
> >> ventry .
> >> ventry .
> >> - ventry .
> >> - ventry .
> >> + handle_vmexit arch_handle_trap
> >> + handle_vmexit irqchip_handle_irq
> >> ventry .
> >> ventry .
> >> @@ -425,8 +425,8 @@ hyp_vectors_hardened:
> >> ventry .
> >> ventry .
> >> - ventry .
> >> - ventry .
> >> + handle_abort_fastpath
> >> + handle_vmexit irqchip_handle_irq
> >> ventry .
> >> ventry .
> >> diff --git a/hypervisor/arch/arm64/include/asm/processor.h
> >> b/hypervisor/arch/arm64/include/asm/processor.h
> >> index b52782b7..e7b048e0 100644
> >> --- a/hypervisor/arch/arm64/include/asm/processor.h
> >> +++ b/hypervisor/arch/arm64/include/asm/processor.h
> >> @@ -34,7 +34,9 @@ union registers {
> >> #define ARM_PARKING_CODE \
> >> 0xd503207f, /* 1: wfi */ \
> >> - 0x17ffffff, /* b 1b */
> >> + 0x17ffffff, /* b 1b */ \
> >> + 0xe320f003, /* 2: wfi */ \
> >> + 0xeafffffd, /* b 2b */
> >> #define dmb(domain) asm volatile("dmb " #domain "\n" : : :
> >> "memory")
> >> #define dsb(domain) asm volatile("dsb " #domain "\n" : : :
> >> "memory") diff --git a/hypervisor/arch/arm64/include/asm/sysregs.h
> >> b/hypervisor/arch/arm64/include/asm/sysregs.h
> >> index 0105b109..62a56743 100644
> >> --- a/hypervisor/arch/arm64/include/asm/sysregs.h
> >> +++ b/hypervisor/arch/arm64/include/asm/sysregs.h
> >> @@ -15,11 +15,13 @@
> >> #define PSR_MODE_MASK 0xf
> >> #define PSR_MODE_EL0t 0x0
> >> +#define PSR_MODE_SVC 0x3
> >> #define PSR_MODE_EL1t 0x4
> >> #define PSR_MODE_EL1h 0x5
> >> #define PSR_MODE_EL2t 0x8
> >> #define PSR_MODE_EL2h 0x9
> >> +#define PSR_32_BIT (1 << 4)
> >> #define PSR_F_BIT (1 << 6)
> >> #define PSR_I_BIT (1 << 7)
> >> #define PSR_A_BIT (1 << 8)
> >> @@ -28,6 +30,8 @@
> >> #define PSR_SS_BIT (1 << 21)
> >> #define RESET_PSR (PSR_D_BIT | PSR_A_BIT | PSR_I_BIT |
> PSR_F_BIT
> >> \
> >> | PSR_MODE_EL1h)
> >> +#define RESET_PSR_AARCH32 (PSR_A_BIT | PSR_I_BIT | PSR_F_BIT
> \
> >> + | PSR_32_BIT | PSR_MODE_SVC)
> >
> > Indention is ugly now, and naming is inconsistent. So let's align
> > everything after RESET_PSR_AARCH32, at least in this block, and rename
> > RESET_PSR to RESET_PSR_AARCH64.
> >
> >> #define MPIDR_CPUID_MASK 0xff00ffffffUL
> >> #define MPIDR_CLUSTERID_MASK 0xff00ffff00UL diff --git
> >> a/include/jailhouse/cell-config.h b/include/jailhouse/cell-config.h
> >> index 6df4a745..6fda32b9 100644
> >> --- a/include/jailhouse/cell-config.h
> >> +++ b/include/jailhouse/cell-config.h
> >> @@ -56,6 +56,10 @@
> >> #define JAILHOUSE_CELL_PASSIVE_COMMREG 0x00000001
> >> #define JAILHOUSE_CELL_TEST_DEVICE 0x00000002
> >> +#define JAILHOUSE_CELL_AARCH32 0x00000004
> >> +
> >> +#define PARKING_ENTRY_ADDR_AARCH64 0x0 #define
> >> +PARKING_ENTRY_ADDR_AARCH32 0x8
> >
> > The last two do not belong here. They are internal to the hypervisor.
> > Move them to asm/processor.h where the code block is also defined.
> >
> > Jan
> >
> >> /*
> >> * The flag JAILHOUSE_CELL_VIRTUAL_CONSOLE_PERMITTED allows
> inmates
> >> to invoke
> >>
> >
>
> --
> You received this message because you are subscribed to the Google Groups
> "Jailhouse" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to [email protected].
> To view this discussion on the web visit
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups
> .google.com%2Fd%2Fmsgid%2Fjailhouse-dev%2Ffccdea7b-8c96-8387-373e-b
> c00b163e4f4%2540web.de&data=02%7C01%7Cpeng.fan%40nxp.com%
> 7C2a9a8a8b6857465ec84808d840e4c636%7C686ea1d3bc2b4c6fa92cd99c5
> c301635%7C0%7C0%7C637330698609625736&sdata=LpYU7aQMQgLB
> a7Y8OGZAPEvuKO7c2o%2FeUK9gpp7wP8k%3D&reserved=0.
--
You received this message because you are subscribed to the Google Groups
"Jailhouse" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
To view this discussion on the web visit
https://groups.google.com/d/msgid/jailhouse-dev/DB6PR0402MB2760A09B1BCEA12BB66B88A8885F0%40DB6PR0402MB2760.eurprd04.prod.outlook.com.
