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@xxxxxx>
>>> Cc: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
>>> Cc: Juergen Gross <jgross@xxxxxxxx>
>>> Cc: xen-devel@xxxxxxxxxxxxxxxxxxxx
>>> ---
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe linux-api" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
