diff mbox

[v2,00/11] qemu: use virtio linux headers in portable code

Message ID 20130526181017.GB3115@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin May 26, 2013, 6:10 p.m. UTC
On Sun, May 26, 2013 at 07:00:58PM +0100, Peter Maydell wrote:
> On 26 May 2013 18:51, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Sun, May 26, 2013 at 04:43:57PM +0100, Peter Maydell wrote:
> >> This series breaks compilation on MacOSX:
> >>
> >>   CC    net/eth.o
> >> In file included from net/eth.c:18:
> >> In file included from /Users/pm215/src/qemu/include/net/eth.h:29:
> >> /Users/pm215/src/qemu/linux-headers/linux/if_ether.h:24:10: fatal
> >> error: 'linux/types.h' file not found
> >> #include <linux/types.h>
> >>          ^
> >> 1 error generated.
> >> make: *** [net/eth.o] Error 1
> 
> > Could be a stale file in your tree ...
> > Did configure get re-run?
> > Could you post the config-host.mak file please?
> 
> Configure did get rerun, but where are you expecting the header
> to pull linux/types.h from?  Your patchset adds files which include
> it but doesn't add any copy itself, so if you're not on a Linux
> host then it just doesn't exist...
> 
> config-host.mak attached.
> 
> thanks
> -- PMM


Ouch. Forgot to git-add them. Thanks.

I'll send a fixed version -
could you please try this patch on top?


    linux-stubs: linux/types.h
    
    This adds a directory with minimal stubs that
    make it possible to use some linux headers
    in qemu in a portable environment.
    
    Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

---

Comments

Paolo Bonzini May 26, 2013, 6:26 p.m. UTC | #1
Il 26/05/2013 20:10, Michael S. Tsirkin ha scritto:
> On Sun, May 26, 2013 at 07:00:58PM +0100, Peter Maydell wrote:
>> On 26 May 2013 18:51, Michael S. Tsirkin <mst@redhat.com> wrote:
>>> On Sun, May 26, 2013 at 04:43:57PM +0100, Peter Maydell wrote:
>>>> This series breaks compilation on MacOSX:
>>>>
>>>>   CC    net/eth.o
>>>> In file included from net/eth.c:18:
>>>> In file included from /Users/pm215/src/qemu/include/net/eth.h:29:
>>>> /Users/pm215/src/qemu/linux-headers/linux/if_ether.h:24:10: fatal
>>>> error: 'linux/types.h' file not found
>>>> #include <linux/types.h>
>>>>          ^
>>>> 1 error generated.
>>>> make: *** [net/eth.o] Error 1
>>
>>> Could be a stale file in your tree ...
>>> Did configure get re-run?
>>> Could you post the config-host.mak file please?
>>
>> Configure did get rerun, but where are you expecting the header
>> to pull linux/types.h from?  Your patchset adds files which include
>> it but doesn't add any copy itself, so if you're not on a Linux
>> host then it just doesn't exist...
>>
>> config-host.mak attached.
>>
>> thanks
>> -- PMM
> 
> 
> Ouch. Forgot to git-add them. Thanks.
> 
> I'll send a fixed version -
> could you please try this patch on top?
> 
> 
>     linux-stubs: linux/types.h
>     
>     This adds a directory with minimal stubs that
>     make it possible to use some linux headers
>     in qemu in a portable environment.
>     
>     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> 
> ---
> 
> diff --git a/linux-stubs/linux/types.h b/linux-stubs/linux/types.h
> new file mode 100644
> index 0000000..8642cbb
> --- /dev/null
> +++ b/linux-stubs/linux/types.h
> @@ -0,0 +1,37 @@
> +#ifndef STUBS_LINUX_TYPES_H
> +#define STUBS_LINUX_TYPES_H
> +
> +#include <stdint.h>
> +
> +/*
> + * Below are Linux-specific types that should never collide with
> + * any application/library that wants linux/types.h.
> + */
> +
> +typedef uint8_t __u8;
> +typedef uint16_t __u16;
> +typedef uint32_t __u32;
> +typedef uint64_t __u64;
> +
> +#ifdef __CHECKER__
> +#define __bitwise__ __attribute__((bitwise))
> +#else
> +#define __bitwise__
> +#endif
> +#ifdef __CHECK_ENDIAN__
> +#define __bitwise __bitwise__
> +#else
> +#define __bitwise
> +#endif
> +
> +typedef __u16 __bitwise __le16;
> +typedef __u16 __bitwise __be16;
> +typedef __u32 __bitwise __le32;
> +typedef __u32 __bitwise __be32;
> +typedef __u64 __bitwise __le64;
> +typedef __u64 __bitwise __be64;
> +
> +typedef __u16 __bitwise __sum16;
> +typedef __u32 __bitwise __wsum;
> +

I don't like defining __-prefixed types.  Can we preprocess
linux-headers to avoid usage of __u8/16/32/64, and to
s,linux/types.h,stdint.h, ?

Paolo

