Re: [PATCH 8/8] virtio_ring.h: do not include <stdint.h> from exported header

Re: [PATCH 8/8] virtio_ring.h: do not include <stdint.h> from exported header

[Arnd Bergmann]

On Tue, Apr 5, 2022 at 7:35 AM Christoph Hellwig <hch@infradead.org> wrote:
>
> On Mon, Apr 04, 2022 at 10:04:02AM +0200, Arnd Bergmann wrote:
> > The header is shared between kernel and other projects using virtio, such as
> > qemu and any boot loaders booting from virtio devices. It's not technically a
> > /kernel/ ABI, but it is an ABI and for practical reasons the kernel version is
> > maintained as the master copy if I understand it correctly.
>
> Besides that fact that as you correctly states these are not a UAPI at
> all, qemu and bootloades are not specific to Linux and can't require a
> specific kernel version.  So the same thing we do for file system
> formats or network protocols applies here:  just copy the damn header.
> And as stated above any reasonably portable userspace needs to have a
> copy anyway.

I think the users all have their own copies, at least the ones I could
find on codesearch.debian.org. However, there are 27 virtio_*.h
files in include/uapi/linux that probably should stay together for
the purpose of defining the virtio protocol, and some others might
be uapi relevant.

I see that at least include/uapi/linux/vhost.h has ioctl() definitions
in it, and includes the virtio_ring.h header indirectly.

Adding the virtio maintainers to Cc to see if they can provide
more background on this.

> If it is just as a "master copy" it can live in drivers/virtio/, just
> like we do for other formats.

It has to be in include/linux/ at least because it's used by a number
of drivers outside of drivers/virtio/.

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

Re: [PATCH 8/8] virtio_ring.h: do not include <stdint.h> from exported header

[Christoph Hellwig]

On Tue, Apr 05, 2022 at 08:29:36AM +0200, Arnd Bergmann wrote:
> I think the users all have their own copies, at least the ones I could
> find on codesearch.debian.org. However, there are 27 virtio_*.h
> files in include/uapi/linux that probably should stay together for
> the purpose of defining the virtio protocol, and some others might
> be uapi relevant.
> 
> I see that at least include/uapi/linux/vhost.h has ioctl() definitions
> in it, and includes the virtio_ring.h header indirectly.

Uhh.  We had a somilar mess (but at a smaller scale) in nvme, where
the uapi nvme.h contained both the UAPI and the protocol definition.
We took a hard break to only have a nvme_ioctl.h in the uapi header
and linux/nvme.h for the protocol.  This did break a bit of userspace
compilation (but not running obviously) at the time, but really made
the headers much easier to main.  Some userspace keeps on copying
nvme.h with the protocol definitions.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 8/8] virtio_ring.h: do not include <stdint.h> from exported header

[Michael S. Tsirkin]

On Tue, Apr 05, 2022 at 12:01:35AM -0700, Christoph Hellwig wrote:
> On Tue, Apr 05, 2022 at 08:29:36AM +0200, Arnd Bergmann wrote:
> > I think the users all have their own copies, at least the ones I could
> > find on codesearch.debian.org. However, there are 27 virtio_*.h
> > files in include/uapi/linux that probably should stay together for
> > the purpose of defining the virtio protocol, and some others might
> > be uapi relevant.
> > 
> > I see that at least include/uapi/linux/vhost.h has ioctl() definitions
> > in it, and includes the virtio_ring.h header indirectly.
> 
> Uhh.  We had a somilar mess (but at a smaller scale) in nvme, where
> the uapi nvme.h contained both the UAPI and the protocol definition.
> We took a hard break to only have a nvme_ioctl.h in the uapi header
> and linux/nvme.h for the protocol.  This did break a bit of userspace
> compilation (but not running obviously) at the time, but really made
> the headers much easier to main.  Some userspace keeps on copying
> nvme.h with the protocol definitions.

So far we are quite happy with the status quo, I don't see any issues
maintaining the headers. And yes, through vhost and vringh they are part
of UAPI.

Yes users have their own copies but they synch with the kernel.

That's generally. Specifically the vring_init thing is a legacy thingy
used by kvmtool and maybe others, and it inits the ring in the way that
vring/virtio expect.  Has been there since day 1 and we are careful not
to add more stuff like that, so I don't see a lot of gain from incurring
this pain for users.

-- 
MST

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

Re: [PATCH 8/8] virtio_ring.h: do not include <stdint.h> from exported header

[Michael S. Tsirkin]

On Tue, Apr 05, 2022 at 08:29:36AM +0200, Arnd Bergmann wrote:
> On Tue, Apr 5, 2022 at 7:35 AM Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Mon, Apr 04, 2022 at 10:04:02AM +0200, Arnd Bergmann wrote:
> > > The header is shared between kernel and other projects using virtio, such as
> > > qemu and any boot loaders booting from virtio devices. It's not technically a
> > > /kernel/ ABI, but it is an ABI and for practical reasons the kernel version is
> > > maintained as the master copy if I understand it correctly.
> >
> > Besides that fact that as you correctly states these are not a UAPI at
> > all, qemu and bootloades are not specific to Linux and can't require a
> > specific kernel version.  So the same thing we do for file system
> > formats or network protocols applies here:  just copy the damn header.
> > And as stated above any reasonably portable userspace needs to have a
> > copy anyway.
> 
> I think the users all have their own copies, at least the ones I could
> find on codesearch.debian.org.

kvmtool does not seem to have its own copy, just grep vring_init.

> However, there are 27 virtio_*.h
> files in include/uapi/linux that probably should stay together for
> the purpose of defining the virtio protocol, and some others might
> be uapi relevant.
> 
> I see that at least include/uapi/linux/vhost.h has ioctl() definitions
> in it, and includes the virtio_ring.h header indirectly.
> 
> Adding the virtio maintainers to Cc to see if they can provide
> more background on this.
> 
> > If it is just as a "master copy" it can live in drivers/virtio/, just
> > like we do for other formats.
> 
> It has to be in include/linux/ at least because it's used by a number
> of drivers outside of drivers/virtio/.
> 
>         Arnd
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
> 

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