From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:45749)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <tiejun.chen@intel.com>) id 1YWEZm-0003Bj-SC
	for qemu-devel@nongnu.org; Thu, 12 Mar 2015 21:39:47 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <tiejun.chen@intel.com>) id 1YWEZi-0004qi-Ai
	for qemu-devel@nongnu.org; Thu, 12 Mar 2015 21:39:46 -0400
Received: from mga14.intel.com ([192.55.52.115]:21577)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <tiejun.chen@intel.com>) id 1YWEZi-0004qS-3W
	for qemu-devel@nongnu.org; Thu, 12 Mar 2015 21:39:42 -0400
Message-ID: <55023FDA.7030204@intel.com>
Date: Fri, 13 Mar 2015 09:39:38 +0800
From: "Chen, Tiejun" <tiejun.chen@intel.com>
MIME-Version: 1.0
References: <1425980538-5508-1-git-send-email-tiejun.chen@intel.com>		
	<1425980538-5508-3-git-send-email-tiejun.chen@intel.com>	
	<1426073641.21353.192.camel@citrix.com>
	<55010596.2030106@intel.com>
	<1426163186.21353.409.camel@citrix.com>
In-Reply-To: <1426163186.21353.409.camel@citrix.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [v2][PATCH 2/2] libxl: introduce gfx_passthru_kind
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Ian Campbell <ian.campbell@citrix.com>
Cc: Ian.Jackson@eu.citrix.com, wei.liu2@citrix.com, qemu-devel@nongnu.org, stefano.stabellini@citrix.com, xen-devel@lists.xen.org

> I don't think you can abort here, since a user can set
> b_info->u.hvm.gfx_passthru_kind to default. You would need to return an
> error.

Then, looks I should do this,

                     LOG(ERROR, "No supported IGD to passthru,"
                         " or please force set gfx_passthru=\"igd\".\n");
                     return NULL;

>
>> @@ -720,6 +720,13 @@ void libxl_mac_copy(libxl_ctx *ctx, libxl_mac *dst,
>> libxl_mac *src);
>>    #define LIBXL_HAVE_PSR_MBM 1
>>    #endif
>>
>> +/*
>> + * LIBXL_HAVE_GFX_PASSTHRU_KIND
>> + *
>> + * If this is defined, the Graphic Device Passthrough Override is
>> supported.
>
> Almost, please also explicitly name the type field as other similar
> comments do for clarity.

Okay, maybe something is like this,

+/*
+ * LIBXL_HAVE_IGD_GFX_PASSTHRU
+ *
+ * If this is defined, the IGD Graphic Device Passthrough is supported.
+ *
+ * LIBXL_HAVE_IGD_GFX_PASSTHRU indicates that the
+ * libxl_device_pci field in the hvm is present in the pci_info structure
+ * fixup_ids[] which contains all supported IGD devices. So wwe use
+ * "igd-passthru=on" specify on the qemu command-line.
+ */
+#define LIBXL_HAVE_IGD_GFX_PASSTHRU 1
+

>> + */

[snip]

> and this should be in libxl_internal.h not here...

Okay.

I mistakenly understand we always have to expose this in libxl.h...

>
>> But looks libxl__gc{} is defined in the libxl_internal.h file... I guess

[snip]

>> +        if (!xlu_cfg_get_long(config, "gfx_passthru", &l, 1)) {
>> +            if (l) {
>> +                libxl_defbool_set(&b_info->u.hvm.gfx_passthru, true);
>> +            } else {
>> +                libxl_defbool_set(&b_info->u.hvm.gfx_passthru, false);
>> +            }
>
> This is exactly the same as:
>          libxl_defbool_set(&b_info->u.hvm.gfx_passthru, l);
>

Sure.

Thanks
Tiejun