Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

[Randy Dunlap]

On 11/27/20 10:30 AM, Enrico Weigelt, metux IT consult wrote:
> diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
> index 5d4de5cd6759..e8414d82cf75 100644
> --- a/drivers/gpio/Kconfig
> +++ b/drivers/gpio/Kconfig
> @@ -1613,4 +1613,13 @@ 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 GPIOs.

	                                         virtio-based

> +
> +	  These virtual gpios can be routed to real GPIOs or attached to

	                GPIOs

> +	  simulators on the host (qemu).
> +
>  endif


-- 
~Randy

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 2305 bytes --]

Hi "Enrico,

I love your patch! Yet something to improve:

[auto build test ERROR on gpio/for-next]
[also build test ERROR on linux/master linus/master v5.10-rc5 next-20201127]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
base:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-gpio.git for-next
config: c6x-randconfig-p002-20201128 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/a134ec4827b0ffb7edd6a292238bd93fb377f127
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Enrico-Weigelt-metux-IT-consult/drivers-gpio-add-virtio-gpio-guest-driver/20201128-023506
        git checkout a134ec4827b0ffb7edd6a292238bd93fb377f127
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=c6x 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> error: include/uapi/linux/virtio_gpio.h: missing "WITH Linux-syscall-note" for SPDX-License-Identifier
   make[2]: *** [scripts/Makefile.headersinst:63: usr/include/linux/virtio_gpio.h] Error 1
   make[2]: Target '__headers' not remade because of errors.
   make[1]: *** [Makefile:1288: headers] Error 2
   arch/c6x/kernel/asm-offsets.c:14:6: warning: no previous prototype for 'foo' [-Wmissing-prototypes]
      14 | void foo(void)
         |      ^~~
   <stdin>:1511:2: warning: #warning syscall clone3 not implemented [-Wcpp]
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [Makefile:185: __sub-make] Error 2
   make: Target 'prepare' not remade because of errors.

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25224 bytes --]

