From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34839) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYOqB-0003GY-Ni for qemu-devel@nongnu.org; Wed, 18 Mar 2015 21:01:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YYOq6-0006BH-Oo for qemu-devel@nongnu.org; Wed, 18 Mar 2015 21:01:39 -0400 Received: from mga03.intel.com ([134.134.136.65]:11580) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YYOq6-0006Ah-JQ for qemu-devel@nongnu.org; Wed, 18 Mar 2015 21:01:34 -0400 Message-ID: <550A1FE7.2050602@intel.com> Date: Thu, 19 Mar 2015 09:01:27 +0800 From: "Chen, Tiejun" MIME-Version: 1.0 References: <1426669601-13891-1-git-send-email-tiejun.chen@intel.com> <1426669601-13891-4-git-send-email-tiejun.chen@intel.com> <1426674083.32192.44.camel@nilsson.home.kraxel.org> In-Reply-To: <1426674083.32192.44.camel@nilsson.home.kraxel.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [v7][PATCH 03/10] piix: create host bridge to passthrough List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: pbonzini@redhat.com, rth@twiddle.net, allen.m.kay@intel.com, qemu-devel@nongnu.org, mst@redhat.com On 2015/3/18 18:21, Gerd Hoffmann wrote: > On Mi, 2015-03-18 at 17:06 +0800, Tiejun Chen wrote: >> Implement a pci host bridge specific to passthrough. Actually >> this just inherits the standard one. And we also just expose >> a minimal real host bridge pci configuration subset. > >> +/* Here we just expose minimal host bridge offset subset. */ >> +static const IGDHostInfo igd_host_bridge_infos[] = { >> + {0x08, 2}, /* revision id */ >> + {0x2c, 2}, /* sybsystem vendor id */ >> + {0x2e, 2}, /* sybsystem id */ >> + {0x50, 2}, /* SNB: processor graphics control register */ >> + {0x52, 2}, /* processor graphics control register */ >> + {0xa4, 4}, /* SNB: graphics base of stolen memory */ >> + {0xa8, 4}, /* SNB: base of GTT stolen memory */ >> +}; > > Hmm, no vendor/device id here? Will the idg guest drivers happily read These two emulated values are already fine. And this is a long story. At the beginning, we were initially trying to expose more, + case 0x00: /* vendor id */ + case 0x02: /* device id */ + case 0x08: /* revision id */ + case 0x2c: /* sybsystem vendor id */ + case 0x2e: /* sybsystem id */ + case 0x50: /* SNB: processor graphics control register */ + case 0x52: /* processor graphics control register */ + case 0xa0: /* top of memory */ + case 0xb0: /* ILK: BSM: should read from dev 2 offset 0x5c */ + case 0x58: /* SNB: PAVPC Offset */ + case 0xa4: /* SNB: graphics base of stolen memory */ + case 0xa8: /* SNB: base of GTT stolen memory */ But this brought some discussions with Paolo and Michael, and then plus our further experiment, now as everyone expect, this is a minimal real host bridge pci configuration subset which we need to expose. > graphics control registers from i440fx even though this chipset never > existed in a igd variant? > > [ just wondering, if it works that way fine, it certainly makes things Yes, it works fine. > easier for the firmware which uses host bridge pci id to figure > whenever it is running @ i440fx or q35 ]. > >> +static int igd_pt_i440fx_initfn(struct PCIDevice *pci_dev) >> +{ >> + uint32_t val = 0; >> + int rc, i, num; >> + int pos, len; >> + >> + num = ARRAY_SIZE(igd_host_bridge_infos); >> + for (i = 0; i < num; i++) { >> + pos = igd_host_bridge_infos[i].offset; >> + len = igd_host_bridge_infos[i].len; >> + rc = host_pci_config_read(pos, len, val); >> + if (rc) { >> + return -ENODEV; >> + } >> + pci_default_write_config(pci_dev, pos, val, len); >> + } >> + >> + return 0; >> +} > > Nothing i440fx specific in here, just copying some host bridge pci > config space bits. I guess we can sub-class the q35 host bridge the > same way and even reuse the init function? This is our another discussion long time ago :) Currently Xen don't run with q35. If I remember those discussions correctly, something is still restricted to Windows. > >> +#define TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE "xen-igd-passthrough-i440FX" > > One xen leftover slipped through here. Fixed. Thanks Tiejun