[Intel-gfx] How to create PCH to support those existing driver

[Kay, Allen M]

> Allen,
> 
> Could you reply this?

Let me summarized what we have discussed and learned so far:

1) Future Windows/Linux IGD drivers will be modified to restrain from accessing MCH/PCH devices.  We are prototyping this in Windows driver right now and will pass the same methodology to Linux driver once we have a workable solution.  The goal is removing all MCH/PCH accesses in the IGD driver.

2) We want the same solution to work in both native and virtualization environments.  Given most driver developers test their changes only in native environment, doing anything specific for virtualization in the driver will cause frequent breakage for virtualization use cases.

3) Back porting this new code to support previous generations of HW will be problematic if not impossible.  Each Windows IGD driver release binary supports two generations of HW.  For example, 15.36 driver supports Broadwell/Haswell, 15.33 driver supports Haswell/IvyBridge, 15.31 driver supports Ivybridge/Sandybridge, etc.   Once the driver is product validated, there is little opportunity to  go back and make high impact changes that might affect stability in native environment.

4) I agree there is little reason to do anything that requires driver changes at this point,  unless it is the final desired solution.

The question is whether/how to support IGD passthrough for previous generations of HW?

1) If we want to support SandyBridge/IvyBridge/Haswell/Broadwell, we will need most of the original QEMU patches.  We might be able to do without igd_pci_write().  I have tested QEMU changes without igd_pci_write() on both Haswell/Broadwell and Windows booted without any problems.  This will limit only read operations which should reduce a quite a bit of risk to the host platform.

2) If we want the upstream QEMU only work with future driver version, then most of the IGD passthrough patch is probably not needed - with exception of opregion mapping handlers.  The downside is products that depend on this feature will need to apply private patches separately to re-enable IGD passthrough capability.

Any advice on how should Tiejun proceed from here?

Allen

