| Message ID | 1555086498-26691-6-git-send-email-Dave.Martin@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: SVE cleanups | expand |
On Fri, Apr 12, 2019 at 05:28:09PM +0100, Dave Martin wrote: > Currently, the SVE register ID macros are not all defined in the > same way, and advertise the fact that FFR maps onto the nonexistent > predicate register P16. This is really just for kernel > convenience, and may lead userspace into bad habits. > > Instead, this patch masks the ID macro arguments so that > architecturally invalid register numbers will not be passed through > any more, and uses a literal KVM_REG_ARM64_SVE_FFR_BASE macro to > define KVM_REG_ARM64_SVE_FFR(), similarly to the way the _ZREG() > and _PREG() macros are defined. > > Rather than plugging in magic numbers for the number of Z- and P- > registers and the maximum possible number of register slices, this > patch provides definitions for those too. Userspace is going to > need them in any case, and it makes sense for them to come from > <uapi/asm/kvm.h>. > > sve_reg_to_region() uses convenience constants that are defined in > a different way, and also makes use of the fact that the FFR IDs > are really contiguous with the P15 IDs, so this patch retains the > existing convenience constants in guest.c, supplemented with a > couple of sanity checks to check for consistency with with the UAPI s/with with/with/ > header. > > Fixes: e1c9c98345b3 ("KVM: arm64/sve: Add SVE support to register access ioctl interface") > Suggested-by: Andrew Jones <drjones@redhat.com> > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > --- > arch/arm64/include/uapi/asm/kvm.h | 32 +++++++++++++++++++++++--------- > arch/arm64/kvm/guest.c | 9 +++++++++ > 2 files changed, 32 insertions(+), 9 deletions(-) > Reviewed-by: Andrew Jones <drjones@redhat.com>
On Mon, Apr 15, 2019 at 05:17:39PM +0200, Andrew Jones wrote: > On Fri, Apr 12, 2019 at 05:28:09PM +0100, Dave Martin wrote: > > Currently, the SVE register ID macros are not all defined in the > > same way, and advertise the fact that FFR maps onto the nonexistent > > predicate register P16. This is really just for kernel > > convenience, and may lead userspace into bad habits. > > > > Instead, this patch masks the ID macro arguments so that > > architecturally invalid register numbers will not be passed through > > any more, and uses a literal KVM_REG_ARM64_SVE_FFR_BASE macro to > > define KVM_REG_ARM64_SVE_FFR(), similarly to the way the _ZREG() > > and _PREG() macros are defined. > > > > Rather than plugging in magic numbers for the number of Z- and P- > > registers and the maximum possible number of register slices, this > > patch provides definitions for those too. Userspace is going to > > need them in any case, and it makes sense for them to come from > > <uapi/asm/kvm.h>. > > > > sve_reg_to_region() uses convenience constants that are defined in > > a different way, and also makes use of the fact that the FFR IDs > > are really contiguous with the P15 IDs, so this patch retains the > > existing convenience constants in guest.c, supplemented with a > > couple of sanity checks to check for consistency with with the UAPI > > s/with with/with/ Good spot, thanks. > > header. > > > > Fixes: e1c9c98345b3 ("KVM: arm64/sve: Add SVE support to register access ioctl interface") > > Suggested-by: Andrew Jones <drjones@redhat.com> > > Signed-off-by: Dave Martin <Dave.Martin@arm.com> > > --- > > arch/arm64/include/uapi/asm/kvm.h | 32 +++++++++++++++++++++++--------- > > arch/arm64/kvm/guest.c | 9 +++++++++ > > 2 files changed, 32 insertions(+), 9 deletions(-) > > > > Reviewed-by: Andrew Jones <drjones@redhat.com> Thanks ---Dave
diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 6963b7e..2a04ef0 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -35,6 +35,7 @@ #include <linux/psci.h> #include <linux/types.h> #include <asm/ptrace.h> +#include <asm/sve_context.h> #define __KVM_HAVE_GUEST_DEBUG #define __KVM_HAVE_IRQ_LINE @@ -233,16 +234,29 @@ struct kvm_vcpu_events { /* Z- and P-regs occupy blocks at the following offsets within this range: */ #define KVM_REG_ARM64_SVE_ZREG_BASE 0 #define KVM_REG_ARM64_SVE_PREG_BASE 0x400 +#define KVM_REG_ARM64_SVE_FFR_BASE 0x600 -#define KVM_REG_ARM64_SVE_ZREG(n, i) (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \ - KVM_REG_ARM64_SVE_ZREG_BASE | \ - KVM_REG_SIZE_U2048 | \ - ((n) << 5) | (i)) -#define KVM_REG_ARM64_SVE_PREG(n, i) (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \ - KVM_REG_ARM64_SVE_PREG_BASE | \ - KVM_REG_SIZE_U256 | \ - ((n) << 5) | (i)) -#define KVM_REG_ARM64_SVE_FFR(i) KVM_REG_ARM64_SVE_PREG(16, i) +#define KVM_ARM64_SVE_NUM_ZREGS __SVE_NUM_ZREGS +#define KVM_ARM64_SVE_NUM_PREGS __SVE_NUM_PREGS + +#define KVM_ARM64_SVE_MAX_SLICES 32 + +#define KVM_REG_ARM64_SVE_ZREG(n, i) \ + (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_ZREG_BASE | \ + KVM_REG_SIZE_U2048 | \ + (((n) & (KVM_ARM64_SVE_NUM_ZREGS - 1)) << 5) | \ + ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1))) + +#define KVM_REG_ARM64_SVE_PREG(n, i) \ + (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_PREG_BASE | \ + KVM_REG_SIZE_U256 | \ + (((n) & (KVM_ARM64_SVE_NUM_PREGS - 1)) << 5) | \ + ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1))) + +#define KVM_REG_ARM64_SVE_FFR(i) \ + (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | KVM_REG_ARM64_SVE_FFR_BASE | \ + KVM_REG_SIZE_U256 | \ + ((i) & (KVM_ARM64_SVE_MAX_SLICES - 1))) /* Vector lengths pseudo-register: */ #define KVM_REG_ARM64_SVE_VLS (KVM_REG_ARM64 | KVM_REG_ARM64_SVE | \ diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c index 4f7b26b..2e449e1 100644 --- a/arch/arm64/kvm/guest.c +++ b/arch/arm64/kvm/guest.c @@ -325,6 +325,15 @@ static int sve_reg_to_region(struct sve_state_reg_region *region, size_t sve_state_size; + const u64 last_preg_id = KVM_REG_ARM64_SVE_PREG(SVE_NUM_PREGS - 1, + SVE_NUM_SLICES - 1); + + /* Verify that the P-regs and FFR really do have contiguous IDs: */ + BUILD_BUG_ON(KVM_REG_ARM64_SVE_FFR(0) != last_preg_id + 1); + + /* Verify that we match the UAPI header: */ + BUILD_BUG_ON(SVE_NUM_SLICES != KVM_ARM64_SVE_MAX_SLICES); + /* Only the first slice ever exists, for now: */ if ((reg->id & SVE_REG_SLICE_MASK) != 0) return -ENOENT;
Currently, the SVE register ID macros are not all defined in the same way, and advertise the fact that FFR maps onto the nonexistent predicate register P16. This is really just for kernel convenience, and may lead userspace into bad habits. Instead, this patch masks the ID macro arguments so that architecturally invalid register numbers will not be passed through any more, and uses a literal KVM_REG_ARM64_SVE_FFR_BASE macro to define KVM_REG_ARM64_SVE_FFR(), similarly to the way the _ZREG() and _PREG() macros are defined. Rather than plugging in magic numbers for the number of Z- and P- registers and the maximum possible number of register slices, this patch provides definitions for those too. Userspace is going to need them in any case, and it makes sense for them to come from <uapi/asm/kvm.h>. sve_reg_to_region() uses convenience constants that are defined in a different way, and also makes use of the fact that the FFR IDs are really contiguous with the P15 IDs, so this patch retains the existing convenience constants in guest.c, supplemented with a couple of sanity checks to check for consistency with with the UAPI header. Fixes: e1c9c98345b3 ("KVM: arm64/sve: Add SVE support to register access ioctl interface") Suggested-by: Andrew Jones <drjones@redhat.com> Signed-off-by: Dave Martin <Dave.Martin@arm.com> --- arch/arm64/include/uapi/asm/kvm.h | 32 +++++++++++++++++++++++--------- arch/arm64/kvm/guest.c | 9 +++++++++ 2 files changed, 32 insertions(+), 9 deletions(-)