[-- Attachment #3: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

[Michael S. Tsirkin]

On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> diff --git a/include/uapi/linux/virtio_ids.h b/include/uapi/linux/virtio_ids.h
> index b052355ac7a3..85772c0bcb4b 100644
> --- a/include/uapi/linux/virtio_ids.h
> +++ b/include/uapi/linux/virtio_ids.h
> @@ -48,5 +48,6 @@
>  #define VIRTIO_ID_FS           26 /* virtio filesystem */
>  #define VIRTIO_ID_PMEM         27 /* virtio pmem */
>  #define VIRTIO_ID_MAC80211_HWSIM 29 /* virtio mac80211-hwsim */
> +#define VIRTIO_ID_GPIO           30 /* virtio GPIO */


Pls remember to reserve the ID with the virtio TC
before using it in the driver. Thanks!

>  #endif /* _LINUX_VIRTIO_IDS_H */
> -- 
> 2.11.0

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

[Jonathan Neuschäfer]

[-- Attachment #1.1: Type: text/plain, Size: 585 bytes --]

Hi,

On Fri, Nov 27, 2020 at 07:30:03PM +0100, Enrico Weigelt, metux IT consult wrote:
> Introducing new gpio driver for virtual GPIO devices via virtio.
> 
> The driver allows routing gpio control into VM guests, eg. brigding
> virtual gpios to specific host gpios, or attaching simulators for
> automatic application testing.
> 
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

is there a spec of the virtio-gpio protocol available? If so, linking it
in the commit message and/or driver/Kconfig would be nice.


Best regards,
Jonathan Neuschäfer

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 183 bytes --]

_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

[Linus Walleij]

Hi Enrico,

On Tue, Jun 15, 2021 at 7:49 PM Enrico Weigelt, metux IT consult
<info@metux.net> wrote:

> Introduce new GPIO driver for virtual GPIO devices via virtio.
>
> The driver implements the virtio-gpio protocol (ID 41), which can be
> used by either VM guests (e.g. bridging virtual gpios from the guest
> to real gpios in the host or attaching simulators for automatic
> application testing), as well as virtio-gpio hardware devices.
>
> Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>

So now there are two contesting patches for this and that creates a
social problem for us as maintainers. I am not too happy about that.

This situation activates the kernel management style document so
I advise involved parties to familiarize themselves with it:
https://www.kernel.org/doc/html/latest/process/management-style.html

Can we get the discussion down to actual technical points?
We really need a virtio GPIO driver, no doubt, so if everyone could
just work toward that goal and compromise with their specific pet
peeves that would be great.

Yours,
Linus Walleij
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

[Viresh Kumar]

On 16-06-21, 10:31, Linus Walleij wrote:
> Hi Enrico,
> 
> On Tue, Jun 15, 2021 at 7:49 PM Enrico Weigelt, metux IT consult
> <info@metux.net> wrote:
> 
> > Introduce new GPIO driver for virtual GPIO devices via virtio.
> >
> > The driver implements the virtio-gpio protocol (ID 41), which can be
> > used by either VM guests (e.g. bridging virtual gpios from the guest
> > to real gpios in the host or attaching simulators for automatic
> > application testing), as well as virtio-gpio hardware devices.
> >
> > Signed-off-by: Enrico Weigelt, metux IT consult <info@metux.net>
> 
> So now there are two contesting patches for this and that creates a
> social problem for us as maintainers. I am not too happy about that.
> 
> This situation activates the kernel management style document so
> I advise involved parties to familiarize themselves with it:
> https://www.kernel.org/doc/html/latest/process/management-style.html
> 
> Can we get the discussion down to actual technical points?

+1

I can not agree more to this.

> We really need a virtio GPIO driver, no doubt, so if everyone could
> just work toward that goal and compromise with their specific pet
> peeves that would be great.

Enrico,

I am not looking to get any credits for the code or spec here. I don't
really care about that. For the very same reason I kept you as the
author of the 1st patch in the kernel series, so git keeps showing you
as the original author.

All I wanted to work on was the backend (in rust). This is what
happened for I2C for example, Jie Deng (Intel) worked on the spec and
Linux driver and I helped review it, make him fix a thing or two and
that's all. I worked on the rust implementation for the backend then.

You only ever sent 1 real versions of the Linux driver, that too
"6-months-back", there were no real blockers anywhere and you never
attempted to upstream anything.

Similarly, you "never" sent the specification properly to the virtio
lists for review. You sent it once as an attachment to an email, which
no one ever received.

When I tried to move this forward, invested a lot of time into making
it better from specification to code, reviews started happening, you
decided to start blocking it again.

You should be rather happy that something that you worked on is making
progress, specially when you didn't get time to do the same.

You wrote this in your patch:

> > Status:
> >     * this driver is now field tested for about 6 month
> >       (against KVM+Qemu as well as some HW/FPGA implementation)

Linux upstream doesn't really care about this, you can ask any Linux
Maintainer about this. If your code and specification isn't doing the
right thing, and isn't good enough, you will be asked to update it
upon reviews.

YOU JUST CAN'T SAY I WON'T because I have products based on this
version.

This is not how any open source project works. The code and
specification here doesn't belong to a single person or company. It is
for everyone's use.

> >     * virtio device ID officially allocated

Correct.

> >     * virtio spec has been submitted to virtio TC

Which specification are you talking about here ? The only
specification I can see on virtio lists is the one I sent.

And the driver you tried to send isn't aligned to that for sure, and
it takes us back from all the improvements I tried to do.

I am not saying that my version of the specification is the best and
there is no flaw in it. There surely is, but that can't be fixed by
sending another version of it. You need to make a technical point
about it and convince people that what you are saying is correct and
it is required for your use-case (not existing downstream solution).

There is no point going backwards now after making so much of
progress. Even if you try to send your version, it will slowly and
slowly reach very close to my latest version of code and spec. Because
your version of the code and spec weren't good enough for everyone. It
doesn't matter if you have real products on your earlier version, you
can keep using that in your downstream solution, but Linux kernel and
specification are going to get an improved version (from yours or
mine, but that doesn't matter here). You need to accept that changes
to that are inevitable since there are many users of gpio-virtio, not
just you and me, but many more (Like Bjorn expressed his interest in
this today for Qcom stuff).

Again, it would be better if you can discuss further on technical
merits or demerits in the currently circulated specification and give
your invaluable suggestions on the same.

Else we will end up spending few more months with just this and it
won't get us anywhere.

Thanks.

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

[Viresh Kumar]

On 16-06-21, 17:04, Enrico Weigelt, metux IT consult wrote:
> Half correct: I sent it to the list, but this wasn't tex'ified yet.
> 
> When we had an email conversation about this, it was about submitting
> the existing spec in a formal correct way. Don't get me wrong: I
> apreciate that somebody's doing the beaurocratic work. But still have
> no idea why you changed it completely, so there's quite nothing left
> but the name and that it somehow does gpio via virtio.

> The one I've resent (now texified) a few days ago. It had been submitted
> in ascii form last year. The answer from virtio TC folks whas that there
> are some formal steps to be done and it needs to be patched int their
> tex document.

Okay, we figured out now that you _haven't_ subscribed to virtio lists
and so your stuff never landed in anyone's inbox. But you did send
something and didn't completely went away.

Since you started this all and still want to do it, I will take my
patches back and let you finish with what you started. I will help
review them.

Please start with specification first, and resend them as soon as
possible. So we can start with reviews there.

Also please cc relevant people directly, like GPIO maintainers in
kernel and few more from CC list of this email, as most of these
people aren't subscribed to virtio lists, they will never get your
patches otherwise. Lets get over this once and for all.

> You sound like a politician that tries to push an hidden agenda,
> made by some secret interest group in the back room, against the
> people - like "resistance is futile".

:)

-- 
viresh
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] drivers: gpio: add virtio-gpio guest driver

[Viresh Kumar]

