From patchwork Wed Feb 11 02:45:27 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tiejun Chen X-Patchwork-Id: 438650 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id BCD9D1401AB for ; Wed, 11 Feb 2015 13:45:59 +1100 (AEDT) Received: from localhost ([::1]:42788 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLNJM-0005TV-E0 for incoming@patchwork.ozlabs.org; Tue, 10 Feb 2015 21:45:56 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36863) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLNJ2-0005CC-HN for qemu-devel@nongnu.org; Tue, 10 Feb 2015 21:45:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YLNIz-0005Pk-8i for qemu-devel@nongnu.org; Tue, 10 Feb 2015 21:45:36 -0500 Received: from mga03.intel.com ([134.134.136.65]:42912) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YLNIy-0005Pb-Oq for qemu-devel@nongnu.org; Tue, 10 Feb 2015 21:45:33 -0500 Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga103.jf.intel.com with ESMTP; 10 Feb 2015 18:40:30 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,555,1418112000"; d="scan'208";a="664623105" Received: from tiejunch-mobl.ccr.corp.intel.com (HELO [10.238.128.132]) ([10.238.128.132]) by fmsmga001.fm.intel.com with ESMTP; 10 Feb 2015 18:45:29 -0800 Message-ID: <54DAC247.6080004@intel.com> Date: Wed, 11 Feb 2015 10:45:27 +0800 From: "Chen, Tiejun" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 MIME-Version: 1.0 To: Ian Campbell References: <1422839843-25622-1-git-send-email-tiejun.chen@intel.com> <20150202121940.GA28773@zion.uk.xensource.com> <21711.29549.892862.333392@mariner.uk.xensource.com> <54D01EAB.1020005@intel.com> <1422961628.9323.32.camel@citrix.com> <54D1770C.4020904@intel.com> <1423046493.17711.28.camel@citrix.com> <54D2C5E5.40407@intel.com> <1423129922.24924.46.camel@citrix.com> <54D41274.9090400@intel.com> <54D8537C.2080805@intel.com> <1423479910.23098.34.camel@citrix.com> In-Reply-To: <1423479910.23098.34.camel@citrix.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 134.134.136.65 Cc: Wei Liu , Ian Jackson , qemu-devel@nongnu.org, xen-devel@lists.xen.org, stefano.stabellini@citrix.com, kraxel@redhat.com Subject: Re: [Qemu-devel] [Xen-devel] [v2][PATCH] libxl: add one machine property to support IGD GFX passthrough X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 2015/2/9 19:05, Ian Campbell wrote: > On Mon, 2015-02-09 at 14:28 +0800, Chen, Tiejun wrote: > >> What about this? > > I've not read the code in detail,since I'm travelling but from a quick > glance it looks to be implementing the sort of thing I meant, thanks. Thanks for your time. > > A couple of higher level comments: > > I'd suggest to put the code for reading the vid/did into a helper > function so it can be reused. Looks good. > > You might like to optionally consider add a forcing option somehow so > that people with new devices not in the list can control things without > the need to recompile (e.g. gfx_passthru_kind_override?). Perhaps that > isn't needed for a first cut though and it would be a libxl API so > thought required. What about 'gfx_passthru_force'? Because what we're doing is, we want to make sure if we have a such a IGD that needs to workaround by posting a parameter to qemu. So in case of non-listed devices we just need to provide a bool to force this regardless of that real device. > > I think it should probably log something at a lowish level when it has > autodetected IGD. > So I tried to rebase that according to your all comments, ("serial", string), ("boot", string), Thanks Tiejun diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c index 98687bd..398d9da 100644 --- a/tools/libxl/libxl_create.c +++ b/tools/libxl/libxl_create.c @@ -361,6 +361,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, libxl_defbool_setdefault(&b_info->u.hvm.nographic, false); libxl_defbool_setdefault(&b_info->u.hvm.gfx_passthru, false); + libxl_defbool_setdefault(&b_info->u.hvm.gfx_passthru_force, false); break; case LIBXL_DOMAIN_TYPE_PV: diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c index 8599a6a..507034f 100644 --- a/tools/libxl/libxl_dm.c +++ b/tools/libxl/libxl_dm.c @@ -710,9 +710,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, flexarray_append(dm_args, "-net"); flexarray_append(dm_args, "none"); } - if (libxl_defbool_val(b_info->u.hvm.gfx_passthru)) { - flexarray_append(dm_args, "-gfx_passthru"); - } } else { if (!sdl && !vnc) { flexarray_append(dm_args, "-nographic"); @@ -757,6 +754,11 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, machinearg, max_ram_below_4g); } } + + if (libxl__is_igd_vga_passthru(gc, guest_config)) { + machinearg = GCSPRINTF("%s,igd-passthru=on", machinearg); + } + flexarray_append(dm_args, machinearg); for (i = 0; b_info->extra_hvm && b_info->extra_hvm[i] != NULL; i++) flexarray_append(dm_args, b_info->extra_hvm[i]); diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h index 934465a..35ec5fc 100644 --- a/tools/libxl/libxl_internal.h +++ b/tools/libxl/libxl_internal.h @@ -1177,6 +1177,9 @@ _hidden int libxl__create_pci_backend(libxl__gc *gc, uint32_t domid, libxl_device_pci *pcidev, int num); _hidden int libxl__device_pci_destroy_all(libxl__gc *gc, uint32_t domid); +_hidden int libxl__is_igd_vga_passthru(libxl__gc *gc, + const libxl_domain_config *d_config); + /*----- xswait: wait for a xenstore node to be suitable -----*/ typedef struct libxl__xswait_state libxl__xswait_state; diff --git a/tools/libxl/libxl_pci.c b/tools/libxl/libxl_pci.c index f3ae132..07b9e22 100644 --- a/tools/libxl/libxl_pci.c +++ b/tools/libxl/libxl_pci.c @@ -491,6 +491,130 @@ static int sysfs_dev_unbind(libxl__gc *gc, libxl_device_pci *pcidev, return 0; } +static unsigned long sysfs_dev_get_vendor(libxl__gc *gc, + libxl_device_pci *pcidev) +{ + char *pci_device_vendor_path = + libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/vendor", + pcidev->domain, pcidev->bus, pcidev->dev, + pcidev->func); + int read_items; + unsigned long pci_device_vendor; + + FILE *f = fopen(pci_device_vendor_path, "r"); + if (!f) { + LOGE(ERROR, + "pci device "PCI_BDF" does not have vendor attribute", + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); + return 0xffff; + } + read_items = fscanf(f, "0x%lx\n", &pci_device_vendor); + fclose(f); + if (read_items != 1) { + LOGE(ERROR, + "cannot read vendor of pci device "PCI_BDF, + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); + return 0xffff; + } + + return pci_device_vendor; +} + +static unsigned long sysfs_dev_get_device(libxl__gc *gc, + libxl_device_pci *pcidev) +{ + char *pci_device_device_path = + libxl__sprintf(gc, SYSFS_PCI_DEV"/"PCI_BDF"/device", + pcidev->domain, pcidev->bus, pcidev->dev, + pcidev->func); + int read_items; + unsigned long pci_device_device; + + FILE *f = fopen(pci_device_device_path, "r"); + if (!f) { + LOGE(ERROR, + "pci device "PCI_BDF" does not have device attribute", + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); + return 0xffff; + } + read_items = fscanf(f, "0x%lx\n", &pci_device_device); + fclose(f); + if (read_items != 1) { + LOGE(ERROR, + "cannot read device of pci device "PCI_BDF, + pcidev->domain, pcidev->bus, pcidev->dev, pcidev->func); + return 0xffff; + } + + return pci_device_device; +} + +static const uint16_t igd_ids[] = { + /* HSW Classic */ + 0x0402, /* HSWGT1D, HSWD_w7 */ + 0x0406, /* HSWGT1M, HSWM_w7 */ + 0x0412, /* HSWGT2D, HSWD_w7 */ + 0x0416, /* HSWGT2M, HSWM_w7 */ + 0x041E, /* HSWGT15D, HSWD_w7 */ + /* HSW ULT */ + 0x0A06, /* HSWGT1UT, HSWM_w7 */ + 0x0A16, /* HSWGT2UT, HSWM_w7 */ + 0x0A26, /* HSWGT3UT, HSWM_w7 */ + 0x0A2E, /* HSWGT3UT28W, HSWM_w7 */ + 0x0A1E, /* HSWGT2UX, HSWM_w7 */ + 0x0A0E, /* HSWGT1ULX, HSWM_w7 */ + /* HSW CRW */ + 0x0D26, /* HSWGT3CW, HSWM_w7 */ + 0x0D22, /* HSWGT3CWDT, HSWD_w7 */ + /* HSW Sever */ + 0x041A, /* HSWSVGT2, HSWD_w7 */ + /* HSW SRVR */ + 0x040A, /* HSWSVGT1, HSWD_w7 */ + /* BSW */ + 0x1606, /* BDWULTGT1, BDWM_w7 */ + 0x1616, /* BDWULTGT2, BDWM_w7 */ + 0x1626, /* BDWULTGT3, BDWM_w7 */ + 0x160E, /* BDWULXGT1, BDWM_w7 */ + 0x161E, /* BDWULXGT2, BDWM_w7 */ + 0x1602, /* BDWHALOGT1, BDWM_w7 */ + 0x1612, /* BDWHALOGT2, BDWM_w7 */ + 0x1622, /* BDWHALOGT3, BDWM_w7 */ + 0x162B, /* BDWHALO28W, BDWM_w7 */ + 0x162A, /* BDWGT3WRKS, BDWM_w7 */ + 0x162D, /* BDWGT3SRVR, BDWM_w7 */ +}; + +int libxl__is_igd_vga_passthru(libxl__gc *gc, + const libxl_domain_config *d_config) +{ + unsigned int i, j, num = ARRAY_SIZE(igd_ids); + libxl_ctx *ctx = libxl__gc_owner(gc); + + if (!libxl_defbool_val(d_config->b_info.u.hvm.gfx_passthru)) + return 0; + + if (libxl_defbool_val(d_config->b_info.u.hvm.gfx_passthru_force)) { + LIBXL__LOG(ctx, LIBXL__LOG_WARNING, "Force to workaround IGD."); + return 1; + } + + for (i = 0 ; i < d_config->num_pcidevs ; i++) { + libxl_device_pci *pcidev = &d_config->pcidevs[i]; + + if (sysfs_dev_get_vendor(gc, pcidev) != 0x8086) /* Intel class */ + continue; + + for (j = 0 ; j < num ; j++) { + if (sysfs_dev_get_device(gc, pcidev) == igd_ids[j]) { /* IGD */ + LIBXL__LOG(ctx, LIBXL__LOG_INFO, "Detected supported IGD."); + return 1; + } + } + } + + return 0; +} + /* * A brief comment about slots. I don't know what slots are for; however, * I have by experimentation determined: diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl index 02be466..d3015bc 100644 --- a/tools/libxl/libxl_types.idl +++ b/tools/libxl/libxl_types.idl @@ -430,6 +430,7 @@ libxl_domain_build_info = Struct("domain_build_info",[ ("spice", libxl_spice_info), ("gfx_passthru", libxl_defbool), + ("gfx_passthru_force", libxl_defbool),