From: "Arnd Bergmann" <arnd@arndb.de>
To: "Thomas Weißschuh" <thomas.weissschuh@linutronix.de>,
"Vinicius Costa Gomes" <vinicius.gomes@intel.com>,
"Dave Jiang" <dave.jiang@intel.com>
Cc: dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] dmaengine: idxd: uapi: use UAPI types
Date: Mon, 22 Dec 2025 10:21:31 +0100 [thread overview]
Message-ID: <a4c50d48-8b5f-4b5f-9a4e-6ea1f9433c0b@app.fastmail.com> (raw)
In-Reply-To: <20251222-uapi-idxd-v1-1-baa183adb20d@linutronix.de>
On Mon, Dec 22, 2025, at 09:04, Thomas Weißschuh wrote:
> Using libc types and headers from the UAPI headers is problematic as it
> introduces a dependency on a full C toolchain.
>
> Use the fixed-width integer types provided by the UAPI headers instead.
>
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
Acked-by: Arnd Bergmann <arnd@arndb.de>
This is certainly an improvement.
Since I'm looking at this file, I'd point out a few other oddities
in the header that have absolutely nothing to do with your patch:
> @@ -176,132 +172,132 @@ enum iax_completion_status {
> #define DSA_COMP_STATUS(status) ((status) & DSA_COMP_STATUS_MASK)
>
> struct dsa_hw_desc {
> - uint32_t pasid:20;
> - uint32_t rsvd:11;
> - uint32_t priv:1;
> - uint32_t flags:24;
> - uint32_t opcode:8;
> - uint64_t completion_addr;
> + __u32 pasid:20;
> + __u32 rsvd:11;
> + __u32 priv:1;
> + __u32 flags:24;
> + __u32 opcode:8;
> + __u64 completion_addr;
Bitfields are usually a bad idea to describe hardware structures
since the actual layout is ABI specific.
> union {
> - uint8_t expected_res;
> + __u8 expected_res;
> /* create delta record */
> struct {
> - uint64_t delta_addr;
> - uint32_t max_delta_size;
> - uint32_t delt_rsvd;
> - uint8_t expected_res_mask;
> + __u64 delta_addr;
> + __u32 max_delta_size;
> + __u32 delt_rsvd;
> + __u8 expected_res_mask;
> };
All the outer structures are marked as __packed, but
the unions and inner structures are not, so this still
ends up with padding, and a -Wpadded warning. This came
up as one of the uapi structures that is incompatible
between x86-64 and compat i386 user space.
This ends up being harmless since other members of the
union are the same 24 byte length. For consistency, I'd
always add explicit padding, and I plan to send a patch
for all uapi structures that need this in the future.
> - uint8_t op_specific[24];
> + __u8 op_specific[24];
> };
> } __attribute__((packed));
The packed attribute here makes the structure unsuitable
for hardware DMA access since the compiler may generate
bytewise access. These should probably all be dropped or
changed into
__attribute__((packed, aligned(8)))
to make the structure itself aligned by the members if
some members need packing.
> * volatile and prevent the compiler from optimize the read.
> */
> struct dsa_completion_record {
> - volatile uint8_t status;
> + volatile __u8 status;
volatile by itself is not really sufficient to do that in
portable code, there should also be a dmb_mb() or similar
around it.
As far as I can tell, the driver cannot ever be used on
anything other than x86, so there is no harm in practice.
Arnd
next prev parent reply other threads:[~2025-12-22 9:21 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-12-22 8:04 Thomas Weißschuh
2025-12-22 9:21 ` Arnd Bergmann [this message]
2025-12-23 11:28 ` Vinod Koul
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=a4c50d48-8b5f-4b5f-9a4e-6ea1f9433c0b@app.fastmail.com \
--to=arnd@arndb.de \
--cc=dave.jiang@intel.com \
--cc=dmaengine@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=thomas.weissschuh@linutronix.de \
--cc=vinicius.gomes@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®