On Wed, Aug 02, 2023 at 08:55:43AM -0700, Jing Zhang wrote: > > > +#define ARM64_FEATURE_ID_SPACE_SIZE (3 * 8 * 8) > > > + > > > +struct feature_id_writable_masks { > > > + __u64 mask[ARM64_FEATURE_ID_SPACE_SIZE]; > > > +}; > > > > This UAPI is rather difficult to extend in the future. We may need to > > support describing the masks of multiple ranges of registers in the > > future. I was thinking something along the lines of: > > > > enum reg_mask_range_idx { > > FEATURE_ID, > > }; > > > > struct reg_mask_range { > > __u64 idx; > > __u64 *masks; > > __u64 rsvd[6]; > > }; > > > Since have the way to map sysregs encoding to the index in the mask > array, we can extend the UAPI by just adding a size field in struct > feature_id_writable_masks like below: > struct feature_id_writable_masks { > __u64 size; > __u64 mask[ARM64_FEATURE_ID_SPACE_SIZE]; > }; > The 'size' field can be used as input for the size of 'mask' array and > output for the number of masks actually read in. > This way, we can freely add more ranges without breaking anything in userspace. > WDYT? Sorry, 'index' is a bit overloaded in this context. The point I was trying to get across is that we might want to describe a completely different range of registers than the feature ID registers in the future. Nonetheless, we shouldn't even presume the shape of future extensions to the ioctl. struct reg_mask_range { __u64 addr; /* pointer to mask array */ __u64 rsvd[7]; }; Then in KVM we should require ::rsvd be zero and fail the ioctl otherwise. -- Thanks, Oliver