mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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

  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®