Re: [PATCH v3 02/22] include/xen/slr-table.h: Secure Launch Resource Table definitions

[Sergii Dmytruk <sergii.dmytruk@xxxxxxxxx>]

On Mon, Jul 07, 2025 at 10:29:46AM +0200, Jan Beulich wrote:
> >> Btw, please don't forget to Cc maintainers of code you're changing / 
> >> adding.
> >
> > What do you mean?  I'm running add_maintainers.pl on the patches.
>
> The Cc: list had none of the REST maintainers. (Whether there's a bug in the
> script I can't tell.)

Oh, looks like adding new entries to MAINTAINERS prevented application
of THE REST.  Will try running the script from master next time to
address this.

> >> ... then isn't used right here, instead requiring a cast somewhere 
> >> (presumably,
> >> as code using this isn't visible in this patch).
> >
> > As was mentioned earlier: because size of a pointer between Xen and a
> > bootloader doesn't necessarily match.  What you're proposing can break
> > under certain conditions.
>
> But the header isn't shared with a bootloader's code base. It's private to
> Xen.

Yes, but sources of Xen be compiled with different size of a pointer
which messes up the interpretation of the data.  I tried using
BUILD_BUG_ON() to enforce the pointer is 64-bit and early code stopped
compiling.  The structures must not vary like that.

> >>> +} __packed;
> >>
> >> I similarly keep wondering why there are all these packed attributes here, 
> >> when
> >> (afaics) all of the structures are defined in a way that any padding is 
> >> explicit
> >> anyway.
> >
> > As before: it won't hurt, it's future-proof, just in case and to let
> > reader of the code know the structure must have no padding.  If you want
> > them gone, I can do that, but I think it will make the code harder to
> > maintain.
>
> Well, if there's a maintenance concern, then I would of course like to learn
> about that. I could see such being the case if the file was imported from
> somewhere. But with neither description nor any code comment not saying so,
> that doesn't look to be the case.
>
> Jan

While there is some synchronization to do, that's not what I meant, I
don't think it warrants writing the header in a special way.  __packed
just relieves anyone from checking for padding, while just to remove the
attribute one needs to go through the structures and make sure nothing
will change.

Regard