On Wed, Aug 02 2023, Oliver Upton <oliver.upton@xxxxxxxxx> wrote: > 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. [I assume rsvd == reserved? I think I have tried to divine further meaning into this for far too long...] Is the idea here for userspace the request a mask array for FEATURE_ID and future ranges separately instead of getting all id-type regs in one go?