Message ID | 10442926ae8a65f716bfc23f32339a6b35e51d5a.1623326176.git.viresh.kumar@linaro.org |
---|---|
State | New |
Headers | show |
Series | gpio: Add virtio based driver | expand |
On Thu, Jun 10, 2021 at 2:18 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > From: "Enrico Weigelt, metux IT consult" <info@metux.net> > > This patch adds a new driver for Virtio based GPIO devices. > > This allows a guest VM running Linux to access GPIO device provided by > the host. It supports all basic operations for now, except interrupts > for the GPIO lines. > > Signed-off-by: "Enrico Weigelt, metux IT consult" <info@metux.net> > Co-developed-by: Viresh Kumar <viresh.kumar@linaro.org> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> Can you give an example of how this would be hooked up to other drivers using those gpios. Can you give an example of how using the "gpio-keys" or "gpio-leds" drivers in combination with virtio-gpio looks like in the DT? Would qemu simply add the required DT properties to the device node that corresponds to the virtio device in this case? From what I can tell, both the mmio and pci variants of virtio can have their dev->of_node populated, but I don't see the logic in register_virtio_device() that looks up the of_node of the virtio_device that the of_gpio code then tries to refer to. Arnd
On 10.06.21 14:16, Viresh Kumar wrote: > From: "Enrico Weigelt, metux IT consult" <info@metux.net> > > This patch adds a new driver for Virtio based GPIO devices. > > This allows a guest VM running Linux to access GPIO device provided by > the host. It supports all basic operations for now, except interrupts > for the GPIO lines. What exactly did you change here from my original driver ? Your already changed the spec havily (w/o consulting me first), so I guess this driver hasn't so much in common w/ my original design. Note that I made my original design decisions for good reaons (see virtio-dev list). It already support async notifications (IOW: irqs), had an easy upgrade path for future additions, etc. Note #2: it's already implemented and running in the field (different kernels, different hypervisors, ...) - it just lacked the going through virtio comitte's formal specification process, which blocked mainlining. Is there anything fundamentally wrong w/ my original design, why you invented a completely new one ? --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On 10.06.21 15:22, Arnd Bergmann wrote: > Can you give an example of how this would be hooked up to other drivers > using those gpios. Can you give an example of how using the "gpio-keys" or > "gpio-leds" drivers in combination with virtio-gpio looks like in the DT? Connecting between self-probing bus'es and DT is generally tricky. IMHO we don't have any generic mechanism for that. I've made a few attempts, but nothing practically useful, which would be accepted by the corresponding maintainers, yet. We'd either need some very special logic in DT probing or pseudo-bus'es for the mapping. (DT wants to do those connections via phandle's, which in turn need the referenced nodes to be present in the DT). > From what I can tell, both the mmio and pci variants of virtio can have their > dev->of_node populated, but I don't see the logic in register_virtio_device() > that looks up the of_node of the virtio_device that the of_gpio code then > tries to refer to. Have you ever successfully bound a virtio device via DT ? --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
Hi Enrico, On Thu, 10 Jun 2021 at 21:24, Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > On 10.06.21 14:16, Viresh Kumar wrote: > > From: "Enrico Weigelt, metux IT consult" <info@metux.net> > > > > This patch adds a new driver for Virtio based GPIO devices. > > > > This allows a guest VM running Linux to access GPIO device provided by > > the host. It supports all basic operations for now, except interrupts > > for the GPIO lines. > > What exactly did you change here from my original driver ? I didn't write it from scratch and used your patch only to start with, and so you are still the Author of this particular patch. This just followed the specification updates and so changes only the stuff that was different from your original specs. Apart from that as you know, the irqs weren't working in your case as they needed to be implemented differently (patch 2 does that) here. > Your already changed the spec havily (w/o consulting me first), Isn't that what we are doing on the list? I believe that's why the lists exist, so people don't need to discuss in person, offline. I am happy to make changes in spec if you want to suggest something and if something breaks it for you. > so I guess this driver hasn't so much in common w/ my original design. It actually has as I said earlier, it is still based on your patch. > Note that I made my original design decisions for good reaons > (see virtio-dev list). I tried to follow your patches from December on the Linux kernel list and the ID allocation one on virtio-comments list. I wasn't able to search for any other attempt at sending the specification, so may have missed something. I do understand that there were reasons why you (and me) chose a particular way of doing things and if there is a good reason of following that, then we can still modify the spec and fix things for everyone. We just need to discuss our reasoning on the list and get through with a specfication which works for everyone as this will become a standard interface for many in the future and it needs to be robust and efficient for everyone. > It already support async notifications > (IOW: irqs), had an easy upgrade path for future additions, etc. I haven't changed irqs path, we still have a separate virtqueue (named "interrupt" vq) which handles just the interrupts now. And so will be faster than what you initially suggested. In your original design you also received the responses for the requests on this virtqueue, wihch I have changed to get the response synchronously on the "command" virtqueue only. This is from one of your earlier replies: " I've been under the impression that queues only work in only one direction. (at least that's what my web research was telling). Could you please give an example how bi-directional transmission within the same queue could look like ? " It is actually possible and the right thing to do here as we aren't starting multiple requests together. The operation needs to be synchronous anyway and both request/response on the same channel work better there. Also that makes the interrupts reach faster, without additional delay due to responses. > Note #2: it's already implemented and running in the field (different > kernels, different hypervisors, ...) - it just lacked the going through > virtio comitte's formal specification process, which blocked mainlining. I understand the pain you would be reqiured to go through because of this, but this is how any open source community will work, like Linux. There will be changes in specification and code once you post it and any solutions already working in the field won't really matter during the discussion. That is why it is always the right thing to _upstream first_, so one can avoid these problems later on. Though I understand that the real world needs to move faster than community. But no one can really help in that. > Is there anything fundamentally wrong w/ my original design, why you > invented a completely new one ? Not much, and I haven't changed a lot as well. Lemme point out few things which have changed in specification since your earlier version (the code just followed the specification, that's it). - config structure - version information is replaced with virtio-features. - Irq support is added as a feature, as you suggested. - extra padding space (24 bytes) removed. If you see this patch we can now read the entire config structure in a single go. Why should anyone be required to copy extra 24 bytes? If we need more fields later, we can always do that with help of another feature-flag. So this is still extendable. - Virtqueues: we still have two virtqueues (command and interrupt), just responses are moved to the command virtqueue only, as that is more efficient. - Request/response: Request layout is same, added a new layout for response as the same layout is inefficient. - IRQ support: Initial specification had no interface for configuring irqs from the guest, I added that. That's all I can gather right now. I don't think that's a lot and it is mostly improvements only. But if there is a good reason on why we should do things differently, we can still discuss that. I am open to suggestions. Thanks -- Viresh
On Thu, Jun 10, 2021 at 04:00:39PM +0000, Enrico Weigelt, metux IT consult via Stratos-dev wrote: > On 10.06.21 15:22, Arnd Bergmann wrote: > > > Can you give an example of how this would be hooked up to other drivers > > using those gpios. Can you give an example of how using the "gpio-keys" or > > "gpio-leds" drivers in combination with virtio-gpio looks like in the DT? > > Connecting between self-probing bus'es and DT is generally tricky. IMHO > we don't have any generic mechanism for that. DT does have a generic description of PCI endpoints, which virtio-iommu relies on to express the relation between IOMMU and endpoint nodes [1]. I think the problem here is similar: the client node needs a phandle to the GPIO controller which may use virtio-pci transport? Note that it mostly works if the device is on the root PCI bus. Behind a bridge the OS may change the device's bus number as needed, so the BDF reference in DT is only valid if the software providing the DT description (VMM or firmware) initializes bus numbers accordingly (and I don't remember if Linux supports this case well). Thanks, Jean [1] Documentation/devicetree/bindings/virtio/iommu.txt > > I've made a few attempts, but nothing practically useful, which would be > accepted by the corresponding maintainers, yet. We'd either need some > very special logic in DT probing or pseudo-bus'es for the mapping. > (DT wants to do those connections via phandle's, which in turn need the > referenced nodes to be present in the DT). > > > From what I can tell, both the mmio and pci variants of virtio can have their > > dev->of_node populated, but I don't see the logic in register_virtio_device() > > that looks up the of_node of the virtio_device that the of_gpio code then > > tries to refer to. > > Have you ever successfully bound a virtio device via DT ? > > > --mtx > > -- > --- > Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert > werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren > GPG/PGP-Schlüssel zu. > --- > Enrico Weigelt, metux IT consult > Free software and Linux embedded engineering > info@metux.net -- +49-151-27565287 > -- > Stratos-dev mailing list > Stratos-dev@op-lists.linaro.org > https://op-lists.linaro.org/mailman/listinfo/stratos-dev
On Thu, Jun 10, 2021 at 7:03 PM Jean-Philippe Brucker <jean-philippe@linaro.org> wrote: > > On Thu, Jun 10, 2021 at 04:00:39PM +0000, Enrico Weigelt, metux IT consult via Stratos-dev wrote: > > On 10.06.21 15:22, Arnd Bergmann wrote: > > > > > Can you give an example of how this would be hooked up to other drivers > > > using those gpios. Can you give an example of how using the "gpio-keys" or > > > "gpio-leds" drivers in combination with virtio-gpio looks like in the DT? > > > > Connecting between self-probing bus'es and DT is generally tricky. IMHO > > we don't have any generic mechanism for that. > > DT does have a generic description of PCI endpoints, which virtio-iommu > relies on to express the relation between IOMMU and endpoint nodes [1]. > I think the problem here is similar: the client node needs a phandle to > the GPIO controller which may use virtio-pci transport? Right, the code to set dev->of_node is fairly simple, the device probe just needs to scan for child nodes. Aside from PCI, similar code exists for USB and MMC/SDIO, which are usually discoverable but sometimes need additional properties. > Note that it mostly works if the device is on the root PCI bus. Behind a > bridge the OS may change the device's bus number as needed, so the BDF > reference in DT is only valid if the software providing the DT description > (VMM or firmware) initializes bus numbers accordingly (and I don't > remember if Linux supports this case well). I think you can mark the host bridge as "probe-only" to prevent the OS (at least Linux) from renumbering the buses. The part I did not find though is assigning dev->of_node in the virtio_device to a child of the PCI device node. Arnd
Hi Viresh! thanks for working on this, it's a really interesting driver. My first question is conceptual: We previously have Geerts driver for virtualization: drivers/gpio/gpio-aggregator.c The idea with the aggregator is that a host script sets up a unique gpiochip for the virtualized instance using some poking in sysfs and pass that to the virtual machine. So this is Linux acting as virtualization host by definition. I think virtio is more abstract and intended for the usecase where the hypervisor is not Linux, so this should be mentioned in the commit, possibly also in Kconfig so users immediately know what usecases the two different drivers are for. Possibly both could be used: aggregator to pick out the GPIOs you want into a synthetic GPIO chip, and the actual talk between the hypervisor/host and the guest using virtio, even with linux-on-linux. Yet another usecase would be to jit this with remoteproc/rpmsg and let a specific signal processor or real-time executive on another CPU with a few GPIOs around present these to Linux using this mechanism. Well that would certainly interest Bjorn and other rpmsg stakeholders, so they should have a look so that this provides what they need they day they need it. (CCed Bjorn and also Google who may want this for their Android emulators.) On Thu, Jun 10, 2021 at 2:16 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > +static const char **parse_gpio_names(struct virtio_device *vdev, > + struct virtio_gpio_config *config) I really like this end-to-end plug-and-play that even provides the names over virtio. I think my patch to the gpiolib to make it mandatory for names to be unique per-chip made it in, but please make that part of the spec so that we don't get the problem with non-unique names here. I suppose the spec can be augmented later to also accept config settings like open drain pull up/down etc but no need to specify more than the basic for now. But to be able to add more in the future, the client needs some kind of query mechanism or version number so the driver can adapt and not announce something the underlying virtio device cannot do. Do we have this? A bitmask for features, a version number that increase monotonically for new features to be presented or similar? Because otherwise we have to bump this: +#define VIRTIO_ID_GPIO 41 /* virtio GPIO */ every time we add something new (and we will). Yours, Linus Walleij
Hi Linus, On 10-06-21, 22:46, Linus Walleij wrote: > Hi Viresh! > > thanks for working on this, it's a really interesting driver. > > My first question is conceptual: > > We previously have Geerts driver for virtualization: > drivers/gpio/gpio-aggregator.c > > The idea with the aggregator is that a host script sets up a > unique gpiochip for the virtualized instance using some poking > in sysfs and pass that to the virtual machine. > So this is Linux acting as virtualization host by definition. > > I think virtio is more abstract and intended for the usecase > where the hypervisor is not Linux, so this should be mentioned > in the commit, possibly also in Kconfig so users immediately > know what usecases the two different drivers are for. Well, not actually. The host can actually be anything. It can be a Xen based dom0, which runs some proprietary firmware, or Qemu running over Linux. It is left for the host to decide how it wants to club together the GPIO pins from host and access them, with Linux host userspace it would be playing with /dev/gpiochipN, while for a raw one it may be accessing registers directly. And so the backend running at host, needs to pass the gpiochip configurations and only the host understand it. The way I test it for now is by running this with Qemu over my x86 box, so my host side is indeed playing with sysfs Linux. > Possibly both could be used: aggregator to pick out the GPIOs > you want into a synthetic GPIO chip, and the actual talk > between the hypervisor/host and the guest using virtio, even > with linux-on-linux. Not sure if I understand the aggregator thing for now, but we see the backend running at host (which talks to this Linux driver at guest) as a userspace thing and not a kernel driver. Not sure if aggregator can be used like that, but anyway.. > Yet another usecase would be to jit this with remoteproc/rpmsg > and let a specific signal processor or real-time executive on > another CPU with a few GPIOs around present these to > Linux using this mechanism. Well that would certainly interest > Bjorn and other rpmsg stakeholders, so they should have > a look so that this provides what they need they day they > need it. (CCed Bjorn and also Google who may want this for > their Android emulators.) I am not very clear on the rpmsg thing, I know couple of folks at project Stratos were talking about it :) @Alex, want to chime in here for me ? :) > On Thu, Jun 10, 2021 at 2:16 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > +static const char **parse_gpio_names(struct virtio_device *vdev, > > + struct virtio_gpio_config *config) > > I really like this end-to-end plug-and-play that even provides > the names over virtio. The credit goes to Enrico for this :) > I think my patch to the gpiolib to make it mandatory for names to > be unique per-chip made it in, but please make that part of the spec > so that we don't get the problem with non-unique names here. Oh, that's nice. I will surely do that. > I suppose the spec can be augmented later to also accept config > settings like open drain pull up/down etc but no need to specify > more than the basic for now. That's the plan. > But to be able to add more in the future, the client needs some > kind of query mechanism or version number so the driver can > adapt and not announce something the underlying virtio device > cannot do. Do we have this? A bitmask for features, a version > number that increase monotonically for new features to be > presented or similar? > > Because otherwise we have to bump this: > +#define VIRTIO_ID_GPIO 41 /* virtio GPIO */ > > every time we add something new (and we will). Yes, Virtio presents features for this. The patch 2/3 already uses one for IRQs. We won't need to bump up the IDs :) -- viresh
Hi Viresh, Linus, On Fri, Jun 11, 2021 at 5:56 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 10-06-21, 22:46, Linus Walleij wrote: > > thanks for working on this, it's a really interesting driver. > > > > My first question is conceptual: > > > > We previously have Geerts driver for virtualization: > > drivers/gpio/gpio-aggregator.c > > > > The idea with the aggregator is that a host script sets up a > > unique gpiochip for the virtualized instance using some poking > > in sysfs and pass that to the virtual machine. > > So this is Linux acting as virtualization host by definition. The gpio-aggregator is running on the host... > > I think virtio is more abstract and intended for the usecase > > where the hypervisor is not Linux, so this should be mentioned > > in the commit, possibly also in Kconfig so users immediately > > know what usecases the two different drivers are for. ... while the virtio-gpio driver is meant for the guest kernel. I my PoC "[PATCH QEMU v2 0/5] Add a GPIO backend"[1], I didn't have a virtio transport, but just hooked into the PL061 GPIO emulation in QEMU. The PL061 QEMU driver talked to the GPIO backend, which talked to /dev/gpiochipN on the host. > Well, not actually. > > The host can actually be anything. It can be a Xen based dom0, which > runs some proprietary firmware, or Qemu running over Linux. > > It is left for the host to decide how it wants to club together the > GPIO pins from host and access them, with Linux host userspace it > would be playing with /dev/gpiochipN, while for a raw one it may > be accessing registers directly. > > And so the backend running at host, needs to pass the gpiochip > configurations and only the host understand it. So QEMU has to translate the virtio-gpio communication to e.g. /dev/gpiochipN on the host (or a different backend on non-Linux or bare-metal HV). > The way I test it for now is by running this with Qemu over my x86 > box, so my host side is indeed playing with sysfs Linux. Can you please share a link to the QEMU patches? > > Possibly both could be used: aggregator to pick out the GPIOs > > you want into a synthetic GPIO chip, and the actual talk > > between the hypervisor/host and the guest using virtio, even > > with linux-on-linux. > > Not sure if I understand the aggregator thing for now, but we see the > backend running at host (which talks to this Linux driver at guest) as > a userspace thing and not a kernel driver. Not sure if aggregator can > be used like that, but anyway.. The GPIO aggregator came into play after talking to Alexander Graf and Peter Maydell. To reduce the attack surface, they didn't want QEMU to be responsible for exporting to the guest a subset of all GPIOs of a gpiochip, only a full gpiochip. However, the full gpiochip may contain critical GPIOs you do not want the guest to tamper with. Hence the GPIO aggregator was born, to take care of aggregating all GPIOs you want to export to a guest into a new virtual gpiochip. You can find more information about the GPIO Aggregator's use cases in "[PATCH v7 0/6] gpio: Add GPIO Aggregator"[2]. [1] https://lore.kernel.org/linux-gpio/20200423090118.11199-1-geert+renesas@glider.be [2] https://lore.kernel.org/linux-doc/20200511145257.22970-1-geert+renesas@glider.be/ Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 11-06-21, 09:42, Geert Uytterhoeven wrote: > Hi Viresh, Linus, > > On Fri, Jun 11, 2021 at 5:56 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 10-06-21, 22:46, Linus Walleij wrote: > > > thanks for working on this, it's a really interesting driver. > > > > > > My first question is conceptual: > > > > > > We previously have Geerts driver for virtualization: > > > drivers/gpio/gpio-aggregator.c > > > > > > The idea with the aggregator is that a host script sets up a > > > unique gpiochip for the virtualized instance using some poking > > > in sysfs and pass that to the virtual machine. > > > So this is Linux acting as virtualization host by definition. > > The gpio-aggregator is running on the host... > > > > I think virtio is more abstract and intended for the usecase > > > where the hypervisor is not Linux, so this should be mentioned > > > in the commit, possibly also in Kconfig so users immediately > > > know what usecases the two different drivers are for. > > ... while the virtio-gpio driver is meant for the guest kernel. > > I my PoC "[PATCH QEMU v2 0/5] Add a GPIO backend"[1], I didn't have > a virtio transport, but just hooked into the PL061 GPIO emulation > in QEMU. The PL061 QEMU driver talked to the GPIO backend, which > talked to /dev/gpiochipN on the host. Hmm, interesting. > > Well, not actually. > > > > The host can actually be anything. It can be a Xen based dom0, which > > runs some proprietary firmware, or Qemu running over Linux. > > > > It is left for the host to decide how it wants to club together the > > GPIO pins from host and access them, with Linux host userspace it > > would be playing with /dev/gpiochipN, while for a raw one it may > > be accessing registers directly. > > > > And so the backend running at host, needs to pass the gpiochip > > configurations and only the host understand it. > > So QEMU has to translate the virtio-gpio communication to e.g. > /dev/gpiochipN on the host (or a different backend on non-Linux or > bare-metal HV). No, QEMU passes the raw messages to the backend daemon running in host userspace (which shares a socket with qemu). The backend understands the virtio/vhost protocols and so won't be required to change at all if we move from Qemu to something else. And that's what we (Linaro) are looking to do here with Project Stratos. Create virtio based hypervisor agnostic backends. > > The way I test it for now is by running this with Qemu over my x86 > > box, so my host side is indeed playing with sysfs Linux. > > Can you please share a link to the QEMU patches? Unfortunately, they aren't in good shape right now and the backend is a bit hacky (Just checking the data paths, but not touching /dev/gpiochipN at all for now). I didn't implement one as I am going to implement the backend in Rust and not Qemu. So it doesn't depend on Qemu at all. To give you an idea of the whole thing, here is what we have done for I2c for example, GPIO one will look very similar. The Qemu patches: https://yhbt.net/lore/all/cover.1617278395.git.viresh.kumar@linaro.org/T/ The stuff from tools/vhost-user-i2c/ directory (or patch 4/6) isn't used anymore and the following Rust implementation replaces it: https://github.com/vireshk/vhost-device/tree/master/src/i2c I can share the GPIO code once I have the Rust implementation ready. > The GPIO aggregator came into play after talking to Alexander Graf and > Peter Maydell. To reduce the attack surface, they didn't want QEMU > to be responsible for exporting to the guest a subset of all GPIOs of > a gpiochip, only a full gpiochip. However, the full gpiochip may > contain critical GPIOs you do not want the guest to tamper with. > Hence the GPIO aggregator was born, to take care of aggregating all > GPIOs you want to export to a guest into a new virtual gpiochip. > > You can find more information about the GPIO Aggregator's use cases in > "[PATCH v7 0/6] gpio: Add GPIO Aggregator"[2]. So I was actually looking to do some kind of aggregation on the host side's backend daemon to share only a subset of GPIO pins, I will see if that is something I can reuse. Thanks for sharing details. -- viresh
Hi Viresh, On Fri, Jun 11, 2021 at 10:01 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 11-06-21, 09:42, Geert Uytterhoeven wrote: > > On Fri, Jun 11, 2021 at 5:56 AM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > On 10-06-21, 22:46, Linus Walleij wrote: > > > > thanks for working on this, it's a really interesting driver. > > > > > > > > My first question is conceptual: > > > > > > > > We previously have Geerts driver for virtualization: > > > > drivers/gpio/gpio-aggregator.c > > > > > > > > The idea with the aggregator is that a host script sets up a > > > > unique gpiochip for the virtualized instance using some poking > > > > in sysfs and pass that to the virtual machine. > > > > So this is Linux acting as virtualization host by definition. > > > > The gpio-aggregator is running on the host... > > > > > > I think virtio is more abstract and intended for the usecase > > > > where the hypervisor is not Linux, so this should be mentioned > > > > in the commit, possibly also in Kconfig so users immediately > > > > know what usecases the two different drivers are for. > > > > ... while the virtio-gpio driver is meant for the guest kernel. > > > > I my PoC "[PATCH QEMU v2 0/5] Add a GPIO backend"[1], I didn't have > > a virtio transport, but just hooked into the PL061 GPIO emulation > > in QEMU. The PL061 QEMU driver talked to the GPIO backend, which > > talked to /dev/gpiochipN on the host. > > Hmm, interesting. > > > > Well, not actually. > > > > > > The host can actually be anything. It can be a Xen based dom0, which > > > runs some proprietary firmware, or Qemu running over Linux. > > > > > > It is left for the host to decide how it wants to club together the > > > GPIO pins from host and access them, with Linux host userspace it > > > would be playing with /dev/gpiochipN, while for a raw one it may > > > be accessing registers directly. > > > > > > And so the backend running at host, needs to pass the gpiochip > > > configurations and only the host understand it. > > > > So QEMU has to translate the virtio-gpio communication to e.g. > > /dev/gpiochipN on the host (or a different backend on non-Linux or > > bare-metal HV). > > No, QEMU passes the raw messages to the backend daemon running in host > userspace (which shares a socket with qemu). The backend understands > the virtio/vhost protocols and so won't be required to change at all > if we move from Qemu to something else. And that's what we (Linaro) > are looking to do here with Project Stratos. > > Create virtio based hypervisor agnostic backends. OK, IC. > > > The way I test it for now is by running this with Qemu over my x86 > > > box, so my host side is indeed playing with sysfs Linux. > > > > Can you please share a link to the QEMU patches? > > Unfortunately, they aren't in good shape right now and the backend is > a bit hacky (Just checking the data paths, but not touching > /dev/gpiochipN at all for now). > > I didn't implement one as I am going to implement the backend in Rust > and not Qemu. So it doesn't depend on Qemu at all. OK. > To give you an idea of the whole thing, here is what we have done for > I2c for example, GPIO one will look very similar. Oh, you also have i2c. Nice! > The Qemu patches: > > https://yhbt.net/lore/all/cover.1617278395.git.viresh.kumar@linaro.org/T/ > > The stuff from tools/vhost-user-i2c/ directory (or patch 4/6) isn't > used anymore and the following Rust implementation replaces it: > > https://github.com/vireshk/vhost-device/tree/master/src/i2c Thanks for the links! > > The GPIO aggregator came into play after talking to Alexander Graf and > > Peter Maydell. To reduce the attack surface, they didn't want QEMU > > to be responsible for exporting to the guest a subset of all GPIOs of > > a gpiochip, only a full gpiochip. However, the full gpiochip may > > contain critical GPIOs you do not want the guest to tamper with. > > Hence the GPIO aggregator was born, to take care of aggregating all > > GPIOs you want to export to a guest into a new virtual gpiochip. > > > > You can find more information about the GPIO Aggregator's use cases in > > "[PATCH v7 0/6] gpio: Add GPIO Aggregator"[2]. > > So I was actually looking to do some kind of aggregation on the host > side's backend daemon to share only a subset of GPIO pins, I will see > if that is something I can reuse. Thanks for sharing details. The same reasoning can apply to your backend daemon, so when using the GPIO aggregator, you can just control a full gpiochip, without having to implement access control on individual GPIO lines. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On 11.06.21 09:42, Geert Uytterhoeven wrote: hi, > I my PoC "[PATCH QEMU v2 0/5] Add a GPIO backend"[1], I didn't have > a virtio transport, but just hooked into the PL061 GPIO emulation > in QEMU. The PL061 QEMU driver talked to the GPIO backend, which > talked to /dev/gpiochipN on the host. for qemu side you might be interested in my patch queue from last year (including the virtio-gpio implementation) - it also introduces an gpio backend subsys that allows attaching simulation gpio's to various backends. so far just implemented a dummy backend (that can be manipulated by qemu console) and using it just in the virtio-gpio device emulation. https://github.com/oss-qm/qemu/tree/wip/gpio-v2 > So QEMU has to translate the virtio-gpio communication to e.g. > /dev/gpiochipN on the host (or a different backend on non-Linux or > bare-metal HV). For qemu case, yes, depending on your actual configuration. You can attach the virtual device to any gpio backend you like (once it's actually implemented). Yet only implemented the dummy, which doesn't speak to a real hosts gpio, but can be used simulations like HIL. --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On 11.06.21 10:01, Viresh Kumar wrote: > No, QEMU passes the raw messages to the backend daemon running in host > userspace (which shares a socket with qemu). The backend understands > the virtio/vhost protocols and so won't be required to change at all > if we move from Qemu to something else. And that's what we (Linaro) > are looking to do here with Project Stratos. Note that this is completely different from my approach that I've posted in autumn last year. Viresh's protocol hasn't much in common with mine. --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On Mon, Jun 14, 2021 at 11:08 AM Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > > On 11.06.21 10:01, Viresh Kumar wrote: > > > No, QEMU passes the raw messages to the backend daemon running in host > > userspace (which shares a socket with qemu). The backend understands > > the virtio/vhost protocols and so won't be required to change at all > > if we move from Qemu to something else. And that's what we (Linaro) > > are looking to do here with Project Stratos. > > Note that this is completely different from my approach that I've posted > in autumn last year. Viresh's protocol hasn't much in common with mine. That's why we have a thing called standard. And AFAIU virtio API/ABIs should be officially registered and standardized. -- With Best Regards, Andy Shevchenko
On 14-06-21, 10:07, Enrico Weigelt, metux IT consult wrote: > On 11.06.21 10:01, Viresh Kumar wrote: > > > No, QEMU passes the raw messages to the backend daemon running in host > > userspace (which shares a socket with qemu). The backend understands > > the virtio/vhost protocols and so won't be required to change at all > > if we move from Qemu to something else. And that's what we (Linaro) > > are looking to do here with Project Stratos. This is one of the ways of using it, via a backend running in host userspace, maybe there are other ways of making this work which I may not have explored, like handling this within qemu. > Note that this is completely different from my approach that I've posted > in autumn last year. Viresh's protocol hasn't much in common with mine. The protocol hasn't changed in a sense on how it can be used. Yes few things have moved around, new operations were introduced, but nothing that will make a change at this level. We are trying to use the protocol in one of the possible ways and yours can be different. -- viresh
On 14-06-21, 11:12, Andy Shevchenko wrote: > On Mon, Jun 14, 2021 at 11:08 AM Enrico Weigelt, metux IT consult > <lkml@metux.net> wrote: > > > > On 11.06.21 10:01, Viresh Kumar wrote: > > > > > No, QEMU passes the raw messages to the backend daemon running in host > > > userspace (which shares a socket with qemu). The backend understands > > > the virtio/vhost protocols and so won't be required to change at all > > > if we move from Qemu to something else. And that's what we (Linaro) > > > are looking to do here with Project Stratos. > > > > Note that this is completely different from my approach that I've posted > > in autumn last year. Viresh's protocol hasn't much in common with mine. > > That's why we have a thing called standard. And AFAIU virtio API/ABIs > should be officially registered and standardized. Yes and here is the latest version (which is based on the work done by Enrico earlier). It isn't accepted yet and is under review. https://lists.oasis-open.org/archives/virtio-comment/202106/msg00022.html -- viresh
On 14.06.21 10:12, Andy Shevchenko wrote: > That's why we have a thing called standard. And AFAIU virtio API/ABIs > should be officially registered and standardized. Absolutely. I've submitted my spec to virtio folks last nov, but this wasn't in form of patch against their tex-based documentation yet (just ascii text), thus laying around in the pipeline. (meanwhile the actual implementation is running in the field for over 6 month now) Viresh picked it up, but made something totally different out of it. I wonder why he didn't consult me first. All that needed to be done was translated the ascii documentation into tex and rephrase a bit in order to match the formal terminology and structure used in virtio specs. This makes me very unhappy. --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On 14-06-21, 10:03, Enrico Weigelt, metux IT consult wrote: > for qemu side you might be interested in my patch queue from last year > (including the virtio-gpio implementation) - it also introduces an > gpio backend subsys that allows attaching simulation gpio's to various > backends. so far just implemented a dummy backend (that can be > manipulated by qemu console) and using it just in the virtio-gpio > device emulation. > > https://github.com/oss-qm/qemu/tree/wip/gpio-v2 Interesting, so this is a qemu-specific backend you have here. The way we are looking to write the backend (in Project Stratos at Linaro) is to make it hypervisor agnostic. So any hypervisor that understands the vhost protocol can use our backend without changing a single line of code. The backend will not be part of any hypervisor's/VMM's source code. FWIW, this doesn't add anything special to the virtio protocol (like GPIO). Here is a C based backend we have for I2C: https://yhbt.net/lore/all/cover.1617278395.git.viresh.kumar@linaro.org/T/#m3b5044bad9769b170f505e63bd081eb27cef8db2 I started with keeping code in QEMU itself but now replaced it with one in RUST. https://github.com/vireshk/vhost-device/tree/master/src/i2c -- viresh
On 14.06.21 11:12, Viresh Kumar wrote: >> Note that this is completely different from my approach that I've posted >> in autumn last year. Viresh's protocol hasn't much in common with mine. > > The protocol hasn't changed in a sense on how it can be used. Sorry, but that's not correct. It's an entirely different protocol, entirely different signal flow, different data structures, different queue layout, ... in no way compatible. The only thing they've got in common is they're both do gpios via virtio. --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On 14-06-21, 11:17, Enrico Weigelt, metux IT consult wrote: > On 14.06.21 10:12, Andy Shevchenko wrote: > > > That's why we have a thing called standard. And AFAIU virtio API/ABIs > > should be officially registered and standardized. > > Absolutely. > > I've submitted my spec to virtio folks last nov, but this wasn't in form > of patch against their tex-based documentation yet (just ascii text), > thus laying around in the pipeline. > > (meanwhile the actual implementation is running in the field for over > 6 month now) > > Viresh picked it up, but made something totally different out of it. > I wonder why he didn't consult me first. Here I asked you on 26th of May, if you would be fine if I take it forward as you didn't do anything with it formally in the last 5-6 months (Yes I know you were busy with other stuff). https://lore.kernel.org/linux-doc/20210526033206.5v362hdywb55msve@vireshk-i7/ You chose not to reply. I did the same before picking up the kernel code. You chose not to reply. I am not inclined to do offlist discussions as they aren't fruitful eventually, and it is better to do these discussions over open lists, so others can chime in as well. > All that needed to be done was > translated the ascii documentation into tex and rephrase a bit in order > to match the formal terminology and structure used in virtio specs. Not really. There were things lagging, which are normally caught in reviews and fixed/updated. But that never happened in this case. You only sent the specs once to virtio list, that too as an attachment and it never made it to the virtio guys there (as the list doesn't take attachments). https://www.mail-archive.com/virtio-dev@lists.oasis-open.org/msg06946.html > This makes me very unhappy. I know you have solutions running in products which depend on the layout of the config structure or request/responses, based on your original write-up, but unless something is merged in open source code or specification, it is going to get changed, normally because of reviews. And you will be required to change things based on reviews, you just can't say that I have products running the code and so I won't change it. That is why people say, Upstream first. So you don't get into this mess. Yes, this is tough and that's the way it is. You keep saying that I have changed the original specification too much, which is incorrect, I tried to point out earlier what all is changed and why. https://lore.kernel.org/linux-gpio/CAKohpokxSoSVtAJkL-T_OOoS8dW-gYG1Gs5=_DwebvJETE48Xw@mail.gmail.com/ You chose not to reply to that. Lemme say this again. This is a generic protocol we are trying to define here. It needs to take everyone's view into account and their use cases. We are required here to collaborate. A solution thought to be good enough for one person or use-case, isn't going to fly. The changes I did to it are required to make it useful for the generic solution for a protocol. I am sure there would be shortcomings, like the one identified by Jean-Philippe Brucker, where he asked to add offset of the gpios-name thing. He made a sensible suggestion, which I am required to incorporate. I just can't reply to him and say I won't change it because I have already designed my product based on this. Lets try to improve the code and specification here and make it work for both of us by giving very specific feedback on wherever you think the protocol or code is not efficient or correct. Being unhappy isn't going make anything better. -- viresh
On 10-06-21, 15:22, Arnd Bergmann wrote: > Can you give an example of how this would be hooked up to other drivers > using those gpios. Can you give an example of how using the "gpio-keys" or > "gpio-leds" drivers in combination with virtio-gpio looks like in the DT? > > Would qemu simply add the required DT properties to the device node that > corresponds to the virtio device in this case? > > From what I can tell, both the mmio and pci variants of virtio can have their > dev->of_node populated, but I don't see the logic in register_virtio_device() > that looks up the of_node of the virtio_device that the of_gpio code then > tries to refer to. To be honest, I haven't tried this yet and I was expecting it to be already taken care of. I was relying on the DTB automatically generated by Qemu to get the driver probed and didn't have a look at it as well. I now understand that it won't be that straight forward. The same must be true for adding an i2c device to an i2c bus over virtio (The way I tested that earlier was by using the sysfs file to add a device to a bus). This may be something lacking generally for virtio-pci thing, not sure though. -- viresh
On Mon, Jun 14, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 10-06-21, 15:22, Arnd Bergmann wrote: > > Can you give an example of how this would be hooked up to other drivers > > using those gpios. Can you give an example of how using the "gpio-keys" or > > "gpio-leds" drivers in combination with virtio-gpio looks like in the DT? > > > > Would qemu simply add the required DT properties to the device node that > > corresponds to the virtio device in this case? > > > > From what I can tell, both the mmio and pci variants of virtio can have their > > dev->of_node populated, but I don't see the logic in register_virtio_device() > > that looks up the of_node of the virtio_device that the of_gpio code then > > tries to refer to. > > To be honest, I haven't tried this yet and I was expecting it to be > already taken care of. I was relying on the DTB automatically > generated by Qemu to get the driver probed and didn't have a look at > it as well. > > I now understand that it won't be that straight forward. The same must > be true for adding an i2c device to an i2c bus over virtio (The way I > tested that earlier was by using the sysfs file to add a device to a > bus). Yes, correct, we had the same discussion about i2c. Again, this is relatively straightforward when the controller and the device attached to it (i2c controller/client or gpio controller/function) are both emulated by qemu, but a lot harder when the controller and device are implemented in different programs. > This may be something lacking generally for virtio-pci thing, not > sure though. I think most importantly we need a DT binding to describe what device nodes are supposed to look like underneath a virtio-mmio or virtio-pci device in order for a hypervisor to pass down the information to a guest OS in a generic way. We can probably borrow the USB naming, and replace compatible="usbVID,PID" with compatible="virtioDID", with the device ID in hexadecimal digits, such as "virtio22" for I2C (virtio device ID 34 == 0x22) if we decide to have a sub-node under the device, or we just point dev->of_node of the virtio device to the platform/pci device that is its parent in Linux. Adding the Linux guest code to the virtio layer should be fairly straightforward, and I suppose it could be mostly copied from the corresponding code that added this for mmc in commit 25185f3f31c9 ("mmc: Add SDIO function devicetree subnode parsing") and for USB in commit 69bec7259853 ("USB: core: let USB device know device node") and 1a7e3948cb9f ("USB: add device-tree support for interfaces"). Arnd
On Mon, 14 Jun 2021 at 14:33, Arnd Bergmann <arnd@kernel.org> wrote: > > On Mon, Jun 14, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > On 10-06-21, 15:22, Arnd Bergmann wrote: > > > Can you give an example of how this would be hooked up to other drivers > > > using those gpios. Can you give an example of how using the "gpio-keys" or > > > "gpio-leds" drivers in combination with virtio-gpio looks like in the DT? > > > > > > Would qemu simply add the required DT properties to the device node that > > > corresponds to the virtio device in this case? > > > > > > From what I can tell, both the mmio and pci variants of virtio can have their > > > dev->of_node populated, but I don't see the logic in register_virtio_device() > > > that looks up the of_node of the virtio_device that the of_gpio code then > > > tries to refer to. > > > > To be honest, I haven't tried this yet and I was expecting it to be > > already taken care of. I was relying on the DTB automatically > > generated by Qemu to get the driver probed and didn't have a look at > > it as well. > > > > I now understand that it won't be that straight forward. The same must > > be true for adding an i2c device to an i2c bus over virtio (The way I > > tested that earlier was by using the sysfs file to add a device to a > > bus). > > Yes, correct, we had the same discussion about i2c. Again, this is > relatively straightforward when the controller and the device attached > to it (i2c controller/client or gpio controller/function) are both emulated > by qemu, but a lot harder when the controller and device are > implemented in different programs. > > > This may be something lacking generally for virtio-pci thing, not > > sure though. > > I think most importantly we need a DT binding to describe what device > nodes are supposed to look like underneath a virtio-mmio or > virtio-pci device in order for a hypervisor to pass down the > information to a guest OS in a generic way. We can probably borrow > the USB naming, and replace compatible="usbVID,PID" with > compatible="virtioDID", with the device ID in hexadecimal digits, > such as "virtio22" for I2C (virtio device ID 34 == 0x22) if we decide > to have a sub-node under the device, or we just point dev->of_node > of the virtio device to the platform/pci device that is its parent > in Linux. > > Adding the Linux guest code to the virtio layer should be fairly > straightforward, and I suppose it could be mostly copied from the > corresponding code that added this for mmc in commit 25185f3f31c9 > ("mmc: Add SDIO function devicetree subnode parsing") and for USB > in commit 69bec7259853 ("USB: core: let USB device know device > node") and 1a7e3948cb9f ("USB: add device-tree support for > interfaces"). And something similar is also done with SCMI protocols which are defined in a SCMI node. A typical example: cpu@0 { ... clocks = <&scmi_dvfs 0>; ... }; deviceX: deviceX@YYYYYYY { ... clocks = <&scmi_clk 0>; ... }; scmi: scmi { compatible = "arm,scmi-virtio"; #address-cells = <1>; #size-cells = <0>; scmi_devpd: protocol@11 { reg = <0x11>; #power-domain-cells = <1>; }; scmi_clk: protocol@14 { reg = <0x14>; #clock-cells = <1>; }; scmi_sensors: protocol@15 { reg = <0x15>; #thermal-sensor-cells = <1>; }; scmi_dvfs: protocol@13 { reg = <0x13>; #clock-cells = <1>; }; }; > > Arnd
On Mon, Jun 14, 2021 at 2:50 PM Vincent Guittot via Stratos-dev <stratos-dev@op-lists.linaro.org> wrote:> > On Mon, 14 Jun 2021 at 14:33, Arnd Bergmann <arnd@kernel.org> wrote: > > On Mon, Jun 14, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > I think most importantly we need a DT binding to describe what device > > nodes are supposed to look like underneath a virtio-mmio or > > virtio-pci device in order for a hypervisor to pass down the > > information to a guest OS in a generic way. We can probably borrow > > the USB naming, and replace compatible="usbVID,PID" with > > compatible="virtioDID", with the device ID in hexadecimal digits, > > such as "virtio22" for I2C (virtio device ID 34 == 0x22) if we decide > > to have a sub-node under the device, or we just point dev->of_node > > of the virtio device to the platform/pci device that is its parent > > in Linux. > > > > Adding the Linux guest code to the virtio layer should be fairly > > straightforward, and I suppose it could be mostly copied from the > > corresponding code that added this for mmc in commit 25185f3f31c9 > > ("mmc: Add SDIO function devicetree subnode parsing") and for USB > > in commit 69bec7259853 ("USB: core: let USB device know device > > node") and 1a7e3948cb9f ("USB: add device-tree support for > > interfaces"). > > And something similar is also done with SCMI protocols which are > defined in a SCMI node. A typical example: > > cpu@0 { > ... > clocks = <&scmi_dvfs 0>; > ... > }; > > deviceX: deviceX@YYYYYYY { > ... > clocks = <&scmi_clk 0>; > ... > }; > > scmi: scmi { > compatible = "arm,scmi-virtio"; > #address-cells = <1>; > #size-cells = <0>; > > scmi_devpd: protocol@11 { > reg = <0x11>; > #power-domain-cells = <1>; > }; > > scmi_clk: protocol@14 { > reg = <0x14>; > #clock-cells = <1>; > }; > > scmi_sensors: protocol@15 { > reg = <0x15>; > #thermal-sensor-cells = <1>; > }; > > scmi_dvfs: protocol@13 { > reg = <0x13>; > #clock-cells = <1>; > }; > }; But this example seem to be completely different from the ones I mentioned: The scmi node that you have here looks like it shows up under the root of the device tree, not below the virtio device that implements the scmi transport. Arnd
On Mon, 14 Jun 2021 at 15:00, Arnd Bergmann <arnd@kernel.org> wrote: > > On Mon, Jun 14, 2021 at 2:50 PM Vincent Guittot via Stratos-dev > <stratos-dev@op-lists.linaro.org> wrote:> > > On Mon, 14 Jun 2021 at 14:33, Arnd Bergmann <arnd@kernel.org> wrote: > > > On Mon, Jun 14, 2021 at 12:23 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > > > > I think most importantly we need a DT binding to describe what device > > > nodes are supposed to look like underneath a virtio-mmio or > > > virtio-pci device in order for a hypervisor to pass down the > > > information to a guest OS in a generic way. We can probably borrow > > > the USB naming, and replace compatible="usbVID,PID" with > > > compatible="virtioDID", with the device ID in hexadecimal digits, > > > such as "virtio22" for I2C (virtio device ID 34 == 0x22) if we decide > > > to have a sub-node under the device, or we just point dev->of_node > > > of the virtio device to the platform/pci device that is its parent > > > in Linux. > > > > > > Adding the Linux guest code to the virtio layer should be fairly > > > straightforward, and I suppose it could be mostly copied from the > > > corresponding code that added this for mmc in commit 25185f3f31c9 > > > ("mmc: Add SDIO function devicetree subnode parsing") and for USB > > > in commit 69bec7259853 ("USB: core: let USB device know device > > > node") and 1a7e3948cb9f ("USB: add device-tree support for > > > interfaces"). > > > > And something similar is also done with SCMI protocols which are > > defined in a SCMI node. A typical example: > > > > cpu@0 { > > ... > > clocks = <&scmi_dvfs 0>; > > ... > > }; > > > > deviceX: deviceX@YYYYYYY { > > ... > > clocks = <&scmi_clk 0>; > > ... > > }; > > > > scmi: scmi { > > compatible = "arm,scmi-virtio"; > > #address-cells = <1>; > > #size-cells = <0>; > > > > scmi_devpd: protocol@11 { > > reg = <0x11>; > > #power-domain-cells = <1>; > > }; > > > > scmi_clk: protocol@14 { > > reg = <0x14>; > > #clock-cells = <1>; > > }; > > > > scmi_sensors: protocol@15 { > > reg = <0x15>; > > #thermal-sensor-cells = <1>; > > }; > > > > scmi_dvfs: protocol@13 { > > reg = <0x13>; > > #clock-cells = <1>; > > }; > > }; > > But this example seem to be completely different from the ones I mentioned: > The scmi node that you have here looks like it shows up under the root of the > device tree, not below the virtio device that implements the scmi transport. I was thinking of something like below: deviceX: deviceX@YYYYYYY { ... gpio = <&virtio_gpio 0>; ... }; virtio_mmio@a000000 { dma-coherent; interrupts = <0x0 0x10 0x1>; reg = <0x0 0xa000000 0x0 0x200>; compatible = "virtio,mmio"; virtio_gpio: protocol@22 { reg = <0x22>; }; }; > > Arnd
On Mon, Jun 14, 2021 at 3:24 PM Vincent Guittot <vincent.guittot@linaro.org> wrote: > On Mon, 14 Jun 2021 at 15:00, Arnd Bergmann <arnd@kernel.org> wrote: > > On Mon, Jun 14, 2021 at 2:50 PM Vincent Guittot via Stratos-dev <stratos-dev@op-lists.linaro.org> wrote:> > > > > But this example seem to be completely different from the ones I mentioned: > > The scmi node that you have here looks like it shows up under the root of the > > device tree, not below the virtio device that implements the scmi transport. > > I was thinking of something like below: > > deviceX: deviceX@YYYYYYY { > ... > gpio = <&virtio_gpio 0>; > ... > }; > > virtio_mmio@a000000 { > dma-coherent; > interrupts = <0x0 0x10 0x1>; > reg = <0x0 0xa000000 0x0 0x200>; > compatible = "virtio,mmio"; > > virtio_gpio: protocol@22 { > reg = <0x22>; > }; Encoding the device ID as "reg" seems somewhat odd, especially since there can only be one child for each virtio device. The other bus types use the "compatible" property instead of "reg" for this purpose. This is still redundant, since the type is also known from the contents, but it seems less unusual. The gpio node in the example is usually called "gpio" or "gpio-controller", and it would then need the "gpio-controller" and "#gpio-cells" properties so other nodes can refer to it by phandle. Arnd
On Mon, 14 Jun 2021 at 22:56, Arnd Bergmann <arnd@kernel.org> wrote: > > On Mon, Jun 14, 2021 at 3:24 PM Vincent Guittot > <vincent.guittot@linaro.org> wrote: > > On Mon, 14 Jun 2021 at 15:00, Arnd Bergmann <arnd@kernel.org> wrote: > > > On Mon, Jun 14, 2021 at 2:50 PM Vincent Guittot via Stratos-dev <stratos-dev@op-lists.linaro.org> wrote:> > > > > > > But this example seem to be completely different from the ones I mentioned: > > > The scmi node that you have here looks like it shows up under the root of the > > > device tree, not below the virtio device that implements the scmi transport. > > > > I was thinking of something like below: > > > > deviceX: deviceX@YYYYYYY { > > ... > > gpio = <&virtio_gpio 0>; > > ... > > }; > > > > virtio_mmio@a000000 { > > dma-coherent; > > interrupts = <0x0 0x10 0x1>; > > reg = <0x0 0xa000000 0x0 0x200>; > > compatible = "virtio,mmio"; > > > > virtio_gpio: protocol@22 { > > reg = <0x22>; > > }; > > Encoding the device ID as "reg" seems somewhat odd, especially since there > can only be one child for each virtio device. The other bus types use the > "compatible" property instead of "reg" for this purpose. This is still > redundant, > since the type is also known from the contents, but it seems less unusual. At least this ensures to match directly the protocol id instead of mapping a compatible string with the protocol id. > > The gpio node in the example is usually called "gpio" or "gpio-controller", and > it would then need the "gpio-controller" and "#gpio-cells" properties so other > nodes can refer to it by phandle. yes, This short example is just to show what I mean. > > Arnd
On 11-06-21, 10:22, Geert Uytterhoeven wrote: > The same reasoning can apply to your backend daemon, so when using > the GPIO aggregator, you can just control a full gpiochip, without > having to implement access control on individual GPIO lines. I tried to look at it and it surely looks very temping and may fit well and reduce size of my backend :) I am now wondering how interrupts can be made to work here. Do you have anything in mind for that ? GPIO sysfs already supports interrupts, just that you need to register irq for the specific GPIO pins inside the aggregator ? -- viresh
Hi Viresh, On Tue, Jun 15, 2021 at 1:15 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > On 11-06-21, 10:22, Geert Uytterhoeven wrote: > > The same reasoning can apply to your backend daemon, so when using > > the GPIO aggregator, you can just control a full gpiochip, without > > having to implement access control on individual GPIO lines. > > I tried to look at it and it surely looks very temping and may fit > well and reduce size of my backend :) > > I am now wondering how interrupts can be made to work here. Do you > have anything in mind for that ? > > GPIO sysfs already supports interrupts, just that you need to register > irq for the specific GPIO pins inside the aggregator ? So far I hadn't considered interrupts. Will think about it... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
On Tue, Jun 15, 2021 at 1:15 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > I am now wondering how interrupts can be made to work here. Do you > have anything in mind for that ? The aggregator does not aggregate interrupts only lines for now. > GPIO sysfs already supports interrupts, (...) I hope that doesn't make you tempted to use sysfs to get interrupts, those are awful, they use sysfs_notify_dirent() which means that if two IRQs happen in fast succession you will miss one of them and think it was only one. The GPIO character device supports low latency events forwarding interrupts to userspace including QEMU & similar, timestamps the events as close in time as possible to when they actually happen (which is necessary for any kind of industrial control) and will never miss an event if the hardware can register it. See: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/gpio/gpio-event-mon.c Yours, Linus Walleij
On 15-06-21, 22:03, Linus Walleij wrote: > On Tue, Jun 15, 2021 at 1:15 PM Viresh Kumar <viresh.kumar@linaro.org> wrote: > > > I am now wondering how interrupts can be made to work here. Do you > > have anything in mind for that ? > > The aggregator does not aggregate interrupts only lines for now. > > > GPIO sysfs already supports interrupts, (...) > > I hope that doesn't make you tempted to use sysfs to get interrupts, > those are awful, they use sysfs_notify_dirent() which means that > if two IRQs happen in fast succession you will miss one of them > and think it was only one. > > The GPIO character device supports low latency events forwarding > interrupts to userspace including QEMU & similar, timestamps the > events as close in time as possible to when they actually happen > (which is necessary for any kind of industrial control) and will never > miss an event if the hardware can register it. See: > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/tools/gpio/gpio-event-mon.c The current version of backend (I am working on) will be Linux userspace based, so I would be required to get an interrupt there. I was planning to use /dev/gpiochipN only for now and not a sysfs file, so yes it should be fine. -- viresh
On Thu 10 Jun 15:46 CDT 2021, Linus Walleij wrote: [..] > Yet another usecase would be to jit this with remoteproc/rpmsg > and let a specific signal processor or real-time executive on > another CPU with a few GPIOs around present these to > Linux using this mechanism. Well that would certainly interest > Bjorn and other rpmsg stakeholders, so they should have > a look so that this provides what they need they day they > need it. (CCed Bjorn and also Google who may want this for > their Android emulators.) > Right, your typical Qualcomm platform has a dedicated sensor subsystem, with some CPU core with dedicated I2C controllers and GPIOs for processing sensor input while the rest of the SoC is in deep sleep. Combined with the virtio-i2c effort this could provide an alternative by simply tunneling the busses and GPIOs into Linux and use standard iio drivers, for cases where this suits your product requirements better. And I've seen similar interest from others in the community as well. Regards, Bjorn
On 16.06.21 05:30, Bjorn Andersson wrote: > Combined with the virtio-i2c effort this could provide an alternative by > simply tunneling the busses and GPIOs into Linux and use standard iio > drivers, for cases where this suits your product requirements better. So, you wanna use virtio as logical interface between the two CPUs ? Interesting idea. Usually folks use rpmsg for those things. What is running on the secondary CPU ? Some OS like Linux or some bare metal stuff ? What kind of CPU is that anyways ? --mtx -- --- Hinweis: unverschlüsselte E-Mails können leicht abgehört und manipuliert werden ! Für eine vertrauliche Kommunikation senden Sie bitte ihren GPG/PGP-Schlüssel zu. --- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287
On Wed, Jun 16, 2021 at 5:52 PM Enrico Weigelt, metux IT consult <lkml@metux.net> wrote: > On 16.06.21 05:30, Bjorn Andersson wrote: > > > Combined with the virtio-i2c effort this could provide an alternative by > > simply tunneling the busses and GPIOs into Linux and use standard iio > > drivers, for cases where this suits your product requirements better. > > So, you wanna use virtio as logical interface between the two CPUs ? > Interesting idea. Usually folks use rpmsg for those things. rpmsg is using shared memory as transport mechanism and virtio is providing this. rpmsg is conceptually a child of virtio: when the subsystem was proposed by TI Arnd noted that virtio has large similarities in shared memory transport and as the potential reuse for buffer handling etc was excellent virtio was used as a base for rpmsg/remoteproc work. Yours, Linus Walleij
On Wed 16 Jun 10:52 CDT 2021, Enrico Weigelt, metux IT consult wrote: > On 16.06.21 05:30, Bjorn Andersson wrote: > > > Combined with the virtio-i2c effort this could provide an alternative by > > simply tunneling the busses and GPIOs into Linux and use standard iio > > drivers, for cases where this suits your product requirements better. > > So, you wanna use virtio as logical interface between the two CPUs ? > Interesting idea. Usually folks use rpmsg for those things. > rpmsg is a layer on top of virtio, so this would be an extension of the existing model. There's been discussions (and I believe some implementations) related to bridging I2C requests over rpmsg, but I think it's preferable to standardize around the virtio based bearer directly. > What is running on the secondary CPU ? Some OS like Linux or some bare > metal stuff ? What kind of CPU is that anyways ? > These ideas revolves around platforms that implements something like the "Android Sensor Hub", which provides some resource constraint co-processor that deals with sensor device interaction and processing of the data without waking up the power-hungry ARM cores. Given the focus on power consumption I would guess that these are not going to run Linux. Core-wise I've seen this implemented using primarily ARM and Hexagon cores. Regards, Bjorn
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig index e3607ec4c2e8..7f12fb65d130 100644 --- a/drivers/gpio/Kconfig +++ b/drivers/gpio/Kconfig @@ -1633,6 +1633,15 @@ config GPIO_MOCKUP tools/testing/selftests/gpio/gpio-mockup.sh. Reference the usage in it. +config GPIO_VIRTIO + tristate "VirtIO GPIO support" + depends on VIRTIO + help + Say Y here to enable guest support for virtio-based GPIO controllers. + + These virtual GPIOs can be routed to real GPIOs or attached to + simulators on the host (like QEMU). + endmenu endif diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile index c58a90a3c3b1..ace004c80871 100644 --- a/drivers/gpio/Makefile +++ b/drivers/gpio/Makefile @@ -162,6 +162,7 @@ obj-$(CONFIG_GPIO_UCB1400) += gpio-ucb1400.o obj-$(CONFIG_GPIO_UNIPHIER) += gpio-uniphier.o obj-$(CONFIG_GPIO_VF610) += gpio-vf610.o obj-$(CONFIG_GPIO_VIPERBOARD) += gpio-viperboard.o +obj-$(CONFIG_GPIO_VIRTIO) += gpio-virtio.o obj-$(CONFIG_GPIO_VISCONTI) += gpio-visconti.o obj-$(CONFIG_GPIO_VR41XX) += gpio-vr41xx.o obj-$(CONFIG_GPIO_VX855) += gpio-vx855.o diff --git a/drivers/gpio/gpio-virtio.c b/drivers/gpio/gpio-virtio.c new file mode 100644 index 000000000000..1ba8ddf4693a --- /dev/null +++ b/drivers/gpio/gpio-virtio.c @@ -0,0 +1,326 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * GPIO driver for virtio-based virtual GPIO controllers + * + * Copyright (C) 2021 metux IT consult + * Enrico Weigelt, metux IT consult <info@metux.net> + * + * Copyright (C) 2021 Linaro. + * Viresh Kumar <viresh.kumar@linaro.org> + */ + +#include <linux/completion.h> +#include <linux/err.h> +#include <linux/gpio/driver.h> +#include <linux/io.h> +#include <linux/kernel.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/virtio_config.h> +#include <uapi/linux/virtio_gpio.h> +#include <uapi/linux/virtio_ids.h> + +struct virtio_gpio { + struct virtio_device *vdev; + struct mutex lock; + struct gpio_chip gc; + + struct completion completion; + struct virtqueue *command_vq; + struct virtio_gpio_request creq; + struct virtio_gpio_response cres; + + /* This must be kept as the last entry here, hint: gpio_names[0] */ + struct virtio_gpio_config config; +}; + +#define gpio_chip_to_vgpio(chip) container_of(chip, struct virtio_gpio, gc) + +static int virtio_gpio_req(struct virtio_gpio *vgpio, u16 type, u16 gpio, + u8 txdata, u8 *rxdata) +{ + struct virtio_gpio_response *res = &vgpio->cres; + struct virtio_gpio_request *req = &vgpio->creq; + struct scatterlist *sgs[2], req_sg, res_sg; + struct device *dev = &vgpio->vdev->dev; + unsigned long time_left; + unsigned int len; + int ret; + + req->type = cpu_to_le16(type); + req->gpio = cpu_to_le16(gpio); + req->data = txdata; + + sg_init_one(&req_sg, req, sizeof(*req)); + sg_init_one(&res_sg, res, sizeof(*res)); + sgs[0] = &req_sg; + sgs[1] = &res_sg; + + mutex_lock(&vgpio->lock); + ret = virtqueue_add_sgs(vgpio->command_vq, sgs, 1, 1, res, GFP_KERNEL); + if (ret) { + dev_err(dev, "failed to add request to vq\n"); + goto out; + } + + reinit_completion(&vgpio->completion); + virtqueue_kick(vgpio->command_vq); + + time_left = wait_for_completion_timeout(&vgpio->completion, HZ / 10); + if (!time_left) { + dev_err(dev, "virtio GPIO backend timeout\n"); + return -ETIMEDOUT; + } + + WARN_ON(res != virtqueue_get_buf(vgpio->command_vq, &len)); + if (unlikely(res->status != VIRTIO_GPIO_STATUS_OK)) { + dev_err(dev, "virtio GPIO request failed: %d\n", gpio); + return -EINVAL; + } + + if (rxdata) + *rxdata = res->data; + +out: + mutex_unlock(&vgpio->lock); + + return ret; +} + +static int virtio_gpio_request(struct gpio_chip *gc, unsigned int gpio) +{ + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); + + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_ACTIVATE, gpio, 0, NULL); +} + +static void virtio_gpio_free(struct gpio_chip *gc, unsigned int gpio) +{ + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); + + virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DEACTIVATE, gpio, 0, NULL); +} + +static int virtio_gpio_get_direction(struct gpio_chip *gc, unsigned int gpio) +{ + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); + u8 direction; + int ret; + + ret = virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_GET_DIRECTION, gpio, 0, + &direction); + if (ret) + return ret; + + return direction; +} + +static int virtio_gpio_direction_input(struct gpio_chip *gc, unsigned int gpio) +{ + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); + + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DIRECTION_IN, gpio, 0, + NULL); +} + +static int virtio_gpio_direction_output(struct gpio_chip *gc, unsigned int gpio, + int value) +{ + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); + + return virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_DIRECTION_OUT, gpio, (u8) + value, NULL); +} + +static int virtio_gpio_get(struct gpio_chip *gc, unsigned int gpio) +{ + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); + u8 value; + int ret; + + ret = virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_GET_VALUE, gpio, 0, &value); + if (ret) + return ret; + + return value; +} + +static void virtio_gpio_set(struct gpio_chip *gc, unsigned int gpio, int value) +{ + struct virtio_gpio *vgpio = gpio_chip_to_vgpio(gc); + + virtio_gpio_req(vgpio, VIRTIO_GPIO_REQ_SET_VALUE, gpio, value, NULL); +} + +static void virtio_gpio_command(struct virtqueue *vq) +{ + struct virtio_gpio *vgpio = vq->vdev->priv; + + complete(&vgpio->completion); +} + +static int virtio_gpio_alloc_vqs(struct virtio_device *vdev) +{ + struct virtio_gpio *vgpio = vdev->priv; + const char * const names[] = { "command" }; + vq_callback_t *cbs[] = { + virtio_gpio_command, + }; + struct virtqueue *vqs[1] = {NULL}; + int ret; + + ret = virtio_find_vqs(vdev, 1, vqs, cbs, names, NULL); + if (ret) { + dev_err(&vdev->dev, "failed to allocate vqs: %d\n", ret); + return ret; + } + + vgpio->command_vq = vqs[0]; + + /* Mark the device ready to perform operations from within probe() */ + virtio_device_ready(vgpio->vdev); + return 0; +} + +static void virtio_gpio_free_vqs(struct virtio_device *vdev) +{ + vdev->config->reset(vdev); + vdev->config->del_vqs(vdev); +} + +static const char **parse_gpio_names(struct virtio_device *vdev, + struct virtio_gpio_config *config) +{ + struct device *dev = &vdev->dev; + char *str = config->gpio_names; + const char **names; + int i, len; + + if (!config->gpio_names_size) + return NULL; + + names = devm_kcalloc(dev, config->ngpio, sizeof(names), GFP_KERNEL); + if (!names) + return ERR_PTR(-ENOMEM); + + /* NULL terminate the string instead of checking it */ + config->gpio_names[config->gpio_names_size - 1] = '\0'; + + for (i = 0; i < config->ngpio; i++) { + /* + * We have already marked the last byte with '\0' + * earlier, this shall be enough here. + */ + if (str == config->gpio_names + config->gpio_names_size) { + dev_err(dev, "Invalid gpio_names\n"); + return ERR_PTR(-EINVAL); + } + + len = strlen(str); + if (!len) { + dev_err(dev, "Missing GPIO name: %d\n", i); + return ERR_PTR(-EINVAL); + } + + names[i] = str; + str += len + 1; + } + + return names; +} + +static int virtio_gpio_probe(struct virtio_device *vdev) +{ + struct virtio_gpio_config *config; + struct device *dev = &vdev->dev; + struct virtio_gpio *vgpio; + u32 size; + int ret; + + virtio_cread(vdev, struct virtio_gpio_config, gpio_names_size, &size); + size = cpu_to_le32(size); + + vgpio = devm_kzalloc(dev, sizeof(*vgpio) + size, GFP_KERNEL); + if (!vgpio) + return -ENOMEM; + + config = &vgpio->config; + + /* Read configuration */ + virtio_cread_bytes(vdev, 0, config, sizeof(*config) + size); + + /* NULL terminate the string instead of checking it */ + config->name[sizeof(config->name) - 1] = '\0'; + config->ngpio = cpu_to_le16(config->ngpio); + config->gpio_names_size = cpu_to_le32(config->gpio_names_size); + + if (!config->ngpio) { + dev_err(dev, "Number of GPIOs can't be zero\n"); + return -EINVAL; + } + + vgpio->vdev = vdev; + vgpio->gc.request = virtio_gpio_request; + vgpio->gc.free = virtio_gpio_free; + vgpio->gc.get_direction = virtio_gpio_get_direction; + vgpio->gc.direction_input = virtio_gpio_direction_input; + vgpio->gc.direction_output = virtio_gpio_direction_output; + vgpio->gc.get = virtio_gpio_get; + vgpio->gc.set = virtio_gpio_set; + vgpio->gc.ngpio = config->ngpio; + vgpio->gc.base = -1; /* Allocate base dynamically */ + vgpio->gc.label = config->name[0] ? + config->name : dev_name(dev); + vgpio->gc.parent = dev; + vgpio->gc.owner = THIS_MODULE; + vgpio->gc.can_sleep = true; + vgpio->gc.names = parse_gpio_names(vdev, config); + if (IS_ERR(vgpio->gc.names)) + return PTR_ERR(vgpio->gc.names); + + vdev->priv = vgpio; + mutex_init(&vgpio->lock); + init_completion(&vgpio->completion); + + ret = virtio_gpio_alloc_vqs(vdev); + if (ret) + return ret; + + ret = gpiochip_add(&vgpio->gc); + if (ret) { + virtio_gpio_free_vqs(vdev); + dev_err(dev, "Failed to add virtio-gpio controller\n"); + } + + return ret; +} + +static void virtio_gpio_remove(struct virtio_device *vdev) +{ + struct virtio_gpio *vgpio = vdev->priv; + + gpiochip_remove(&vgpio->gc); + virtio_gpio_free_vqs(vdev); +} + +static const struct virtio_device_id id_table[] = { + { VIRTIO_ID_GPIO, VIRTIO_DEV_ANY_ID }, + {}, +}; +MODULE_DEVICE_TABLE(virtio, id_table); + +static struct virtio_driver virtio_gpio_driver = { + .id_table = id_table, + .probe = virtio_gpio_probe, + .remove = virtio_gpio_remove, + .driver = { + .name = KBUILD_MODNAME, + .owner = THIS_MODULE, + }, +}; +module_virtio_driver(virtio_gpio_driver); + +MODULE_AUTHOR("Enrico Weigelt, metux IT consult <info@metux.net>"); +MODULE_AUTHOR("Viresh Kumar <viresh.kumar@linaro.org>"); +MODULE_DESCRIPTION("VirtIO GPIO driver"); +MODULE_LICENSE("GPL"); diff --git a/include/uapi/linux/virtio_gpio.h b/include/uapi/linux/virtio_gpio.h new file mode 100644 index 000000000000..e805e33a79cb --- /dev/null +++ b/include/uapi/linux/virtio_gpio.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */ + +#ifndef _LINUX_VIRTIO_GPIO_H +#define _LINUX_VIRTIO_GPIO_H + +#include <linux/types.h> + +/* Virtio GPIO request types */ +#define VIRTIO_GPIO_REQ_ACTIVATE 0x01 +#define VIRTIO_GPIO_REQ_DEACTIVATE 0x02 +#define VIRTIO_GPIO_REQ_GET_DIRECTION 0x03 +#define VIRTIO_GPIO_REQ_DIRECTION_IN 0x04 +#define VIRTIO_GPIO_REQ_DIRECTION_OUT 0x05 +#define VIRTIO_GPIO_REQ_GET_VALUE 0x06 +#define VIRTIO_GPIO_REQ_SET_VALUE 0x07 + +/* Possible values of the status field */ +#define VIRTIO_GPIO_STATUS_OK 0x0 +#define VIRTIO_GPIO_STATUS_ERR 0x1 + +struct virtio_gpio_config { + char name[32]; + __u16 ngpio; + __u16 padding; + __u32 gpio_names_size; + char gpio_names[0]; +} __packed; + +/* Virtio GPIO Request / Response */ +struct virtio_gpio_request { + __u16 type; + __u16 gpio; + __u8 data; +}; + +struct virtio_gpio_response { + __u8 status; + __u8 data; +}; + +#endif /* _LINUX_VIRTIO_GPIO_H */ diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h index b89391dff6c9..0c24e8ae2499 100644 --- a/include/uapi/linux/virtio_ids.h +++ b/include/uapi/linux/virtio_ids.h @@ -55,5 +55,6 @@ #define VIRTIO_ID_PMEM 27 /* virtio pmem */ #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */ #define VIRTIO_ID_I2C_ADAPTER 34 /* virtio i2c adapter */ +#define VIRTIO_ID_GPIO 41 /* virtio GPIO */ #endif /* _LINUX_VIRTIO_IDS_H */