From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:60426)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Ian.Campbell@citrix.com>) id 1YWMZb-0001rQ-PT
	for qemu-devel@nongnu.org; Fri, 13 Mar 2015 06:12:09 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <Ian.Campbell@citrix.com>) id 1YWMZX-0008OR-Sl
	for qemu-devel@nongnu.org; Fri, 13 Mar 2015 06:12:07 -0400
Received: from smtp02.citrix.com ([66.165.176.63]:49537)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <Ian.Campbell@citrix.com>) id 1YWMZX-0008OM-Lw
	for qemu-devel@nongnu.org; Fri, 13 Mar 2015 06:12:03 -0400
Message-ID: <1426241479.32572.94.camel@citrix.com>
From: Ian Campbell <ian.campbell@citrix.com>
Date: Fri, 13 Mar 2015 10:11:19 +0000
In-Reply-To: <55023FDA.7030204@intel.com>
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> <55023FDA.7030204@intel.com>
Content-Type: text/plain; charset="UTF-8"
MIME-Version: 1.0
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: "Chen, Tiejun" <tiejun.chen@intel.com>
Cc: Ian.Jackson@eu.citrix.com, wei.liu2@citrix.com, qemu-devel@nongnu.org, stefano.stabellini@citrix.com, xen-devel@lists.xen.org

On Fri, 2015-03-13 at 09:39 +0800, Chen, Tiejun wrote:
> > 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;

If I remember the context correctly this is in the autodetect case, so I
think shouldn't mention IGD. Something like "Unable to detect graphics
passthru kind, please set gfx_passthru_kind. See xl.cfg(5) for more
information."

> >
> >> @@ -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.

This:

/*
 * libxl_domain_build_info has the u.hvm.gfx_passthru_kind field and
 * the libxl_gfx_passthru_kind enumeration is defined. 
 */
#define LIBXL_HAVE_GFX_PASSTHRU_KIND

Users don't care about the internal details, just about the existence of
the support.

Ian.