| Message ID | 1548658727-14271-5-git-send-email-amit.kachhap@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add ARMv8.3 pointer authentication for kvm guest | expand |
Hi Amit, On 28/01/2019 06:58, Amit Daniel Kachhap wrote: > This feature will allow the KVM guest to allow the handling of > pointer authentication instructions or to treat them as undefined > if not set. It uses the existing vcpu API KVM_ARM_VCPU_INIT to > supply this parameter instead of creating a new API. > > A new register is not created to pass this parameter via > SET/GET_ONE_REG interface as just a flag (KVM_ARM_VCPU_PTRAUTH) > supplied is enough to enable this feature. > I had a bit of trouble parsing this last sentence. I'd suggest rewording it as "No additional register is created for the SET/GET_ONE_REG API, instead just pass this parameter as a flag". > Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Cc: Marc Zyngier <marc.zyngier@arm.com> > Cc: Christoffer Dall <christoffer.dall@arm.com> > Cc: Kristina Martsenko <kristina.martsenko@arm.com> > Cc: kvmarm@lists.cs.columbia.edu > Cc: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> > Cc: Will Deacon <will.deacon@arm.com> > --- > Documentation/arm64/pointer-authentication.txt | 9 +++++---- > Documentation/virtual/kvm/api.txt | 4 ++++ > arch/arm/include/asm/kvm_host.h | 4 ++++ > arch/arm64/include/asm/kvm_host.h | 7 ++++--- > arch/arm64/include/uapi/asm/kvm.h | 1 + > arch/arm64/kvm/handle_exit.c | 3 ++- > arch/arm64/kvm/hyp/ptrauth-sr.c | 13 +++++++++++++ > arch/arm64/kvm/reset.c | 3 +++ > include/uapi/linux/kvm.h | 1 + > 9 files changed, 37 insertions(+), 8 deletions(-) > > diff --git a/Documentation/arm64/pointer-authentication.txt b/Documentation/arm64/pointer-authentication.txt > index a25cd21..0529a7d 100644 > --- a/Documentation/arm64/pointer-authentication.txt > +++ b/Documentation/arm64/pointer-authentication.txt > @@ -82,7 +82,8 @@ pointers). > Virtualization > -------------- > > -Pointer authentication is not currently supported in KVM guests. KVM > -will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of > -the feature will result in an UNDEFINED exception being injected into > -the guest. > +Pointer authentication is enabled in KVM guest when virtual machine is > +created by passing a flag (KVM_ARM_VCPU_PTRAUTH) requesting this feature > +to be enabled. Without this flag, pointer authentication is not enabled > +in KVM guests and attempted use of the feature will result in an UNDEFINED > +exception being injected into the guest. > diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt > index 356156f..1e646fb 100644 > --- a/Documentation/virtual/kvm/api.txt > +++ b/Documentation/virtual/kvm/api.txt > @@ -2642,6 +2642,10 @@ Possible features: > Depends on KVM_CAP_ARM_PSCI_0_2. > - KVM_ARM_VCPU_PMU_V3: Emulate PMUv3 for the CPU. > Depends on KVM_CAP_ARM_PMU_V3. > + - KVM_ARM_VCPU_PTRAUTH: Emulate Pointer authentication for the CPU. > + Depends on KVM_CAP_ARM_PTRAUTH and only on arm64 architecture. If > + set, then the KVM guest allows the execution of pointer authentication > + instructions or treats them as undefined if not set. I'd suggest "then the KVM guest allows the execution of pointer authentication instructions. Otherwise, KVM treats these instructions as undefined." > > > 4.83 KVM_ARM_PREFERRED_TARGET > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h > index b200c14..b6950df 100644 > --- a/arch/arm/include/asm/kvm_host.h > +++ b/arch/arm/include/asm/kvm_host.h > @@ -346,6 +346,10 @@ static inline int kvm_arm_have_ssbd(void) > static inline void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu) {} > static inline void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu) {} > static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) {} > +static inline bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) > +{ > + return false; > +} > > #define __KVM_HAVE_ARCH_VM_ALLOC > struct kvm *kvm_arch_alloc_vm(void); > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index c798d0c..4a6ec40 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -43,7 +43,7 @@ > > #define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS > > -#define KVM_VCPU_MAX_FEATURES 4 > +#define KVM_VCPU_MAX_FEATURES 5 > > #define KVM_REQ_SLEEP \ > KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP) > @@ -453,14 +453,15 @@ static inline bool kvm_arch_requires_vhe(void) > > void kvm_arm_vcpu_ptrauth_enable(struct kvm_vcpu *vcpu); > void kvm_arm_vcpu_ptrauth_disable(struct kvm_vcpu *vcpu); > +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu); > > static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) > { > /* Disable ptrauth and use it in a lazy context via traps */ > - if (has_vhe() && kvm_supports_ptrauth()) > + if (has_vhe() && kvm_supports_ptrauth() > + && kvm_arm_vcpu_ptrauth_allowed(vcpu)) > kvm_arm_vcpu_ptrauth_disable(vcpu); > } > - > void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu); > > static inline void kvm_arch_hardware_unsetup(void) {} > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > index 97c3478..5f82ca1 100644 > --- a/arch/arm64/include/uapi/asm/kvm.h > +++ b/arch/arm64/include/uapi/asm/kvm.h > @@ -102,6 +102,7 @@ struct kvm_regs { > #define KVM_ARM_VCPU_EL1_32BIT 1 /* CPU running a 32bit VM */ > #define KVM_ARM_VCPU_PSCI_0_2 2 /* CPU uses PSCI v0.2 */ > #define KVM_ARM_VCPU_PMU_V3 3 /* Support guest PMUv3 */ > +#define KVM_ARM_VCPU_PTRAUTH 4 /* VCPU uses address authentication */ > > struct kvm_vcpu_init { > __u32 target; > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > index 5b980e7..c0e5dcd 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -179,7 +179,8 @@ static int handle_sve(struct kvm_vcpu *vcpu, struct kvm_run *run) > */ > void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu) > { > - if (has_vhe() && kvm_supports_ptrauth()) > + if (has_vhe() && kvm_supports_ptrauth() > + && kvm_arm_vcpu_ptrauth_allowed(vcpu)) > kvm_arm_vcpu_ptrauth_enable(vcpu); > else > kvm_inject_undefined(vcpu); > diff --git a/arch/arm64/kvm/hyp/ptrauth-sr.c b/arch/arm64/kvm/hyp/ptrauth-sr.c > index 0576c01..369624f 100644 > --- a/arch/arm64/kvm/hyp/ptrauth-sr.c > +++ b/arch/arm64/kvm/hyp/ptrauth-sr.c > @@ -42,3 +42,16 @@ void __no_ptrauth __hyp_text __ptrauth_switch_to_host(struct kvm_vcpu *vcpu, > ptrauth_keys_store((struct ptrauth_keys *) &guest_ctxt->sys_regs[APIAKEYLO_EL1]); > ptrauth_keys_switch((struct ptrauth_keys *) &host_ctxt->sys_regs[APIAKEYLO_EL1]); > } > + > +/** > + * kvm_arm_vcpu_ptrauth_allowed - checks if ptrauth feature is present in vcpu > + * > + * @vcpu: The VCPU pointer > + * > + * This function will be used to enable/disable ptrauth in guest as configured > + * by the KVM userspace API. > + */ > +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) Nit: I find the "allowed" a bit strange. Maybe something like "kvm_arm_vcpu_has_ptrauth()" ? > +{ > + return test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features); > +} > diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c > index b72a3dd..987e0c3c 100644 > --- a/arch/arm64/kvm/reset.c > +++ b/arch/arm64/kvm/reset.c > @@ -91,6 +91,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_PTRAUTH: > + r = kvm_supports_ptrauth(); Even if we don't have VHE? Cheers,
Hi Amit, On 28/01/2019 06:58, Amit Daniel Kachhap wrote: > This feature will allow the KVM guest to allow the handling of > pointer authentication instructions or to treat them as undefined > if not set. It uses the existing vcpu API KVM_ARM_VCPU_INIT to > supply this parameter instead of creating a new API. > > A new register is not created to pass this parameter via > SET/GET_ONE_REG interface as just a flag (KVM_ARM_VCPU_PTRAUTH) > supplied is enough to enable this feature. > diff --git a/Documentation/arm64/pointer-authentication.txt b/Documentation/arm64/pointer-authentication.txt > index a25cd21..0529a7d 100644 > --- a/Documentation/arm64/pointer-authentication.txt > +++ b/Documentation/arm64/pointer-authentication.txt > @@ -82,7 +82,8 @@ pointers). > Virtualization > -------------- > > -Pointer authentication is not currently supported in KVM guests. KVM > -will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of > -the feature will result in an UNDEFINED exception being injected into > -the guest. > +Pointer authentication is enabled in KVM guest when virtual machine is > +created by passing a flag (KVM_ARM_VCPU_PTRAUTH) Isn't that a VCPU flag? Shouldn't this be when each VCPU is created? > requesting this feature > +to be enabled. Without this flag, pointer authentication is not enabled > +in KVM guests and attempted use of the feature will result in an UNDEFINED > +exception being injected into the guest. ... what happens if KVM's user-space enables ptrauth on some vcpus, but not on others? You removed the id-register suppression in the previous patch, but it doesn't get hooked up to kvm_arm_vcpu_ptrauth_allowed() here. (you could add kvm_arm_vcpu_ptrauth_allowed() earlier, and default it to true to make it easier). Doesn't this mean that if the CPU supports pointer auth, but user-space doesn't specify this flag, the guest gets mysterious undef's whenever it tries to use the advertised feature? (whether we support big/little virtual-machines is probably a separate issue, but the id registers need to be consistent with our trap-and-undef behaviour) > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index c798d0c..4a6ec40 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -453,14 +453,15 @@ static inline bool kvm_arch_requires_vhe(void) > > void kvm_arm_vcpu_ptrauth_enable(struct kvm_vcpu *vcpu); > void kvm_arm_vcpu_ptrauth_disable(struct kvm_vcpu *vcpu); > +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu); > > static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) > { > /* Disable ptrauth and use it in a lazy context via traps */ > - if (has_vhe() && kvm_supports_ptrauth()) > + if (has_vhe() && kvm_supports_ptrauth() > + && kvm_arm_vcpu_ptrauth_allowed(vcpu)) > kvm_arm_vcpu_ptrauth_disable(vcpu); > } > - > void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu); > > diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c > index 5b980e7..c0e5dcd 100644 > --- a/arch/arm64/kvm/handle_exit.c > +++ b/arch/arm64/kvm/handle_exit.c > @@ -179,7 +179,8 @@ static int handle_sve(struct kvm_vcpu *vcpu, struct kvm_run *run) > */ > void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu) > { > - if (has_vhe() && kvm_supports_ptrauth()) > + if (has_vhe() && kvm_supports_ptrauth() > + && kvm_arm_vcpu_ptrauth_allowed(vcpu)) Duplication. If has_vhe() moved into kvm_supports_ptrauth(), and kvm_supports_ptrauth() was called from kvm_arm_vcpu_ptrauth_allowed() it would be clearer that use of this feature was becoming user-controlled policy. (We don't need to list the dependencies at every call site) > diff --git a/arch/arm64/kvm/hyp/ptrauth-sr.c b/arch/arm64/kvm/hyp/ptrauth-sr.c > index 0576c01..369624f 100644 > --- a/arch/arm64/kvm/hyp/ptrauth-sr.c > +++ b/arch/arm64/kvm/hyp/ptrauth-sr.c > @@ -42,3 +42,16 @@ void __no_ptrauth __hyp_text __ptrauth_switch_to_host(struct kvm_vcpu *vcpu, > ptrauth_keys_store((struct ptrauth_keys *) &guest_ctxt->sys_regs[APIAKEYLO_EL1]); > ptrauth_keys_switch((struct ptrauth_keys *) &host_ctxt->sys_regs[APIAKEYLO_EL1]); > } > + > +/** > + * kvm_arm_vcpu_ptrauth_allowed - checks if ptrauth feature is present in vcpu ('enabled by KVM's user-space' may be clearer. 'Present in vcpu' could be down to a cpufeature thing) > + * > + * @vcpu: The VCPU pointer > + * > + * This function will be used to enable/disable ptrauth in guest as configured ... but it just tests the bit ... > + * by the KVM userspace API. > + */ > +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) > +{ > + return test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features); > +} Thanks, James
On 28/01/2019 06:58, Amit Daniel Kachhap wrote: > This feature will allow the KVM guest to allow the handling of > pointer authentication instructions or to treat them as undefined > if not set. It uses the existing vcpu API KVM_ARM_VCPU_INIT to > supply this parameter instead of creating a new API. > > A new register is not created to pass this parameter via > SET/GET_ONE_REG interface as just a flag (KVM_ARM_VCPU_PTRAUTH) > supplied is enough to enable this feature. [...] > diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h > index b200c14..b6950df 100644 > --- a/arch/arm/include/asm/kvm_host.h > +++ b/arch/arm/include/asm/kvm_host.h > @@ -346,6 +346,10 @@ static inline int kvm_arm_have_ssbd(void) > static inline void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu) {} > static inline void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu) {} > static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) {} > +static inline bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) > +{ > + return false; > +} It seems like this is only ever called from arm64 code, so do we need an arch/arm/ definition? > +/** > + * kvm_arm_vcpu_ptrauth_allowed - checks if ptrauth feature is present in vcpu > + * > + * @vcpu: The VCPU pointer > + * > + * This function will be used to enable/disable ptrauth in guest as configured > + * by the KVM userspace API. > + */ > +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) > +{ > + return test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features); > +} I'm not sure, but should there also be something like if (test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features) && !kvm_supports_ptrauth()) return -EINVAL; in kvm_reset_vcpu? Thanks, Kristina
Hi, On 1/31/19 9:57 PM, James Morse wrote: > Hi Amit, > > On 28/01/2019 06:58, Amit Daniel Kachhap wrote: >> This feature will allow the KVM guest to allow the handling of >> pointer authentication instructions or to treat them as undefined >> if not set. It uses the existing vcpu API KVM_ARM_VCPU_INIT to >> supply this parameter instead of creating a new API. >> >> A new register is not created to pass this parameter via >> SET/GET_ONE_REG interface as just a flag (KVM_ARM_VCPU_PTRAUTH) >> supplied is enough to enable this feature. > > >> diff --git a/Documentation/arm64/pointer-authentication.txt > b/Documentation/arm64/pointer-authentication.txt >> index a25cd21..0529a7d 100644 >> --- a/Documentation/arm64/pointer-authentication.txt >> +++ b/Documentation/arm64/pointer-authentication.txt >> @@ -82,7 +82,8 @@ pointers). >> Virtualization >> -------------- >> >> -Pointer authentication is not currently supported in KVM guests. KVM >> -will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of >> -the feature will result in an UNDEFINED exception being injected into >> -the guest. >> +Pointer authentication is enabled in KVM guest when virtual machine is >> +created by passing a flag (KVM_ARM_VCPU_PTRAUTH) > > Isn't that a VCPU flag? Shouldn't this be when each VCPU is created? Yes it is a VCPU flag. > > >> requesting this feature >> +to be enabled. Without this flag, pointer authentication is not enabled >> +in KVM guests and attempted use of the feature will result in an UNDEFINED >> +exception being injected into the guest. > > ... what happens if KVM's user-space enables ptrauth on some vcpus, but not on > others? Yes seems to be issue. Let me check more on this if there are other ways of passing the userspace parameter such as in CREATE_VM type ioctl. > > You removed the id-register suppression in the previous patch, but it doesn't > get hooked up to kvm_arm_vcpu_ptrauth_allowed() here. (you could add > kvm_arm_vcpu_ptrauth_allowed() earlier, and default it to true to make it easier). > > Doesn't this mean that if the CPU supports pointer auth, but user-space doesn't > specify this flag, the guest gets mysterious undef's whenever it tries to use > the advertised feature? Agree, ID registers should be masked when userspace disables it. > > (whether we support big/little virtual-machines is probably a separate issue, > but the id registers need to be consistent with our trap-and-undef behaviour) > > >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h >> index c798d0c..4a6ec40 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -453,14 +453,15 @@ static inline bool kvm_arch_requires_vhe(void) >> >> void kvm_arm_vcpu_ptrauth_enable(struct kvm_vcpu *vcpu); >> void kvm_arm_vcpu_ptrauth_disable(struct kvm_vcpu *vcpu); >> +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu); >> >> static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) >> { >> /* Disable ptrauth and use it in a lazy context via traps */ >> - if (has_vhe() && kvm_supports_ptrauth()) >> + if (has_vhe() && kvm_supports_ptrauth() >> + && kvm_arm_vcpu_ptrauth_allowed(vcpu)) >> kvm_arm_vcpu_ptrauth_disable(vcpu); >> } >> - >> void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu); >> > >> diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c >> index 5b980e7..c0e5dcd 100644 >> --- a/arch/arm64/kvm/handle_exit.c >> +++ b/arch/arm64/kvm/handle_exit.c >> @@ -179,7 +179,8 @@ static int handle_sve(struct kvm_vcpu *vcpu, struct kvm_run *run) >> */ >> void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu) >> { >> - if (has_vhe() && kvm_supports_ptrauth()) >> + if (has_vhe() && kvm_supports_ptrauth() >> + && kvm_arm_vcpu_ptrauth_allowed(vcpu)) > > Duplication. If has_vhe() moved into kvm_supports_ptrauth(), and > kvm_supports_ptrauth() was called from kvm_arm_vcpu_ptrauth_allowed() it would > be clearer that use of this feature was becoming user-controlled policy. > > (We don't need to list the dependencies at every call site) ok. > > >> diff --git a/arch/arm64/kvm/hyp/ptrauth-sr.c b/arch/arm64/kvm/hyp/ptrauth-sr.c >> index 0576c01..369624f 100644 >> --- a/arch/arm64/kvm/hyp/ptrauth-sr.c >> +++ b/arch/arm64/kvm/hyp/ptrauth-sr.c >> @@ -42,3 +42,16 @@ void __no_ptrauth __hyp_text __ptrauth_switch_to_host(struct kvm_vcpu *vcpu, >> ptrauth_keys_store((struct ptrauth_keys *) &guest_ctxt->sys_regs[APIAKEYLO_EL1]); >> ptrauth_keys_switch((struct ptrauth_keys *) &host_ctxt->sys_regs[APIAKEYLO_EL1]); >> } >> + >> +/** >> + * kvm_arm_vcpu_ptrauth_allowed - checks if ptrauth feature is present in vcpu > > ('enabled by KVM's user-space' may be clearer. 'Present in vcpu' could be down > to a cpufeature thing) ok. > > >> + * >> + * @vcpu: The VCPU pointer >> + * >> + * This function will be used to enable/disable ptrauth in guest as configured > > ... but it just tests the bit ... > >> + * by the KVM userspace API. >> + */ >> +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) >> +{ >> + return test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features); >> +} > > > Thanks, > > James > //Amit
Hi, On 2/13/19 11:05 PM, Kristina Martsenko wrote: > On 28/01/2019 06:58, Amit Daniel Kachhap wrote: >> This feature will allow the KVM guest to allow the handling of >> pointer authentication instructions or to treat them as undefined >> if not set. It uses the existing vcpu API KVM_ARM_VCPU_INIT to >> supply this parameter instead of creating a new API. >> >> A new register is not created to pass this parameter via >> SET/GET_ONE_REG interface as just a flag (KVM_ARM_VCPU_PTRAUTH) >> supplied is enough to enable this feature. > > [...] > >> diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h >> index b200c14..b6950df 100644 >> --- a/arch/arm/include/asm/kvm_host.h >> +++ b/arch/arm/include/asm/kvm_host.h >> @@ -346,6 +346,10 @@ static inline int kvm_arm_have_ssbd(void) >> static inline void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu) {} >> static inline void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu) {} >> static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) {} >> +static inline bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) >> +{ >> + return false; >> +} > > It seems like this is only ever called from arm64 code, so do we need an > arch/arm/ definition? Yes not required. Nice catch. > >> +/** >> + * kvm_arm_vcpu_ptrauth_allowed - checks if ptrauth feature is present in vcpu >> + * >> + * @vcpu: The VCPU pointer >> + * >> + * This function will be used to enable/disable ptrauth in guest as configured >> + * by the KVM userspace API. >> + */ >> +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) >> +{ >> + return test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features); >> +} > > I'm not sure, but should there also be something like > > if (test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features) && > !kvm_supports_ptrauth()) > return -EINVAL; > > in kvm_reset_vcpu? Yes makes sense. I missed it and with Dave martin patch this may be done cleanly. Thanks, Amit D > > Thanks, > Kristina >
diff --git a/Documentation/arm64/pointer-authentication.txt b/Documentation/arm64/pointer-authentication.txt index a25cd21..0529a7d 100644 --- a/Documentation/arm64/pointer-authentication.txt +++ b/Documentation/arm64/pointer-authentication.txt @@ -82,7 +82,8 @@ pointers). Virtualization -------------- -Pointer authentication is not currently supported in KVM guests. KVM -will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of -the feature will result in an UNDEFINED exception being injected into -the guest. +Pointer authentication is enabled in KVM guest when virtual machine is +created by passing a flag (KVM_ARM_VCPU_PTRAUTH) requesting this feature +to be enabled. Without this flag, pointer authentication is not enabled +in KVM guests and attempted use of the feature will result in an UNDEFINED +exception being injected into the guest. diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt index 356156f..1e646fb 100644 --- a/Documentation/virtual/kvm/api.txt +++ b/Documentation/virtual/kvm/api.txt @@ -2642,6 +2642,10 @@ Possible features: Depends on KVM_CAP_ARM_PSCI_0_2. - KVM_ARM_VCPU_PMU_V3: Emulate PMUv3 for the CPU. Depends on KVM_CAP_ARM_PMU_V3. + - KVM_ARM_VCPU_PTRAUTH: Emulate Pointer authentication for the CPU. + Depends on KVM_CAP_ARM_PTRAUTH and only on arm64 architecture. If + set, then the KVM guest allows the execution of pointer authentication + instructions or treats them as undefined if not set. 4.83 KVM_ARM_PREFERRED_TARGET diff --git a/arch/arm/include/asm/kvm_host.h b/arch/arm/include/asm/kvm_host.h index b200c14..b6950df 100644 --- a/arch/arm/include/asm/kvm_host.h +++ b/arch/arm/include/asm/kvm_host.h @@ -346,6 +346,10 @@ static inline int kvm_arm_have_ssbd(void) static inline void kvm_vcpu_load_sysregs(struct kvm_vcpu *vcpu) {} static inline void kvm_vcpu_put_sysregs(struct kvm_vcpu *vcpu) {} static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) {} +static inline bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) +{ + return false; +} #define __KVM_HAVE_ARCH_VM_ALLOC struct kvm *kvm_arch_alloc_vm(void); diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index c798d0c..4a6ec40 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -43,7 +43,7 @@ #define KVM_MAX_VCPUS VGIC_V3_MAX_CPUS -#define KVM_VCPU_MAX_FEATURES 4 +#define KVM_VCPU_MAX_FEATURES 5 #define KVM_REQ_SLEEP \ KVM_ARCH_REQ_FLAGS(0, KVM_REQUEST_WAIT | KVM_REQUEST_NO_WAKEUP) @@ -453,14 +453,15 @@ static inline bool kvm_arch_requires_vhe(void) void kvm_arm_vcpu_ptrauth_enable(struct kvm_vcpu *vcpu); void kvm_arm_vcpu_ptrauth_disable(struct kvm_vcpu *vcpu); +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu); static inline void kvm_arm_vcpu_ptrauth_reset(struct kvm_vcpu *vcpu) { /* Disable ptrauth and use it in a lazy context via traps */ - if (has_vhe() && kvm_supports_ptrauth()) + if (has_vhe() && kvm_supports_ptrauth() + && kvm_arm_vcpu_ptrauth_allowed(vcpu)) kvm_arm_vcpu_ptrauth_disable(vcpu); } - void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu); static inline void kvm_arch_hardware_unsetup(void) {} diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 97c3478..5f82ca1 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -102,6 +102,7 @@ struct kvm_regs { #define KVM_ARM_VCPU_EL1_32BIT 1 /* CPU running a 32bit VM */ #define KVM_ARM_VCPU_PSCI_0_2 2 /* CPU uses PSCI v0.2 */ #define KVM_ARM_VCPU_PMU_V3 3 /* Support guest PMUv3 */ +#define KVM_ARM_VCPU_PTRAUTH 4 /* VCPU uses address authentication */ struct kvm_vcpu_init { __u32 target; diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c index 5b980e7..c0e5dcd 100644 --- a/arch/arm64/kvm/handle_exit.c +++ b/arch/arm64/kvm/handle_exit.c @@ -179,7 +179,8 @@ static int handle_sve(struct kvm_vcpu *vcpu, struct kvm_run *run) */ void kvm_arm_vcpu_ptrauth_trap(struct kvm_vcpu *vcpu) { - if (has_vhe() && kvm_supports_ptrauth()) + if (has_vhe() && kvm_supports_ptrauth() + && kvm_arm_vcpu_ptrauth_allowed(vcpu)) kvm_arm_vcpu_ptrauth_enable(vcpu); else kvm_inject_undefined(vcpu); diff --git a/arch/arm64/kvm/hyp/ptrauth-sr.c b/arch/arm64/kvm/hyp/ptrauth-sr.c index 0576c01..369624f 100644 --- a/arch/arm64/kvm/hyp/ptrauth-sr.c +++ b/arch/arm64/kvm/hyp/ptrauth-sr.c @@ -42,3 +42,16 @@ void __no_ptrauth __hyp_text __ptrauth_switch_to_host(struct kvm_vcpu *vcpu, ptrauth_keys_store((struct ptrauth_keys *) &guest_ctxt->sys_regs[APIAKEYLO_EL1]); ptrauth_keys_switch((struct ptrauth_keys *) &host_ctxt->sys_regs[APIAKEYLO_EL1]); } + +/** + * kvm_arm_vcpu_ptrauth_allowed - checks if ptrauth feature is present in vcpu + * + * @vcpu: The VCPU pointer + * + * This function will be used to enable/disable ptrauth in guest as configured + * by the KVM userspace API. + */ +bool kvm_arm_vcpu_ptrauth_allowed(struct kvm_vcpu *vcpu) +{ + return test_bit(KVM_ARM_VCPU_PTRAUTH, vcpu->arch.features); +} diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c index b72a3dd..987e0c3c 100644 --- a/arch/arm64/kvm/reset.c +++ b/arch/arm64/kvm/reset.c @@ -91,6 +91,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_PTRAUTH: + r = kvm_supports_ptrauth(); + break; default: r = 0; } diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h index 6d4ea4b..a553477 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_PTRAUTH 168 #ifdef KVM_CAP_IRQ_ROUTING
This feature will allow the KVM guest to allow the handling of pointer authentication instructions or to treat them as undefined if not set. It uses the existing vcpu API KVM_ARM_VCPU_INIT to supply this parameter instead of creating a new API. A new register is not created to pass this parameter via SET/GET_ONE_REG interface as just a flag (KVM_ARM_VCPU_PTRAUTH) supplied is enough to enable this feature. Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Cc: Marc Zyngier <marc.zyngier@arm.com> Cc: Christoffer Dall <christoffer.dall@arm.com> Cc: Kristina Martsenko <kristina.martsenko@arm.com> Cc: kvmarm@lists.cs.columbia.edu Cc: Ramana Radhakrishnan <ramana.radhakrishnan@arm.com> Cc: Will Deacon <will.deacon@arm.com> --- Documentation/arm64/pointer-authentication.txt | 9 +++++---- Documentation/virtual/kvm/api.txt | 4 ++++ arch/arm/include/asm/kvm_host.h | 4 ++++ arch/arm64/include/asm/kvm_host.h | 7 ++++--- arch/arm64/include/uapi/asm/kvm.h | 1 + arch/arm64/kvm/handle_exit.c | 3 ++- arch/arm64/kvm/hyp/ptrauth-sr.c | 13 +++++++++++++ arch/arm64/kvm/reset.c | 3 +++ include/uapi/linux/kvm.h | 1 + 9 files changed, 37 insertions(+), 8 deletions(-)