linux-kernel - Re: [PATCH net-next v2] net: dsa: mv88e6xxx: extend PTP gettime function to read system clock

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: <20190805174051.GW24275@lunn.ch>
Date:   Mon, 5 Aug 2019 19:40:51 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Hubert Feurstein <h.feurstein@...il.com>
Cc:     netdev <netdev@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        Richard Cochran <richardcochran@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH net-next v2] net: dsa: mv88e6xxx: extend PTP gettime
 function to read system clock

> +static int mv88e6xxx_mdiobus_write_nested(struct mv88e6xxx_chip
> *chip, int addr, u32 regnum, u16 val)
> +{
> +       int err;
> +
> +       BUG_ON(in_interrupt());
> +
> +       mutex_lock_nested(&chip->bus->mdio_lock, MDIO_MUTEX_NESTED);
> +       ptp_read_system_prets(chip->ptp_sts);
> +       err = __mdiobus_write(chip->bus, addr, regnum, val);
> +       ptp_read_system_postts(chip->ptp_sts);
> +       mutex_unlock(&chip->bus->mdio_lock);
> +
> +       return err;
> +}
> +
> static int mv88e6xxx_smi_direct_write(struct mv88e6xxx_chip *chip,
>                                      int dev, int reg, u16 data)
> {
>        int ret;
> 
> -       ret = mdiobus_write_nested_ptp(chip->bus, dev, reg, data,
> chip->ptp_sts);
> +       ret = mv88e6xxx_mdiobus_write_nested(chip, dev, reg, data);
>        if (ret < 0)
>                return ret;
> 
> The result was:
> Min:  -8052
> Max:  9988
> StdDev: 2490.17
> Count: 3592
> 
> It got improved, but you still have the unpredictable latencies caused by the
> mdio_done-completion (=> wait_for_completion_timeout) in imx_fec.

O.K. So lets think about a more generic solution we can use inside the
mdio bus driver. I don't know if adding an sts pointer to struct
device will be accepted. But adding one to struct mii_bus should be
O.K. It can be assigned to once the mdio_lock is taken, to avoid race
conditions. Add mdio_ptp_read_system_prets(bus) and
mdio_ptp_read_system_postts(bus) which the bus driver can use.

We also need a fallback in case the bus driver does not use them, so
something like:

mdiobus_write_sts(...)
{
        int retval;

        BUG_ON(in_interrupt());

        mutex_lock(&bus->mdio_lock);
	bus->sts = sts;
	sts->post_ts = 0;

	ktime_get_real_ts64(&sts->pre_ts);

        retval = __mdiobus_write(bus, addr, regnum, val);

	if (!sts->post_ts)
	   ktime_get_real_ts64(sts->post_ts)

	bus->sts = NULL;
        mutex_unlock(&bus->mdio_lock);

        return retval;
}

So at worse case, we get the time around the whole write operation,
but the MDIO bus driver can overwrite the pre_ts and set post_ts,
using mdio_ptp_read_system_prets(bus) and
mdio_ptp_read_system_postts(bus).

A similar scheme could be implemented to SPI devices, if the SPI
maintainer would accepted a sts pointer in the SPI bus driver
structure.

	Andrew