| Message ID | 1550519559-15915-25-git-send-email-Dave.Martin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: SVE guest support | expand |
Hi Dave, On 18/02/2019 19:52, Dave Martin wrote: > To provide a uniform way to check for KVM SVE support amongst other > features, this patch adds a suitable capability KVM_CAP_ARM_SVE, > and reports it as present when SVE is available. > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > --- > arch/arm64/kvm/reset.c | 8 ++++++++ > include/uapi/linux/kvm.h | 1 + > 2 files changed, 9 insertions(+) > > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c > index e67cd2e..f636b34 100644 > --- a/arch/arm64/kvm/reset.c > +++ b/arch/arm64/kvm/reset.c > @@ -35,6 +35,7 @@ > #include <asm/kvm_asm.h> > #include <asm/kvm_coproc.h> > #include <asm/kvm_mmu.h> > +#include <asm/virt.h> > > /* Maximum phys_shift supported for any VM on this host */ > static u32 kvm_ipa_limit; > @@ -93,6 +94,9 @@ int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext) > case KVM_CAP_ARM_VM_IPA_SIZE: > r = kvm_ipa_limit; > break; > + case KVM_CAP_ARM_SVE: > + r = system_supports_sve(); > + break; > default: > r = 0; > } > @@ -105,6 +109,10 @@ static int kvm_reset_sve(struct kvm_vcpu *vcpu) > if (!system_supports_sve()) > return -EINVAL; > > + /* Verify that KVM startup enforced this when SVE was detected: */ > + if (WARN_ON(!has_vhe())) > + return -EINVAL; I'm wondering, wouldn't it make more sense to check for this when userland tries to set KVM_ARM_VCPU_SVE? Otherwise: Reviewed-by: Julien Thierry <julien.thierry@arm.com> Cheers,
On Fri, Feb 22, 2019 at 09:10:46AM +0000, Julien Thierry wrote: > Hi Dave, > > On 18/02/2019 19:52, Dave Martin wrote: > > To provide a uniform way to check for KVM SVE support amongst other > > features, this patch adds a suitable capability KVM_CAP_ARM_SVE, > > and reports it as present when SVE is available. > > > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > > --- > > arch/arm64/kvm/reset.c | 8 ++++++++ > > include/uapi/linux/kvm.h | 1 + > > 2 files changed, 9 insertions(+) > > > > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c > > index e67cd2e..f636b34 100644 > > --- a/arch/arm64/kvm/reset.c > > +++ b/arch/arm64/kvm/reset.c > > @@ -35,6 +35,7 @@ > > #include <asm/kvm_asm.h> > > #include <asm/kvm_coproc.h> > > #include <asm/kvm_mmu.h> > > +#include <asm/virt.h> > > > > /* Maximum phys_shift supported for any VM on this host */ > > static u32 kvm_ipa_limit; > > @@ -93,6 +94,9 @@ int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext) > > case KVM_CAP_ARM_VM_IPA_SIZE: > > r = kvm_ipa_limit; > > break; > > + case KVM_CAP_ARM_SVE: > > + r = system_supports_sve(); > > + break; > > default: > > r = 0; > > } > > @@ -105,6 +109,10 @@ static int kvm_reset_sve(struct kvm_vcpu *vcpu) > > if (!system_supports_sve()) > > return -EINVAL; > > > > + /* Verify that KVM startup enforced this when SVE was detected: */ > > + if (WARN_ON(!has_vhe())) > > + return -EINVAL; > > I'm wondering, wouldn't it make more sense to check for this when > userland tries to set KVM_ARM_VCPU_SVE? > > Reviewed-by: Julien Thierry <julien.thierry@arm.com> I have a vague memory of these being some sort of reason for this, but looking at the code now I can't see why the check can't be moved to kvm_reset_vcpu(). How does the following look? The effective is no different, but the code arrangement may be a bit less surprising this way: int kvm_reset_vcpu(struct kvm_vcpu *vcpu) { [...] if (test_bit(KVM_ARM_VCPU_SVE, vcpu->arch.features)) { if (!system_supports_sve()) return -EINVAL; if (WARN_ON(!has_vhe())) return -EINVAL; /* KVM statup should enforce this on SVE hardware: */ ret = kvm_reset_sve(vcpu); if (ret) return ret; } This function is bascially the arch backend for KVM_ARM_VCPU_INIT, so I don't see a better place to do this right now. Adding an extra hook just for this kind of thing feels like overkill... Cheers ---Dave
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c index e67cd2e..f636b34 100644 --- a/arch/arm64/kvm/reset.c +++ b/arch/arm64/kvm/reset.c @@ -35,6 +35,7 @@ #include <asm/kvm_asm.h> #include <asm/kvm_coproc.h> #include <asm/kvm_mmu.h> +#include <asm/virt.h> /* Maximum phys_shift supported for any VM on this host */ static u32 kvm_ipa_limit; @@ -93,6 +94,9 @@ int kvm_arch_vm_ioctl_check_extension(struct kvm *kvm, long ext) case KVM_CAP_ARM_VM_IPA_SIZE: r = kvm_ipa_limit; break; + case KVM_CAP_ARM_SVE: + r = system_supports_sve(); + break; default: r = 0; } @@ -105,6 +109,10 @@ static int kvm_reset_sve(struct kvm_vcpu *vcpu) if (!system_supports_sve()) return -EINVAL; + /* Verify that KVM startup enforced this when SVE was detected: */ + if (WARN_ON(!has_vhe())) + return -EINVAL; + /* If resetting an already-configured vcpu, just zero the SVE regs: */ if (vcpu->arch.sve_state) { size_t size = vcpu_sve_state_size(vcpu); diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index dc77a5a..4ea0d92 100644 --- a/include/uapi/linux/kvm.h +++ b/include/uapi/linux/kvm.h @@ -988,6 +988,7 @@ struct kvm_ppc_resize_hpt { #define KVM_CAP_ARM_VM_IPA_SIZE 165 #define KVM_CAP_MANUAL_DIRTY_LOG_PROTECT 166 #define KVM_CAP_HYPERV_CPUID 167 +#define KVM_CAP_ARM_SVE 168 #ifdef KVM_CAP_IRQ_ROUTING
To provide a uniform way to check for KVM SVE support amongst other features, this patch adds a suitable capability KVM_CAP_ARM_SVE, and reports it as present when SVE is available. Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- arch/arm64/kvm/reset.c | 8 ++++++++ include/uapi/linux/kvm.h | 1 + 2 files changed, 9 insertions(+)