| Message ID | 20170806164428.2273-28-mikko.rapeli@iki.fi (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 06/08/17 18:44, Mikko Rapeli wrote: > Both are needed to compile <xen/gntdev.h> in userspace. Fixes these > userspace compile errors: > > xen/gntdev.h:151:4: error: unknown type name ‘grant_ref_t’ > grant_ref_t ref; > ^ > xen/gntdev.h:153:4: error: unknown type name ‘domid_t’ > domid_t domid; > ^ > > Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi> > Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> > Cc: Juergen Gross <jgross@suse.com> > Cc: xen-devel@lists.xenproject.org > --- > include/uapi/xen/gntdev.h | 6 ++++++ > include/xen/interface/grant_table.h | 6 +----- > 2 files changed, 7 insertions(+), 5 deletions(-) > > diff --git a/include/uapi/xen/gntdev.h b/include/uapi/xen/gntdev.h > index d0661977667e..f208706b07c9 100644 > --- a/include/uapi/xen/gntdev.h > +++ b/include/uapi/xen/gntdev.h > @@ -34,6 +34,12 @@ > #define __LINUX_PUBLIC_GNTDEV_H__ > > #include <linux/types.h> > +#include <xen/privcmd.h> > + > +/* > + * Reference to a grant entry in a specified domain's grant table. > + */ > +typedef __u32 grant_ref_t; > > struct ioctl_gntdev_grant_ref { > /* The domain ID of the grant to be mapped. */ > diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h > index 7fb7112d667c..29b3724aec2c 100644 > --- a/include/xen/interface/grant_table.h > +++ b/include/xen/interface/grant_table.h > @@ -29,6 +29,7 @@ > #define __XEN_PUBLIC_GRANT_TABLE_H__ > > #include <xen/interface/xen.h> > +#include <uapi/xen/gntdev.h> /* for grant_ref_t */ Hmm, I really don't like this. We import the Xen headers from the Xen source repository. The Xen specific types shouldn't be taken from non-Xen headers. > > /*********************************** > * GRANT TABLE REPRESENTATION > @@ -85,11 +86,6 @@ > */ > > /* > - * Reference to a grant entry in a specified domain's grant table. > - */ > -typedef uint32_t grant_ref_t; > - > -/* > * A grant table comprises a packed array of grant entries in one or more > * page frames shared between Xen and a guest. > * [XEN]: This field is written by Xen and read by the sharing guest. > I'd rather use __u32 instead of grant_ref_t in the uapi headers. Boris, what do you think? Juergen
On 08/21/2017 09:33 AM, Juergen Gross wrote: > On 06/08/17 18:44, Mikko Rapeli wrote: >> Both are needed to compile <xen/gntdev.h> in userspace. Fixes these >> userspace compile errors: >> >> xen/gntdev.h:151:4: error: unknown type name ‘grant_ref_t’ >> grant_ref_t ref; >> ^ >> xen/gntdev.h:153:4: error: unknown type name ‘domid_t’ >> domid_t domid; >> ^ >> >> Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi> >> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> >> Cc: Juergen Gross <jgross@suse.com> >> Cc: xen-devel@lists.xenproject.org >> --- >> include/uapi/xen/gntdev.h | 6 ++++++ >> include/xen/interface/grant_table.h | 6 +----- >> 2 files changed, 7 insertions(+), 5 deletions(-) >> >> diff --git a/include/uapi/xen/gntdev.h b/include/uapi/xen/gntdev.h >> index d0661977667e..f208706b07c9 100644 >> --- a/include/uapi/xen/gntdev.h >> +++ b/include/uapi/xen/gntdev.h >> @@ -34,6 +34,12 @@ >> #define __LINUX_PUBLIC_GNTDEV_H__ >> >> #include <linux/types.h> >> +#include <xen/privcmd.h> >> + >> +/* >> + * Reference to a grant entry in a specified domain's grant table. >> + */ >> +typedef __u32 grant_ref_t; >> >> struct ioctl_gntdev_grant_ref { >> /* The domain ID of the grant to be mapped. */ >> diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h >> index 7fb7112d667c..29b3724aec2c 100644 >> --- a/include/xen/interface/grant_table.h >> +++ b/include/xen/interface/grant_table.h >> @@ -29,6 +29,7 @@ >> #define __XEN_PUBLIC_GRANT_TABLE_H__ >> >> #include <xen/interface/xen.h> >> +#include <uapi/xen/gntdev.h> /* for grant_ref_t */ > Hmm, I really don't like this. We import the Xen headers from the Xen > source repository. The Xen specific types shouldn't be taken from > non-Xen headers. > >> >> /*********************************** >> * GRANT TABLE REPRESENTATION >> @@ -85,11 +86,6 @@ >> */ >> >> /* >> - * Reference to a grant entry in a specified domain's grant table. >> - */ >> -typedef uint32_t grant_ref_t; >> - >> -/* >> * A grant table comprises a packed array of grant entries in one or more >> * page frames shared between Xen and a guest. >> * [XEN]: This field is written by Xen and read by the sharing guest. >> > I'd rather use __u32 instead of grant_ref_t in the uapi headers. > > Boris, what do you think? I agree that including a non-Xen file into include/xen/interface is not especially desirable. Can we ifdef __KERNEL__grant_ref_t usage in uapi header, with else clause using __u32? And maybe stick BUILD_BUG_ON somewhere to check that sizes are still the same ? -boris -boris
On 21/08/17 16:31, Boris Ostrovsky wrote: > On 08/21/2017 09:33 AM, Juergen Gross wrote: >> On 06/08/17 18:44, Mikko Rapeli wrote: >>> Both are needed to compile <xen/gntdev.h> in userspace. Fixes these >>> userspace compile errors: >>> >>> xen/gntdev.h:151:4: error: unknown type name ‘grant_ref_t’ >>> grant_ref_t ref; >>> ^ >>> xen/gntdev.h:153:4: error: unknown type name ‘domid_t’ >>> domid_t domid; >>> ^ >>> >>> Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi> >>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> >>> Cc: Juergen Gross <jgross@suse.com> >>> Cc: xen-devel@lists.xenproject.org >>> --- >>> include/uapi/xen/gntdev.h | 6 ++++++ >>> include/xen/interface/grant_table.h | 6 +----- >>> 2 files changed, 7 insertions(+), 5 deletions(-) >>> >>> diff --git a/include/uapi/xen/gntdev.h b/include/uapi/xen/gntdev.h >>> index d0661977667e..f208706b07c9 100644 >>> --- a/include/uapi/xen/gntdev.h >>> +++ b/include/uapi/xen/gntdev.h >>> @@ -34,6 +34,12 @@ >>> #define __LINUX_PUBLIC_GNTDEV_H__ >>> >>> #include <linux/types.h> >>> +#include <xen/privcmd.h> >>> + >>> +/* >>> + * Reference to a grant entry in a specified domain's grant table. >>> + */ >>> +typedef __u32 grant_ref_t; >>> >>> struct ioctl_gntdev_grant_ref { >>> /* The domain ID of the grant to be mapped. */ >>> diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h >>> index 7fb7112d667c..29b3724aec2c 100644 >>> --- a/include/xen/interface/grant_table.h >>> +++ b/include/xen/interface/grant_table.h >>> @@ -29,6 +29,7 @@ >>> #define __XEN_PUBLIC_GRANT_TABLE_H__ >>> >>> #include <xen/interface/xen.h> >>> +#include <uapi/xen/gntdev.h> /* for grant_ref_t */ >> Hmm, I really don't like this. We import the Xen headers from the Xen >> source repository. The Xen specific types shouldn't be taken from >> non-Xen headers. >> >>> >>> /*********************************** >>> * GRANT TABLE REPRESENTATION >>> @@ -85,11 +86,6 @@ >>> */ >>> >>> /* >>> - * Reference to a grant entry in a specified domain's grant table. >>> - */ >>> -typedef uint32_t grant_ref_t; >>> - >>> -/* >>> * A grant table comprises a packed array of grant entries in one or more >>> * page frames shared between Xen and a guest. >>> * [XEN]: This field is written by Xen and read by the sharing guest. >>> >> I'd rather use __u32 instead of grant_ref_t in the uapi headers. >> >> Boris, what do you think? > > I agree that including a non-Xen file into include/xen/interface is not > especially desirable. > > Can we ifdef __KERNEL__grant_ref_t usage in uapi header, with else > clause using __u32? And maybe stick BUILD_BUG_ON somewhere to check that > sizes are still the same ? I'd rather use __u32 unconditionally and have the BUILD_BUG_ON in the grant or privcmd driver. Otherwise we'd depend on the sequence of #includes (what happens if a driver includes the uapi header before xen/interface/xen.h?) Juergen
On 08/21/2017 10:46 AM, Juergen Gross wrote: > On 21/08/17 16:31, Boris Ostrovsky wrote: >> On 08/21/2017 09:33 AM, Juergen Gross wrote: >>> On 06/08/17 18:44, Mikko Rapeli wrote: >>>> Both are needed to compile <xen/gntdev.h> in userspace. Fixes these >>>> userspace compile errors: >>>> >>>> xen/gntdev.h:151:4: error: unknown type name ‘grant_ref_t’ >>>> grant_ref_t ref; >>>> ^ >>>> xen/gntdev.h:153:4: error: unknown type name ‘domid_t’ >>>> domid_t domid; >>>> ^ >>>> >>>> Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi> >>>> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> >>>> Cc: Juergen Gross <jgross@suse.com> >>>> Cc: xen-devel@lists.xenproject.org >>>> --- >>>> include/uapi/xen/gntdev.h | 6 ++++++ >>>> include/xen/interface/grant_table.h | 6 +----- >>>> 2 files changed, 7 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/include/uapi/xen/gntdev.h b/include/uapi/xen/gntdev.h >>>> index d0661977667e..f208706b07c9 100644 >>>> --- a/include/uapi/xen/gntdev.h >>>> +++ b/include/uapi/xen/gntdev.h >>>> @@ -34,6 +34,12 @@ >>>> #define __LINUX_PUBLIC_GNTDEV_H__ >>>> >>>> #include <linux/types.h> >>>> +#include <xen/privcmd.h> >>>> + >>>> +/* >>>> + * Reference to a grant entry in a specified domain's grant table. >>>> + */ >>>> +typedef __u32 grant_ref_t; >>>> >>>> struct ioctl_gntdev_grant_ref { >>>> /* The domain ID of the grant to be mapped. */ >>>> diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h >>>> index 7fb7112d667c..29b3724aec2c 100644 >>>> --- a/include/xen/interface/grant_table.h >>>> +++ b/include/xen/interface/grant_table.h >>>> @@ -29,6 +29,7 @@ >>>> #define __XEN_PUBLIC_GRANT_TABLE_H__ >>>> >>>> #include <xen/interface/xen.h> >>>> +#include <uapi/xen/gntdev.h> /* for grant_ref_t */ >>> Hmm, I really don't like this. We import the Xen headers from the Xen >>> source repository. The Xen specific types shouldn't be taken from >>> non-Xen headers. >>> >>>> >>>> /*********************************** >>>> * GRANT TABLE REPRESENTATION >>>> @@ -85,11 +86,6 @@ >>>> */ >>>> >>>> /* >>>> - * Reference to a grant entry in a specified domain's grant table. >>>> - */ >>>> -typedef uint32_t grant_ref_t; >>>> - >>>> -/* >>>> * A grant table comprises a packed array of grant entries in one or more >>>> * page frames shared between Xen and a guest. >>>> * [XEN]: This field is written by Xen and read by the sharing guest. >>>> >>> I'd rather use __u32 instead of grant_ref_t in the uapi headers. >>> >>> Boris, what do you think? >> I agree that including a non-Xen file into include/xen/interface is not >> especially desirable. >> >> Can we ifdef __KERNEL__grant_ref_t usage in uapi header, with else >> clause using __u32? And maybe stick BUILD_BUG_ON somewhere to check that >> sizes are still the same ? > I'd rather use __u32 unconditionally and have the BUILD_BUG_ON in the > grant or privcmd driver. > > Otherwise we'd depend on the sequence of #includes (what happens if a > driver includes the uapi header before xen/interface/xen.h?) OK, good point. -boris
diff --git a/include/uapi/xen/gntdev.h b/include/uapi/xen/gntdev.h index d0661977667e..f208706b07c9 100644 --- a/include/uapi/xen/gntdev.h +++ b/include/uapi/xen/gntdev.h @@ -34,6 +34,12 @@ #define __LINUX_PUBLIC_GNTDEV_H__ #include <linux/types.h> +#include <xen/privcmd.h> + +/* + * Reference to a grant entry in a specified domain's grant table. + */ +typedef __u32 grant_ref_t; struct ioctl_gntdev_grant_ref { /* The domain ID of the grant to be mapped. */ diff --git a/include/xen/interface/grant_table.h b/include/xen/interface/grant_table.h index 7fb7112d667c..29b3724aec2c 100644 --- a/include/xen/interface/grant_table.h +++ b/include/xen/interface/grant_table.h @@ -29,6 +29,7 @@ #define __XEN_PUBLIC_GRANT_TABLE_H__ #include <xen/interface/xen.h> +#include <uapi/xen/gntdev.h> /* for grant_ref_t */ /*********************************** * GRANT TABLE REPRESENTATION @@ -85,11 +86,6 @@ */ /* - * Reference to a grant entry in a specified domain's grant table. - */ -typedef uint32_t grant_ref_t; - -/* * A grant table comprises a packed array of grant entries in one or more * page frames shared between Xen and a guest. * [XEN]: This field is written by Xen and read by the sharing guest.
Both are needed to compile <xen/gntdev.h> in userspace. Fixes these userspace compile errors: xen/gntdev.h:151:4: error: unknown type name ‘grant_ref_t’ grant_ref_t ref; ^ xen/gntdev.h:153:4: error: unknown type name ‘domid_t’ domid_t domid; ^ Signed-off-by: Mikko Rapeli <mikko.rapeli@iki.fi> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Juergen Gross <jgross@suse.com> Cc: xen-devel@lists.xenproject.org --- include/uapi/xen/gntdev.h | 6 ++++++ include/xen/interface/grant_table.h | 6 +----- 2 files changed, 7 insertions(+), 5 deletions(-)