> -----Original Message-----
> From: Chen, Tiejun
> Sent: Monday, August 18, 2014 6:39 PM
> To: Michael S. Tsirkin
> Cc: Paolo Bonzini; Kay, Allen M; Wang, Yong Y; Don Dutile; Jesse Barnes;
> Konrad Rzeszutek Wilk; qemu-devel at nongnu.org; xen-
> devel at lists.xensource.com; intel-gfx
> Subject: Re: How to create PCH to support those existing driver
> 
> 
> 
> On 2014/8/18 17:58, Michael S. Tsirkin wrote:
> > On Mon, Aug 18, 2014 at 05:01:25PM +0800, Chen, Tiejun wrote:
> >> On 2014/8/18 16:21, Michael S. Tsirkin wrote:
> >>> On Mon, Aug 18, 2014 at 11:06:29AM +0800, Chen, Tiejun wrote:
> >>>> On 2014/8/17 18:32, Michael S. Tsirkin wrote:
> >>>>> On Fri, Aug 15, 2014 at 09:58:40AM +0800, Chen, Tiejun wrote:
> >>>>>> Michael and Paolo,
> >>>>>
> >>>>> Please re-post discussion on list. These off list ones are just
> >>>>> wasting time since they invariably have to be repeated on list again.
> >>>>
> >>>> Okay, now just reissue this discussion to all related guys. And do
> >>>> you think we need to discuss in public, qemu and xen mail list?
> >>>
> >>> Absolutely.
> >>
> >> Now -CC qemu, xen and intel-gfx.
> >>
> >> If I'm missing someone important please tell me as well.
> >>
> >>>
> >>>>>
> >>>>>> After I created that new machine specific to IGD passthrough,
> >>>>>> xenigd, now I will step next to register the PCH.
> >>>>>>
> >>>>>> IIRC, our complete solution should be as follows:
> >>>>>>
> >>>>>> #1 create a new machine based on piix, xenigd
> >>>>>>
> >>>>>> This is done with Michael help.
> >>>>>>
> >>>>>> #2 register ISA bridge
> >>>>>>
> >>>>>> 1> Its still fixed at 1f.0.
> >>>>>> 2> ISA bridge's vendor_id/device_id should be emulated but then
> >>>>>>
> >>>>>> 	subsystem_vendor_id = PCI_VENDOR_ID_XEN;
> >>>>>> 	subsystem_device_id = ISA Bridge's real device id
> >>>>>>
> >>>>>> This mean we need to change driver to walk with this way.
> >>>>>> For example, in
> >>>>>> case of Linux native driver,
> >>>>>>
> >>>>>> intel_detect_pch()
> >>>>>> {
> >>>>>> 	...
> >>>>>> 	if (pch->subsystem_vendor == PCI_VENDOR_ID_XEN)
> >>>>>> 		id = pch->subsystem_device &
> INTEL_PCH_DEVICE_ID_MASK;
> >>>>>>
> >>>>>> Then driver can get that real device id by 'subsystem_device', right?
> >>>>>>
> >>>>>> This is fine now but how to support those existing drivers which
> >>>>>> are just
> >>>>
> >>>> Here correct one point, we don't need to care about supporting the
> >>>> legacy driver since the legacy driver still should work
> >>>> qemu-traditional. So we just make sure the existing driver with
> >>>> this subsystem_id way can support those existing and legacy platform.
> >>>>
> >>>> Now this is clear to me.
> >>>>
> >>>>>> dependent on checking real vendor_id/device_id directly,
> >>>>>>
> >>>>>> 	if (pch->vendor == PCI_VENDOR_ID_INTEL) {
> >>>>>> 		unsigned short id = pch->device &
> INTEL_PCH_DEVICE_ID_MASK
> >>>>>>
> >>>>>> Maybe I'm missing something, please hint me.
> >>>>>>
> >>>>>> Thanks
> >>>>>> Tiejun
> >>>>>
> >>>>> The subsystem id was just one idea.
> >>>>
> >>>> But from that email thread, "RH / Intel Virtualization Engineering
> >>>> Meeting - Minutes 7/10", I didn't see other idea we should prefer
> currently.
> >>>>
> >>>>> What was finally agreed for future drivers is that guests will get
> >>>>> all information they need from the video card, this ID hack was
> >>>>> needed only for very old legacy devices.
> >>>>> http://article.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel
> >>>>> /42258 So this is for newer guests, they will work without need
> >>>>> for hacks, like any other device.
> >>>>>
> >>>>
> >>>> Actually we had a meeting to discuss our future solution, but seems
> >>>> you were on vacation at that moment :)
> >>>>
> >>>> In that meeting we had an agreement between us and some upstream
> guys.
> >>>>
> >>>> We will have such a PCI capability structure in this PCI device to
> >>>> represent all information in the future. This make sens to Intel as well.
> >>>>
> >>>> Maybe Allen or Paolo known more details.
> >>>>
> >>>> But obviously this a long-term solution, so currently we will work
> >>>> with this subsystem_id way temporarily. And this way is accepted by
> >>>> those guys in the meeting.
> >>>
> >>>
> >>> I don't see the point really. If you are modifying the driver,
> >>
> >> Yes, we need to modify something in the driver.
> >>
> >>> why not modify it to its final form.
> >>
> >> What's your final form?
> >
> > Avoid poking at other devices besides the passed through card.
> > Get everything from BAR and other registers of the card.
> 
> Allen,
> 
> Could you reply this?
> 
> >
> >> As I track that email thread, seems the follows is just a way you
> >> guys achieve a better agreement.
> >>
> >> "
> >>> why not set the subsys vid to the RH/Quamranet/Virtio VID, so it's
> >>> obvious for the use-match?
> >>
> >> That's exactly the suggestion.  Though upstream they might be using
> >> the XenSource id since the patches were for Xen.
> >>
> >> Paolo
> >> "
> >> Or I'm missing something?
> >>
> >> Thanks
> >> Tiejun
> >
> >
> > I thought the point of this work is to make existing linux/windows
> > drivers work. Is it or isn't it?
> 
> Yes. We need change driver as I said previously like this,
> 
>  >>>>>> 2> ISA bridge's vendor_id/device_id should be emulated but then
>  >>>>>>
>  >>>>>> 	subsystem_vendor_id = PCI_VENDOR_ID_XEN;
>  >>>>>> 	subsystem_device_id = ISA Bridge's real device id
>  >>>>>>
>  >>>>>> This mean we need to change driver to walk with this way.
>  >>>>>> For example, in
>  >>>>>> case of Linux native driver,
>  >>>>>>
>  >>>>>> intel_detect_pch()
>  >>>>>> {
>  >>>>>> 	...
>  >>>>>> 	if (pch->subsystem_vendor == PCI_VENDOR_ID_XEN)
>  >>>>>> 		id = pch->subsystem_device &
> INTEL_PCH_DEVICE_ID_MASK;
> 
> This is a minimal change to driver as we discussed.
> 
> >
> > Wrt changing drivers, change them to behave sanely, like all other
> > drivers, and avoid poking at the chipset.
> > http://article.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/422
> > 58
> > Seems to suggest one way to do this.
> > Can what is suggested there work for existing devices?
> >
> 
> Again, Allen,
> 
> Are you sure if this suggestion can work?
> 
> Thanks
> Tiejun
> 
> >
> >>>
> >>>>>
> >>>>> For existing drivers: Vendor ID is intel anyway.
> >>>>
> >>>> Yes.
> >>>>
> >>>>> For device ID, override it through a property or something. But I
> >>>>> think poking at the real host from qemu is a mistake though, host
> >>>>> is not protected by iommu.
> >>>>> Two possible suggestions were to reverse-detect id of the device
> >>>>> from the card that is assigned,
> >>>>
> >>>> I guess you're saying pci_get_device(vendor/devices_ids), right?
> >>>>
> >>>>> or just make it a user property, and move the smarts to
> >>>>> management.
> >>>>
> >>>> Sorry could you elaborate this way in detail?
> >>>>
> >>>> Thanks
> >>>> Tiejun
> >>>
> >>>
> >>> Will do but let's do it on the mailing list.
> >>>
> >
> >