netdev - Re: [RFC PATCH fwctl 3/5] pds_fwctl: initial driver framework

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250214005520.GE3886819@nvidia.com>
Date: Thu, 13 Feb 2025 20:55:20 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: "Nelson, Shannon" <shannon.nelson@....com>
Cc: Jonathan Cameron <Jonathan.Cameron@...wei.com>,
	andrew.gospodarek@...adcom.com, aron.silverton@...cle.com,
	dan.j.williams@...el.com, daniel.vetter@...ll.ch,
	dave.jiang@...el.com, dsahern@...nel.org, gospo@...adcom.com,
	hch@...radead.org, itayavr@...dia.com, jiri@...dia.com,
	kuba@...nel.org, lbloch@...dia.com, leonro@...dia.com,
	saeedm@...dia.com, linux-cxl@...r.kernel.org,
	linux-rdma@...r.kernel.org, netdev@...r.kernel.org,
	brett.creeley@....com
Subject: Re: [RFC PATCH fwctl 3/5] pds_fwctl: initial driver framework

On Thu, Feb 13, 2025 at 03:06:14PM -0800, Nelson, Shannon wrote:

> > > +/**
> > > + * struct pds_fwctl_cmd - Firmware control command structure
> > > + * @opcode: Opcode
> > > + * @rsvd:   Word boundary padding
> > > + * @ep:     Endpoint identifier.
> > > + * @op:     Operation identifier.
> > > + */
> > > +struct pds_fwctl_cmd {
> > > +     u8     opcode;
> > > +     u8     rsvd[3];
> > > +     __le32 ep;
> > > +     __le32 op;
> > > +} __packed;
> > None of these actually need to be packed given explicit padding to
> > natural alignment of all fields.  Arguably it does no harm though
> > so up to you.
> 
> Old belt-and-suspenders habits...

In that case it is worth knowing that __packed also changes the
assumed alignment of the struct. You can access a __packed struct at
any byte.

On x86 this is mostly meaningless, but on other arches it can effect
code generation as the compiler will have to assume that, say, a 64
bit load is not naturally aligned and emit a more expensive sequence
to load it.

Which is why you occasionally see things like:
  __attribute__ ((packed,aligned(8)));

Which says that there is no padding inside the struct, but also that
the compiler can assume a guaranteed starting alignment for the
memory.

Jason