Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1716459 > unrolled thread
| Started by | Juergen Gross <jgross@suse.com> |
|---|---|
| First post | 2017-08-21 15:40 +0200 |
| Last post | 2017-08-21 17:10 +0200 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH v06 27/36] uapi xen/gntdev.h: include xen/privcmd.h and define grant_ref_t Juergen Gross <jgross@suse.com> - 2017-08-21 15:40 +0200
Re: [PATCH v06 27/36] uapi xen/gntdev.h: include xen/privcmd.h and define grant_ref_t Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-08-21 16:40 +0200
Re: [PATCH v06 27/36] uapi xen/gntdev.h: include xen/privcmd.h and define grant_ref_t Juergen Gross <jgross@suse.com> - 2017-08-21 16:50 +0200
Re: [PATCH v06 27/36] uapi xen/gntdev.h: include xen/privcmd.h and define grant_ref_t Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2017-08-21 17:10 +0200
| From | Juergen Gross <jgross@suse.com> |
|---|---|
| Date | 2017-08-21 15:40 +0200 |
| Subject | Re: [PATCH v06 27/36] uapi xen/gntdev.h: include xen/privcmd.h and define grant_ref_t |
| Message-ID | <ugV5T-4XE-5@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Boris Ostrovsky <boris.ostrovsky@oracle.com> |
|---|---|
| Date | 2017-08-21 16:40 +0200 |
| Message-ID | <ugW1X-5yk-1@gated-at.bofh.it> |
| In reply to | #1716459 |
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
[toc] | [prev] | [next] | [standalone]
| From | Juergen Gross <jgross@suse.com> |
|---|---|
| Date | 2017-08-21 16:50 +0200 |
| Message-ID | <ugWbE-5BL-11@gated-at.bofh.it> |
| In reply to | #1716552 |
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
[toc] | [prev] | [next] | [standalone]
| From | Boris Ostrovsky <boris.ostrovsky@oracle.com> |
|---|---|
| Date | 2017-08-21 17:10 +0200 |
| Message-ID | <ugWv0-5Xs-21@gated-at.bofh.it> |
| In reply to | #1716563 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web