Re: [PATCH] public: xen: Define missing guest handle for int32_t

[Julien Grall <julien@xxxxxxx>]

Hi Stefano,

On 17/04/2024 19:49, Stefano Stabellini wrote:
> On Wed, 17 Apr 2024, Julien Grall wrote:
> > Hi Michal,
> >
> > On 17/04/2024 13:14, Michal Orzel wrote:
> > > Commit afab29d0882f ("public: s/int/int32_t") replaced int with int32_t
> > > in XEN_GUEST_HANDLE() in memory.h but there is no guest handle defined
> > > for it. This results in a build failure. Example on Arm:
> > >
> > > ./include/public/arch-arm.h:205:41: error: unknown type name
> > > ‘__guest_handle_64_int32_t’
> > >     205 | #define __XEN_GUEST_HANDLE(name)        __guest_handle_64_ ## name
> > >         |                                         ^~~~~~~~~~~~~~~~~~
> > > ./include/public/arch-arm.h:206:41: note: in expansion of macro
> > > ‘__XEN_GUEST_HANDLE’
> > >     206 | #define XEN_GUEST_HANDLE(name)          __XEN_GUEST_HANDLE(name)
> > >         |                                         ^~~~~~~~~~~~~~~~~~
> > > ./include/public/memory.h:277:5: note: in expansion of macro
> > > ‘XEN_GUEST_HANDLE’
> > >     277 |     XEN_GUEST_HANDLE(int32_t) errs;
> > >
> > > Fix it. Also, drop guest handle definition for int given no further use.
> > >
> > > Fixes: afab29d0882f ("public: s/int/int32_t")
> > > Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>
>
> > So it turned out that I committed v1 from Stefano. I was meant to commit the
> > patch at all, but I think I started with a dirty staging :(. Sorry for that.
> >
> > I have reverted Stefano's commit for now so we can take the correct patch.
> >
> > Now, from my understanding, Andrew suggested on Matrix that this solution may
> > actually be a good way to handle GUEST_HANLDEs (they were removed in v2).
> > Maybe this can be folded in Stefano's patch?
> v1 together with Michal's fix is correct. Also v2 alone is correct, or
> v2 with Michal's fix is also correct.
I am slightly confused, v2 + Michal's fix means that 
XEN_GUEST_HANDLE(int) is removed and we introduce XEN_GUEST_INT(int32_t) 
with no user. So wouldn't this break the build?
> My preference is v2 with Michal's fix, they can be committed as separate
> patches. Also the others options are fine.
I am fine if you want to commit them separately. However, I am not sure 
your suggestion about using v2 + Michal's fix is actually correct.
Cheers,

--
Julien Grall