From mboxrd@z Thu Jan 1 00:00:00 1970 From: Pasi =?iso-8859-1?Q?K=E4rkk=E4inen?= Subject: Re: [PATCH 3/3] qemu-xen-trad: IGD passthrough: Expose vendor specific pci cap on host bridge. Date: Tue, 16 Jul 2013 01:55:20 +0300 Message-ID: <20130715225520.GH2924@reaktio.net> References: <20130621180335.GC15809@phenom.dumpdata.com> <51C9A44E.1050309@citrix.com> <20130625145438.GA28904@phenom.dumpdata.com> <51C9B0B9.4000409@citrix.com> <20130626125301.GB4222@phenom.dumpdata.com> <20130701130637.GA10934@phenom.dumpdata.com> <20130715160633.GF2924@reaktio.net> <51E435BF.6010708@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <51E435BF.6010708@citrix.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: Ross Philipson Cc: Ian Campbell , "G.R." , xen-devel , Stefano Stabellini , Jan Beulich , Jean Guyader List-Id: xen-devel@lists.xenproject.org On Mon, Jul 15, 2013 at 01:47:43PM -0400, Ross Philipson wrote: > On 07/15/2013 12:06 PM, Pasi K=E4rkk=E4inen wrote: > >On Mon, Jul 01, 2013 at 09:06:37AM -0400, Konrad Rzeszutek Wilk wrote: > >>>>>>>Or just scan through the capabilities, and chain only the ones > >>>>>>>that we want to "Whitelist" and the rest are to be blacklisted. > >>>>>>>The rest can also have its values set to some bogus value (0xdeadb= eef?) > >>>>>>>Perhaps only when built with 'debug=3Dy'. > >>>>>> > >>>>>> > >>>>>>That sounds about right. Back when I first did the patch (in an old= qemu) > >>>>>>there were no other capabilities on the piix4 host bridge so it was= simple. > >>>>>>Not sure if other capabilities will be an issue now. > >>>>> > >>>>>It's still the case as for the IVB host bridge. > >>>>>And from what I can find from the datasheet for the Haswell, it's > >>>>>still the case. > >>>>> > >>>>>Note that the datasheet explicitly documents the offset of the > >>>>>CAPABILITY registers. > >>>>>I guess there will be code that rely on this offset that been public= ly > >>>>>documented. > >>>>> > >>>>>Btw. Ross, now that you appears to be the original author (sorry for > >>>>>mess you up with Jean), > >>>>>could you also comment on my rework proposal? Jan believe the current > >>>>>form is not clean enough. > >>>>> > >>>>>Currently we use a whitelist of registers to pass-through.How do you > >>>>>come up with the current list? > >>>>>The shadow copy way appears to work for the current list. > >>>> > >>>>OK. > >>>>>But what if we are going to need some special registers that cannot = be > >>>>>handled well? (e.g. has side effect for reading and cannot perform > >>>>>read-back?) > >>>> > >>>>Hopefully the i915 driver in Linux will help in figuring out which > >>>>ones of those are needed? > >>>I remember the vendor cap fix only helps windows guest. > >> > >>How was that diagnosed? Perhaps that information can be part of the sou= rce > >>code to help in the future with diagnosiing which caps are needed and > >>which ones can be blacklisted? > >> > > > >I guess that's a question mostly for Ross/Jean as they're the original a= uthors of the patch? > = > We discovered the issue with Windows guests running the vendor > drivers for the passed in IGD graphics device. Under certain > circumstances (resuming from S3/S4 IIRC), the guest would BSOD. I > finally tracked it down to a bad state in the resuming driver > because it was not coded to handle the vendor capabilities not being > present on the host bridge. BTW, those capabilities are flags > indicating what features the IGD card has - their exact meaning is > of course proprietary. > = > I cannot say it was only a problem on Windows but rather that that > is the only place we ever saw it. > = > I never saw any other capabilities on the hosts bridges at that > time, just vendor ones so the patch just handled that. If there were > other capabilities, I would think it would have to be determined on > a case by case basis whether they were included. Inclusion of each > new type would have different ramifications it seems. > = Thanks for the explanation. I guess parts of that should go to the patch de= scription aswell.. -- Pasi