| Message ID | 20210519202253.76782-16-agraf@csgraf.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hvf: Implement Apple Silicon Support | expand |
On Wed, May 19, 2021 at 10:22:49PM +0200, Alexander Graf wrote: > Now that we have working system register sync, we push more target CPU > properties into the virtual machine. That might be useful in some > situations, but is not the typical case that users want. > > So let's add a -cpu host option that allows them to explicitly pass all > CPU capabilities of their host CPU into the guest. > > Signed-off-by: Alexander Graf <agraf@csgraf.de> > Acked-by: Roman Bolshakov <r.bolshakov@yadro.com> > > --- > > v6 -> v7: > > - Move function define to own header > - Do not propagate SVE features for HVF > - Remove stray whitespace change > - Verify that EL0 and EL1 do not allow AArch32 mode > - Only probe host CPU features once > --- > target/arm/cpu.c | 9 ++++-- > target/arm/cpu.h | 2 ++ > target/arm/hvf/hvf.c | 72 ++++++++++++++++++++++++++++++++++++++++++++ > target/arm/hvf_arm.h | 19 ++++++++++++ > target/arm/kvm_arm.h | 2 -- > 5 files changed, 100 insertions(+), 4 deletions(-) > create mode 100644 target/arm/hvf_arm.h Reviewed-by: Sergio Lopez <slp@redhat.com> Tested-by: Sergio Lopez <slp@redhat.com>
On Wed, 19 May 2021 at 21:23, Alexander Graf <agraf@csgraf.de> wrote: > > Now that we have working system register sync, we push more target CPU > properties into the virtual machine. That might be useful in some > situations, but is not the typical case that users want. > > So let's add a -cpu host option that allows them to explicitly pass all > CPU capabilities of their host CPU into the guest. > > Signed-off-by: Alexander Graf <agraf@csgraf.de> > Acked-by: Roman Bolshakov <r.bolshakov@yadro.com> > > --- > > v6 -> v7: > > - Move function define to own header > - Do not propagate SVE features for HVF > - Remove stray whitespace change > - Verify that EL0 and EL1 do not allow AArch32 mode > - Only probe host CPU features once > +static void hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > +{ > + ARMISARegisters host_isar; Can you zero-initialize this (with "= { }"), please? That way we know we have zeroes in the aarch32 ID fields rather than random junk later... > + const struct isar_regs { > + int reg; > + uint64_t *val; > + } regs[] = { > + { HV_SYS_REG_ID_AA64PFR0_EL1, &host_isar.id_aa64pfr0 }, > + { HV_SYS_REG_ID_AA64PFR1_EL1, &host_isar.id_aa64pfr1 }, > + { HV_SYS_REG_ID_AA64DFR0_EL1, &host_isar.id_aa64dfr0 }, > + { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, > + { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, > + { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, > + { HV_SYS_REG_ID_AA64MMFR0_EL1, &host_isar.id_aa64mmfr0 }, > + { HV_SYS_REG_ID_AA64MMFR1_EL1, &host_isar.id_aa64mmfr1 }, > + { HV_SYS_REG_ID_AA64MMFR2_EL1, &host_isar.id_aa64mmfr2 }, > + }; > + hv_vcpu_t fd; > + hv_vcpu_exit_t *exit; > + int i; > + > + ahcf->dtb_compatible = "arm,arm-v8"; > + ahcf->features = (1ULL << ARM_FEATURE_V8) | > + (1ULL << ARM_FEATURE_NEON) | > + (1ULL << ARM_FEATURE_AARCH64) | > + (1ULL << ARM_FEATURE_PMU) | > + (1ULL << ARM_FEATURE_GENERIC_TIMER); > + > + /* We set up a small vcpu to extract host registers */ > + > + assert_hvf_ok(hv_vcpu_create(&fd, &exit, NULL)); > + for (i = 0; i < ARRAY_SIZE(regs); i++) { > + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, regs[i].reg, regs[i].val)); > + } > + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, HV_SYS_REG_MIDR_EL1, &ahcf->midr)); > + assert_hvf_ok(hv_vcpu_destroy(fd)); > + > + ahcf->isar = host_isar; > + ahcf->reset_sctlr = 0x00c50078; Why this value in particular? Could we just ask the scratch HVF CPU for the value of SCTLR_EL1 rather than hardcoding something? > + > + /* Make sure we don't advertise AArch32 support for EL0/EL1 */ > + g_assert((host_isar.id_aa64pfr0 & 0xff) == 0x11); This shouldn't really be an assert, I think. error_report() something and return false, and then arm_cpu_realizefn() will fail, which should cause us to exit. > +} > + > +void hvf_arm_set_cpu_features_from_host(ARMCPU *cpu) > +{ > + if (!arm_host_cpu_features.dtb_compatible) { > + if (!hvf_enabled()) { > + cpu->host_cpu_probe_failed = true; > + return; > + } > + hvf_arm_get_host_cpu_features(&arm_host_cpu_features); > + } > + > + cpu->dtb_compatible = arm_host_cpu_features.dtb_compatible; > + cpu->isar = arm_host_cpu_features.isar; > + cpu->env.features = arm_host_cpu_features.features; > + cpu->midr = arm_host_cpu_features.midr; > + cpu->reset_sctlr = arm_host_cpu_features.reset_sctlr; > +} > + > void hvf_arch_vcpu_destroy(CPUState *cpu) > { > } thanks -- PMM
On 15.06.21 12:56, Peter Maydell wrote: > On Wed, 19 May 2021 at 21:23, Alexander Graf <agraf@csgraf.de> wrote: >> Now that we have working system register sync, we push more target CPU >> properties into the virtual machine. That might be useful in some >> situations, but is not the typical case that users want. >> >> So let's add a -cpu host option that allows them to explicitly pass all >> CPU capabilities of their host CPU into the guest. >> >> Signed-off-by: Alexander Graf <agraf@csgraf.de> >> Acked-by: Roman Bolshakov <r.bolshakov@yadro.com> >> >> --- >> >> v6 -> v7: >> >> - Move function define to own header >> - Do not propagate SVE features for HVF >> - Remove stray whitespace change >> - Verify that EL0 and EL1 do not allow AArch32 mode >> - Only probe host CPU features once >> +static void hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >> +{ >> + ARMISARegisters host_isar; > Can you zero-initialize this (with "= { }"), please? That way we > know we have zeroes in the aarch32 ID fields rather than random junk later... > >> + const struct isar_regs { >> + int reg; >> + uint64_t *val; >> + } regs[] = { >> + { HV_SYS_REG_ID_AA64PFR0_EL1, &host_isar.id_aa64pfr0 }, >> + { HV_SYS_REG_ID_AA64PFR1_EL1, &host_isar.id_aa64pfr1 }, >> + { HV_SYS_REG_ID_AA64DFR0_EL1, &host_isar.id_aa64dfr0 }, >> + { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, >> + { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, >> + { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, >> + { HV_SYS_REG_ID_AA64MMFR0_EL1, &host_isar.id_aa64mmfr0 }, >> + { HV_SYS_REG_ID_AA64MMFR1_EL1, &host_isar.id_aa64mmfr1 }, >> + { HV_SYS_REG_ID_AA64MMFR2_EL1, &host_isar.id_aa64mmfr2 }, >> + }; >> + hv_vcpu_t fd; >> + hv_vcpu_exit_t *exit; >> + int i; >> + >> + ahcf->dtb_compatible = "arm,arm-v8"; >> + ahcf->features = (1ULL << ARM_FEATURE_V8) | >> + (1ULL << ARM_FEATURE_NEON) | >> + (1ULL << ARM_FEATURE_AARCH64) | >> + (1ULL << ARM_FEATURE_PMU) | >> + (1ULL << ARM_FEATURE_GENERIC_TIMER); >> + >> + /* We set up a small vcpu to extract host registers */ >> + >> + assert_hvf_ok(hv_vcpu_create(&fd, &exit, NULL)); >> + for (i = 0; i < ARRAY_SIZE(regs); i++) { >> + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, regs[i].reg, regs[i].val)); >> + } >> + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, HV_SYS_REG_MIDR_EL1, &ahcf->midr)); >> + assert_hvf_ok(hv_vcpu_destroy(fd)); >> + >> + ahcf->isar = host_isar; >> + ahcf->reset_sctlr = 0x00c50078; > Why this value in particular? Could we just ask the scratch HVF CPU > for the value of SCTLR_EL1 rather than hardcoding something? The fresh scratch hvf CPU has 0 as SCTLR. But I'm happy to put an actual M1 copy of it here. > >> + >> + /* Make sure we don't advertise AArch32 support for EL0/EL1 */ >> + g_assert((host_isar.id_aa64pfr0 & 0xff) == 0x11); > This shouldn't really be an assert, I think. error_report() something > and return false, and then arm_cpu_realizefn() will fail, which should > cause us to exit. I don't follow. We're filling in the -cpu host CPU template here. There is no error path anywhere we could take. Or are you suggesting we only error on realize? I don't see any obvious way how we could tell the realize function that we don't want to expose AArch32 support for -cpu host. This is a case that on today's systems can't happen - M1 does not support AArch32 anywhere. So that assert could only ever hit if you run macOS on non-Apple hardware (in which case I doubt hvf works as intended) or if a new Apple CPU starts supporting AArch32 (again, very unlikely). So overall, I think the assert here is not too bad :) Alex
On Sun, 12 Sept 2021 at 21:23, Alexander Graf <agraf@csgraf.de> wrote: > > > On 15.06.21 12:56, Peter Maydell wrote: > > On Wed, 19 May 2021 at 21:23, Alexander Graf <agraf@csgraf.de> wrote: > >> Now that we have working system register sync, we push more target CPU > >> properties into the virtual machine. That might be useful in some > >> situations, but is not the typical case that users want. > >> > >> So let's add a -cpu host option that allows them to explicitly pass all > >> CPU capabilities of their host CPU into the guest. > >> > >> Signed-off-by: Alexander Graf <agraf@csgraf.de> > >> Acked-by: Roman Bolshakov <r.bolshakov@yadro.com> > >> > >> --- > >> > >> v6 -> v7: > >> > >> - Move function define to own header > >> - Do not propagate SVE features for HVF > >> - Remove stray whitespace change > >> - Verify that EL0 and EL1 do not allow AArch32 mode > >> - Only probe host CPU features once > >> +static void hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > >> +{ > >> + ARMISARegisters host_isar; > > Can you zero-initialize this (with "= { }"), please? That way we > > know we have zeroes in the aarch32 ID fields rather than random junk later... > > > >> + const struct isar_regs { > >> + int reg; > >> + uint64_t *val; > >> + } regs[] = { > >> + { HV_SYS_REG_ID_AA64PFR0_EL1, &host_isar.id_aa64pfr0 }, > >> + { HV_SYS_REG_ID_AA64PFR1_EL1, &host_isar.id_aa64pfr1 }, > >> + { HV_SYS_REG_ID_AA64DFR0_EL1, &host_isar.id_aa64dfr0 }, > >> + { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, > >> + { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, > >> + { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, > >> + { HV_SYS_REG_ID_AA64MMFR0_EL1, &host_isar.id_aa64mmfr0 }, > >> + { HV_SYS_REG_ID_AA64MMFR1_EL1, &host_isar.id_aa64mmfr1 }, > >> + { HV_SYS_REG_ID_AA64MMFR2_EL1, &host_isar.id_aa64mmfr2 }, > >> + }; > >> + hv_vcpu_t fd; > >> + hv_vcpu_exit_t *exit; > >> + int i; > >> + > >> + ahcf->dtb_compatible = "arm,arm-v8"; > >> + ahcf->features = (1ULL << ARM_FEATURE_V8) | > >> + (1ULL << ARM_FEATURE_NEON) | > >> + (1ULL << ARM_FEATURE_AARCH64) | > >> + (1ULL << ARM_FEATURE_PMU) | > >> + (1ULL << ARM_FEATURE_GENERIC_TIMER); > >> + > >> + /* We set up a small vcpu to extract host registers */ > >> + > >> + assert_hvf_ok(hv_vcpu_create(&fd, &exit, NULL)); > >> + for (i = 0; i < ARRAY_SIZE(regs); i++) { > >> + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, regs[i].reg, regs[i].val)); > >> + } > >> + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, HV_SYS_REG_MIDR_EL1, &ahcf->midr)); > >> + assert_hvf_ok(hv_vcpu_destroy(fd)); > >> + > >> + ahcf->isar = host_isar; > >> + ahcf->reset_sctlr = 0x00c50078; > > Why this value in particular? Could we just ask the scratch HVF CPU > > for the value of SCTLR_EL1 rather than hardcoding something? > > > The fresh scratch hvf CPU has 0 as SCTLR. Yuck. That's pretty unhelpful of hvf, since it's not an architecturally valid thing for a freshly reset EL1-only CPU to have as its SCTLR (some bits are supposed to be RES1 or reset-to-1). In that case we do need to set this to a known value here, I guess -- but we should have a comment explaining why we do it and what bits we're setting. > >> + /* Make sure we don't advertise AArch32 support for EL0/EL1 */ > >> + g_assert((host_isar.id_aa64pfr0 & 0xff) == 0x11); > > This shouldn't really be an assert, I think. error_report() something > > and return false, and then arm_cpu_realizefn() will fail, which should > > cause us to exit. > > > I don't follow. We're filling in the -cpu host CPU template here. There > is no error path anywhere we could take. Look at how the kvm_arm_get_host_cpu_features() error handling works: it returns a bool. The caller (kvm_arm_set_cpu_features_from_host()) checks the return value, and if the function failed it sets the cpu->host_cpu_probe_failed flag, which then results in realize failing. (You'll want to update the arm_cpu_realizefn to allow hvf as well as kvm for that error message.) > This is a case that on today's systems can't happen - M1 does not > support AArch32 anywhere. So that assert could only ever hit if you run > macOS on non-Apple hardware (in which case I doubt hvf works as > intended) or if a new Apple CPU starts supporting AArch32 (again, very > unlikely). > > So overall, I think the assert here is not too bad :) Well, probably not, but you need the error handling path anyway for the boring case of "oops, this syscall failed". -- PMM
On 13.09.21 10:28, Peter Maydell wrote: > On Sun, 12 Sept 2021 at 21:23, Alexander Graf <agraf@csgraf.de> wrote: >> >> On 15.06.21 12:56, Peter Maydell wrote: >>> On Wed, 19 May 2021 at 21:23, Alexander Graf <agraf@csgraf.de> wrote: >>>> Now that we have working system register sync, we push more target CPU >>>> properties into the virtual machine. That might be useful in some >>>> situations, but is not the typical case that users want. >>>> >>>> So let's add a -cpu host option that allows them to explicitly pass all >>>> CPU capabilities of their host CPU into the guest. >>>> >>>> Signed-off-by: Alexander Graf <agraf@csgraf.de> >>>> Acked-by: Roman Bolshakov <r.bolshakov@yadro.com> >>>> >>>> --- >>>> >>>> v6 -> v7: >>>> >>>> - Move function define to own header >>>> - Do not propagate SVE features for HVF >>>> - Remove stray whitespace change >>>> - Verify that EL0 and EL1 do not allow AArch32 mode >>>> - Only probe host CPU features once >>>> +static void hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >>>> +{ >>>> + ARMISARegisters host_isar; >>> Can you zero-initialize this (with "= { }"), please? That way we >>> know we have zeroes in the aarch32 ID fields rather than random junk later... >>> >>>> + const struct isar_regs { >>>> + int reg; >>>> + uint64_t *val; >>>> + } regs[] = { >>>> + { HV_SYS_REG_ID_AA64PFR0_EL1, &host_isar.id_aa64pfr0 }, >>>> + { HV_SYS_REG_ID_AA64PFR1_EL1, &host_isar.id_aa64pfr1 }, >>>> + { HV_SYS_REG_ID_AA64DFR0_EL1, &host_isar.id_aa64dfr0 }, >>>> + { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, >>>> + { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, >>>> + { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, >>>> + { HV_SYS_REG_ID_AA64MMFR0_EL1, &host_isar.id_aa64mmfr0 }, >>>> + { HV_SYS_REG_ID_AA64MMFR1_EL1, &host_isar.id_aa64mmfr1 }, >>>> + { HV_SYS_REG_ID_AA64MMFR2_EL1, &host_isar.id_aa64mmfr2 }, >>>> + }; >>>> + hv_vcpu_t fd; >>>> + hv_vcpu_exit_t *exit; >>>> + int i; >>>> + >>>> + ahcf->dtb_compatible = "arm,arm-v8"; >>>> + ahcf->features = (1ULL << ARM_FEATURE_V8) | >>>> + (1ULL << ARM_FEATURE_NEON) | >>>> + (1ULL << ARM_FEATURE_AARCH64) | >>>> + (1ULL << ARM_FEATURE_PMU) | >>>> + (1ULL << ARM_FEATURE_GENERIC_TIMER); >>>> + >>>> + /* We set up a small vcpu to extract host registers */ >>>> + >>>> + assert_hvf_ok(hv_vcpu_create(&fd, &exit, NULL)); >>>> + for (i = 0; i < ARRAY_SIZE(regs); i++) { >>>> + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, regs[i].reg, regs[i].val)); >>>> + } >>>> + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, HV_SYS_REG_MIDR_EL1, &ahcf->midr)); >>>> + assert_hvf_ok(hv_vcpu_destroy(fd)); >>>> + >>>> + ahcf->isar = host_isar; >>>> + ahcf->reset_sctlr = 0x00c50078; >>> Why this value in particular? Could we just ask the scratch HVF CPU >>> for the value of SCTLR_EL1 rather than hardcoding something? >> >> The fresh scratch hvf CPU has 0 as SCTLR. > Yuck. That's pretty unhelpful of hvf, since it's not an > architecturally valid thing for a freshly reset EL1-only > CPU to have as its SCTLR (some bits are supposed to be RES1 > or reset-to-1). In that case we do need to set this to a known > value here, I guess -- but we should have a comment explaining > why we do it and what bits we're setting. Ok :) > >>>> + /* Make sure we don't advertise AArch32 support for EL0/EL1 */ >>>> + g_assert((host_isar.id_aa64pfr0 & 0xff) == 0x11); >>> This shouldn't really be an assert, I think. error_report() something >>> and return false, and then arm_cpu_realizefn() will fail, which should >>> cause us to exit. >> >> I don't follow. We're filling in the -cpu host CPU template here. There >> is no error path anywhere we could take. > Look at how the kvm_arm_get_host_cpu_features() error handling works: > it returns a bool. The caller (kvm_arm_set_cpu_features_from_host()) > checks the return value, and if the function failed it sets > the cpu->host_cpu_probe_failed flag, which then results in realize > failing. (You'll want to update the arm_cpu_realizefn to allow hvf > as well as kvm for that error message.) Sure, happy to adjust accordingly :) > >> This is a case that on today's systems can't happen - M1 does not >> support AArch32 anywhere. So that assert could only ever hit if you run >> macOS on non-Apple hardware (in which case I doubt hvf works as >> intended) or if a new Apple CPU starts supporting AArch32 (again, very >> unlikely). >> >> So overall, I think the assert here is not too bad :) > Well, probably not, but you need the error handling path > anyway for the boring case of "oops, this syscall failed". Why? You only get to this code path if you already selected -accel hvf. If even a simple "create scratch vcpu" syscall failed then, pretty failure when you define -cpu host is the last thing you care about. Any CPU creation would fail. Alex
On Mon, 13 Sept 2021 at 11:46, Alexander Graf <agraf@csgraf.de> wrote: > Why? You only get to this code path if you already selected -accel hvf. > If even a simple "create scratch vcpu" syscall failed then, pretty > failure when you define -cpu host is the last thing you care about. Any > CPU creation would fail. General design principle -- low level functions should report errors upwards, not barf and exit. -- PMM
On 13.09.21 12:52, Peter Maydell wrote: > On Mon, 13 Sept 2021 at 11:46, Alexander Graf <agraf@csgraf.de> wrote: >> Why? You only get to this code path if you already selected -accel hvf. >> If even a simple "create scratch vcpu" syscall failed then, pretty >> failure when you define -cpu host is the last thing you care about. Any >> CPU creation would fail. > General design principle -- low level functions should report > errors upwards, not barf and exit. Usually I would agree with you, but here it really does not make sense and would make the code significantly harder to read. Alex
On Mon, 13 Sept 2021 at 12:46, Alexander Graf <agraf@csgraf.de> wrote: > > > On 13.09.21 12:52, Peter Maydell wrote: > > On Mon, 13 Sept 2021 at 11:46, Alexander Graf <agraf@csgraf.de> wrote: > >> Why? You only get to this code path if you already selected -accel hvf. > >> If even a simple "create scratch vcpu" syscall failed then, pretty > >> failure when you define -cpu host is the last thing you care about. Any > >> CPU creation would fail. > > General design principle -- low level functions should report > > errors upwards, not barf and exit. > > > Usually I would agree with you, but here it really does not make sense > and would make the code significantly harder to read. It's an unnecessary difference from how we've structured the KVM code. I don't like those. Every time you put one in to the code you write you can be fairly sure I'm going to question it during review... I want to be able to look at the hvf code and say "ah, yes, this is just the hvf version of the kvm code we already have". -- PMM
On 13.09.21 13:48, Peter Maydell wrote: > On Mon, 13 Sept 2021 at 12:46, Alexander Graf <agraf@csgraf.de> wrote: >> >> On 13.09.21 12:52, Peter Maydell wrote: >>> On Mon, 13 Sept 2021 at 11:46, Alexander Graf <agraf@csgraf.de> wrote: >>>> Why? You only get to this code path if you already selected -accel hvf. >>>> If even a simple "create scratch vcpu" syscall failed then, pretty >>>> failure when you define -cpu host is the last thing you care about. Any >>>> CPU creation would fail. >>> General design principle -- low level functions should report >>> errors upwards, not barf and exit. >> >> Usually I would agree with you, but here it really does not make sense >> and would make the code significantly harder to read. > It's an unnecessary difference from how we've structured the > KVM code. I don't like those. Every time you put one in to > the code you write you can be fairly sure I'm going to question > it during review... I want to be able to look at the hvf code > and say "ah, yes, this is just the hvf version of the kvm code > we already have". I'll follow the KVM pattern then ... Alex
diff --git a/target/arm/cpu.c b/target/arm/cpu.c index 4eb0d2f85c..762d8a6d26 100644 --- a/target/arm/cpu.c +++ b/target/arm/cpu.c @@ -39,6 +39,7 @@ #include "sysemu/tcg.h" #include "sysemu/hw_accel.h" #include "kvm_arm.h" +#include "hvf_arm.h" #include "disas/capstone.h" #include "fpu/softfloat.h" @@ -1998,15 +1999,19 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data) #endif /* CONFIG_TCG */ } -#ifdef CONFIG_KVM +#if defined(CONFIG_KVM) || defined(CONFIG_HVF) static void arm_host_initfn(Object *obj) { ARMCPU *cpu = ARM_CPU(obj); +#ifdef CONFIG_KVM kvm_arm_set_cpu_features_from_host(cpu); if (arm_feature(&cpu->env, ARM_FEATURE_AARCH64)) { aarch64_add_sve_properties(obj); } +#else + hvf_arm_set_cpu_features_from_host(cpu); +#endif arm_cpu_post_init(obj); } @@ -2066,7 +2071,7 @@ static void arm_cpu_register_types(void) { type_register_static(&arm_cpu_type_info); -#ifdef CONFIG_KVM +#if defined(CONFIG_KVM) || defined(CONFIG_HVF) type_register_static(&host_arm_cpu_type_info); #endif } diff --git a/target/arm/cpu.h b/target/arm/cpu.h index 616b393253..4360e77183 100644 --- a/target/arm/cpu.h +++ b/target/arm/cpu.h @@ -2977,6 +2977,8 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool kvm_sync); #define ARM_CPU_TYPE_NAME(name) (name ARM_CPU_TYPE_SUFFIX) #define CPU_RESOLVING_TYPE TYPE_ARM_CPU +#define TYPE_ARM_HOST_CPU "host-" TYPE_ARM_CPU + #define cpu_signal_handler cpu_arm_signal_handler #define cpu_list arm_cpu_list diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index 67002efd36..bce46f3ed8 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -17,6 +17,7 @@ #include "sysemu/hvf.h" #include "sysemu/hvf_int.h" #include "sysemu/hw_accel.h" +#include "hvf_arm.h" #include <mach/mach_time.h> @@ -44,6 +45,16 @@ #define TMR_CTL_IMASK (1 << 1) #define TMR_CTL_ISTATUS (1 << 2) +typedef struct ARMHostCPUFeatures { + ARMISARegisters isar; + uint64_t features; + uint64_t midr; + uint32_t reset_sctlr; + const char *dtb_compatible; +} ARMHostCPUFeatures; + +static ARMHostCPUFeatures arm_host_cpu_features; + struct hvf_reg_match { int reg; uint64_t offset; @@ -390,6 +401,67 @@ static uint64_t hvf_get_reg(CPUState *cpu, int rt) return val; } +static void hvf_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) +{ + ARMISARegisters host_isar; + const struct isar_regs { + int reg; + uint64_t *val; + } regs[] = { + { HV_SYS_REG_ID_AA64PFR0_EL1, &host_isar.id_aa64pfr0 }, + { HV_SYS_REG_ID_AA64PFR1_EL1, &host_isar.id_aa64pfr1 }, + { HV_SYS_REG_ID_AA64DFR0_EL1, &host_isar.id_aa64dfr0 }, + { HV_SYS_REG_ID_AA64DFR1_EL1, &host_isar.id_aa64dfr1 }, + { HV_SYS_REG_ID_AA64ISAR0_EL1, &host_isar.id_aa64isar0 }, + { HV_SYS_REG_ID_AA64ISAR1_EL1, &host_isar.id_aa64isar1 }, + { HV_SYS_REG_ID_AA64MMFR0_EL1, &host_isar.id_aa64mmfr0 }, + { HV_SYS_REG_ID_AA64MMFR1_EL1, &host_isar.id_aa64mmfr1 }, + { HV_SYS_REG_ID_AA64MMFR2_EL1, &host_isar.id_aa64mmfr2 }, + }; + hv_vcpu_t fd; + hv_vcpu_exit_t *exit; + int i; + + ahcf->dtb_compatible = "arm,arm-v8"; + ahcf->features = (1ULL << ARM_FEATURE_V8) | + (1ULL << ARM_FEATURE_NEON) | + (1ULL << ARM_FEATURE_AARCH64) | + (1ULL << ARM_FEATURE_PMU) | + (1ULL << ARM_FEATURE_GENERIC_TIMER); + + /* We set up a small vcpu to extract host registers */ + + assert_hvf_ok(hv_vcpu_create(&fd, &exit, NULL)); + for (i = 0; i < ARRAY_SIZE(regs); i++) { + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, regs[i].reg, regs[i].val)); + } + assert_hvf_ok(hv_vcpu_get_sys_reg(fd, HV_SYS_REG_MIDR_EL1, &ahcf->midr)); + assert_hvf_ok(hv_vcpu_destroy(fd)); + + ahcf->isar = host_isar; + ahcf->reset_sctlr = 0x00c50078; + + /* Make sure we don't advertise AArch32 support for EL0/EL1 */ + g_assert((host_isar.id_aa64pfr0 & 0xff) == 0x11); +} + +void hvf_arm_set_cpu_features_from_host(ARMCPU *cpu) +{ + if (!arm_host_cpu_features.dtb_compatible) { + if (!hvf_enabled()) { + cpu->host_cpu_probe_failed = true; + return; + } + hvf_arm_get_host_cpu_features(&arm_host_cpu_features); + } + + cpu->dtb_compatible = arm_host_cpu_features.dtb_compatible; + cpu->isar = arm_host_cpu_features.isar; + cpu->env.features = arm_host_cpu_features.features; + cpu->midr = arm_host_cpu_features.midr; + cpu->reset_sctlr = arm_host_cpu_features.reset_sctlr; +} + void hvf_arch_vcpu_destroy(CPUState *cpu) { } diff --git a/target/arm/hvf_arm.h b/target/arm/hvf_arm.h new file mode 100644 index 0000000000..603074a331 --- /dev/null +++ b/target/arm/hvf_arm.h @@ -0,0 +1,19 @@ +/* + * QEMU Hypervisor.framework (HVF) support -- ARM specifics + * + * Copyright (c) 2021 Alexander Graf + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + * + */ + +#ifndef QEMU_HVF_ARM_H +#define QEMU_HVF_ARM_H + +#include "qemu/accel.h" +#include "cpu.h" + +void hvf_arm_set_cpu_features_from_host(struct ARMCPU *cpu); + +#endif diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index 34f8daa377..828dca4a4a 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -214,8 +214,6 @@ bool kvm_arm_create_scratch_host_vcpu(const uint32_t *cpus_to_try, */ void kvm_arm_destroy_scratch_host_vcpu(int *fdarray); -#define TYPE_ARM_HOST_CPU "host-" TYPE_ARM_CPU - /** * ARMHostCPUFeatures: information about the host CPU (identified * by asking the host kernel)