linux-kernel - Re: [PATCH 03/14] mailbox: pcc: Drop unnecessary endianness conversion of pcc

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: <20250305103435.nl5q2uvyqftal7en@bogus>
Date: Wed, 5 Mar 2025 10:34:35 +0000
From: Sudeep Holla <sudeep.holla@....com>
To: "lihuisong (C)" <lihuisong@...wei.com>
Cc: linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	Jassi Brar <jassisinghbrar@...il.com>,
	Sudeep Holla <sudeep.holla@....com>,
	Adam Young <admiyo@...amperecomputing.com>
Subject: Re: [PATCH 03/14] mailbox: pcc: Drop unnecessary endianness
 conversion of pcc_hdr.flags

On Wed, Mar 05, 2025 at 12:02:13PM +0800, lihuisong (C) wrote:
> 
> 在 2025/3/3 18:51, Sudeep Holla 写道:
> > The Sparse static checker flags a type mismatch warning related to
> > endianness conversion:
> > 
> >    |  warning: incorrect type in argument 1 (different base types)
> >    |     expected restricted __le32 const [usertype] *p
> >    |     got unsigned int *
> > 
> > This is because an explicit endianness conversion (le32_to_cpu()) was
> > applied unnecessarily to a pcc_hdr.flags field that is already in
> > little-endian format.
> > 
> > The PCC driver is only enabled on little-endian kernels due to its
> > dependency on ACPI and EFI, making the explicit conversion unnecessary.
> How to confirm ACPI works only on little-endian?
> > 
> > The redundant conversion occurs in pcc_chan_check_and_ack() for the
> > pcc_hdr.flags field. Drop this unnecessary endianness conversion of
> > pcc_hdr.flags.
> > 
> > Signed-off-by: Sudeep Holla <sudeep.holla@....com>
> > ---
> >   drivers/mailbox/pcc.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
> > index 4c582fa2b8bf4c9a9368dba8220f567555dba963..c87a5b7fa6eaf7bcabe0d55f844961c499376938 100644
> > --- a/drivers/mailbox/pcc.c
> > +++ b/drivers/mailbox/pcc.c
> > @@ -292,7 +292,7 @@ static void check_and_ack(struct pcc_chan_info *pchan, struct mbox_chan *chan)
> >   	 *
> >   	 * The PCC master subspace channel clears chan_in_use to free channel.
> >   	 */
> > -	if (le32_to_cpup(&pcc_hdr.flags) & PCC_ACK_FLAG_MASK)
> > +	if (pcc_hdr.flags & PCC_ACK_FLAG_MASK)
> It's recommanded to delete PCC_ACK_FLAG_MASK and use
> PCC_CMD_COMPLETION_NOTIFY.
> They are from the same place, namely, 'Initiator Responder Communications
> Channel Flags'.

Good point, I will use PCC_CMD_COMPLETION_NOTIFY and drop even the definition
of PCC_ACK_FLAG_MASK as it got added for no good reason.

--
Regards,
Sudeep