Re: [RFC PATCH 1/6] xen/domctl: Refine grant_opts into grant_version

["Alejandro Vallejo" <alejandro.vallejo@xxxxxxxxx>]

Hi,

On Wed Oct 30, 2024 at 9:08 AM GMT, Jan Beulich wrote:
> On 29.10.2024 19:16, Alejandro Vallejo wrote:
> > grant_opts is overoptimizing for space packing in a hypercall that
> > doesn't warrant the effort. Tweak the ABI without breaking it in order
> > to remove the bitfield by extending it to 8 bits.
> > 
> > Xen only supports little-endian systems, so the transformation from
> > uint32_t to uint8_t followed by 3 octets worth of padding is not an ABI
> > breakage.
> > 
> > No functional change
> > 
> > Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>
> > ---
> >  xen/include/public/domctl.h | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
>
> This isn't a complete patch, is it? I expect it'll break the build without
> users of the field also adjusted.

Indeed. The non-RFC version would have everything folded in one. I just wanted
to avoid Cc-ing everyone in MAINTAINERS for the same single RFC patch. It's
split by (rough) maintained area.

>
> > --- a/xen/include/public/domctl.h
> > +++ b/xen/include/public/domctl.h
> > @@ -90,11 +90,18 @@ struct xen_domctl_createdomain {
> >      int32_t max_grant_frames;
> >      int32_t max_maptrack_frames;
> >  
> > -/* Grant version, use low 4 bits. */
> > -#define XEN_DOMCTL_GRANT_version_mask    0xf
> > -#define XEN_DOMCTL_GRANT_version(v)      ((v) & 
> > XEN_DOMCTL_GRANT_version_mask)
> > +    /*
> > +     * Maximum grant table version the domain can be configured with.
> > +     *
> > +     * Domains always start with v1 (if CONFIG_GRANT_TABLE) and can be 
> > bumped
> > +     * to use up to `max_grant_version` via GNTTABOP_set_version.
> > +     *
> > +     * Must be zero iff !CONFIG_GRANT_TABLE.
> > +     */
> > +    uint8_t max_grant_version;
> >  
> > -    uint32_t grant_opts;
> > +    /* Unused */
> > +    uint8_t rsvd0[3];
> >  
> >  /*
> >   * Enable altp2m mixed mode.
>
> Just to mention it: I think while binary compatible, this is still on the edge
> of needing an interface version bump. We may get away without as users of the
> removed identifiers will still notice by way of observing build failures.
>
> Jan

If users are forced to rebuild either way, might as well prevent existing
binaries from breaking. There ought to be a strict distinction between ABI and
API compatibility because, while they typically move in lockstep, they don't
always (and this is one such an example).

Regardless, this is a discussion for the final patch if we get there.

Cheers,
Alejandro