From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54873) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XM6pK-0001zB-72 for qemu-devel@nongnu.org; Mon, 25 Aug 2014 22:49:47 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XM6pF-0002pf-2s for qemu-devel@nongnu.org; Mon, 25 Aug 2014 22:49:42 -0400 Received: from mga09.intel.com ([134.134.136.24]:12159) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XM6pE-0002pX-TH for qemu-devel@nongnu.org; Mon, 25 Aug 2014 22:49:37 -0400 Message-ID: <53FBF5BD.6060104@intel.com> Date: Tue, 26 Aug 2014 10:49:33 +0800 From: "Chen, Tiejun" MIME-Version: 1.0 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> <20140824111206.GB9561@redhat.com> <53FAC70F.1080201@intel.com> In-Reply-To: <53FAC70F.1080201@intel.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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: "Michael S. Tsirkin" Cc: xen-devel@lists.xensource.com, allen.m.kay@intel.com, qemu-devel@nongnu.org, Konrad Rzeszutek Wilk On 2014/8/25 13:18, Chen, Tiejun wrote: > On 2014/8/24 19:12, Michael S. Tsirkin wrote: >> 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. > > Okay. > >> >>>>> >>>>> 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? >> > > Here what is your so-called reverse logic as I already asked you > previously? Do you mean I should list all PCHs with a combo illustrated > with the vendor/device id in advance? Then look up if we can find a Michael, Could you explain this exactly? Then I can try follow-up your idea ASAP if this is necessary and possible. Thanks Tiejun > matched PCH? If yes, what is that benefit you expect in passthrough > case? Shouldn't we pass these info to VM directly in passthrough case? > > Thanks > Tiejun > > >