netdev - 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAAVpwAvVO7RGLGMXCBxCD35kKCLmZEkeXuERG0C2GHP54kCGJw@mail.gmail.com>
Date: Wed, 16 Apr 2025 20:27:46 +0200
From: Ivan Vecera <ivecera@...hat.com>
To: Andrew Lunn <andrew@...n.ch>
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

On Wed, Apr 16, 2025 at 7:32 PM Andrew Lunn <andrew@...n.ch> wrote:
>
> > +/**
> > + * zl3073x_mb_dpll_read - read given DPLL configuration to mailbox
> > + * @zldev: pointer to device structure
> > + * @index: DPLL index
> > + *
> > + * Reads configuration of given DPLL into DPLL mailbox.
> > + *
> > + * Context: Process context. Expects zldev->regmap_lock to be held by caller.
> > + * Return: 0 on success, <0 on error
> > + */
> > +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index)
> > +{
> > +     int rc;
>
> lockdep_assert_held(zldev->regmap_lock) is stronger than having a
> comment. When talking about i2c and spi devices, it costs nothing, and
> catches bugs early.

Makes sense to put the assert here...

Will add.

>
> > +/*
> > + * Mailbox operations
> > + */
> > +int zl3073x_mb_dpll_read(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_dpll_write(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_output_read(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_output_write(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_ref_read(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_ref_write(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_synth_read(struct zl3073x_dev *zldev, u8 index);
> > +int zl3073x_mb_synth_write(struct zl3073x_dev *zldev, u8 index);
>
> I assume these are the only valid ways to access a mailbox?
>
> If so:
>
> > +static inline __maybe_unused int
> > +zl3073x_mb_read_ref_mb_mask(struct zl3073x_dev *zldev, u16 *value)
> > +{
> > +     __be16 temp;
> > +     int rc;
> > +
> > +     lockdep_assert_held(&zldev->mailbox_lock);
> > +     rc = regmap_bulk_read(zldev->regmap, ZL_REG_REF_MB_MASK, &temp,
> > +                           sizeof(temp));
> > +     if (rc)
> > +             return rc;
> > +
> > +     *value = be16_to_cpu(temp);
> > +     return rc;
> > +}
>
> These helpers can be made local to the core. You can then drop the
> lockdep_assert_held() from here, since the only way to access them is
> via the API you defined above, and add the checks in those API
> functions.

This cannot be done this way... the above API just simplifies the
operation of read and write latch registers from/to mailbox.

Whole operation is described in the commit description.

E.g. read something about DPLL1
1. Call zl3073x_mb_dpll_read(..., 1)
   This selects DPLL1 in the DPLL mailbox and performs read operation
and waits for finish
2. Call zl3073x_mb_read_dpll_mode()
   This reads dpll_mode latch register

write:
1. Call zl3073x_mb_write_dpll_mode(...)
   This writes mode to dpll_mode latch register
2. Call zl3073x_mb_dpll_read(..., 1)
   This writes all info from latch registers to DPLL1

The point is that between step 1 and 2 nobody else cannot touch
latch_registers or mailbox select register and op semaphore.

Thanks,
Ivan