Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754040AbdHUPAl (ORCPT ); Mon, 21 Aug 2017 11:00:41 -0400 Received: from aserp1040.oracle.com ([141.146.126.69]:31608 "EHLO aserp1040.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753899AbdHUO77 (ORCPT ); Mon, 21 Aug 2017 10:59:59 -0400 Subject: Re: [PATCH v06 27/36] uapi xen/gntdev.h: include xen/privcmd.h and define grant_ref_t To: Juergen Gross , Mikko Rapeli , linux-kernel@vger.kernel.org, linux-api@vger.kernel.org References: <20170806164428.2273-1-mikko.rapeli@iki.fi> <20170806164428.2273-28-mikko.rapeli@iki.fi> Cc: xen-devel@lists.xenproject.org, Andrew Morton , Masahiro Yamada From: Boris Ostrovsky Message-ID: <1e7f62c1-d1d8-a45f-d0e0-a88f618e4f2a@oracle.com> Date: Mon, 21 Aug 2017 10:59:36 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Source-IP: userv0022.oracle.com [156.151.31.74] Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3113 Lines: 87 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 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 >>>> Cc: Boris Ostrovsky >>>> Cc: Juergen Gross >>>> 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 >>>> +#include >>>> + >>>> +/* >>>> + * 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 >>>> +#include /* 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