> +#endif
> 
> 
>
Michael S. Tsirkin May 26, 2013, 6:37 p.m. UTC | #2
On Sun, May 26, 2013 at 08:26:14PM +0200, Paolo Bonzini wrote:
> Il 26/05/2013 20:10, Michael S. Tsirkin ha scritto:
> > On Sun, May 26, 2013 at 07:00:58PM +0100, Peter Maydell wrote:
> >> On 26 May 2013 18:51, Michael S. Tsirkin <mst@redhat.com> wrote:
> >>> On Sun, May 26, 2013 at 04:43:57PM +0100, Peter Maydell wrote:
> >>>> This series breaks compilation on MacOSX:
> >>>>
> >>>>   CC    net/eth.o
> >>>> In file included from net/eth.c:18:
> >>>> In file included from /Users/pm215/src/qemu/include/net/eth.h:29:
> >>>> /Users/pm215/src/qemu/linux-headers/linux/if_ether.h:24:10: fatal
> >>>> error: 'linux/types.h' file not found
> >>>> #include <linux/types.h>
> >>>>          ^
> >>>> 1 error generated.
> >>>> make: *** [net/eth.o] Error 1
> >>
> >>> Could be a stale file in your tree ...
> >>> Did configure get re-run?
> >>> Could you post the config-host.mak file please?
> >>
> >> Configure did get rerun, but where are you expecting the header
> >> to pull linux/types.h from?  Your patchset adds files which include
> >> it but doesn't add any copy itself, so if you're not on a Linux
> >> host then it just doesn't exist...
> >>
> >> config-host.mak attached.
> >>
> >> thanks
> >> -- PMM
> > 
> > 
> > Ouch. Forgot to git-add them. Thanks.
> > 
> > I'll send a fixed version -
> > could you please try this patch on top?
> > 
> > 
> >     linux-stubs: linux/types.h
> >     
> >     This adds a directory with minimal stubs that
> >     make it possible to use some linux headers
> >     in qemu in a portable environment.
> >     
> >     Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > 
> > ---
> > 
> > diff --git a/linux-stubs/linux/types.h b/linux-stubs/linux/types.h
> > new file mode 100644
> > index 0000000..8642cbb
> > --- /dev/null
> > +++ b/linux-stubs/linux/types.h
> > @@ -0,0 +1,37 @@
> > +#ifndef STUBS_LINUX_TYPES_H
> > +#define STUBS_LINUX_TYPES_H
> > +
> > +#include <stdint.h>
> > +
> > +/*
> > + * Below are Linux-specific types that should never collide with
> > + * any application/library that wants linux/types.h.
> > + */
> > +
> > +typedef uint8_t __u8;
> > +typedef uint16_t __u16;
> > +typedef uint32_t __u32;
> > +typedef uint64_t __u64;
> > +
> > +#ifdef __CHECKER__
> > +#define __bitwise__ __attribute__((bitwise))
> > +#else
> > +#define __bitwise__
> > +#endif
> > +#ifdef __CHECK_ENDIAN__
> > +#define __bitwise __bitwise__
> > +#else
> > +#define __bitwise
> > +#endif
> > +
> > +typedef __u16 __bitwise __le16;
> > +typedef __u16 __bitwise __be16;
> > +typedef __u32 __bitwise __le32;
> > +typedef __u32 __bitwise __be32;
> > +typedef __u64 __bitwise __le64;
> > +typedef __u64 __bitwise __be64;
> > +
> > +typedef __u16 __bitwise __sum16;
> > +typedef __u32 __bitwise __wsum;
> > +
> 
> I don't like defining __-prefixed types.  Can we preprocess
> linux-headers to avoid usage of __u8/16/32/64, and to
> s,linux/types.h,stdint.h, ?
> 
> Paolo

Let's not be purists, and do the practical thing.

When I suggested this you didn't like it:
>> I can think of two ways:
>>     - strip linux/types.h in update_headers
>>     - add a stub linux/types.h for non linux platforms
>
>The latter, using stdint.h types, would be fine.

And now, I think it's cleaner to keep it as is.
Most likely, these specific __ types work fine, *if* we
see a problem we can think what's to be done.
In particular, there are __le/__be tags there which can
be useful to do static code analysis.

What's the worst case failure scanario?
Conflict with some system standard header on some
strange target OS, this results in a failed build,
and we go and try to fix it, almost surely with
ifndef around some of the stubs. Why worry now?

> > +#endif
> > 
> > 
> >
Paolo Bonzini May 26, 2013, 6:53 p.m. UTC | #3
Il 26/05/2013 20:37, Michael S. Tsirkin ha scritto:
>> > I don't like defining __-prefixed types.  Can we preprocess
>> > linux-headers to avoid usage of __u8/16/32/64, and to
>> > s,linux/types.h,stdint.h, ?
>> > 
>> > Paolo
> Let's not be purists, and do the practical thing.
> 
> When I suggested this you didn't like it:
>>> >> I can think of two ways:
>>> >>     - strip linux/types.h in update_headers
>>> >>     - add a stub linux/types.h for non linux platforms
>> >
>> >The latter, using stdint.h types, would be fine.

