| Message ID | 20220213035753.34577-1-akihiko.odaki@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] target/arm: Support PSCI 1.1 and SMCCC 1.0 | expand |
On Sun, 13 Feb 2022 at 03:58, Akihiko Odaki <akihiko.odaki@gmail.com> wrote: > > Support the latest PSCI on TCG and HVF. A 64-bit function called from > AArch32 now returns NOT_SUPPORTED, which is necessary to adhere to SMC > Calling Convention 1.0. It is still not compliant with SMCCC 1.3 since > they do not implement mandatory functions. > > Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> > --- Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0 for any user-visible changes. (I noticed while reviewing this that we report KVM's PSCI via the DTB as only 0.2 even if KVM's actually implementing better than that; I'll write a patch to clean that up.) -- PMM
On Sun, 13 Feb 2022 at 03:58, Akihiko Odaki <akihiko.odaki@gmail.com> wrote: > > Support the latest PSCI on TCG and HVF. A 64-bit function called from > AArch32 now returns NOT_SUPPORTED, which is necessary to adhere to SMC > Calling Convention 1.0. It is still not compliant with SMCCC 1.3 since > they do not implement mandatory functions. > /* PSCI v0.2 return values used by TCG emulation of PSCI */ > > /* No Trusted OS migration to worry about when offlining CPUs */ > #define QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED 2 > > -/* We implement version 0.2 only */ > -#define QEMU_PSCI_0_2_RET_VERSION_0_2 2 > +#define QEMU_PSCI_VERSION_0_1 0x00001 > +#define QEMU_PSCI_VERSION_0_2 0x00002 > +#define QEMU_PSCI_VERSION_1_1 0x10001 Just noticed that there's a minor issue with this change -- it deletes the definition of QEMU_PSCI_0_2_RET_VERSION_0_2, but it is still used below: > > MISMATCH_CHECK(QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED, PSCI_0_2_TOS_MP); > MISMATCH_CHECK(QEMU_PSCI_0_2_RET_VERSION_0_2, here ^^ which means that this breaks compilation on Arm hosts. I'll squash in the fix: --- a/target/arm/kvm-consts.h +++ b/target/arm/kvm-consts.h @@ -98,8 +98,11 @@ MISMATCH_CHECK(QEMU_PSCI_1_0_FN_PSCI_FEATURES, PSCI_1_0_FN_PSCI_FEATURES); #define QEMU_PSCI_VERSION_1_1 0x10001 MISMATCH_CHECK(QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED, PSCI_0_2_TOS_MP); -MISMATCH_CHECK(QEMU_PSCI_0_2_RET_VERSION_0_2, +/* We don't bother to check every possible version value */ +MISMATCH_CHECK(QEMU_PSCI_VERSION_0_2, (PSCI_VERSION_MAJOR(0) | PSCI_VERSION_MINOR(2))); +MISMATCH_CHECK(QEMU_PSCI_VERSION_1_1, + (PSCI_VERSION_MAJOR(1) | PSCI_VERSION_MINOR(1))); /* PSCI return values (inclusive of all PSCI versions) */ #define QEMU_PSCI_RET_SUCCESS 0 thanks -- PMM
On Thu, 24 Feb 2022 at 13:25, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Sun, 13 Feb 2022 at 03:58, Akihiko Odaki <akihiko.odaki@gmail.com> wrote: > > > > Support the latest PSCI on TCG and HVF. A 64-bit function called from > > AArch32 now returns NOT_SUPPORTED, which is necessary to adhere to SMC > > Calling Convention 1.0. It is still not compliant with SMCCC 1.3 since > > they do not implement mandatory functions. > > > /* PSCI v0.2 return values used by TCG emulation of PSCI */ > > > > /* No Trusted OS migration to worry about when offlining CPUs */ > > #define QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED 2 > > > > -/* We implement version 0.2 only */ > > -#define QEMU_PSCI_0_2_RET_VERSION_0_2 2 > > +#define QEMU_PSCI_VERSION_0_1 0x00001 > > +#define QEMU_PSCI_VERSION_0_2 0x00002 > > +#define QEMU_PSCI_VERSION_1_1 0x10001 > > Just noticed that there's a minor issue with this change -- it > deletes the definition of QEMU_PSCI_0_2_RET_VERSION_0_2, but > it is still used below: > > > > > MISMATCH_CHECK(QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED, PSCI_0_2_TOS_MP); > > MISMATCH_CHECK(QEMU_PSCI_0_2_RET_VERSION_0_2, > > here ^^ which means that this breaks compilation on Arm hosts. > > I'll squash in the fix: > > --- a/target/arm/kvm-consts.h > +++ b/target/arm/kvm-consts.h > @@ -98,8 +98,11 @@ MISMATCH_CHECK(QEMU_PSCI_1_0_FN_PSCI_FEATURES, > PSCI_1_0_FN_PSCI_FEATURES); > #define QEMU_PSCI_VERSION_1_1 0x10001 > > MISMATCH_CHECK(QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED, PSCI_0_2_TOS_MP); > -MISMATCH_CHECK(QEMU_PSCI_0_2_RET_VERSION_0_2, > +/* We don't bother to check every possible version value */ > +MISMATCH_CHECK(QEMU_PSCI_VERSION_0_2, > (PSCI_VERSION_MAJOR(0) | PSCI_VERSION_MINOR(2))); > +MISMATCH_CHECK(QEMU_PSCI_VERSION_1_1, > + (PSCI_VERSION_MAJOR(1) | PSCI_VERSION_MINOR(1))); Ha, turns out the existing check line was wrong : it ORs together the major and minor, which only works if the major happens to be 0. Actually working lines: MISMATCH_CHECK(QEMU_PSCI_VERSION_0_2, PSCI_VERSION(0, 2)); MISMATCH_CHECK(QEMU_PSCI_VERSION_1_1, PSCI_VERSION(1, 1)); -- PMM
On 2022/02/24 21:53, Peter Maydell wrote: > On Sun, 13 Feb 2022 at 03:58, Akihiko Odaki <akihiko.odaki@gmail.com> wrote: >> >> Support the latest PSCI on TCG and HVF. A 64-bit function called from >> AArch32 now returns NOT_SUPPORTED, which is necessary to adhere to SMC >> Calling Convention 1.0. It is still not compliant with SMCCC 1.3 since >> they do not implement mandatory functions. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> >> --- > > Applied, thanks. > > Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0 > for any user-visible changes. > > (I noticed while reviewing this that we report KVM's PSCI via > the DTB as only 0.2 even if KVM's actually implementing better > than that; I'll write a patch to clean that up.) > > -- PMM I don't have an account on https://wiki.qemu.org/ so can you create one? I'll update the changelog once I get access to the account. Regards, Akihiko Odaki
On Fri, 25 Feb 2022 at 03:36, Akihiko Odaki <akihiko.odaki@gmail.com> wrote: > > On 2022/02/24 21:53, Peter Maydell wrote: > > On Sun, 13 Feb 2022 at 03:58, Akihiko Odaki <akihiko.odaki@gmail.com> wrote: > >> > >> Support the latest PSCI on TCG and HVF. A 64-bit function called from > >> AArch32 now returns NOT_SUPPORTED, which is necessary to adhere to SMC > >> Calling Convention 1.0. It is still not compliant with SMCCC 1.3 since > >> they do not implement mandatory functions. > >> > >> Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> > >> --- > > > > Applied, thanks. > > > > Please update the changelog at https://wiki.qemu.org/ChangeLog/7.0 > > for any user-visible changes. > > > > (I noticed while reviewing this that we report KVM's PSCI via > > the DTB as only 0.2 even if KVM's actually implementing better > > than that; I'll write a patch to clean that up.) > I don't have an account on https://wiki.qemu.org/ so can you create one? > I'll update the changelog once I get access to the account. Oops, I accidentally used my canned-email-reply for "applied a pull request" when I meant to use "applied a patch to target-arm.next". You don't need to update the changelog -- I'll do that when I next send a pull request for the arm tree and it gets merged. Sorry for the confusion. -- PMM
diff --git a/hw/arm/boot.c b/hw/arm/boot.c index b1e95978f26..0eeef94ceb5 100644 --- a/hw/arm/boot.c +++ b/hw/arm/boot.c @@ -488,9 +488,15 @@ static void fdt_add_psci_node(void *fdt) } qemu_fdt_add_subnode(fdt, "/psci"); - if (armcpu->psci_version == 2) { - const char comp[] = "arm,psci-0.2\0arm,psci"; - qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp)); + if (armcpu->psci_version == QEMU_PSCI_VERSION_0_2 || + armcpu->psci_version == QEMU_PSCI_VERSION_1_1) { + if (armcpu->psci_version == QEMU_PSCI_VERSION_0_2) { + const char comp[] = "arm,psci-0.2\0arm,psci"; + qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp)); + } else { + const char comp[] = "arm,psci-1.0\0arm,psci-0.2\0arm,psci"; + qemu_fdt_setprop(fdt, "/psci", "compatible", comp, sizeof(comp)); + } cpu_off_fn = QEMU_PSCI_0_2_FN_CPU_OFF; if (arm_feature(&armcpu->env, ARM_FEATURE_AARCH64)) { diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 5a9c02a2561..307a83a7bb6 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -1110,11 +1110,12 @@ static void arm_cpu_initfn(Object *obj) * picky DTB consumer will also provide a helpful error message. */ cpu->dtb_compatible = "qemu,unknown"; - cpu->psci_version = 1; /* By default assume PSCI v0.1 */ + cpu->psci_version = QEMU_PSCI_VERSION_0_1; /* By default assume PSCI v0.1 */ cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE; if (tcg_enabled() || hvf_enabled()) { - cpu->psci_version = 2; /* TCG and HVF implement PSCI 0.2 */ + /* TCG and HVF implement PSCI 1.1 */ + cpu->psci_version = QEMU_PSCI_VERSION_1_1; } } diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index 0dc96560d34..1701fb8bbdb 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -653,7 +653,7 @@ static bool hvf_handle_psci_call(CPUState *cpu) switch (param[0]) { case QEMU_PSCI_0_2_FN_PSCI_VERSION: - ret = QEMU_PSCI_0_2_RET_VERSION_0_2; + ret = QEMU_PSCI_VERSION_1_1; break; case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE: ret = QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED; /* No trusted OS */ @@ -721,6 +721,31 @@ static bool hvf_handle_psci_call(CPUState *cpu) case QEMU_PSCI_0_2_FN_MIGRATE: ret = QEMU_PSCI_RET_NOT_SUPPORTED; break; + case QEMU_PSCI_1_0_FN_PSCI_FEATURES: + switch (param[1]) { + case QEMU_PSCI_0_2_FN_PSCI_VERSION: + case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE: + case QEMU_PSCI_0_2_FN_AFFINITY_INFO: + case QEMU_PSCI_0_2_FN64_AFFINITY_INFO: + case QEMU_PSCI_0_2_FN_SYSTEM_RESET: + case QEMU_PSCI_0_2_FN_SYSTEM_OFF: + case QEMU_PSCI_0_1_FN_CPU_ON: + case QEMU_PSCI_0_2_FN_CPU_ON: + case QEMU_PSCI_0_2_FN64_CPU_ON: + case QEMU_PSCI_0_1_FN_CPU_OFF: + case QEMU_PSCI_0_2_FN_CPU_OFF: + case QEMU_PSCI_0_1_FN_CPU_SUSPEND: + case QEMU_PSCI_0_2_FN_CPU_SUSPEND: + case QEMU_PSCI_0_2_FN64_CPU_SUSPEND: + case QEMU_PSCI_1_0_FN_PSCI_FEATURES: + ret = 0; + break; + case QEMU_PSCI_0_1_FN_MIGRATE: + case QEMU_PSCI_0_2_FN_MIGRATE: + default: + ret = QEMU_PSCI_RET_NOT_SUPPORTED; + } + break; default: return false; } diff --git a/target/arm/kvm-consts.h b/target/arm/kvm-consts.h index 580f1c1fee0..ee877aa3a5c 100644 --- a/target/arm/kvm-consts.h +++ b/target/arm/kvm-consts.h @@ -77,6 +77,8 @@ MISMATCH_CHECK(QEMU_PSCI_0_1_FN_MIGRATE, KVM_PSCI_FN_MIGRATE); #define QEMU_PSCI_0_2_FN64_AFFINITY_INFO QEMU_PSCI_0_2_FN64(4) #define QEMU_PSCI_0_2_FN64_MIGRATE QEMU_PSCI_0_2_FN64(5) +#define QEMU_PSCI_1_0_FN_PSCI_FEATURES QEMU_PSCI_0_2_FN(10) + MISMATCH_CHECK(QEMU_PSCI_0_2_FN_CPU_SUSPEND, PSCI_0_2_FN_CPU_SUSPEND); MISMATCH_CHECK(QEMU_PSCI_0_2_FN_CPU_OFF, PSCI_0_2_FN_CPU_OFF); MISMATCH_CHECK(QEMU_PSCI_0_2_FN_CPU_ON, PSCI_0_2_FN_CPU_ON); @@ -84,14 +86,16 @@ MISMATCH_CHECK(QEMU_PSCI_0_2_FN_MIGRATE, PSCI_0_2_FN_MIGRATE); MISMATCH_CHECK(QEMU_PSCI_0_2_FN64_CPU_SUSPEND, PSCI_0_2_FN64_CPU_SUSPEND); MISMATCH_CHECK(QEMU_PSCI_0_2_FN64_CPU_ON, PSCI_0_2_FN64_CPU_ON); MISMATCH_CHECK(QEMU_PSCI_0_2_FN64_MIGRATE, PSCI_0_2_FN64_MIGRATE); +MISMATCH_CHECK(QEMU_PSCI_1_0_FN_PSCI_FEATURES, PSCI_1_0_FN_PSCI_FEATURES); /* PSCI v0.2 return values used by TCG emulation of PSCI */ /* No Trusted OS migration to worry about when offlining CPUs */ #define QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED 2 -/* We implement version 0.2 only */ -#define QEMU_PSCI_0_2_RET_VERSION_0_2 2 +#define QEMU_PSCI_VERSION_0_1 0x00001 +#define QEMU_PSCI_VERSION_0_2 0x00002 +#define QEMU_PSCI_VERSION_1_1 0x10001 MISMATCH_CHECK(QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED, PSCI_0_2_TOS_MP); MISMATCH_CHECK(QEMU_PSCI_0_2_RET_VERSION_0_2, diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 71c3ca69717..64d48bfb19d 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -864,7 +864,7 @@ int kvm_arch_init_vcpu(CPUState *cs) cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_POWER_OFF; } if (kvm_check_extension(cs->kvm_state, KVM_CAP_ARM_PSCI_0_2)) { - cpu->psci_version = 2; + cpu->psci_version = QEMU_PSCI_VERSION_0_2; cpu->kvm_init_features[0] |= 1 << KVM_ARM_VCPU_PSCI_0_2; } if (!arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { diff --git a/target/arm/psci.c b/target/arm/psci.c index b279c0b9a45..6c1239bb968 100644 --- a/target/arm/psci.c +++ b/target/arm/psci.c @@ -57,7 +57,7 @@ void arm_handle_psci_call(ARMCPU *cpu) { /* * This function partially implements the logic for dispatching Power State - * Coordination Interface (PSCI) calls (as described in ARM DEN 0022B.b), + * Coordination Interface (PSCI) calls (as described in ARM DEN 0022D.b), * to the extent required for bringing up and taking down secondary cores, * and for handling reset and poweroff requests. * Additional information about the calling convention used is available in @@ -80,7 +80,7 @@ void arm_handle_psci_call(ARMCPU *cpu) } if ((param[0] & QEMU_PSCI_0_2_64BIT) && !is_a64(env)) { - ret = QEMU_PSCI_RET_INVALID_PARAMS; + ret = QEMU_PSCI_RET_NOT_SUPPORTED; goto err; } @@ -89,7 +89,7 @@ void arm_handle_psci_call(ARMCPU *cpu) ARMCPU *target_cpu; case QEMU_PSCI_0_2_FN_PSCI_VERSION: - ret = QEMU_PSCI_0_2_RET_VERSION_0_2; + ret = QEMU_PSCI_VERSION_1_1; break; case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE: ret = QEMU_PSCI_0_2_RET_TOS_MIGRATION_NOT_REQUIRED; /* No trusted OS */ @@ -170,6 +170,35 @@ void arm_handle_psci_call(ARMCPU *cpu) } helper_wfi(env, 4); break; + case QEMU_PSCI_1_0_FN_PSCI_FEATURES: + switch (param[1]) { + case QEMU_PSCI_0_2_FN_PSCI_VERSION: + case QEMU_PSCI_0_2_FN_MIGRATE_INFO_TYPE: + case QEMU_PSCI_0_2_FN_AFFINITY_INFO: + case QEMU_PSCI_0_2_FN64_AFFINITY_INFO: + case QEMU_PSCI_0_2_FN_SYSTEM_RESET: + case QEMU_PSCI_0_2_FN_SYSTEM_OFF: + case QEMU_PSCI_0_1_FN_CPU_ON: + case QEMU_PSCI_0_2_FN_CPU_ON: + case QEMU_PSCI_0_2_FN64_CPU_ON: + case QEMU_PSCI_0_1_FN_CPU_OFF: + case QEMU_PSCI_0_2_FN_CPU_OFF: + case QEMU_PSCI_0_1_FN_CPU_SUSPEND: + case QEMU_PSCI_0_2_FN_CPU_SUSPEND: + case QEMU_PSCI_0_2_FN64_CPU_SUSPEND: + case QEMU_PSCI_1_0_FN_PSCI_FEATURES: + if (!(param[1] & QEMU_PSCI_0_2_64BIT) || is_a64(env)) { + ret = 0; + break; + } + /* fallthrough */ + case QEMU_PSCI_0_1_FN_MIGRATE: + case QEMU_PSCI_0_2_FN_MIGRATE: + default: + ret = QEMU_PSCI_RET_NOT_SUPPORTED; + break; + } + break; case QEMU_PSCI_0_1_FN_MIGRATE: case QEMU_PSCI_0_2_FN_MIGRATE: default:
Support the latest PSCI on TCG and HVF. A 64-bit function called from AArch32 now returns NOT_SUPPORTED, which is necessary to adhere to SMC Calling Convention 1.0. It is still not compliant with SMCCC 1.3 since they do not implement mandatory functions. Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com> --- hw/arm/boot.c | 12 +++++++++--- target/arm/cpu.c | 5 +++-- target/arm/hvf/hvf.c | 27 ++++++++++++++++++++++++++- target/arm/kvm-consts.h | 8 ++++++-- target/arm/kvm64.c | 2 +- target/arm/psci.c | 35 ++++++++++++++++++++++++++++++++--- 6 files changed, 77 insertions(+), 12 deletions(-)