From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:41413) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQhPP-0008PZ-Kc for qemu-devel@nongnu.org; Tue, 02 Feb 2016 15:18:44 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aQhPK-0003sZ-KT for qemu-devel@nongnu.org; Tue, 02 Feb 2016 15:18:43 -0500 Received: from mx1.redhat.com ([209.132.183.28]:52208) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aQhPK-0003s8-Cz for qemu-devel@nongnu.org; Tue, 02 Feb 2016 15:18:38 -0500 Message-ID: <1454444316.30910.7.camel@redhat.com> From: Alex Williamson Date: Tue, 02 Feb 2016 13:18:36 -0700 In-Reply-To: <20160202163123.GA3727@morn.lan> References: <1451994098-6972-1-git-send-email-kraxel@redhat.com> <1454009759.7183.7.camel@redhat.com> <1454051359.28516.28.camel@redhat.com> <1454090373.23148.11.camel@redhat.com> <1454330962.10168.34.camel@redhat.com> <1454403380.9300.49.camel@redhat.com> <20160202163123.GA3727@morn.lan> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin O'Connor , Gerd Hoffmann Cc: "igvt-g@ml01.01.org" , "Tian, Kevin" , "xen-devel@lists.xensource.com" , Eduardo Habkost , Stefano Stabellini , seabios@seabios.org, "qemu-devel@nongnu.org" , Cao jin , "vfio-users@redhat.com" , Laszlo Ersek On Tue, 2016-02-02 at 11:31 -0500, Kevin O'Connor wrote: > On Tue, Feb 02, 2016 at 09:56:20AM +0100, Gerd Hoffmann wrote: > > =C2=A0 Hi, > >=C2=A0 > > > > I'd have qemu copy the data on 0xfc write then, so things continu= e to > > > > work without updating seabios.=C2=A0=C2=A0So, the firmware has to= allocate space, > > > > reserve it etc.,=C2=A0=C2=A0and programming the 0xfc register.=C2= =A0=C2=A0Qemu has to make > > > > sure the opregion appears at the address written by the firmware,= by > > > > whatever method it prefers. > > >=C2=A0 > > > Yup. It's Qemu's responsibility to expose opregion content.=C2=A0 > > >=C2=A0 > > > btw, prefer to do copying here. It's pointless to allow write from = guest > > > side. One write example is SWSCI mailbox, thru which gfx driver can > > > trigger some SCI event to communicate with BIOS (specifically ACPI > > > methods here), mostly for some monitor operations. However it's=C2=A0 > > > not a right thing for guest to trigger host SCI and thus kick host=C2= =A0 > > > ACPI methods. > >=C2=A0 > > Thanks. > >=C2=A0 > > So, question again how we do that best.=C2=A0=C2=A0Option one being t= he mmap way, > > i.e. basically what the patches posted by alex are doing.=C2=A0=C2=A0= Option two > > being the fw_cfg way, i.e. place a opregion copy in fw_cfg and have > > seabios not only set 0xfc, but also store the opregion there by copyi= ng > > from fw_cfg. >=C2=A0 > What about option 2a - SeaBIOS copies from fw_cfg to memory and then > programs 0xfc.=C2=A0=C2=A0QEMU can detect the write to 0xfc and choose = to map > that ram (thus completely ignoring the contents that were just copied > in) or it can choose not to map that ram (thus guest uses the contents > just copied in). >=C2=A0 > The advantage of this approach is that it is a bit simpler in the > firmware (no size probing is needed as the size comes from fw_cfg) and > it allows for future flexibility as the choice of mapping can be > deferred. >=C2=A0 > Totally untested seabios code below as example. >=C2=A0 > As an aside, if this type of "program a pci register" with a memory > address becomes common, we could enhance the acpi-style "linker > script" system to automate this.. >=C2=A0 > -Kevin >=C2=A0 >=C2=A0 > static void intel_igd_opregion_setup(struct pci_device *dev, void *arg) > { > =C2=A0=C2=A0=C2=A0=C2=A0struct romfile_s *file =3D romfile_find("etc/ig= d-opregion"); > =C2=A0=C2=A0=C2=A0=C2=A0if (!file) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > =C2=A0=C2=A0=C2=A0=C2=A0void *data =3D memalign_high(PAGE_SIZE, file->s= ize); > =C2=A0=C2=A0=C2=A0=C2=A0if (!data) { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0warn_noalloc(); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > =C2=A0=C2=A0=C2=A0=C2=A0} > =C2=A0=C2=A0=C2=A0=C2=A0int ret =3D file->copy(file, data, file->size); > =C2=A0=C2=A0=C2=A0=C2=A0if (ret < 0) { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0free(data); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > =C2=A0=C2=A0=C2=A0=C2=A0} > =C2=A0=C2=A0=C2=A0=C2=A0pci_config_writel(dev->bdf, 0xFC, (u32)data); > } I posted a v2 of the last QEMU patch and the SeaBIOS patch that takes this approach with an option in QEMU for the direct map.=C2=A0=C2=A0Thank= s, Alex From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alex Williamson Subject: Re: [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset tweaks Date: Tue, 02 Feb 2016 13:18:36 -0700 Message-ID: <1454444316.30910.7.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> <1454090373.23148.11.camel@redhat.com> <1454330962.10168.34.camel@redhat.com> <1454403380.9300.49.camel@redhat.com> <20160202163123.GA3727@morn.lan> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Return-path: In-Reply-To: <20160202163123.GA3727@morn.lan> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org To: Kevin O'Connor , Gerd Hoffmann Cc: "igvt-g@ml01.01.org" , "Tian, Kevin" , "xen-devel@lists.xensource.com" , Eduardo Habkost , Stefano Stabellini , seabios@seabios.org, "qemu-devel@nongnu.org" , Cao jin , "vfio-users@redhat.com" , Laszlo Ersek List-Id: xen-devel@lists.xenproject.org On Tue, 2016-02-02 at 11:31 -0500, Kevin O'Connor wrote: > On Tue, Feb 02, 2016 at 09:56:20AM +0100, Gerd Hoffmann wrote: > > =C2=A0 Hi, > >=C2=A0 > > > > I'd have qemu copy the data on 0xfc write then, so things continu= e to > > > > work without updating seabios.=C2=A0=C2=A0So, the firmware has to= allocate space, > > > > reserve it etc.,=C2=A0=C2=A0and programming the 0xfc register.=C2= =A0=C2=A0Qemu has to make > > > > sure the opregion appears at the address written by the firmware,= by > > > > whatever method it prefers. > > >=C2=A0 > > > Yup. It's Qemu's responsibility to expose opregion content.=C2=A0 > > >=C2=A0 > > > btw, prefer to do copying here. It's pointless to allow write from = guest > > > side. One write example is SWSCI mailbox, thru which gfx driver can > > > trigger some SCI event to communicate with BIOS (specifically ACPI > > > methods here), mostly for some monitor operations. However it's=C2=A0 > > > not a right thing for guest to trigger host SCI and thus kick host=C2= =A0 > > > ACPI methods. > >=C2=A0 > > Thanks. > >=C2=A0 > > So, question again how we do that best.=C2=A0=C2=A0Option one being t= he mmap way, > > i.e. basically what the patches posted by alex are doing.=C2=A0=C2=A0= Option two > > being the fw_cfg way, i.e. place a opregion copy in fw_cfg and have > > seabios not only set 0xfc, but also store the opregion there by copyi= ng > > from fw_cfg. >=C2=A0 > What about option 2a - SeaBIOS copies from fw_cfg to memory and then > programs 0xfc.=C2=A0=C2=A0QEMU can detect the write to 0xfc and choose = to map > that ram (thus completely ignoring the contents that were just copied > in) or it can choose not to map that ram (thus guest uses the contents > just copied in). >=C2=A0 > The advantage of this approach is that it is a bit simpler in the > firmware (no size probing is needed as the size comes from fw_cfg) and > it allows for future flexibility as the choice of mapping can be > deferred. >=C2=A0 > Totally untested seabios code below as example. >=C2=A0 > As an aside, if this type of "program a pci register" with a memory > address becomes common, we could enhance the acpi-style "linker > script" system to automate this.. >=C2=A0 > -Kevin >=C2=A0 >=C2=A0 > static void intel_igd_opregion_setup(struct pci_device *dev, void *arg) > { > =C2=A0=C2=A0=C2=A0=C2=A0struct romfile_s *file =3D romfile_find("etc/ig= d-opregion"); > =C2=A0=C2=A0=C2=A0=C2=A0if (!file) > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > =C2=A0=C2=A0=C2=A0=C2=A0void *data =3D memalign_high(PAGE_SIZE, file->s= ize); > =C2=A0=C2=A0=C2=A0=C2=A0if (!data) { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0warn_noalloc(); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > =C2=A0=C2=A0=C2=A0=C2=A0} > =C2=A0=C2=A0=C2=A0=C2=A0int ret =3D file->copy(file, data, file->size); > =C2=A0=C2=A0=C2=A0=C2=A0if (ret < 0) { > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0free(data); > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0return; > =C2=A0=C2=A0=C2=A0=C2=A0} > =C2=A0=C2=A0=C2=A0=C2=A0pci_config_writel(dev->bdf, 0xFC, (u32)data); > } I posted a v2 of the last QEMU patch and the SeaBIOS patch that takes this approach with an option in QEMU for the direct map.=C2=A0=C2=A0Thank= s, Alex