My fault.  I should have looked at linux/types.h (actually asm-generic/).

Paolo
Michael S. Tsirkin May 26, 2013, 8:02 p.m. UTC | #4
On Sun, May 26, 2013 at 08:53:33PM +0200, Paolo Bonzini wrote:
> Il 26/05/2013 20:37, Michael S. Tsirkin ha scritto:
> >> > I don't like defining __-prefixed types.  Can we preprocess
> >> > linux-headers to avoid usage of __u8/16/32/64, and to
> >> > s,linux/types.h,stdint.h, ?
> >> > 
> >> > Paolo
> > Let's not be purists, and do the practical thing.
> > 
> > When I suggested this you didn't like it:
> >>> >> I can think of two ways:
> >>> >>     - strip linux/types.h in update_headers
> >>> >>     - add a stub linux/types.h for non linux platforms
> >> >
> >> >The latter, using stdint.h types, would be fine.
> 
> My fault.  I should have looked at linux/types.h (actually asm-generic/).
> 
> Paolo

Not really, __uX appear in the headers that were posted.
What I'm saying is - a chance of a conflict is very remote,
if it happens it's a build failure so easy to debug.
Paolo Bonzini May 26, 2013, 8:20 p.m. UTC | #5
Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
> 
> Not really, __uX appear in the headers that were posted.
> What I'm saying is - a chance of a conflict is very remote,
> if it happens it's a build failure so easy to debug.

I'm sure that others will complain, :) but you can go ahead.

Paolo
Anthony Liguori May 26, 2013, 8:49 p.m. UTC | #6
Paolo Bonzini <pbonzini@redhat.com> writes:

> Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
>> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
>> 
>> Not really, __uX appear in the headers that were posted.

Which is a problem because this is a reserved namespace in C99.

>> What I'm saying is - a chance of a conflict is very remote,
>> if it happens it's a build failure so easy to debug.
>
> I'm sure that others will complain, :) but you can go ahead.

I think we should clean up the virtio headers in the kernel first.
Seems like a good thing to do given the standardization effort too.

There are lots of headers in uapi that use the C99 int types so there
doesn't seem to be a reason they couldn't be used in virtio.  fuse.h
even has a:

    #ifdef __KERNEL__
    #include <linux/types.h>
    #else
    #include <stdint.h>
    #endif

Which seems like a reasonable thing to do.

The only other kernel dependency is linux/if_ether.h to define
ETH_ALEN.  But it seems completely reasonable to define a
VIRTIO_NET_MAC_ALEN or something like that.

This would make the virtio headers completely stand alone and includable
in userspace (without violating C99).

Perhaps it's even worth moving the headers from uapi/linux to
uapi/virtio.  Rusty, what do you think?

Regards,

Anhtony Liguori

>
> Paolo
Michael S. Tsirkin May 26, 2013, 9:42 p.m. UTC | #7
On Sun, May 26, 2013 at 03:49:53PM -0500, Anthony Liguori wrote:
> Paolo Bonzini <pbonzini@redhat.com> writes:
> 
> > Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
> >> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
> >> 
> >> Not really, __uX appear in the headers that were posted.
> 
> Which is a problem because this is a reserved namespace in C99.
> 
> >> What I'm saying is - a chance of a conflict is very remote,
> >> if it happens it's a build failure so easy to debug.
> >
> > I'm sure that others will complain, :) but you can go ahead.
> 
> I think we should clean up the virtio headers in the kernel first.
> Seems like a good thing to do given the standardization effort too.
> There are lots of headers in uapi that use the C99 int types

