From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51329) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XLVi4-0000QZ-Sq for qemu-devel@nongnu.org; Sun, 24 Aug 2014 07:11:49 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XLVi0-0007GD-DE for qemu-devel@nongnu.org; Sun, 24 Aug 2014 07:11:44 -0400 Received: from mx1.redhat.com ([209.132.183.28]:51509) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XLVi0-0007Fu-5P for qemu-devel@nongnu.org; Sun, 24 Aug 2014 07:11:40 -0400 Date: Sun, 24 Aug 2014 13:12:06 +0200 From: "Michael S. Tsirkin" Message-ID: <20140824111206.GB9561@redhat.com> References: <1408584508-5946-1-git-send-email-tiejun.chen@intel.com> <1408584508-5946-3-git-send-email-tiejun.chen@intel.com> <20140821161649.GA18265@laptop.dumpdata.com> <53F6978C.9080600@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <53F6978C.9080600@intel.com> Subject: Re: [Qemu-devel] [Xen-devel] [PATCH 2/2] xen:i386:pc_piix: create isa bridge specific to IGD passthrough List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Chen, Tiejun" Cc: xen-devel@lists.xensource.com, allen.m.kay@intel.com, qemu-devel@nongnu.org, Konrad Rzeszutek Wilk On Fri, Aug 22, 2014 at 09:06:20AM +0800, Chen, Tiejun wrote: > On 2014/8/22 0:16, Konrad Rzeszutek Wilk wrote: > >On Thu, Aug 21, 2014 at 09:28:28AM +0800, Tiejun Chen wrote: > >>Currenjly this ISA bridge should be fixed at 1f.0, and pass the > > > >Currently > > Fixed. > > > > >>real vendor/device ids as the driver expect. > > > >Could you add a bit more description to this patch please? Explain > >the rationale, etc. > > So rephrase as follows: > > xen:i386:pc_piix: create isa bridge specific to IGD passthrough > > Currently IGD drivers always need to access PCH by 1f.0, OK > and > identify PCH type with its own real vendor/device ids. This type > value help driver initialize correctly. instead: PCH vendor/device id is used to identify the card. > >> > >>Signed-off-by: Tiejun Chen > >>--- > >> hw/i386/pc_piix.c | 24 +++++++++++++++++++++++- > >> 1 file changed, 23 insertions(+), 1 deletion(-) > >> > >>diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c > >>index 7710724..b131fa3 100644 > >>--- a/hw/i386/pc_piix.c > >>+++ b/hw/i386/pc_piix.c > >>@@ -50,7 +50,8 @@ > >> #include "cpu.h" > >> #include "qemu/error-report.h" > >> #ifdef CONFIG_XEN > >>-# include > >>+#include > >>+#include > >> #endif > >> > >> #define MAX_IDE_BUS 2 > >>@@ -463,6 +464,26 @@ static void pc_xen_hvm_init(MachineState *machine) > >> } > >> } > >> > >>+static void xen_igd_passthrough_isa_bridge_create(PCIBus *bus) > >>+{ > >>+ struct PCIDevice *dev; > >>+ XenHostPCIDevice hdev; > >>+ int r = 0; > >>+ > >>+ /* This shoudl be fixed at 1f.0 then pass vendor/device ids. > > > >should > > > >However I would remove the comment as it does not add anything extra > >to the function. It is pretty clear what it is doing. > > > >What would help is if you said: > > > >Must be fixed at 1f.0 because .. bla blah > > Like the patch description, so what about this, > > /* Currently IGD drivers always need to access PCH by 1f.0, and > * identify PCH type with its own real vendor/device ids. > */ > > Thanks > Tiejun > > > > >>+ */ > >>+ dev = pci_create_simple(bus, PCI_DEVFN(0x1f, 0), > >>+ "xen-igd-passthrough-isa-bridge"); > >>+ if (dev) { > >>+ r = xen_host_pci_device_get(&hdev, 0, 0, PCI_DEVFN(0x1f, 0), 0); > >>+ if (!r) { > >>+ pci_config_set_vendor_id(dev->config, hdev.vendor_id); > >>+ pci_config_set_device_id(dev->config, hdev.device_id); Can you, instead, implement the reverse logic, probing the card and supplying the correct device id for PCH? > >>+ } else > >>+ fprintf(stderr, "xen set xen-igd-passthrough-isa-bridge failed\n"); > >>+ } > >>+} > >>+ > >> static void xen_igd_passthrough_pc_hvm_init(MachineState *machine) > >> { > >> PCIBus *bus; > >>@@ -472,6 +493,7 @@ static void xen_igd_passthrough_pc_hvm_init(MachineState *machine) > >> bus = pci_find_primary_bus(); > >> if (bus != NULL) { > >> pci_create_simple(bus, -1, "xen-platform"); > >>+ xen_igd_passthrough_isa_bridge_create(bus); > >> } > >> } > >> #endif > >>-- > >>1.9.1 > >> > >> > >>_______________________________________________ > >>Xen-devel mailing list > >>Xen-devel@lists.xen.org > >>http://lists.xen.org/xen-devel > >