linux-kernel - Re: [PATCH linux v3 3/6] hwmon: occ: Add I2C transport implementation for SCOM operations

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]

Date:   Sun, 22 Jan 2017 07:53:17 +1100
From:   Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:     Guenter Roeck <linux@...ck-us.net>, eajames.ibm@...il.com
Cc:     devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-hwmon@...r.kernel.org, linux-doc@...r.kernel.org,
        jdelvare@...e.com, corbet@....net, mark.rutland@....com,
        robh+dt@...nel.org, wsa@...-dreams.de, andrew@...id.au,
        joel@....id.au, "Edward A. James" <eajames@...ibm.com>
Subject: Re: [PATCH linux v3 3/6] hwmon: occ: Add I2C transport
 implementation for SCOM operations

On Sat, 2017-01-21 at 10:11 -0800, Guenter Roeck wrote:
> > +int occ_i2c_getscom(void *bus, u32 address, u64 *data)
> > +{
> > +     ssize_t rc;
> > +     u64 buf;
> > +     struct i2c_client *client = bus;
> > +
> > +     rc = i2c_master_send(client, (const char *)&address,
> > sizeof(u32));
> > +     if (rc < 0)
> > +             return rc;
> > +     else if (rc != sizeof(u32))
> > +             return -EIO;
> > +
> > +     rc = i2c_master_recv(client, (char *)&buf, sizeof(u64));
> > +     if (rc < 0)
> > +             return rc;
> > +     else if (rc != sizeof(u64))
> > +             return -EIO;
> > +
> > +     *data = be64_to_cpu(buf);
> > +
> > +     return 0;
> > +}
> > +EXPORT_SYMBOL(occ_i2c_getscom);

Additionally, this assumes that is is the only other user of that i2c
path to P8. Something interleaving will break it. pdbg for example.

Talk to Alistair, the right thing here would probably be to have
a separate driver that provides the XSCOM interface via i2c to both
in-kernel and userspace, with proper arbitration.

An expedient might be to instead have the address and data cycles
of the above be 2 i2c messages in one i2c request, thus being
serialized somewhat atomically with other transactions to that i2c
bus.

You may need to make sure the underlying i2c controller driver supports
dual messages, and if it doesn't fix it. This is rather classic 4-bytes 
subaddress style i2c, it shouldn't be too hard.

Cheers,
Ben.