linux-kernel - Re: [PATCH net-next v2] amd-xgbe: Add PPS periodic output support

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] [day] [month] [year] [list]

Message-ID: <20250825170429.0f08fa1e@kernel.org>
Date: Mon, 25 Aug 2025 17:04:29 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Raju Rangoju <Raju.Rangoju@....com>
Cc: <netdev@...r.kernel.org>, <andrew+netdev@...n.ch>,
 <davem@...emloft.net>, <edumazet@...gle.com>, <pabeni@...hat.com>,
 <richardcochran@...il.com>, <linux-kernel@...r.kernel.org>,
 <Shyam-sundar.S-k@....com>
Subject: Re: [PATCH net-next v2] amd-xgbe: Add PPS periodic output support

On Fri, 22 Aug 2025 16:08:31 +0530 Raju Rangoju wrote:
> +#define PPSx_MASK(x) ({						\
> +	unsigned int __x = (x);					\
> +	GENMASK(PPS_MAXIDX(__x), PPS_MINIDX(__x));		\
> +})
> +#define PPSCMDx(x, val) ({					\
> +	unsigned int __x = (x);					\
> +	GENMASK(PPS_MINIDX(__x) + 3, PPS_MINIDX(__x)) &		\
> +	((val) << PPS_MINIDX(__x));				\
> +})
> +#define TRGTMODSELx(x, val) ({					\
> +	unsigned int __x = (x);					\
> +	GENMASK(PPS_MAXIDX(__x) - 1, PPS_MAXIDX(__x) - 2) &	\
> +	((val) << (PPS_MAXIDX(__x) - 2));			\
> +})

These macros are way too gnarly, please simplify them.
For a start I'm not sure why you're making your life harder and try 
to have a shifted mask. Instead of masking the value before shifting.

>  static int xgbe_enable(struct ptp_clock_info *info,
>  		       struct ptp_clock_request *request, int on)
>  {
> -	return -EOPNOTSUPP;
> +	struct xgbe_prv_data *pdata = container_of(info, struct xgbe_prv_data,
> +						   ptp_clock_info);
> +	struct xgbe_pps_config *pps_cfg;
> +	unsigned long flags;
> +	int ret;
> +
> +	dev_dbg(pdata->dev, "rq->type %d on %d\n", request->type, on);
> +
> +	if (request->type != PTP_CLK_REQ_PEROUT)
> +		return -EOPNOTSUPP;
> +
> +	/* Reject requests with unsupported flags */
> +	if (request->perout.flags)
> +		return -EOPNOTSUPP;

Are you sure kernel can actually send you any flags here?
ops->supported_perout_flags exists

> +	/* Validate index against our limit */
> +	if (request->perout.index >= XGBE_MAX_PPS_OUT)
> +		return -EINVAL;

Are you sure kernel can send you an index higher than what driver
registered as supported?
-- 
pw-bot: cr