On 17-06-21, 11:54, Enrico Weigelt, metux IT consult wrote:
> Actually, I am subscribed in the list. We already had debates on it,
> including on your postings (but also other things).

Right.

> And the ascii
> version of the spec actually landed on the list last year, we had
> discussions about it there.

I tried to search for it earlier, but never found anything on virtio
list.  Maybe I missed it then.

> I've just had the problem that my patches didn't go through, which is
> very strange, since I actually am on the list and other mails of mine
> went through all the time. I'm now suspecting it's triggered by some
> subtle difference between my regular mail clients and git send-email.
> 
> > Since you started this all and still want to do it, I will take my
> > patches back and let you finish with what you started. I will help
> > review them.
> 
> Thank you very much.
> 
> Please don't me wrong, I really don't wanna any kind of power play, just
> wanna get an technically good solution. If there had been any mis-
> understandings at that point, I'm officially saying sorry here.

Its okay, we are both trying to make things better here :)

> Let's be friends.
> 
> You mentioned you've been missing with my spec. Please come foreward and
> tell us what exactly you're missing and what your use cases are.

I have sent a detailed review of your spec patch, lets do it there
point by point :)

> Note that I've intentionally left off certain "more sophisticated"
> functionality we find on *some* gpio controllers, eg. per-line irq
> masking, pinmux settings for several reasons, e.g.:
> 
> * those are only implemented by some hardware
> * often implemented in or at least need to be coordinated with other
>   pieces of hw (e.g. in SoCs, pinmux is usually done in a separate
>   device)
> * it shall be possible to support even the most simple devices and
>   have the more sophisticated things totally optional. minium
>   requirements for silicon implementations should be the lowest possible
>   (IOW: minimal number of logic gates)
> 
> >> You sound like a politician that tries to push an hidden agenda,
> >> made by some secret interest group in the back room, against the
> >> people - like "resistance is futile".
> >
> > :)
> 
> Perhaps I've been a bit overreacting at that point. But: this is really
> that kind of talking we hear from politicians and corporate leaders
> since many years, whenever they wanna push something through that we the
> people don't want. Politicians use that as a social engineering tool for
> demotivating any resistance. Over heare in Germany this even had become
> a meme, and folks from CCC made a radio show about and named by that
> (the German word is "alternativlos" - in english: without any
> alternative). No idea about other countries, maybe it's a cultural
> issue, but over here, those kind of talking had become a red light.
> 
> Of course, I never intended to accuse you of being one of these people.
> Sorry if there's been misunderstanding.

It sounded strange yesterday to be honest, but I have gone past it
already :)
 
> Let's get back to your implementation: you've mentioned you're routing
> raw virtio traffic into userland, to some other process (outside VMMs
> like qemu) - how exactly are you doing that ?
> 
> That could be interesting for completely different scenarios. For
> example, I'm currently exploring how to get VirGL running between separate
> processes running under the same kernel instance (fow now we
> only have the driver side inside VM and the device outside it), means
> driver and device are running as separate processes.
> 
> The primary use case are containers that shall have really GPU generic
> drivers, not knowing anything about the actual hardware on the host.
> Currently, container workloads wanting to use a GPU need to have special
> drivers for exactly the HW the host happens to have. This makes generic,
> portable container images a tuff problem.
> 
> I haven't digged deeply into the matter, but some virtio-tap transport
> could be an relatively easy (probably not the most efficient) way to
> solve this problem. In that scanario it would like this:
> 
> * we have a "virgl server" (could be some X or wayland application, or
>   completely own compositor) opens up the device-end of an "virtio-tap"
>   transport and attaches its virtio-gpio device emulation on it.
> * "virtio-tap" now creates a driver-end, kernel probes an virtio-gpu
>   instance on this (also leading to a new DRI device)
> * container runtime picks the new DRI device and maps it into the
>   container(s)
>   [ yet open question, whether one DRI device for many containers
>     is enough ]
> * container application sees that virtio-gpu DRI device and speaks to
>   it (mesa->virgl backend)
> * the "virgl-server" receives buffers and commands from via virtio and
>   sends them to the host's GL or Gallium API.
> 
> Once we're already there, we might think whether it could make sense
> putting virtio routing into kvm itself, instead of letting qemu catch
> page faults and virtual irqs. Yet have to see whether that's a good
> idea, but I can imagine some performance improvements here.

We (at Linaro) work on software enablement normally and not end
products (rarely that happen though), like framework level work in the
kernel which can later be used by everyone to build their drivers on.

There are many companies like Qualcomm, ST Micro, etc, who want to use
Virtio in general for Automotive or other applications / solution. The
purpose of Project Stratos [1], an initiative of Linaro, is working
towards developing hypervisor agnostic Virtio interfaces and
standards. The end products and applications will be worked on by the
members directly and we need to add basic minimum support, with all
the generally required APIs or interfaces.

-- 
viresh

[1] https://linaro.atlassian.net/wiki/spaces/STR/overview
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization