linux-hardening - Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with register mailboxes

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: <09c3730a-f6f1-4226-ae29-fe02b1663fe7@lunn.ch>
Date: Thu, 17 Apr 2025 15:27:44 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Ivan Vecera <ivecera@...hat.com>
Cc: netdev@...r.kernel.org, Vadim Fedorenko <vadim.fedorenko@...ux.dev>,
	Arkadiusz Kubalewski <arkadiusz.kubalewski@...el.com>,
	Jiri Pirko <jiri@...nulli.us>, Rob Herring <robh@...nel.org>,
	Krzysztof Kozlowski <krzk+dt@...nel.org>,
	Conor Dooley <conor+dt@...nel.org>,
	Prathosh Satish <Prathosh.Satish@...rochip.com>,
	Lee Jones <lee@...nel.org>, Kees Cook <kees@...nel.org>,
	Andy Shevchenko <andy@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Michal Schmidt <mschmidt@...hat.com>, devicetree@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v3 net-next 5/8] mfd: zl3073x: Add functions to work with
 register mailboxes

> Anyway, I have a different idea... completely abstract mailboxes from the
> caller. The mailbox content can be large and the caller is barely interested
> in all registers from the mailbox but this could be resolved this way:
> 
> The proposed API e.g for Ref mailbox:
> 
> int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index,
>                         struct zl3073x_mb_ref *mb);
> int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index,
>                          struct zl3073x_mb_ref *mb);
> 
> struct zl3073x_mb_ref {
> 	u32	flags;
> 	u16	freq_base;
> 	u16	freq_mult;
> 	u16	ratio_m;
> 	u16	ratio_n;
> 	u8	config;
> 	u64	phase_offset_compensation;
> 	u8	sync_ctrl;
> 	u32	esync_div;
> }
> 
> #define ZL3073X_MB_REF_FREQ_BASE			BIT(0)
> #define ZL3073X_MB_REF_FREQ_MULT			BIT(1)
> #define ZL3073X_MB_REF_RATIO_M				BIT(2)
> #define ZL3073X_MB_REF_RATIO_N			 	BIT(3)
> #define ZL3073X_MB_REF_CONFIG			 	BIT(4)
> #define ZL3073X_MB_REF_PHASE_OFFSET_COMPENSATION 	BIT(5)
> #define ZL3073X_MB_REF_SYNC_CTRL			BIT(6)
> #define ZL3073X_MB_REF_ESYNC_DIV			BIT(7)
> 
> Then a reader can read this way (read freq and ratio of 3rd ref):
> {
> 	struct zl3073x_mb_ref mb;
> 	...
> 	mb.flags = ZL3073X_MB_REF_FREQ_BASE |
> 		   ZL3073X_MB_REF_FREQ_MULT |
> 		   ZL3073X_MB_REF_RATIO_M |
> 		   ZL3073X_MB_REF_RATIO_N;
> 	rc = zl3073x_mb_ref_read(zldev, 3, &mb);
> 	if (rc)
> 		return rc;
> 	/* at this point mb fields requested via flags are filled */
> }
> A writer similarly (write config of 5th ref):
> {
> 	struct zl3073x_mb_ref mb;
> 	...
> 	mb.flags = ZL3073X_MB_REF_CONFIG;
> 	mb.config = FIELD_PREP(SOME_MASK, SOME_VALUE);
> 	rc = zl3073x_mb_ref_write(zldev, 5, &mb);
> 	...
> 	/* config of 5th ref was commited */
> }
> 
> The advantages:
> * no explicit locking required from the callers
> * locking is done inside mailbox API
> * each mailbox type can have different mutex so multiple calls for
>   different mailbox types (e.g ref & output) can be done in parallel
> 
> WDYT about this approach?

I would say this is actually your next layer on top of the basic
mailbox API. This makes it more friendly to your sub driver and puts
all the locking in one place where it can easily be reviewed.

One question would be, where does this code belong. Is it in the MFD,
or in the subdrivers? I guess it is in the subdrivers.

	Andrew