From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53857) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aPDKd-0004bR-4G for qemu-devel@nongnu.org; Fri, 29 Jan 2016 12:59:40 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aPDKZ-0004Ud-Mp for qemu-devel@nongnu.org; Fri, 29 Jan 2016 12:59:39 -0500 Received: from mx1.redhat.com ([209.132.183.28]:47176) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aPDKZ-0004UZ-Bc for qemu-devel@nongnu.org; Fri, 29 Jan 2016 12:59:35 -0500 Message-ID: <1454090373.23148.11.camel@redhat.com> From: Alex Williamson Date: Fri, 29 Jan 2016 10:59:33 -0700 In-Reply-To: <1454051359.28516.28.camel@redhat.com> References: <1451994098-6972-1-git-send-email-kraxel@redhat.com> <1454009759.7183.7.camel@redhat.com> <1454051359.28516.28.camel@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: igvt-g@ml01.01.org, xen-devel@lists.xensource.com, Eduardo Habkost , Stefano Stabellini , qemu-devel@nongnu.org, Cao jin , vfio-users@redhat.com On Fri, 2016-01-29 at 08:09 +0100, Gerd Hoffmann wrote: > =C2=A0 Hi, >=C2=A0 > > 1) The OpRegion MemoryRegion is mapped into system_memory through > > programming of the 0xFC config space register. > > =C2=A0a) vfio-pci could pick an address to do this as it is realized. > > =C2=A0b) SeaBIOS/OVMF could program this. > >=C2=A0 > > Discussion: 1.a) Avoids any BIOS dependency, but vfio-pci would need = to > > pick an address and mark it as e820 reserved.=C2=A0=C2=A0I'm not sure= how to pick > > that address. >=C2=A0 > Because of that I'd let the firmware pick the address and program 0xfc > accordingly, i.e. (b).=C2=A0=C2=A0seabios can simply malloc two pages a= nd be done > with it (any ram allocated by seabios will be tagged as e820 reserved). Thanks for the tip that seabios allocated pages automatically become e820 reserved, that simplifies things a bit. > > 2) Read-only mappings version of 1) > >=C2=A0 > > Discussion: Really nothing changes from the issues above, just preven= ts > > any possibility of the guest modifying anything in the host.=C2=A0=C2= =A0Xen > > apparently allows write access to the host page already. >=C2=A0 > I think read-only is out.=C2=A0=C2=A0Probably xen allows write access b= ecause > guest drivers expect they have write access to the opregion, so the > question is ... >=C2=A0 > > 3) Copy OpRegion contents into buffer and do either 1) or 2) above. >=C2=A0 > whenever we give the guest a copy of the host opregion or direct access= . >=C2=A0 > > 4) Copy contents into a guest RAM location, mark it reserved, point t= o > > it via 0xFC config as scratch register. > > =C2=A0a) Done by QEMU (vfio-pci) > > =C2=A0b) Done by SeaBIOS/OVMF > >=C2=A0 > > Discussion: This is the most like real hardware.=C2=A0=C2=A04.a) has = the usual > > issue of how to pick an address, but the benefit of not requiring BIO= S > > changes (simply mark the RAM reserved via existing methods).=C2=A0=C2= =A04.b) would > > require passing a buffer containing the contents of the OpRegion via > > fw_cfg and letting the BIOS do the setup.=C2=A0=C2=A0The latter of co= urse requires > > modifying each BIOS for this support. >=C2=A0 > Maybe we should define the interface as "guest writes 0xfc to pick > address, qemu takes care to place opregion there".=C2=A0=C2=A0That give= s us the > freedom to change the qemu implementation (either copy host opregion or > map the host opregion) without breaking things. Ok, so seabios allocates two pages, writes the base address of those pages to 0xfc and looks to see whether the signature appears at that address due to qemu mapping.=C2=A0=C2=A0It verifies the size and does a free/realloc if not the right size.=C2=A0=C2=A0If the graphics signature = does not appear, free those pages and assume no opregion support.=C2=A0=C2=A0If we= later decide to use a copy, we'd need to disable the 0xfc automagic mapping and probably pass the data via fw_cfg.=C2=A0=C2=A0Sound right? Do guest drivers depend on IGD appearing at 00:02.0?=C2=A0=C2=A0I'm curre= ntly testing for any Intel VGA device, but I wonder if I should only be enabling anything opregion if it also appears at a specific address. > > Of course none of these support hotplug nor really can they since > > reserved memory regions are not dynamic in the architecture. >=C2=A0 > igd is chipset graphics and therefore not hotpluggable anyway (on > physical hardware), I'd be very surprised if the guest drivers are > prepared to handle hotplug. >=C2=A0 > > Another thing I notice in this series is the access to PCI config spa= ce > > of both the host bridge and the LPC bridge.=C2=A0=C2=A0This prevents = unprivileged > > use cases >=C2=A0 > lpc bridge is no problem, only pci id fields are copied over and > unprivileged access is allowed for them. >=C2=A0 > Copying the gfx registers of the host bridge is a problem indeed. I would argue that both are really a problem, libvirt wants to put QEMU in a container that prevents access to any host system files other than those explicitly allowed.=C2=A0=C2=A0Therefore libvirt needs to grant the= process access to the lpc sysfs config file even though it only needs user visible register values. > > Should vfio add > > additional device specific regions to expose the config space of thes= e > > other devices? >=C2=A0 > That is an option.=C2=A0=C2=A0It is not clear yet which route we have t= o take > though.=C2=A0=C2=A0Testing shows that newer linux drivers work fine eve= n without > igd-passthru=3Don tweaks, whereas older linux kernels and windows drive= rs > don't work even with this series applied and igd-passthru=3Don.=C2=A0=C2= =A0I'll go > look at this as soon as I have test hardware (getting some is wip atm). Ok, well we certainly don't need to necessarily tie config space of those two devices together with opregion access, they can be added later, but we should revisit before we make QEMU grab those config space values itself, if we can make that functionality add value.=C2=A0=C2=A0Th= anks, Alex From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks Date: Fri, 29 Jan 2016 10:59:33 -0700 Message-ID: <1454090373.23148.11.camel@redhat.com> References: <1451994098-6972-1-git-send-email-kraxel@redhat.com> <1454009759.7183.7.camel@redhat.com> <1454051359.28516.28.camel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1454051359.28516.28.camel@redhat.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Gerd Hoffmann Cc: igvt-g@ml01.01.org, xen-devel@lists.xensource.com, Eduardo Habkost , Stefano Stabellini , qemu-devel@nongnu.org, Cao jin , vfio-users@redhat.com List-Id: xen-devel@lists.xenproject.org T24gRnJpLCAyMDE2LTAxLTI5IGF0IDA4OjA5ICswMTAwLCBHZXJkIEhvZmZtYW5uIHdyb3RlOgo+ IMKgIEhpLAo+wqAKPiA+IDEpIFRoZSBPcFJlZ2lvbiBNZW1vcnlSZWdpb24gaXMgbWFwcGVkIGlu dG8gc3lzdGVtX21lbW9yeSB0aHJvdWdoCj4gPiBwcm9ncmFtbWluZyBvZiB0aGUgMHhGQyBjb25m aWcgc3BhY2UgcmVnaXN0ZXIuCj4gPiDCoGEpIHZmaW8tcGNpIGNvdWxkIHBpY2sgYW4gYWRkcmVz cyB0byBkbyB0aGlzIGFzIGl0IGlzIHJlYWxpemVkLgo+ID4gwqBiKSBTZWFCSU9TL09WTUYgY291 bGQgcHJvZ3JhbSB0aGlzLgo+ID7CoAo+ID4gRGlzY3Vzc2lvbjogMS5hKSBBdm9pZHMgYW55IEJJ T1MgZGVwZW5kZW5jeSwgYnV0IHZmaW8tcGNpIHdvdWxkIG5lZWQgdG8KPiA+IHBpY2sgYW4gYWRk cmVzcyBhbmQgbWFyayBpdCBhcyBlODIwIHJlc2VydmVkLsKgwqBJJ20gbm90IHN1cmUgaG93IHRv IHBpY2sKPiA+IHRoYXQgYWRkcmVzcy4KPsKgCj4gQmVjYXVzZSBvZiB0aGF0IEknZCBsZXQgdGhl IGZpcm13YXJlIHBpY2sgdGhlIGFkZHJlc3MgYW5kIHByb2dyYW0gMHhmYwo+IGFjY29yZGluZ2x5 LCBpLmUuIChiKS7CoMKgc2VhYmlvcyBjYW4gc2ltcGx5IG1hbGxvYyB0d28gcGFnZXMgYW5kIGJl IGRvbmUKPiB3aXRoIGl0IChhbnkgcmFtIGFsbG9jYXRlZCBieSBzZWFiaW9zIHdpbGwgYmUgdGFn Z2VkIGFzIGU4MjAgcmVzZXJ2ZWQpLgoKVGhhbmtzIGZvciB0aGUgdGlwIHRoYXQgc2VhYmlvcyBh bGxvY2F0ZWQgcGFnZXMgYXV0b21hdGljYWxseSBiZWNvbWUKZTgyMCByZXNlcnZlZCwgdGhhdCBz aW1wbGlmaWVzIHRoaW5ncyBhIGJpdC4KCj4gPiAyKSBSZWFkLW9ubHkgbWFwcGluZ3MgdmVyc2lv biBvZiAxKQo+ID7CoAo+ID4gRGlzY3Vzc2lvbjogUmVhbGx5IG5vdGhpbmcgY2hhbmdlcyBmcm9t IHRoZSBpc3N1ZXMgYWJvdmUsIGp1c3QgcHJldmVudHMKPiA+IGFueSBwb3NzaWJpbGl0eSBvZiB0 aGUgZ3Vlc3QgbW9kaWZ5aW5nIGFueXRoaW5nIGluIHRoZSBob3N0LsKgwqBYZW4KPiA+IGFwcGFy ZW50bHkgYWxsb3dzIHdyaXRlIGFjY2VzcyB0byB0aGUgaG9zdCBwYWdlIGFscmVhZHkuCj7CoAo+ IEkgdGhpbmsgcmVhZC1vbmx5IGlzIG91dC7CoMKgUHJvYmFibHkgeGVuIGFsbG93cyB3cml0ZSBh Y2Nlc3MgYmVjYXVzZQo+IGd1ZXN0IGRyaXZlcnMgZXhwZWN0IHRoZXkgaGF2ZSB3cml0ZSBhY2Nl c3MgdG8gdGhlIG9wcmVnaW9uLCBzbyB0aGUKPiBxdWVzdGlvbiBpcyAuLi4KPsKgCj4gPiAzKSBD b3B5IE9wUmVnaW9uIGNvbnRlbnRzIGludG8gYnVmZmVyIGFuZCBkbyBlaXRoZXIgMSkgb3IgMikg YWJvdmUuCj7CoAo+IHdoZW5ldmVyIHdlIGdpdmUgdGhlIGd1ZXN0IGEgY29weSBvZiB0aGUgaG9z dCBvcHJlZ2lvbiBvciBkaXJlY3QgYWNjZXNzLgo+wqAKPiA+IDQpIENvcHkgY29udGVudHMgaW50 byBhIGd1ZXN0IFJBTSBsb2NhdGlvbiwgbWFyayBpdCByZXNlcnZlZCwgcG9pbnQgdG8KPiA+IGl0 IHZpYSAweEZDIGNvbmZpZyBhcyBzY3JhdGNoIHJlZ2lzdGVyLgo+ID4gwqBhKSBEb25lIGJ5IFFF TVUgKHZmaW8tcGNpKQo+ID4gwqBiKSBEb25lIGJ5IFNlYUJJT1MvT1ZNRgo+ID7CoAo+ID4gRGlz Y3Vzc2lvbjogVGhpcyBpcyB0aGUgbW9zdCBsaWtlIHJlYWwgaGFyZHdhcmUuwqDCoDQuYSkgaGFz IHRoZSB1c3VhbAo+ID4gaXNzdWUgb2YgaG93IHRvIHBpY2sgYW4gYWRkcmVzcywgYnV0IHRoZSBi ZW5lZml0IG9mIG5vdCByZXF1aXJpbmcgQklPUwo+ID4gY2hhbmdlcyAoc2ltcGx5IG1hcmsgdGhl IFJBTSByZXNlcnZlZCB2aWEgZXhpc3RpbmcgbWV0aG9kcykuwqDCoDQuYikgd291bGQKPiA+IHJl cXVpcmUgcGFzc2luZyBhIGJ1ZmZlciBjb250YWluaW5nIHRoZSBjb250ZW50cyBvZiB0aGUgT3BS ZWdpb24gdmlhCj4gPiBmd19jZmcgYW5kIGxldHRpbmcgdGhlIEJJT1MgZG8gdGhlIHNldHVwLsKg wqBUaGUgbGF0dGVyIG9mIGNvdXJzZSByZXF1aXJlcwo+ID4gbW9kaWZ5aW5nIGVhY2ggQklPUyBm b3IgdGhpcyBzdXBwb3J0Lgo+wqAKPiBNYXliZSB3ZSBzaG91bGQgZGVmaW5lIHRoZSBpbnRlcmZh Y2UgYXMgImd1ZXN0IHdyaXRlcyAweGZjIHRvIHBpY2sKPiBhZGRyZXNzLCBxZW11IHRha2VzIGNh cmUgdG8gcGxhY2Ugb3ByZWdpb24gdGhlcmUiLsKgwqBUaGF0IGdpdmVzIHVzIHRoZQo+IGZyZWVk b20gdG8gY2hhbmdlIHRoZSBxZW11IGltcGxlbWVudGF0aW9uIChlaXRoZXIgY29weSBob3N0IG9w cmVnaW9uIG9yCj4gbWFwIHRoZSBob3N0IG9wcmVnaW9uKSB3aXRob3V0IGJyZWFraW5nIHRoaW5n cy4KCk9rLCBzbyBzZWFiaW9zIGFsbG9jYXRlcyB0d28gcGFnZXMsIHdyaXRlcyB0aGUgYmFzZSBh ZGRyZXNzIG9mIHRob3NlCnBhZ2VzIHRvIDB4ZmMgYW5kIGxvb2tzIHRvIHNlZSB3aGV0aGVyIHRo ZSBzaWduYXR1cmUgYXBwZWFycyBhdCB0aGF0CmFkZHJlc3MgZHVlIHRvIHFlbXUgbWFwcGluZy7C oMKgSXQgdmVyaWZpZXMgdGhlIHNpemUgYW5kIGRvZXMgYQpmcmVlL3JlYWxsb2MgaWYgbm90IHRo ZSByaWdodCBzaXplLsKgwqBJZiB0aGUgZ3JhcGhpY3Mgc2lnbmF0dXJlIGRvZXMgbm90CmFwcGVh ciwgZnJlZSB0aG9zZSBwYWdlcyBhbmQgYXNzdW1lIG5vIG9wcmVnaW9uIHN1cHBvcnQuwqDCoElm IHdlIGxhdGVyCmRlY2lkZSB0byB1c2UgYSBjb3B5LCB3ZSdkIG5lZWQgdG8gZGlzYWJsZSB0aGUg MHhmYyBhdXRvbWFnaWMgbWFwcGluZwphbmQgcHJvYmFibHkgcGFzcyB0aGUgZGF0YSB2aWEgZndf Y2ZnLsKgwqBTb3VuZCByaWdodD8KCkRvIGd1ZXN0IGRyaXZlcnMgZGVwZW5kIG9uIElHRCBhcHBl YXJpbmcgYXQgMDA6MDIuMD/CoMKgSSdtIGN1cnJlbnRseQp0ZXN0aW5nIGZvciBhbnkgSW50ZWwg VkdBIGRldmljZSwgYnV0IEkgd29uZGVyIGlmIEkgc2hvdWxkIG9ubHkgYmUKZW5hYmxpbmcgYW55 dGhpbmcgb3ByZWdpb24gaWYgaXQgYWxzbyBhcHBlYXJzIGF0IGEgc3BlY2lmaWMgYWRkcmVzcy4K Cj4gPiBPZiBjb3Vyc2Ugbm9uZSBvZiB0aGVzZSBzdXBwb3J0IGhvdHBsdWcgbm9yIHJlYWxseSBj YW4gdGhleSBzaW5jZQo+ID4gcmVzZXJ2ZWQgbWVtb3J5IHJlZ2lvbnMgYXJlIG5vdCBkeW5hbWlj IGluIHRoZSBhcmNoaXRlY3R1cmUuCj7CoAo+IGlnZCBpcyBjaGlwc2V0IGdyYXBoaWNzIGFuZCB0 aGVyZWZvcmUgbm90IGhvdHBsdWdnYWJsZSBhbnl3YXkgKG9uCj4gcGh5c2ljYWwgaGFyZHdhcmUp LCBJJ2QgYmUgdmVyeSBzdXJwcmlzZWQgaWYgdGhlIGd1ZXN0IGRyaXZlcnMgYXJlCj4gcHJlcGFy ZWQgdG8gaGFuZGxlIGhvdHBsdWcuCj7CoAo+ID4gQW5vdGhlciB0aGluZyBJIG5vdGljZSBpbiB0 aGlzIHNlcmllcyBpcyB0aGUgYWNjZXNzIHRvIFBDSSBjb25maWcgc3BhY2UKPiA+IG9mIGJvdGgg dGhlIGhvc3QgYnJpZGdlIGFuZCB0aGUgTFBDIGJyaWRnZS7CoMKgVGhpcyBwcmV2ZW50cyB1bnBy aXZpbGVnZWQKPiA+IHVzZSBjYXNlcwo+wqAKPiBscGMgYnJpZGdlIGlzIG5vIHByb2JsZW0sIG9u bHkgcGNpIGlkIGZpZWxkcyBhcmUgY29waWVkIG92ZXIgYW5kCj4gdW5wcml2aWxlZ2VkIGFjY2Vz cyBpcyBhbGxvd2VkIGZvciB0aGVtLgo+wqAKPiBDb3B5aW5nIHRoZSBnZnggcmVnaXN0ZXJzIG9m IHRoZSBob3N0IGJyaWRnZSBpcyBhIHByb2JsZW0gaW5kZWVkLgoKSSB3b3VsZCBhcmd1ZSB0aGF0 IGJvdGggYXJlIHJlYWxseSBhIHByb2JsZW0sIGxpYnZpcnQgd2FudHMgdG8gcHV0IFFFTVUKaW4g YSBjb250YWluZXIgdGhhdCBwcmV2ZW50cyBhY2Nlc3MgdG8gYW55IGhvc3Qgc3lzdGVtIGZpbGVz IG90aGVyIHRoYW4KdGhvc2UgZXhwbGljaXRseSBhbGxvd2VkLsKgwqBUaGVyZWZvcmUgbGlidmly dCBuZWVkcyB0byBncmFudCB0aGUgcHJvY2VzcwphY2Nlc3MgdG8gdGhlIGxwYyBzeXNmcyBjb25m aWcgZmlsZSBldmVuIHRob3VnaCBpdCBvbmx5IG5lZWRzIHVzZXIKdmlzaWJsZSByZWdpc3RlciB2 YWx1ZXMuCgo+ID4gU2hvdWxkIHZmaW8gYWRkCj4gPiBhZGRpdGlvbmFsIGRldmljZSBzcGVjaWZp YyByZWdpb25zIHRvIGV4cG9zZSB0aGUgY29uZmlnIHNwYWNlIG9mIHRoZXNlCj4gPiBvdGhlciBk ZXZpY2VzPwo+wqAKPiBUaGF0IGlzIGFuIG9wdGlvbi7CoMKgSXQgaXMgbm90IGNsZWFyIHlldCB3 aGljaCByb3V0ZSB3ZSBoYXZlIHRvIHRha2UKPiB0aG91Z2guwqDCoFRlc3Rpbmcgc2hvd3MgdGhh dCBuZXdlciBsaW51eCBkcml2ZXJzIHdvcmsgZmluZSBldmVuIHdpdGhvdXQKPiBpZ2QtcGFzc3Ro cnU9b24gdHdlYWtzLCB3aGVyZWFzIG9sZGVyIGxpbnV4IGtlcm5lbHMgYW5kIHdpbmRvd3MgZHJp dmVycwo+IGRvbid0IHdvcmsgZXZlbiB3aXRoIHRoaXMgc2VyaWVzIGFwcGxpZWQgYW5kIGlnZC1w YXNzdGhydT1vbi7CoMKgSSdsbCBnbwo+IGxvb2sgYXQgdGhpcyBhcyBzb29uIGFzIEkgaGF2ZSB0 ZXN0IGhhcmR3YXJlIChnZXR0aW5nIHNvbWUgaXMgd2lwIGF0bSkuCgpPaywgd2VsbCB3ZSBjZXJ0 YWlubHkgZG9uJ3QgbmVlZCB0byBuZWNlc3NhcmlseSB0aWUgY29uZmlnIHNwYWNlIG9mCnRob3Nl IHR3byBkZXZpY2VzIHRvZ2V0aGVyIHdpdGggb3ByZWdpb24gYWNjZXNzLCB0aGV5IGNhbiBiZSBh ZGRlZApsYXRlciwgYnV0IHdlIHNob3VsZCByZXZpc2l0IGJlZm9yZSB3ZSBtYWtlIFFFTVUgZ3Jh YiB0aG9zZSBjb25maWcgc3BhY2UKdmFsdWVzIGl0c2VsZiwgaWYgd2UgY2FuIG1ha2UgdGhhdCBm dW5jdGlvbmFsaXR5IGFkZCB2YWx1ZS7CoMKgVGhhbmtzLAoKQWxleAoKCl9fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fClhlbi1kZXZlbCBtYWlsaW5nIGxpc3QK WGVuLWRldmVsQGxpc3RzLnhlbi5vcmcKaHR0cDovL2xpc3RzLnhlbi5vcmcveGVuLWRldmVsCg==