linux-kernel - Re: [PATCH v7] i2c: virtio: add a virtio i2c frontend driver

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: <CAK8P3a0zQG3pH91emqAUsaRx4AZeuOEvSrPzXP9w_XhjU6w3ng@mail.gmail.com>
Date:   Fri, 12 Mar 2021 09:58:08 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Jie Deng <jie.deng@...el.com>
Cc:     Linux I2C <linux-i2c@...r.kernel.org>,
        virtualization@...ts.linux-foundation.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Michael S. Tsirkin" <mst@...hat.com>,
        Wolfram Sang <wsa@...nel.org>,
        Jason Wang <jasowang@...hat.com>,
        Wolfram Sang <wsa+renesas@...g-engineering.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        conghui.chen@...el.com, kblaiech@...lanox.com,
        jarkko.nikula@...ux.intel.com,
        Sergey Semin <Sergey.Semin@...kalelectronics.ru>,
        Mike Rapoport <rppt@...nel.org>, loic.poulain@...aro.org,
        Tali Perry <tali.perry1@...il.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        yu1.wang@...el.com, shuo.a.liu@...el.com,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Stefan Hajnoczi <stefanha@...hat.com>,
        Paolo Bonzini <pbonzini@...hat.com>
Subject: Re: [PATCH v7] i2c: virtio: add a virtio i2c frontend driver

On Fri, Mar 12, 2021 at 2:33 PM Jie Deng <jie.deng@...el.com> wrote:

> +
> +/**
> + * struct virtio_i2c_req - the virtio I2C request structure
> + * @out_hdr: the OUT header of the virtio I2C message
> + * @buf: the buffer into which data is read, or from which it's written
> + * @in_hdr: the IN header of the virtio I2C message
> + */
> +struct virtio_i2c_req {
> +       struct virtio_i2c_out_hdr out_hdr;
> +       uint8_t *buf;
> +       struct virtio_i2c_in_hdr in_hdr;
> +};

The simpler request structure clearly looks better than the previous version,
but I think I found another problem here, at least a theoretical one:

When you map the headers into the DMA address space, they should
be in separate cache lines, to allow the DMA mapping interfaces to
perform cache management on each one without accidentally clobbering
another member.

So far I think there is an assumption that virtio buffers are always
on cache-coherent devices, but if you ever have a virtio-i2c device
backend on a physical interconnect that is not cache coherent (e.g. a
microcontroller that shares the memory bus), this breaks down.

You could avoid this by either allocating arrays of each type separately,
or by marking each member that you pass to the device as
____cacheline_aligned.

      Arnd