| Message ID | 20190621163422.6127-2-drjones@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm/kvm: enable SVE in guests | expand |
Hi Drew, On 6/21/19 6:34 PM, Andrew Jones wrote: > If -cpu <cpu>,aarch64=off is used then KVM must also be used, and it > and the host must support running the vcpu in 32-bit mode. Also, if > -cpu <cpu>,aarch64=on is used, then it doesn't matter if kvm is > enabled or not. > > Signed-off-by: Andrew Jones <drjones@redhat.com> > --- > target/arm/cpu64.c | 12 ++++++------ > target/arm/kvm64.c | 11 +++++++++++ > target/arm/kvm_arm.h | 14 ++++++++++++++ > 3 files changed, 31 insertions(+), 6 deletions(-) > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > index 1901997a0645..946994838d8a 100644 > --- a/target/arm/cpu64.c > +++ b/target/arm/cpu64.c > @@ -407,13 +407,13 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) > * restriction allows us to avoid fixing up functionality that assumes a > * uniform execution state like do_interrupt. > */> - if (!kvm_enabled()) { > - error_setg(errp, "'aarch64' feature cannot be disabled " > - "unless KVM is enabled"); > - return; > - } > - > if (value == false) { > + if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { > + error_setg(errp, "'aarch64' feature cannot be disabled " > + "unless KVM is enabled and 32-bit EL1 " > + "is supported"); > + return; > + } > unset_feature(&cpu->env, ARM_FEATURE_AARCH64); > } else { > set_feature(&cpu->env, ARM_FEATURE_AARCH64); > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > index 22d19c9aec6f..45ccda589903 100644 > --- a/target/arm/kvm64.c > +++ b/target/arm/kvm64.c > @@ -24,7 +24,9 @@ > #include "exec/gdbstub.h" > #include "sysemu/sysemu.h" > #include "sysemu/kvm.h" > +#include "sysemu/kvm_int.h" > #include "kvm_arm.h" > +#include "hw/boards.h" > #include "internals.h" > > static bool have_guest_debug; > @@ -593,6 +595,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > return true; > } > > +bool kvm_arm_aarch32_supported(CPUState *cpu) > +{ > + KVMState *s = KVM_STATE(current_machine->accelerator); > + int ret; > + > + ret = kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); > + return ret > 0; nit: return kvm_check_extension() should be sufficient > +} > + > #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 > > int kvm_arch_init_vcpu(CPUState *cs) > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > index 2a07333c615f..812125f805a1 100644 > --- a/target/arm/kvm_arm.h > +++ b/target/arm/kvm_arm.h > @@ -207,6 +207,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf); > */ > void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); > > +/** > + * kvm_arm_aarch32_supported: > + * @cs: CPUState use kernel-doc comment style? > + * > + * Returns true if the KVM VCPU can enable AArch32 mode and false > + * otherwise. > + */ > +bool kvm_arm_aarch32_supported(CPUState *cs); > + > /** > * kvm_arm_get_max_vm_ipa_size - Returns the number of bits in the > * IPA address space supported by KVM > @@ -247,6 +256,11 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) > cpu->host_cpu_probe_failed = true; > } > > +static inline bool kvm_arm_aarch32_supported(CPUState *cs) > +{ > + return false; > +} > + > static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) > { > return -ENOENT; > Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric
On Tue, Jun 25, 2019 at 11:35:12AM +0200, Auger Eric wrote: > Hi Drew, > > On 6/21/19 6:34 PM, Andrew Jones wrote: > > If -cpu <cpu>,aarch64=off is used then KVM must also be used, and it > > and the host must support running the vcpu in 32-bit mode. Also, if > > -cpu <cpu>,aarch64=on is used, then it doesn't matter if kvm is > > enabled or not. > > > > Signed-off-by: Andrew Jones <drjones@redhat.com> > > > > --- > > target/arm/cpu64.c | 12 ++++++------ > > target/arm/kvm64.c | 11 +++++++++++ > > target/arm/kvm_arm.h | 14 ++++++++++++++ > > 3 files changed, 31 insertions(+), 6 deletions(-) > > > > diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > > index 1901997a0645..946994838d8a 100644 > > --- a/target/arm/cpu64.c > > +++ b/target/arm/cpu64.c > > @@ -407,13 +407,13 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) > > * restriction allows us to avoid fixing up functionality that assumes a > > * uniform execution state like do_interrupt. > > */> - if (!kvm_enabled()) { > > - error_setg(errp, "'aarch64' feature cannot be disabled " > > - "unless KVM is enabled"); > > - return; > > - } > > - > > if (value == false) { > > + if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { > > + error_setg(errp, "'aarch64' feature cannot be disabled " > > + "unless KVM is enabled and 32-bit EL1 " > > + "is supported"); > > + return; > > + } > > unset_feature(&cpu->env, ARM_FEATURE_AARCH64); > > } else { > > set_feature(&cpu->env, ARM_FEATURE_AARCH64); > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > > index 22d19c9aec6f..45ccda589903 100644 > > --- a/target/arm/kvm64.c > > +++ b/target/arm/kvm64.c > > @@ -24,7 +24,9 @@ > > #include "exec/gdbstub.h" > > #include "sysemu/sysemu.h" > > #include "sysemu/kvm.h" > > +#include "sysemu/kvm_int.h" > > #include "kvm_arm.h" > > +#include "hw/boards.h" > > #include "internals.h" > > > > static bool have_guest_debug; > > @@ -593,6 +595,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > > return true; > > } > > > > +bool kvm_arm_aarch32_supported(CPUState *cpu) > > +{ > > + KVMState *s = KVM_STATE(current_machine->accelerator); > > + int ret; > > + > > + ret = kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); > > + return ret > 0; > nit: return kvm_check_extension() should be sufficient Ah yes, I forgot kvm_check_extension() already converts negative error codes to zero. I'll fix that for v3. > > +} > > + > > #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 > > > > int kvm_arch_init_vcpu(CPUState *cs) > > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > > index 2a07333c615f..812125f805a1 100644 > > --- a/target/arm/kvm_arm.h > > +++ b/target/arm/kvm_arm.h > > @@ -207,6 +207,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf); > > */ > > void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); > > > > +/** > > + * kvm_arm_aarch32_supported: > > + * @cs: CPUState > use kernel-doc comment style? This file (kvm_arm.h) doesn't appear to have a super consistent comment style. I see some use @var: for the parameters and some have 'Returns: ...' lines as well. I'm happy to do whatever the maintainers prefer. For now I was just trying to mimic whatever caught my eye. > > + * > > + * Returns true if the KVM VCPU can enable AArch32 mode and false > > + * otherwise. > > + */ > > +bool kvm_arm_aarch32_supported(CPUState *cs); > > + > > /** > > * kvm_arm_get_max_vm_ipa_size - Returns the number of bits in the > > * IPA address space supported by KVM > > @@ -247,6 +256,11 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) > > cpu->host_cpu_probe_failed = true; > > } > > > > +static inline bool kvm_arm_aarch32_supported(CPUState *cs) > > +{ > > + return false; > > +} > > + > > static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) > > { > > return -ENOENT; > > > Reviewed-by: Eric Auger <eric.auger@redhat.com> > Thanks, drew
Hi Drew, On 6/25/19 3:34 PM, Andrew Jones wrote: > On Tue, Jun 25, 2019 at 11:35:12AM +0200, Auger Eric wrote: >> Hi Drew, >> >> On 6/21/19 6:34 PM, Andrew Jones wrote: >>> If -cpu <cpu>,aarch64=off is used then KVM must also be used, and it >>> and the host must support running the vcpu in 32-bit mode. Also, if s/and it// >>> -cpu <cpu>,aarch64=on is used, then it doesn't matter if kvm is >>> enabled or not. >>> >>> Signed-off-by: Andrew Jones <drjones@redhat.com> >> >> >>> --- >>> target/arm/cpu64.c | 12 ++++++------ >>> target/arm/kvm64.c | 11 +++++++++++ >>> target/arm/kvm_arm.h | 14 ++++++++++++++ >>> 3 files changed, 31 insertions(+), 6 deletions(-) >>> >>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c >>> index 1901997a0645..946994838d8a 100644 >>> --- a/target/arm/cpu64.c >>> +++ b/target/arm/cpu64.c >>> @@ -407,13 +407,13 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) >>> * restriction allows us to avoid fixing up functionality that assumes a >>> * uniform execution state like do_interrupt. >>> */> - if (!kvm_enabled()) { >>> - error_setg(errp, "'aarch64' feature cannot be disabled " >>> - "unless KVM is enabled"); >>> - return; >>> - } >>> - >>> if (value == false) { >>> + if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { >>> + error_setg(errp, "'aarch64' feature cannot be disabled " >>> + "unless KVM is enabled and 32-bit EL1 " >>> + "is supported"); >>> + return; >>> + } >>> unset_feature(&cpu->env, ARM_FEATURE_AARCH64); >>> } else { >>> set_feature(&cpu->env, ARM_FEATURE_AARCH64); >>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c >>> index 22d19c9aec6f..45ccda589903 100644 >>> --- a/target/arm/kvm64.c >>> +++ b/target/arm/kvm64.c >>> @@ -24,7 +24,9 @@ >>> #include "exec/gdbstub.h" >>> #include "sysemu/sysemu.h" >>> #include "sysemu/kvm.h" >>> +#include "sysemu/kvm_int.h" >>> #include "kvm_arm.h" >>> +#include "hw/boards.h" By the way those two new headers are not needed by this patch >>> #include "internals.h" >>> >>> static bool have_guest_debug; >>> @@ -593,6 +595,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >>> return true; >>> } >>> >>> +bool kvm_arm_aarch32_supported(CPUState *cpu) >>> +{ >>> + KVMState *s = KVM_STATE(current_machine->accelerator); >>> + int ret; >>> + >>> + ret = kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); >>> + return ret > 0; >> nit: return kvm_check_extension() should be sufficient > > Ah yes, I forgot kvm_check_extension() already converts negative > error codes to zero. I'll fix that for v3. > >>> +} >>> + >>> #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 >>> >>> int kvm_arch_init_vcpu(CPUState *cs) >>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h >>> index 2a07333c615f..812125f805a1 100644 >>> --- a/target/arm/kvm_arm.h >>> +++ b/target/arm/kvm_arm.h >>> @@ -207,6 +207,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf); >>> */ >>> void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); >>> >>> +/** >>> + * kvm_arm_aarch32_supported: >>> + * @cs: CPUState >> use kernel-doc comment style? > > This file (kvm_arm.h) doesn't appear to have a super consistent comment > style. I see some use @var: for the parameters and some have 'Returns: > ...' lines as well. I'm happy to do whatever the maintainers prefer. For > now I was just trying to mimic whatever caught my eye.> >>> + * >>> + * Returns true if the KVM VCPU can enable AArch32 mode and false >>> + * otherwise. >>> + */ >>> +bool kvm_arm_aarch32_supported(CPUState *cs); >>> + >>> /** >>> * kvm_arm_get_max_vm_ipa_size - Returns the number of bits in the >>> * IPA address space supported by KVM >>> @@ -247,6 +256,11 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) >>> cpu->host_cpu_probe_failed = true; >>> } >>> >>> +static inline bool kvm_arm_aarch32_supported(CPUState *cs) >>> +{ >>> + return false; >>> +} >>> + >>> static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) >>> { >>> return -ENOENT; >>> >> Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks Eric >> > > Thanks, > drew >
On Wed, Jul 24, 2019 at 02:51:15PM +0200, Auger Eric wrote: > Hi Drew, > > On 6/25/19 3:34 PM, Andrew Jones wrote: > > On Tue, Jun 25, 2019 at 11:35:12AM +0200, Auger Eric wrote: > >> Hi Drew, > >> > >> On 6/21/19 6:34 PM, Andrew Jones wrote: > >>> If -cpu <cpu>,aarch64=off is used then KVM must also be used, and it > >>> and the host must support running the vcpu in 32-bit mode. Also, if > s/and it// "and it and the host" means "and KVM and the host", as 'it' refers to the last subject, which is KVM. I wanted to point out both the host (machine) and KVM (version of kernel with KVM) need to support the feature. > >>> -cpu <cpu>,aarch64=on is used, then it doesn't matter if kvm is > >>> enabled or not. > >>> > >>> Signed-off-by: Andrew Jones <drjones@redhat.com> > >> > >> > >>> --- > >>> target/arm/cpu64.c | 12 ++++++------ > >>> target/arm/kvm64.c | 11 +++++++++++ > >>> target/arm/kvm_arm.h | 14 ++++++++++++++ > >>> 3 files changed, 31 insertions(+), 6 deletions(-) > >>> > >>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c > >>> index 1901997a0645..946994838d8a 100644 > >>> --- a/target/arm/cpu64.c > >>> +++ b/target/arm/cpu64.c > >>> @@ -407,13 +407,13 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) > >>> * restriction allows us to avoid fixing up functionality that assumes a > >>> * uniform execution state like do_interrupt. > >>> */> - if (!kvm_enabled()) { > >>> - error_setg(errp, "'aarch64' feature cannot be disabled " > >>> - "unless KVM is enabled"); > >>> - return; > >>> - } > >>> - > >>> if (value == false) { > >>> + if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { > >>> + error_setg(errp, "'aarch64' feature cannot be disabled " > >>> + "unless KVM is enabled and 32-bit EL1 " > >>> + "is supported"); > >>> + return; > >>> + } > >>> unset_feature(&cpu->env, ARM_FEATURE_AARCH64); > >>> } else { > >>> set_feature(&cpu->env, ARM_FEATURE_AARCH64); > >>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c > >>> index 22d19c9aec6f..45ccda589903 100644 > >>> --- a/target/arm/kvm64.c > >>> +++ b/target/arm/kvm64.c > >>> @@ -24,7 +24,9 @@ > >>> #include "exec/gdbstub.h" > >>> #include "sysemu/sysemu.h" > >>> #include "sysemu/kvm.h" > >>> +#include "sysemu/kvm_int.h" > >>> #include "kvm_arm.h" > >>> +#include "hw/boards.h" > By the way those two new headers are not needed by this patch Really? current_machine is defined in hw/boards.h and KVM_STATE is defined in sysemu/kvm_int.h. > >>> #include "internals.h" > >>> > >>> static bool have_guest_debug; > >>> @@ -593,6 +595,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) > >>> return true; > >>> } > >>> > >>> +bool kvm_arm_aarch32_supported(CPUState *cpu) > >>> +{ > >>> + KVMState *s = KVM_STATE(current_machine->accelerator); > >>> + int ret; > >>> + > >>> + ret = kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); > >>> + return ret > 0; > >> nit: return kvm_check_extension() should be sufficient > > > > Ah yes, I forgot kvm_check_extension() already converts negative > > error codes to zero. I'll fix that for v3. > > > >>> +} > >>> + > >>> #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 > >>> > >>> int kvm_arch_init_vcpu(CPUState *cs) > >>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > >>> index 2a07333c615f..812125f805a1 100644 > >>> --- a/target/arm/kvm_arm.h > >>> +++ b/target/arm/kvm_arm.h > >>> @@ -207,6 +207,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf); > >>> */ > >>> void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); > >>> > >>> +/** > >>> + * kvm_arm_aarch32_supported: > >>> + * @cs: CPUState > >> use kernel-doc comment style? > > > > This file (kvm_arm.h) doesn't appear to have a super consistent comment > > style. I see some use @var: for the parameters and some have 'Returns: > > ...' lines as well. I'm happy to do whatever the maintainers prefer. For > > now I was just trying to mimic whatever caught my eye.> > >>> + * > >>> + * Returns true if the KVM VCPU can enable AArch32 mode and false > >>> + * otherwise. > >>> + */ > >>> +bool kvm_arm_aarch32_supported(CPUState *cs); > >>> + > >>> /** > >>> * kvm_arm_get_max_vm_ipa_size - Returns the number of bits in the > >>> * IPA address space supported by KVM > >>> @@ -247,6 +256,11 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) > >>> cpu->host_cpu_probe_failed = true; > >>> } > >>> > >>> +static inline bool kvm_arm_aarch32_supported(CPUState *cs) > >>> +{ > >>> + return false; > >>> +} > >>> + > >>> static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) > >>> { > >>> return -ENOENT; > >>> > >> Reviewed-by: Eric Auger <eric.auger@redhat.com> Thanks, drew
Hi Drew, On 7/24/19 3:52 PM, Andrew Jones wrote: > On Wed, Jul 24, 2019 at 02:51:15PM +0200, Auger Eric wrote: >> Hi Drew, >> >> On 6/25/19 3:34 PM, Andrew Jones wrote: >>> On Tue, Jun 25, 2019 at 11:35:12AM +0200, Auger Eric wrote: >>>> Hi Drew, >>>> >>>> On 6/21/19 6:34 PM, Andrew Jones wrote: >>>>> If -cpu <cpu>,aarch64=off is used then KVM must also be used, and it >>>>> and the host must support running the vcpu in 32-bit mode. Also, if >> s/and it// > > "and it and the host" means "and KVM and the host", as 'it' refers to the > last subject, which is KVM. I wanted to point out both the host (machine) > and KVM (version of kernel with KVM) need to support the feature. hum ok > >>>>> -cpu <cpu>,aarch64=on is used, then it doesn't matter if kvm is >>>>> enabled or not. >>>>> >>>>> Signed-off-by: Andrew Jones <drjones@redhat.com> >>>> >>>> >>>>> --- >>>>> target/arm/cpu64.c | 12 ++++++------ >>>>> target/arm/kvm64.c | 11 +++++++++++ >>>>> target/arm/kvm_arm.h | 14 ++++++++++++++ >>>>> 3 files changed, 31 insertions(+), 6 deletions(-) >>>>> >>>>> diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c >>>>> index 1901997a0645..946994838d8a 100644 >>>>> --- a/target/arm/cpu64.c >>>>> +++ b/target/arm/cpu64.c >>>>> @@ -407,13 +407,13 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) >>>>> * restriction allows us to avoid fixing up functionality that assumes a >>>>> * uniform execution state like do_interrupt. >>>>> */> - if (!kvm_enabled()) { >>>>> - error_setg(errp, "'aarch64' feature cannot be disabled " >>>>> - "unless KVM is enabled"); >>>>> - return; >>>>> - } >>>>> - >>>>> if (value == false) { >>>>> + if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { >>>>> + error_setg(errp, "'aarch64' feature cannot be disabled " >>>>> + "unless KVM is enabled and 32-bit EL1 " >>>>> + "is supported"); >>>>> + return; >>>>> + } >>>>> unset_feature(&cpu->env, ARM_FEATURE_AARCH64); >>>>> } else { >>>>> set_feature(&cpu->env, ARM_FEATURE_AARCH64); >>>>> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c >>>>> index 22d19c9aec6f..45ccda589903 100644 >>>>> --- a/target/arm/kvm64.c >>>>> +++ b/target/arm/kvm64.c >>>>> @@ -24,7 +24,9 @@ >>>>> #include "exec/gdbstub.h" >>>>> #include "sysemu/sysemu.h" >>>>> #include "sysemu/kvm.h" >>>>> +#include "sysemu/kvm_int.h" >>>>> #include "kvm_arm.h" >>>>> +#include "hw/boards.h" >> By the way those two new headers are not needed by this patch > > Really? > > current_machine is defined in hw/boards.h and KVM_STATE is defined > in sysemu/kvm_int.h. argh my bad. Sorry for the noise Eric > >>>>> #include "internals.h" >>>>> >>>>> static bool have_guest_debug; >>>>> @@ -593,6 +595,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) >>>>> return true; >>>>> } >>>>> >>>>> +bool kvm_arm_aarch32_supported(CPUState *cpu) >>>>> +{ >>>>> + KVMState *s = KVM_STATE(current_machine->accelerator); >>>>> + int ret; >>>>> + >>>>> + ret = kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); >>>>> + return ret > 0; >>>> nit: return kvm_check_extension() should be sufficient >>> >>> Ah yes, I forgot kvm_check_extension() already converts negative >>> error codes to zero. I'll fix that for v3. >>> >>>>> +} >>>>> + >>>>> #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 >>>>> >>>>> int kvm_arch_init_vcpu(CPUState *cs) >>>>> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h >>>>> index 2a07333c615f..812125f805a1 100644 >>>>> --- a/target/arm/kvm_arm.h >>>>> +++ b/target/arm/kvm_arm.h >>>>> @@ -207,6 +207,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf); >>>>> */ >>>>> void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); >>>>> >>>>> +/** >>>>> + * kvm_arm_aarch32_supported: >>>>> + * @cs: CPUState >>>> use kernel-doc comment style? >>> >>> This file (kvm_arm.h) doesn't appear to have a super consistent comment >>> style. I see some use @var: for the parameters and some have 'Returns: >>> ...' lines as well. I'm happy to do whatever the maintainers prefer. For >>> now I was just trying to mimic whatever caught my eye.> >>>>> + * >>>>> + * Returns true if the KVM VCPU can enable AArch32 mode and false >>>>> + * otherwise. >>>>> + */ >>>>> +bool kvm_arm_aarch32_supported(CPUState *cs); >>>>> + >>>>> /** >>>>> * kvm_arm_get_max_vm_ipa_size - Returns the number of bits in the >>>>> * IPA address space supported by KVM >>>>> @@ -247,6 +256,11 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) >>>>> cpu->host_cpu_probe_failed = true; >>>>> } >>>>> >>>>> +static inline bool kvm_arm_aarch32_supported(CPUState *cs) >>>>> +{ >>>>> + return false; >>>>> +} >>>>> + >>>>> static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) >>>>> { >>>>> return -ENOENT; >>>>> >>>> Reviewed-by: Eric Auger <eric.auger@redhat.com> > > Thanks, > drew >
diff --git a/target/arm/cpu64.c b/target/arm/cpu64.c index 1901997a0645..946994838d8a 100644 --- a/target/arm/cpu64.c +++ b/target/arm/cpu64.c @@ -407,13 +407,13 @@ static void aarch64_cpu_set_aarch64(Object *obj, bool value, Error **errp) * restriction allows us to avoid fixing up functionality that assumes a * uniform execution state like do_interrupt. */ - if (!kvm_enabled()) { - error_setg(errp, "'aarch64' feature cannot be disabled " - "unless KVM is enabled"); - return; - } - if (value == false) { + if (!kvm_enabled() || !kvm_arm_aarch32_supported(CPU(cpu))) { + error_setg(errp, "'aarch64' feature cannot be disabled " + "unless KVM is enabled and 32-bit EL1 " + "is supported"); + return; + } unset_feature(&cpu->env, ARM_FEATURE_AARCH64); } else { set_feature(&cpu->env, ARM_FEATURE_AARCH64); diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c index 22d19c9aec6f..45ccda589903 100644 --- a/target/arm/kvm64.c +++ b/target/arm/kvm64.c @@ -24,7 +24,9 @@ #include "exec/gdbstub.h" #include "sysemu/sysemu.h" #include "sysemu/kvm.h" +#include "sysemu/kvm_int.h" #include "kvm_arm.h" +#include "hw/boards.h" #include "internals.h" static bool have_guest_debug; @@ -593,6 +595,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf) return true; } +bool kvm_arm_aarch32_supported(CPUState *cpu) +{ + KVMState *s = KVM_STATE(current_machine->accelerator); + int ret; + + ret = kvm_check_extension(s, KVM_CAP_ARM_EL1_32BIT); + return ret > 0; +} + #define ARM_CPU_ID_MPIDR 3, 0, 0, 0, 5 int kvm_arch_init_vcpu(CPUState *cs) diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h index 2a07333c615f..812125f805a1 100644 --- a/target/arm/kvm_arm.h +++ b/target/arm/kvm_arm.h @@ -207,6 +207,15 @@ bool kvm_arm_get_host_cpu_features(ARMHostCPUFeatures *ahcf); */ void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu); +/** + * kvm_arm_aarch32_supported: + * @cs: CPUState + * + * Returns true if the KVM VCPU can enable AArch32 mode and false + * otherwise. + */ +bool kvm_arm_aarch32_supported(CPUState *cs); + /** * kvm_arm_get_max_vm_ipa_size - Returns the number of bits in the * IPA address space supported by KVM @@ -247,6 +256,11 @@ static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) cpu->host_cpu_probe_failed = true; } +static inline bool kvm_arm_aarch32_supported(CPUState *cs) +{ + return false; +} + static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) { return -ENOENT;
If -cpu <cpu>,aarch64=off is used then KVM must also be used, and it and the host must support running the vcpu in 32-bit mode. Also, if -cpu <cpu>,aarch64=on is used, then it doesn't matter if kvm is enabled or not. Signed-off-by: Andrew Jones <drjones@redhat.com> --- target/arm/cpu64.c | 12 ++++++------ target/arm/kvm64.c | 11 +++++++++++ target/arm/kvm_arm.h | 14 ++++++++++++++ 3 files changed, 31 insertions(+), 6 deletions(-)