Re: [Qemu-devel] How to create PCH to support those existing driver

Re: [Qemu-devel] How to create PCH to support those existing driver

[Chen, Tiejun]

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?

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

>
>>>
>>> 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.
>

Re: [Qemu-devel] How to create PCH to support those existing driver

[Michael S. Tsirkin]

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.

> 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?

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/42258
Seems to suggest one way to do this.
Can what is suggested there work for existing devices?


> >
> >>>
> >>>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.
> >

Re: [Qemu-devel] How to create PCH to support those existing driver

[Chen, Tiejun]

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/42258
> 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.
>>>
>
>

Re: [Qemu-devel] 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@nongnu.org; xen-
> devel@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.
> >>>
> >
> >

Re: [Qemu-devel] How to create PCH to support those existing driver

[Michael S. Tsirkin]

On Tue, Aug 19, 2014 at 09:24:03PM +0000, Kay, Allen M wrote:
> > 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.

Excellent. I was thinking about changing host's driver to do the writes
in a safe manner, but if don't need that, all the better.
As a next step, we need to limit read operations to specific set of registers
that we can validate.
We can't just pass through reads blindly to host, pci reads have side-effects
and host chipset isn't protected by the iommu.
Since these are legacy devices and drivers, it should be possible to
enumerate all registers that they need.


> 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

I'm fine with either trying to make existing windows and linux drivers
work, or waiting for future drivers.

To me, quick hacks that need minor changes
in driver but don't avoid poking at MCH/PCH don't seem to have value,
I know I proposed some of these myself but this was before I
realised a cleaner solution is possible.


-- 
MST

Re: [Qemu-devel] How to create PCH to support those existing driver

[Kay, Allen M]

> -----Original Message-----
> From: Michael S. Tsirkin [mailto:mst@redhat.com]
> Sent: Tuesday, August 19, 2014 2:51 PM
> To: Kay, Allen M
> Cc: Chen, Tiejun; Paolo Bonzini; Wang, Yong Y; Don Dutile; Jesse Barnes;
> Konrad Rzeszutek Wilk; qemu-devel@nongnu.org; xen-
> devel@lists.xensource.com; intel-gfx; Stefano Stabellini
> (Stefano.Stabellini@citrix.com)
> Subject: Re: How to create PCH to support those existing driver
> 
> On Tue, Aug 19, 2014 at 09:24:03PM +0000, Kay, Allen M wrote:
> > > 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.
> 
> Excellent. I was thinking about changing host's driver to do the writes in a
> safe manner, but if don't need that, all the better.
> As a next step, we need to limit read operations to specific set of registers
> that we can validate.
> We can't just pass through reads blindly to host, pci reads have side-effects
> and host chipset isn't protected by the iommu.
> Since these are legacy devices and drivers, it should be possible to
> enumerate all registers that they need.
> 

If we limit platform support to HSW/BDW the number of register reads is quite small.  I believe some of the register reads are for the old Ironlake platforms.  I will work with Tiejun to get the smaller set for HSW/BDW systems.

> 
> > 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
> 
> I'm fine with either trying to make existing windows and linux drivers work,
> or waiting for future drivers.
> 
> To me, quick hacks that need minor changes in driver but don't avoid poking
> at MCH/PCH don't seem to have value, I know I proposed some of these
> myself but this was before I realised a cleaner solution is possible.
> 
> 
> --
> MST

Allen

Re: [Qemu-devel] How to create PCH to support those existing driver

[Chen, Tiejun]

On 2014/8/20 5:51, Michael S. Tsirkin wrote:
> On Tue, Aug 19, 2014 at 09:24:03PM +0000, Kay, Allen M wrote:
>>> 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.
>
> Excellent. I was thinking about changing host's driver to do the writes
> in a safe manner, but if don't need that, all the better.
> As a next step, we need to limit read operations to specific set of registers
> that we can validate.
> We can't just pass through reads blindly to host, pci reads have side-effects
> and host chipset isn't protected by the iommu.
> Since these are legacy devices and drivers, it should be possible to
> enumerate all registers that they need.
>
>
>> 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
>
> I'm fine with either trying to make existing windows and linux drivers
> work, or waiting for future drivers.

Michael and Allen,

As I concern, now we may not take a consideration of supporting those 
existing drivers, just leave to qemu-traditional in Xen code repertory.

I think what we should do here just focus on supporting all platforms 
including those legacy platforms.

>
> To me, quick hacks that need minor changes
> in driver but don't avoid poking at MCH/PCH don't seem to have value,

So to me, that subsystem id way is more clear rather than others because 
I'm not sure its really possible not to poke MCH/PCH theoretically both 
Windows and Linux driver.

Allen,

I think Michael is saying this,

http://article.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/42258

What about your opinion?

> I know I proposed some of these myself but this was before I
> realised a cleaner solution is possible.
>

I think all we are fine if this really come true :)

Tiejun