Re: [PATCH] xen/public: partially revert commit 7c7f7e8fba01

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.02.2022 11:36, Juergen Gross wrote:
> Commit 7c7f7e8fba01 changed xen/include/public/memory.h in an incompatible
> way. Unfortunately the changed parts were already in use in the Linux
> kernel, so an update of the header in the kernel would result in a build
> breakage.
> 
> Even when removing its usage from the kernel the used flag bit should be
> marked as reserved in order to avoid to give it a different semantic in
> the future.
> 
> Do a partial revert of said commit in order to enable the kernel to take
> an updated version of memory.h.

I don't think it should be a partial revert, and as said on irc I'm of
the opinion that ...

> Fixes: 7c7f7e8fba01 ("include/public/memory.h: remove the 
> XENMEM_rsrc_acq_caller_owned flag")

... it's 0e2e54966af5 which should have taken measures to protect
against re-use of the flag as an output.

> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -662,7 +662,17 @@ struct xen_mem_acquire_resource {
>       * two calls.
>       */
>      uint32_t nr_frames;
> -    uint32_t pad;
> +
> +    /*
> +     * OUT - Must be zero on entry. On return this may contain a bitwise
> +     *       OR of the following values.
> +     */
> +    uint32_t flags;
> +
> +    /* No longer supported - will be never set */
> +#define _XENMEM_rsrc_acq_caller_owned 0
> +#define XENMEM_rsrc_acq_caller_owned (1u << _XENMEM_rsrc_acq_caller_owned)

I think this goes too far: Neither do we want to re-introduce the
#define-s, nor should we re-fix the purpose of the padding field
to be OUT (only). All we need to make sure is that the field
coming in as zero won't get responded to by setting bit 0 of it.
Imo this can only reasonably be done by way of adding a comment.
This comment may, in turn, mention XENMEM_rsrc_acq_caller_owned
of course.

Btw., if the field was to become OUT-only again, I think you'd
also need to revert the change to xen/common/compat/memory.c. At
least to not leave a trap for someone to later fall into.

Jan