| Message ID | 20220110214755.810343-1-venture@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Adds designware i2c module and adds it to virt arm | expand |
On Mon, 10 Jan 2022 at 21:47, Patrick Venture <venture@google.com> wrote: > > This patch series introduces a new i2c module, namely the designware one and further enables this (optionally) for the virt-arm machine. > > Chris Rauer (2): > hw/i2c: Add designware i2c controller. > hw/arm: Enable smbus on arm virt machine. I need to see a pretty strong justification for why we should be adding new kinds of devices to the virt machine, given that it increases complexity and potential attack surface for using it with KVM; this cover letter doesn't seem to provide any... thanks -- PMM
On Thu, 13 Jan 2022 at 11:48, Peter Maydell <peter.maydell@linaro.org> wrote: > > On Mon, 10 Jan 2022 at 21:47, Patrick Venture <venture@google.com> wrote: > > > > This patch series introduces a new i2c module, namely the designware one and further enables this (optionally) for the virt-arm machine. > > > > Chris Rauer (2): > > hw/i2c: Add designware i2c controller. > > hw/arm: Enable smbus on arm virt machine. > > I need to see a pretty strong justification for why we > should be adding new kinds of devices to the virt machine, > given that it increases complexity and potential attack > surface for using it with KVM; this cover letter doesn't > seem to provide any... Forgot to mention, but my prefered approach for providing an i2c controller on the virt board would be to have a PCI i2c controller: that way users who do need it can plug it in with a -device command line option, and users who don't need it never have to worry about it. (We seem to have an ICH9-SMB PCI device already; I have no idea if it's suitable.) thanks -- PMM
>I need to see a pretty strong justification for why we should be adding new kinds of devices to the virt machine, The designware i2c controller is a very common controller on many ARM SoCs. It has device tree bindings and ACPI bindings which makes it ideal for this platform. >Forgot to mention, but my prefered approach for providing >an i2c controller on the virt board would be to have a >PCI i2c controller: that way users who do need it can plug it >in with a -device command line option, and users who don't >need it never have to worry about it. The device is enabled by a machine parameter, “-machine virt,smbus=true”, and is disabled by default. > (We seem to have an ICH9-SMB PCI device already; I have no idea if it's suitable.) I didn't find that device suitable because it is part of the Intel Southbridge, which may have some Intel platform quirks, and we don't need all the things in that IO hub. -Chris
On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote: > >> I need to see a pretty strong justification for why we should be >> adding new kinds of devices to the virt machine, > > The designware i2c controller is a very common controller on many > ARM SoCs. It has device tree bindings and ACPI bindings which > makes it ideal for this platform. No, I mean, why do we need an i2c controller on the virt board at all ? > >Forgot to mention, but my prefered approach for providing > >an i2c controller on the virt board would be to have a > >PCI i2c controller: that way users who do need it can plug it > >in with a -device command line option, and users who don't > >need it never have to worry about it. > > (We seem to have an ICH9-SMB PCI device already; I have no idea if it's suitable.) > I didn't find that device suitable because it is part of the Intel > Southbridge, which may have some Intel platform quirks, and > we don't need all the things in that IO hub. That's a pity. Is there a different PCI I2C controller we could model ? thanks -- PMM
>>> I need to see a pretty strong justification for why we should be >>> adding new kinds of devices to the virt machine, >> >> The designware i2c controller is a very common controller on many >> ARM SoCs. It has device tree bindings and ACPI bindings which >> makes it ideal for this platform. > >No, I mean, why do we need an i2c controller on the virt >board at all ? The use case we are interested in is to enable IPMI SSIF on the arm virt machine which is why I was interested in something with ACPI support. That particular IPMI SSIF enablement patch is not in this series but I think it could be pulled into the series if necessary. > > >Forgot to mention, but my prefered approach for providing > > >an i2c controller on the virt board would be to have a > > >PCI i2c controller: that way users who do need it can plug it > > >in with a -device command line option, and users who don't > > >need it never have to worry about it. > > > > (We seem to have an ICH9-SMB PCI device already; I have no idea if it's suitable.) > > I didn't find that device suitable because it is part of the Intel > > Southbridge, which may have some Intel platform quirks, and > > we don't need all the things in that IO hub. > > That's a pity. Is there a different PCI I2C controller we could model ? I’m not aware of any standalone PCI I2C controllers. I’ve seen I2C controllers on PCI devices with other things but I don’t think those could be used for IPMI SSIF or other general I2C use cases. Do you know of a particular device I should take a look at? -Chris On Wed, Jan 26, 2022 at 10:03 AM Peter Maydell <peter.maydell@linaro.org> wrote: > On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote: > > > >> I need to see a pretty strong justification for why we should be > >> adding new kinds of devices to the virt machine, > > > > The designware i2c controller is a very common controller on many > > ARM SoCs. It has device tree bindings and ACPI bindings which > > makes it ideal for this platform. > > No, I mean, why do we need an i2c controller on the virt > board at all ? > > > >Forgot to mention, but my prefered approach for providing > > >an i2c controller on the virt board would be to have a > > >PCI i2c controller: that way users who do need it can plug it > > >in with a -device command line option, and users who don't > > >need it never have to worry about it. > > > > (We seem to have an ICH9-SMB PCI device already; I have no idea if > it's suitable.) > > I didn't find that device suitable because it is part of the Intel > > Southbridge, which may have some Intel platform quirks, and > > we don't need all the things in that IO hub. > > That's a pity. Is there a different PCI I2C controller we could model ? > > thanks > -- PMM >
+Enrico Weigelt On 26/1/22 19:03, Peter Maydell wrote: > On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote: >> >>> I need to see a pretty strong justification for why we should be >>> adding new kinds of devices to the virt machine, >> >> The designware i2c controller is a very common controller on many >> ARM SoCs. It has device tree bindings and ACPI bindings which >> makes it ideal for this platform. > > No, I mean, why do we need an i2c controller on the virt > board at all ? > >>> Forgot to mention, but my prefered approach for providing >>> an i2c controller on the virt board would be to have a >>> PCI i2c controller: that way users who do need it can plug it >>> in with a -device command line option, and users who don't >>> need it never have to worry about it. > >>> (We seem to have an ICH9-SMB PCI device already; I have no idea if it's suitable.) >> I didn't find that device suitable because it is part of the Intel >> Southbridge, which may have some Intel platform quirks, and >> we don't need all the things in that IO hub. > > That's a pity. Is there a different PCI I2C controller we could model ? What about using virtio-gpio & bitbang I2C? - virtio-gpio https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/ - bitbang I2C already in: hw/i2c/bitbang_i2c.c Regards, Phil.
Hi Phil, > What about using virtio-gpio & bitbang I2C? > > - virtio-gpio > https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/ > > - bitbang I2C already in: hw/i2c/bitbang_i2c.c Sorry for the delay. That looks like it might be doable with a bit more work creating the ACPI entries for the bitbang I2C. This definitely seems more appropriate on the ARM_VIRT platform instead of putting a specific controller in. For my uses, I will have to stick with the designware controller since it matches the system I'm emulating a little more closely. We can hold back the designware stuff until another SoC platform is interested in using this controller (since it seems like it is a common one). Hopefully someone will find another use for the controller patches someday. Thanks again for looking at our patches. -Chris On Wed, Jan 26, 2022 at 3:42 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote: > +Enrico Weigelt > > On 26/1/22 19:03, Peter Maydell wrote: > > On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote: > >> > >>> I need to see a pretty strong justification for why we should be > >>> adding new kinds of devices to the virt machine, > >> > >> The designware i2c controller is a very common controller on many > >> ARM SoCs. It has device tree bindings and ACPI bindings which > >> makes it ideal for this platform. > > > > No, I mean, why do we need an i2c controller on the virt > > board at all ? > > > >>> Forgot to mention, but my prefered approach for providing > >>> an i2c controller on the virt board would be to have a > >>> PCI i2c controller: that way users who do need it can plug it > >>> in with a -device command line option, and users who don't > >>> need it never have to worry about it. > > > >>> (We seem to have an ICH9-SMB PCI device already; I have no idea if > it's suitable.) > >> I didn't find that device suitable because it is part of the Intel > >> Southbridge, which may have some Intel platform quirks, and > >> we don't need all the things in that IO hub. > > > > That's a pity. Is there a different PCI I2C controller we could model ? > > What about using virtio-gpio & bitbang I2C? > > - virtio-gpio > https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/ > > - bitbang I2C already in: hw/i2c/bitbang_i2c.c > > Regards, > > Phil. >
On Mon, Feb 21, 2022 at 09:47:27AM -0800, Chris Rauer wrote: > Hi Phil, > > > What about using virtio-gpio & bitbang I2C? > > > > - virtio-gpio > > https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/ > > > > - bitbang I2C already in: hw/i2c/bitbang_i2c.c > Sorry for the delay. > > That looks like it might be doable with a bit more work creating the ACPI > entries for the bitbang I2C. This definitely seems more appropriate on the > ARM_VIRT platform instead of putting a specific controller in. I would think the efficiency of this would be horrible. > > For my uses, I will have to stick with the designware controller since it > matches the system I'm emulating a little more closely. We can hold back > the designware stuff until another SoC platform is interested in using this > controller (since it seems like it is a common one). Hopefully someone > will find another use for the controller patches someday. I should have chimed in sooner. The i.MX i2c device has ACPI and OF support, I believe, and it's already available in qemu. I don't think the Intel smbus device would work. The designware one is a pretty common and general one, it's a little suprising that it hasn't already been done on qemu. I would be ok with this but Peter has the big picture here. -corey > > Thanks again for looking at our patches. > > -Chris > > > On Wed, Jan 26, 2022 at 3:42 PM Philippe Mathieu-Daudé <f4bug@amsat.org> > wrote: > > > +Enrico Weigelt > > > > On 26/1/22 19:03, Peter Maydell wrote: > > > On Wed, 26 Jan 2022 at 17:12, Chris Rauer <crauer@google.com> wrote: > > >> > > >>> I need to see a pretty strong justification for why we should be > > >>> adding new kinds of devices to the virt machine, > > >> > > >> The designware i2c controller is a very common controller on many > > >> ARM SoCs. It has device tree bindings and ACPI bindings which > > >> makes it ideal for this platform. > > > > > > No, I mean, why do we need an i2c controller on the virt > > > board at all ? > > > > > >>> Forgot to mention, but my prefered approach for providing > > >>> an i2c controller on the virt board would be to have a > > >>> PCI i2c controller: that way users who do need it can plug it > > >>> in with a -device command line option, and users who don't > > >>> need it never have to worry about it. > > > > > >>> (We seem to have an ICH9-SMB PCI device already; I have no idea if > > it's suitable.) > > >> I didn't find that device suitable because it is part of the Intel > > >> Southbridge, which may have some Intel platform quirks, and > > >> we don't need all the things in that IO hub. > > > > > > That's a pity. Is there a different PCI I2C controller we could model ? > > > > What about using virtio-gpio & bitbang I2C? > > > > - virtio-gpio > > https://lore.kernel.org/qemu-devel/20201127182917.2387-5-info@metux.net/ > > > > - bitbang I2C already in: hw/i2c/bitbang_i2c.c > > > > Regards, > > > > Phil. > >