I found 4:
$ grep -l uint include/uapi/linux/*h
include/uapi/linux/dm-log-userspace.h
include/uapi/linux/fuse.h
include/uapi/linux/jffs2.h
include/uapi/linux/pps.h
include/uapi/linux/rds.h
include/uapi/linux/sctp.h

That's not really lots.

> so there
> doesn't seem to be a reason they couldn't be used in virtio.  fuse.h
> even has a:
> 
>     #ifdef __KERNEL__
>     #include <linux/types.h>
>     #else
>     #include <stdint.h>
>     #endif
> 
> Which seems like a reasonable thing to do.

In kernel, we really want to use things like endian-ness
checks (and, we really should have them in userspace too!).
So we want __le32 and other kernel goodies
like that. stdint.h won't cut it.

> The only other kernel dependency is linux/if_ether.h to define
> ETH_ALEN.  But it seems completely reasonable to define a
> VIRTIO_NET_MAC_ALEN or something like that.

Ugh. Not really reasonable IMO. We also use ETH_P_IP in code,
would like to get rid of redefining that too.
But we can have our own linux/if_ether.h for non-linux hosts,
just with a
couple of macros like that, it's not a big deal.

> This would make the virtio headers completely stand alone and includable
> in userspace (without violating C99).
> 
> Perhaps it's even worth moving the headers from uapi/linux to
> uapi/virtio.  Rusty, what do you think?
> 
> Regards,
> 
> Anhtony Liguori
> 
> >
> > Paolo
Anthony Liguori May 27, 2013, 12:55 a.m. UTC | #8
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Sun, May 26, 2013 at 03:49:53PM -0500, Anthony Liguori wrote:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>> 
>> > Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
>> >> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
>> >> 
>> >> Not really, __uX appear in the headers that were posted.
>> 
>> Which is a problem because this is a reserved namespace in C99.
>> 
>> >> What I'm saying is - a chance of a conflict is very remote,
>> >> if it happens it's a build failure so easy to debug.
>> >
>> > I'm sure that others will complain, :) but you can go ahead.
>> 
>> I think we should clean up the virtio headers in the kernel first.
>> Seems like a good thing to do given the standardization effort too.
>> There are lots of headers in uapi that use the C99 int types
>
> I found 4:
> $ grep -l uint include/uapi/linux/*h
> include/uapi/linux/dm-log-userspace.h
> include/uapi/linux/fuse.h
> include/uapi/linux/jffs2.h
> include/uapi/linux/pps.h
> include/uapi/linux/rds.h
> include/uapi/linux/sctp.h
>
> That's not really lots.
>
>> so there
>> doesn't seem to be a reason they couldn't be used in virtio.  fuse.h
>> even has a:
>> 
>>     #ifdef __KERNEL__
>>     #include <linux/types.h>
>>     #else
>>     #include <stdint.h>
>>     #endif
>> 
>> Which seems like a reasonable thing to do.
>
> In kernel, we really want to use things like endian-ness
> checks (and, we really should have them in userspace too!).
> So we want __le32 and other kernel goodies
> like that. stdint.h won't cut it.

With the spec being standardized, I think having a stand alone set of
headers is a good thing.  Licensing is problematic here too.

If virtio headers depend on other Linux headers, then it really doesn't
matter if they are BSD licensed if you need a GPL header (like
linux/if_ether.h).

Now, we can certainly debate the copyrightability of these defines and
what have you but if the headers are meant to be 1) consumed outside the
kernel 2) licensed under a different license than the general kernel
then depending on kernel goodies is the wrong strategy.

>> The only other kernel dependency is linux/if_ether.h to define
>> ETH_ALEN.  But it seems completely reasonable to define a
>> VIRTIO_NET_MAC_ALEN or something like that.
>
> Ugh. Not really reasonable IMO. We also use ETH_P_IP in code,
> would like to get rid of redefining that too.
> But we can have our own linux/if_ether.h for non-linux hosts,
> just with a
> couple of macros like that, it's not a big deal.

See above.

Regards,

Anthony Liguori

>
>> This would make the virtio headers completely stand alone and includable
>> in userspace (without violating C99).
>> 
>> Perhaps it's even worth moving the headers from uapi/linux to
>> uapi/virtio.  Rusty, what do you think?
>> 
>> Regards,
>> 
>> Anhtony Liguori
>> 
>> >
>> > Paolo
Peter Maydell May 27, 2013, 10:19 a.m. UTC | #9
On 26 May 2013 19:10, Michael S. Tsirkin <mst@redhat.com> wrote:
> Ouch. Forgot to git-add them. Thanks.
>
> I'll send a fixed version -
> could you please try this patch on top?

With this extra patch MacOSX compiles.

thanks
-- PMM
Rusty Russell May 27, 2013, 11:15 a.m. UTC | #10
Anthony Liguori <anthony@codemonkey.ws> writes:
> Paolo Bonzini <pbonzini@redhat.com> writes:
>
>> Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
>>> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
>>> 
>>> Not really, __uX appear in the headers that were posted.
>
> Which is a problem because this is a reserved namespace in C99.

Personally, I find it hard to care.  What matters is not what the
standard has carved out, but whether we have clashes, reserved namespace
or no.  And that won't happen for these.

If someone wants to convert all the kernel headers, I won't NAK it
though.

> Perhaps it's even worth moving the headers from uapi/linux to
> uapi/virtio.  Rusty, what do you think?

Hmm, #include <virtio/virtio_net.h> etc would be worthwhile if that also
worked on FreeBSD.  Bryan CC'd...

Cheers,
Rusty.
Michael S. Tsirkin May 27, 2013, 3:02 p.m. UTC | #11
On Sun, May 26, 2013 at 07:55:25PM -0500, Anthony Liguori wrote:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Sun, May 26, 2013 at 03:49:53PM -0500, Anthony Liguori wrote:
> >> Paolo Bonzini <pbonzini@redhat.com> writes:
> >> 
> >> > Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
> >> >> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
> >> >> 
> >> >> Not really, __uX appear in the headers that were posted.
> >> 
> >> Which is a problem because this is a reserved namespace in C99.
> >> 
> >> >> What I'm saying is - a chance of a conflict is very remote,
> >> >> if it happens it's a build failure so easy to debug.
> >> >
> >> > I'm sure that others will complain, :) but you can go ahead.
> >> 
> >> I think we should clean up the virtio headers in the kernel first.
> >> Seems like a good thing to do given the standardization effort too.
> >> There are lots of headers in uapi that use the C99 int types
> >
> > I found 4:
> > $ grep -l uint include/uapi/linux/*h
> > include/uapi/linux/dm-log-userspace.h
> > include/uapi/linux/fuse.h
> > include/uapi/linux/jffs2.h
> > include/uapi/linux/pps.h
> > include/uapi/linux/rds.h
> > include/uapi/linux/sctp.h
> >
> > That's not really lots.
> >
> >> so there
> >> doesn't seem to be a reason they couldn't be used in virtio.  fuse.h
> >> even has a:
> >> 
> >>     #ifdef __KERNEL__
> >>     #include <linux/types.h>
> >>     #else
> >>     #include <stdint.h>
> >>     #endif
> >> 
> >> Which seems like a reasonable thing to do.
> >
> > In kernel, we really want to use things like endian-ness
> > checks (and, we really should have them in userspace too!).
> > So we want __le32 and other kernel goodies
> > like that. stdint.h won't cut it.
> 
> With the spec being standardized, I think having a stand alone set of
> headers is a good thing.

Sure, that's possible. We'll have to find some way to
preserve the endian-ness annotations, I think.
And then import them into kernel/qemu with some script, converting
to kernel/qemu style and annotations?

>  Licensing is problematic here too.
> 
> If virtio headers depend on other Linux headers, then it really doesn't
> matter if they are BSD licensed if you need a GPL header (like
> linux/if_ether.h).
> 
> Now, we can certainly debate the copyrightability of these defines and
> what have you but if the headers are meant to be 1) consumed outside the
> kernel 2) licensed under a different license than the general kernel
> then depending on kernel goodies is the wrong strategy.

Well specifically if_ether.h says GPLv2+ so it's OK for QEMU.
Do you mean for some other non GPL app?

> >> The only other kernel dependency is linux/if_ether.h to define
> >> ETH_ALEN.  But it seems completely reasonable to define a
> >> VIRTIO_NET_MAC_ALEN or something like that.
> >
> > Ugh. Not really reasonable IMO. We also use ETH_P_IP in code,
> > would like to get rid of redefining that too.
> > But we can have our own linux/if_ether.h for non-linux hosts,
> > just with a
> > couple of macros like that, it's not a big deal.
> 
> See above.
> 
> Regards,
> 
> Anthony Liguori
> 
> >
> >> This would make the virtio headers completely stand alone and includable
> >> in userspace (without violating C99).
> >> 
> >> Perhaps it's even worth moving the headers from uapi/linux to
> >> uapi/virtio.  Rusty, what do you think?
> >> 
> >> Regards,
> >> 
> >> Anhtony Liguori
> >> 
> >> >
> >> > Paolo
Anthony Liguori May 27, 2013, 4:14 p.m. UTC | #12
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Sun, May 26, 2013 at 07:55:25PM -0500, Anthony Liguori wrote:
>> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> 
>> > On Sun, May 26, 2013 at 03:49:53PM -0500, Anthony Liguori wrote:
>> >> Paolo Bonzini <pbonzini@redhat.com> writes:
>> >> 
>> >> > Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
>> >> >> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
>> >> >> 
>> >> >> Not really, __uX appear in the headers that were posted.
>> >> 
>> >> Which is a problem because this is a reserved namespace in C99.
>> >> 
>> >> >> What I'm saying is - a chance of a conflict is very remote,
>> >> >> if it happens it's a build failure so easy to debug.
>> >> >
>> >> > I'm sure that others will complain, :) but you can go ahead.
>> >> 
>> >> I think we should clean up the virtio headers in the kernel first.
>> >> Seems like a good thing to do given the standardization effort too.
>> >> There are lots of headers in uapi that use the C99 int types
>> >
>> > I found 4:
>> > $ grep -l uint include/uapi/linux/*h
>> > include/uapi/linux/dm-log-userspace.h
>> > include/uapi/linux/fuse.h
>> > include/uapi/linux/jffs2.h
>> > include/uapi/linux/pps.h
>> > include/uapi/linux/rds.h
>> > include/uapi/linux/sctp.h
>> >
>> > That's not really lots.
>> >
>> >> so there
>> >> doesn't seem to be a reason they couldn't be used in virtio.  fuse.h
>> >> even has a:
>> >> 
>> >>     #ifdef __KERNEL__
>> >>     #include <linux/types.h>
>> >>     #else
>> >>     #include <stdint.h>
>> >>     #endif
>> >> 
>> >> Which seems like a reasonable thing to do.
>> >
>> > In kernel, we really want to use things like endian-ness
>> > checks (and, we really should have them in userspace too!).
>> > So we want __le32 and other kernel goodies
>> > like that. stdint.h won't cut it.
>> 
>> With the spec being standardized, I think having a stand alone set of
>> headers is a good thing.
>
> Sure, that's possible. We'll have to find some way to
> preserve the endian-ness annotations, I think.
> And then import them into kernel/qemu with some script, converting
> to kernel/qemu style and annotations?

See below.

>>  Licensing is problematic here too.
>> 
>> If virtio headers depend on other Linux headers, then it really doesn't
>> matter if they are BSD licensed if you need a GPL header (like
>> linux/if_ether.h).
>> 
>> Now, we can certainly debate the copyrightability of these defines and
>> what have you but if the headers are meant to be 1) consumed outside the
>> kernel 2) licensed under a different license than the general kernel
>> then depending on kernel goodies is the wrong strategy.
>
> Well specifically if_ether.h says GPLv2+ so it's OK for QEMU.
> Do you mean for some other non GPL app?

Ignore QEMU for the moment.

The headers say they are BSD licensed... but they include a GPLv2+
header.  Doesn't make a lot of sense, does it?

Again, I think it's something pretty basic here.  Either (1) these are
kernel/userspace headers that are meant to be consumed through libc or
whatever (2) these are kernel-private headers not to be consumed outside
of the kernel (3) these are headers meant to be copied widely and used
as a reference implementation of virtio.

If (1) or (2), copying them into QEMU via a magic sanitizing script is
wrong.  We should just keep doing what we're doing.

If (3), then we should clean up the virtio headers to be license clean
and usable outside of the kernel.  We shouldn't try to solve this
problem in QEMU (via scripts) if we can just solve it in the kernel.

Regards,

Anthony Liguori

>
>> >> The only other kernel dependency is linux/if_ether.h to define
>> >> ETH_ALEN.  But it seems completely reasonable to define a
>> >> VIRTIO_NET_MAC_ALEN or something like that.
>> >
>> > Ugh. Not really reasonable IMO. We also use ETH_P_IP in code,
>> > would like to get rid of redefining that too.
>> > But we can have our own linux/if_ether.h for non-linux hosts,
>> > just with a
>> > couple of macros like that, it's not a big deal.
>> 
>> See above.
>> 
>> Regards,
>> 
>> Anthony Liguori
>> 
>> >
>> >> This would make the virtio headers completely stand alone and includable
>> >> in userspace (without violating C99).
>> >> 
>> >> Perhaps it's even worth moving the headers from uapi/linux to
>> >> uapi/virtio.  Rusty, what do you think?
>> >> 
>> >> Regards,
>> >> 
>> >> Anhtony Liguori
>> >> 
>> >> >
>> >> > Paolo
Anthony Liguori May 28, 2013, 2:55 a.m. UTC | #13
Rusty Russell <rusty@rustcorp.com.au> writes:

> Anthony Liguori <anthony@codemonkey.ws> writes:
>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>
>>> Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
>>>> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
>>>> 
>>>> Not really, __uX appear in the headers that were posted.
>>
>> Which is a problem because this is a reserved namespace in C99.
>
> Personally, I find it hard to care.  What matters is not what the
> standard has carved out, but whether we have clashes, reserved namespace
> or no.  And that won't happen for these.
>
> If someone wants to convert all the kernel headers, I won't NAK it
> though.

virtio headers are special.  Linux headers are normally only consumed in
the kernel or in a userspace application running on Linux.

virtio headers may be used either in a userspace application running on
!Linux (we need to support QEMU on Windows) or even in a foreign kernel.

linux/types.h is unusable outside of Linux because it pulls in a bunch
of other headers.  If you look at Michael's patch, he adds his own
version of types.h.  It's unfortunate for every user of the headers to
do this.

Regards,

Anthony Liguori

>> Perhaps it's even worth moving the headers from uapi/linux to
>> uapi/virtio.  Rusty, what do you think?
>
> Hmm, #include <virtio/virtio_net.h> etc would be worthwhile if that also
> worked on FreeBSD.  Bryan CC'd...
>
> Cheers,
> Rusty.
Michael S. Tsirkin May 28, 2013, 6:23 a.m. UTC | #14
On Mon, May 27, 2013 at 11:14:47AM -0500, Anthony Liguori wrote:
> > Well specifically if_ether.h says GPLv2+ so it's OK for QEMU.
> > Do you mean for some other non GPL app?
> 
> Ignore QEMU for the moment.
> 
> The headers say they are BSD licensed... but they include a GPLv2+
> header.

Above is a bit misleading.  To be precise, they don't include a GPLv2+
header. One header merely includes this line:
#include <linux/if_ether.h>
Rusty Russell May 29, 2013, 12:14 a.m. UTC | #15
Anthony Liguori <anthony@codemonkey.ws> writes:
> The headers say they are BSD licensed... but they include a GPLv2+
> header.  Doesn't make a lot of sense, does it?

It makes perfect sense: you're overthinking it.  It just means that
copying the BSD headers outside Linux is encouraged.

And it's clearly nonsensical to claim the GPL on kernel headers means
that userspace needs to be GPL.  So please ignore this licensing
red-herring.

And we'll bikeshed the headers in the standard when we have to :)  They
certainly don't need to be cut&paste into the kernel sources.

Cheers,
Rusty.
Rusty Russell May 29, 2013, 12:17 a.m. UTC | #16
Anthony Liguori <anthony@codemonkey.ws> writes:
> Rusty Russell <rusty@rustcorp.com.au> writes:
>
>> Anthony Liguori <anthony@codemonkey.ws> writes:
>>> Paolo Bonzini <pbonzini@redhat.com> writes:
>>>
>>>> Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
>>>>> > My fault.  I should have looked at linux/types.h (actually asm-generic/).
>>>>> 
>>>>> Not really, __uX appear in the headers that were posted.
>>>
>>> Which is a problem because this is a reserved namespace in C99.
>>
>> Personally, I find it hard to care.  What matters is not what the
>> standard has carved out, but whether we have clashes, reserved namespace
>> or no.  And that won't happen for these.
>>
>> If someone wants to convert all the kernel headers, I won't NAK it
>> though.
>
> virtio headers are special.  Linux headers are normally only consumed in
> the kernel or in a userspace application running on Linux.
>
> virtio headers may be used either in a userspace application running on
> !Linux (we need to support QEMU on Windows) or even in a foreign kernel.

No.

s/virtio/SCSI/.  s/virtio/if_eth/.  s/virtio/TCPIP/.

Cheers,
Rusty.
Bryan Venteicher May 29, 2013, 5:17 a.m. UTC | #17
On Mon, May 27, 2013 at 6:15 AM, Rusty Russell <rusty@rustcorp.com.au>wrote:

> Anthony Liguori <anthony@codemonkey.ws> writes:
> > Paolo Bonzini <pbonzini@redhat.com> writes:
> >
> >> Il 26/05/2013 22:02, Michael S. Tsirkin ha scritto:
> >>> > My fault.  I should have looked at linux/types.h (actually
> asm-generic/).
> >>>
> >>> Not really, __uX appear in the headers that were posted.
> >
> > Which is a problem because this is a reserved namespace in C99.
>
> Personally, I find it hard to care.  What matters is not what the
> standard has carved out, but whether we have clashes, reserved namespace
> or no.  And that won't happen for these.
>
> If someone wants to convert all the kernel headers, I won't NAK it
> though.
>
> > Perhaps it's even worth moving the headers from uapi/linux to
> > uapi/virtio.  Rusty, what do you think?
>
> Hmm, #include <virtio/virtio_net.h> etc would be worthwhile if that also
> worked on FreeBSD.  Bryan CC'd...
>
>
I've only done minor work on the VirtIO headers when importing them to
FreeBSD - mostly converting the _XX types to the preferred C99 variants,
along with some misc nits. I'm not too concerned with keeping the
headers identical to what is in Linux; I manually merge in required changes
when supporting a new feature and this hasn't been an issue. I'm content as
long as they remain BSD licensed. Growing GPL'ed #includes is a bit
worrisome, but I have a hard time foreseeing what the VirtIO files
could possibly depend on that isn't trivial.

I don't think I have enough context to understand the  ' #include
<virtio/virtio_net.h> etc' suggestion ... In FreeBSD, the VirtIO headers
files exist only in the source tree along side the corresponding device,
ie. sys/dev/virtio/network/virtio_net.h, sys/dev/virtio/pci/virtio_pci.h,
etc. The FreeBSD hypervisor (bhyve) just duplicates the
needed definitions/defines. This will be fixed at some point, but bhyve's
VirtIO is so barebones there is bigger fish to fry.


> Cheers,
> Rusty.
>
Anthony Liguori May 29, 2013, 1:05 p.m. UTC | #18
Rusty Russell <rusty@rustcorp.com.au> writes:

> Anthony Liguori <anthony@codemonkey.ws> writes:
>> The headers say they are BSD licensed... but they include a GPLv2+
>> header.  Doesn't make a lot of sense, does it?
>
> It makes perfect sense: you're overthinking it.  It just means that
> copying the BSD headers outside Linux is encouraged.

Copying by hand and modifying.  This patch series attempts to do it
automatically within QEMU.

> And it's clearly nonsensical to claim the GPL on kernel headers means
> that userspace needs to be GPL.  So please ignore this licensing
> red-herring.

You're missing context, I suspect.  This series is trying to
automatically copy the headers from Linux.  We currently have a manually
copied version.

The headers are not currently well suited for automatic copying because
(1) they use kernel types (2) they pull in dependencies from throughout
the kernel.

This discussion comes down to whether we want to make it easier to
automatically copy the headers or do we maintain the status quo and
require manual munging to pull in the headers.

Regards,

Anthony Liguori

>
> And we'll bikeshed the headers in the standard when we have to :)  They
> certainly don't need to be cut&paste into the kernel sources.
>
> Cheers,
> Rusty.
Michael S. Tsirkin May 29, 2013, 2:09 p.m. UTC | #19
On Wed, May 29, 2013 at 08:05:29AM -0500, Anthony Liguori wrote:
> Rusty Russell <rusty@rustcorp.com.au> writes:
> 
> > Anthony Liguori <anthony@codemonkey.ws> writes:
> >> The headers say they are BSD licensed... but they include a GPLv2+
> >> header.  Doesn't make a lot of sense, does it?
> >
> > It makes perfect sense: you're overthinking it.  It just means that
> > copying the BSD headers outside Linux is encouraged.
> 
> Copying by hand and modifying.  This patch series attempts to do it
> automatically within QEMU.
> 
> > And it's clearly nonsensical to claim the GPL on kernel headers means
> > that userspace needs to be GPL.  So please ignore this licensing
> > red-herring.
> 
> You're missing context, I suspect.  This series is trying to
> automatically copy the headers from Linux.  We currently have a manually
> copied version.
> 
> The headers are not currently well suited for automatic copying because
> (1) they use kernel types (2) they pull in dependencies from throughout
> the kernel.
> 
> This discussion comes down to whether we want to make it easier to
> automatically copy the headers or do we maintain the status quo and
> require manual munging to pull in the headers.
> 
> Regards,
> 
> Anthony Liguori

Okay, what if I

1. add a stub if_ether.h with a couple of macros we want
2. replace the types during copying

Would this address all concerns?

> >
> > And we'll bikeshed the headers in the standard when we have to :)  They
> > certainly don't need to be cut&paste into the kernel sources.
> >
> > Cheers,
> > Rusty.
Anthony Liguori May 29, 2013, 2:58 p.m. UTC | #20
"Michael S. Tsirkin" <mst@redhat.com> writes:

> On Wed, May 29, 2013 at 08:05:29AM -0500, Anthony Liguori wrote:
>> Rusty Russell <rusty@rustcorp.com.au> writes:
>> 
>> > Anthony Liguori <anthony@codemonkey.ws> writes:
>> >> The headers say they are BSD licensed... but they include a GPLv2+
>> >> header.  Doesn't make a lot of sense, does it?
>> >
>> > It makes perfect sense: you're overthinking it.  It just means that
>> > copying the BSD headers outside Linux is encouraged.
>> 
>> Copying by hand and modifying.  This patch series attempts to do it
>> automatically within QEMU.
>> 
>> > And it's clearly nonsensical to claim the GPL on kernel headers means
>> > that userspace needs to be GPL.  So please ignore this licensing
>> > red-herring.
>> 
>> You're missing context, I suspect.  This series is trying to
>> automatically copy the headers from Linux.  We currently have a manually
>> copied version.
>> 
>> The headers are not currently well suited for automatic copying because
>> (1) they use kernel types (2) they pull in dependencies from throughout
>> the kernel.
>> 
>> This discussion comes down to whether we want to make it easier to
>> automatically copy the headers or do we maintain the status quo and
>> require manual munging to pull in the headers.
>> 
>> Regards,
>> 
>> Anthony Liguori
>
> Okay, what if I
>
> 1. add a stub if_ether.h with a couple of macros we want
> 2. replace the types during copying
>
> Would this address all concerns?

If Rusty doesn't like the idea of making the headers usable directly,
then I don't object to having stubs/scripts to sanitize them in QEMU.

Regards,

Anthony Liguori

>
>> >
>> > And we'll bikeshed the headers in the standard when we have to :)  They
>> > certainly don't need to be cut&paste into the kernel sources.
>> >
>> > Cheers,
>> > Rusty.
diff mbox

Patch

diff --git a/linux-stubs/linux/types.h b/linux-stubs/linux/types.h
new file mode 100644
index 0000000..8642cbb
--- /dev/null
+++ b/linux-stubs/linux/types.h
@@ -0,0 +1,37 @@ 
+#ifndef STUBS_LINUX_TYPES_H
+#define STUBS_LINUX_TYPES_H
+
+#include <stdint.h>
+
+/*
+ * Below are Linux-specific types that should never collide with
+ * any application/library that wants linux/types.h.
+ */
+
+typedef uint8_t __u8;
+typedef uint16_t __u16;
+typedef uint32_t __u32;
+typedef uint64_t __u64;
+
+#ifdef __CHECKER__
+#define __bitwise__ __attribute__((bitwise))
+#else
+#define __bitwise__
+#endif
+#ifdef __CHECK_ENDIAN__
+#define __bitwise __bitwise__
+#else
+#define __bitwise
+#endif
+
+typedef __u16 __bitwise __le16;
+typedef __u16 __bitwise __be16;
+typedef __u32 __bitwise __le32;
+typedef __u32 __bitwise __be32;
+typedef __u64 __bitwise __le64;
+typedef __u64 __bitwise __be64;
+
+typedef __u16 __bitwise __sum16;
+typedef __u32 __bitwise __wsum;
+
+#endif