Hi Shaoqin,
On Wed, Aug 9, 2023 at 9:27 PM Shaoqin Huang <shahuang@xxxxxxxxxx> wrote:
>
> Hi Jing,
>
> On 8/1/23 23:19, Jing Zhang wrote:
> > Add a VM ioctl to allow userspace to get writable masks for feature ID
> > registers in below system register space:
> > op0 = 3, op1 = {0, 1, 3}, CRn = 0, CRm = {0 - 7}, op2 = {0 - 7}
> > This is used to support mix-and-match userspace and kernels for writable
> > ID registers, where userspace may want to know upfront whether it can
> > actually tweak the contents of an idreg or not.
> >
> > Suggested-by: Marc Zyngier <maz@xxxxxxxxxx>
> > Suggested-by: Cornelia Huck <cohuck@xxxxxxxxxx>
> > Signed-off-by: Jing Zhang <jingzhangos@xxxxxxxxxx>
> > ---
> > arch/arm64/include/asm/kvm_host.h | 2 ++
> > arch/arm64/include/uapi/asm/kvm.h | 25 +++++++++++++++
> > arch/arm64/kvm/arm.c | 3 ++
> > arch/arm64/kvm/sys_regs.c | 51 +++++++++++++++++++++++++++++++
> > include/uapi/linux/kvm.h | 2 ++
> > 5 files changed, 83 insertions(+)
> >
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index d3dd05bbfe23..3996a3707f4e 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -1074,6 +1074,8 @@ int kvm_vm_ioctl_mte_copy_tags(struct kvm *kvm,
> > struct kvm_arm_copy_mte_tags *copy_tags);
> > int kvm_vm_ioctl_set_counter_offset(struct kvm *kvm,
> > struct kvm_arm_counter_offset *offset);
> > +int kvm_vm_ioctl_get_feature_id_writable_masks(struct kvm *kvm,
> > + u64 __user *masks);
> >
> > /* Guest/host FPSIMD coordination helpers */
> > int kvm_arch_vcpu_run_map_fp(struct kvm_vcpu *vcpu);
> > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h
> > index f7ddd73a8c0f..2970c0d792ee 100644
> > --- a/arch/arm64/include/uapi/asm/kvm.h
> > +++ b/arch/arm64/include/uapi/asm/kvm.h
> > @@ -505,6 +505,31 @@ struct kvm_smccc_filter {
> > #define KVM_HYPERCALL_EXIT_SMC (1U << 0)
> > #define KVM_HYPERCALL_EXIT_16BIT (1U << 1)
> >
> > +/* Get feature ID registers userspace writable mask. */
> > +/*
> > + * From DDI0487J.a, D19.2.66 ("ID_AA64MMFR2_EL1, AArch64 Memory Model
> > + * Feature Register 2"):
> > + *
> > + * "The Feature ID space is defined as the System register space in
> > + * AArch64 with op0==3, op1=={0, 1, 3}, CRn==0, CRm=={0-7},
> > + * op2=={0-7}."
> > + *
> > + * This covers all R/O registers that indicate anything useful feature
> > + * wise, including the ID registers.
> > + */
> > +#define ARM64_FEATURE_ID_SPACE_IDX(op0, op1, crn, crm, op2) \
> > + ({ \
> > + __u64 __op1 = (op1) & 3; \
> > + __op1 -= (__op1 == 3); \
> > + (__op1 << 6 | ((crm) & 7) << 3 | (op2)); \
> > + })
> > +
> > +#define ARM64_FEATURE_ID_SPACE_SIZE (3 * 8 * 8)
> > +
> > +struct feature_id_writable_masks {
> > + __u64 mask[ARM64_FEATURE_ID_SPACE_SIZE];
> > +};
> > +
> > #endif
> >
> > #endif /* __ARM_KVM_H__ */
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 72dc53a75d1c..c9cd14057c58 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -1630,6 +1630,9 @@ int kvm_arch_vm_ioctl(struct file *filp, unsigned int ioctl, unsigned long arg)
> >
> > return kvm_vm_set_attr(kvm, &attr);
> > }
> > + case KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS: {
> > + return kvm_vm_ioctl_get_feature_id_writable_masks(kvm, argp);
> > + }
> > default:
> > return -EINVAL;
> > }
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 2ca2973abe66..d9317b640ba5 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -3560,6 +3560,57 @@ int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
> > return write_demux_regids(uindices);
> > }
> >
> > +#define ARM64_FEATURE_ID_SPACE_INDEX(r) \
> > + ARM64_FEATURE_ID_SPACE_IDX(sys_reg_Op0(r), \
> > + sys_reg_Op1(r), \
> > + sys_reg_CRn(r), \
> > + sys_reg_CRm(r), \
> > + sys_reg_Op2(r))
> > +
> > +static bool is_feature_id_reg(u32 encoding)
> > +{
> > + return (sys_reg_Op0(encoding) == 3 &&
> > + (sys_reg_Op1(encoding) < 2 || sys_reg_Op1(encoding) == 3) &&
> > + sys_reg_CRn(encoding) == 0 &&
> > + sys_reg_CRm(encoding) <= 7);
> > +}
>
> A fool question.
>
> In the code base, there is a function is_id_reg() to check if it's a
> id_reg, what's the difference between the is_feature_id_reg() and the
> is_id_reg()?
>
> /*
> * Return true if the register's (Op0, Op1, CRn, CRm, Op2) is
> * (3, 0, 0, crm, op2), where 1<=crm<8, 0<=op2<8.
> */
> static inline bool is_id_reg(u32 id)
> {
> return (sys_reg_Op0(id) == 3 && sys_reg_Op1(id) == 0 &&
> sys_reg_CRn(id) == 0 && sys_reg_CRm(id) >= 1 &&
> sys_reg_CRm(id) < 8);
> }
>
> Thanks,
> Shaoqin
>
Basically, is_feature_id_reg() is used to check whether the reg is in
the feature ID system register space. You can refer to page 6788 at
DDI0487J.a, D19.2.66 for the feature ID space range as below:
The Feature ID space is defined as the System register space in
AArch64 with op0==3, op1=={0,
1, 3}, CRn==0, CRm=={0-7}, op2=={0-7}.
But is_id_reg() is used to check whether the reg is one of the ID_*
regs which have been defined in the previous feature ID space range.
You can refer to the table D18-2 at page D18-6308 for the encoding.
Agreed that the names are somewhat confusing.
> > +
> > +int kvm_vm_ioctl_get_feature_id_writable_masks(struct kvm *kvm, u64 __user *masks)
> > +{
> > + /* Wipe the whole thing first */
> > + for (int i = 0; i < ARM64_FEATURE_ID_SPACE_SIZE; i++)
> > + if (put_user(0, masks + i))
> > + return -EFAULT;
> > +
> > + for (int i = 0; i < ARRAY_SIZE(sys_reg_descs); i++) {
> > + const struct sys_reg_desc *reg = &sys_reg_descs[i];
> > + u32 encoding = reg_to_encoding(reg);
> > + u64 val;
> > +
> > + if (!is_feature_id_reg(encoding) || !reg->set_user)
> > + continue;
> > +
> > + /*
> > + * For ID registers, we return the writable mask. Other feature
> > + * registers return a full 64bit mask. That's not necessary
> > + * compliant with a given revision of the architecture, but the
> > + * RES0/RES1 definitions allow us to do that.
> > + */
> > + if (is_id_reg(encoding)) {
> > + if (!reg->val)
> > + continue;
> > + val = reg->val;
> > + } else {
> > + val = ~0UL;
> > + }
> > +
> > + if (put_user(val, (masks + ARM64_FEATURE_ID_SPACE_INDEX(encoding))))
> > + return -EFAULT;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > int __init kvm_sys_reg_table_init(void)
> > {
> > struct sys_reg_params params;
> > diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> > index f089ab290978..86ffdf134eb8 100644
> > --- a/include/uapi/linux/kvm.h
> > +++ b/include/uapi/linux/kvm.h
> > @@ -1555,6 +1555,8 @@ struct kvm_s390_ucas_mapping {
> > #define KVM_ARM_MTE_COPY_TAGS _IOR(KVMIO, 0xb4, struct kvm_arm_copy_mte_tags)
> > /* Available with KVM_CAP_COUNTER_OFFSET */
> > #define KVM_ARM_SET_COUNTER_OFFSET _IOW(KVMIO, 0xb5, struct kvm_arm_counter_offset)
> > +#define KVM_ARM_GET_FEATURE_ID_WRITABLE_MASKS \
> > + _IOR(KVMIO, 0xb6, struct feature_id_writable_masks)
> >
> > /* ioctl for vm fd */
> > #define KVM_CREATE_DEVICE _IOWR(KVMIO, 0xe0, struct kvm_create_device)
>
> --
> Shaoqin
>
Jing
