On Sat, Dec 01, 2018 at 12:22:17AM +0000, Chris Co wrote:
> > If using EFI_ACPI prefix, these #defines really should be in edk2 MdePkg.
> > And
> > CSRT itself is, so that might not be a bad idea.
> >
> > > +
> > > +#pragma pack(push, 1)
> >
> > I don't see this #pragma making any difference to the structs below, can it
> > be
> > dropped?
>
> The pragma pack is defensive. Without it, we rely on the compiler
> packing structures by default and this may not happen on 64 bit
> compiles.
I understand that is what the pragma does. My comment was because all
of the variables in the below structs are naturally aligned.
The reason I dislike its use when effectively a no-op, is that it
makes it look like it it isn't a no-op.
If it covers a larger set of structs, some of which require the packed
attribute I'm OK with that. But I'm not a fan of adding it "just in
case" without contemplating the statement's (lack of) effect.
Regards,
Leif
> I have addressed the remaining feedback and will resubmit with v2.
>
> Thanks,
> Chris
>
> > > +//-------------------------------------------------------------------
> > > +----- // CSRT Resource Group header 24 bytes long
> > > +//-------------------------------------------------------------------
> > > +-----
> > > +typedef struct {
> > > + UINT32 Length;
> > > + UINT32 VendorID;
> > > + UINT32 SubVendorId;
> > > + UINT16 DeviceId;
> > > + UINT16 SubdeviceId;
> > > + UINT16 Revision;
> > > + UINT16 Reserved;
> > > + UINT32 SharedInfoLength;
> > > +} EFI_ACPI_5_0_CSRT_RESOURCE_GROUP_HEADER;
> > > +
> > > +//-------------------------------------------------------------------
> > > +----- // CSRT Resource Descriptor 12 bytes total
> > > +//-------------------------------------------------------------------
> > > +-----
> > > +typedef struct {
> > > + UINT32 Length;
> > > + UINT16 ResourceType;
> > > + UINT16 ResourceSubType;
> > > + UINT32 UID;
> > > +} EFI_ACPI_5_0_CSRT_RESOURCE_DESCRIPTOR_HEADER;
> > > +#pragma pack (pop)
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
