All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Chen, Tiejun" <tiejun.chen@intel.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	"Wang, Yong Y" <yong.y.wang@intel.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Don Dutile <ddutile@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: How to create PCH to support those existing driver
Date: Mon, 18 Aug 2014 11:58:05 +0200	[thread overview]
Message-ID: <20140818095805.GB26139@redhat.com> (raw)
In-Reply-To: <53F1C0E5.3080605@intel.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com>
To: "Chen, Tiejun" <tiejun.chen@intel.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>,
	"Kay, Allen M" <allen.m.kay@intel.com>,
	"Wang, Yong Y" <yong.y.wang@intel.com>,
	Jesse Barnes <jbarnes@virtuousgeek.org>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>,
	Don Dutile <ddutile@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [Qemu-devel] How to create PCH to support those existing driver
Date: Mon, 18 Aug 2014 11:58:05 +0200	[thread overview]
Message-ID: <20140818095805.GB26139@redhat.com> (raw)
In-Reply-To: <53F1C0E5.3080605@intel.com>

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

  reply	other threads:[~2014-08-18  9:57 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <6AF484C0160C61439DE06F17668F3BCB5331D9AD@ORSMSX114.amr.corp.intel.com>
     [not found] ` <B1FF49C389A4BE4F935FFB8048EF1E34028478EA@CRSMSX101.amr.corp.intel.com>
     [not found]   ` <53BED3D4.8040702@redhat.com>
     [not found]     ` <003AAFE53969E14CB1F09B6FD68C3CD478E91417@ORSMSX106.amr.corp.intel.com>
     [not found]       ` <53ED6950.6090101@intel.com>
     [not found]         ` <20140817103245.GD21622@redhat.com>
     [not found]           ` <53F16DB5.8070809@intel.com>
     [not found]             ` <20140818082148.GA26139@redhat.com>
2014-08-18  9:01               ` How to create PCH to support those existing driver Chen, Tiejun
2014-08-18  9:01                 ` [Qemu-devel] " Chen, Tiejun
2014-08-18  9:58                 ` Michael S. Tsirkin [this message]
2014-08-18  9:58                   ` Michael S. Tsirkin
2014-08-19  1:39                   ` Chen, Tiejun
2014-08-19  1:39                     ` [Qemu-devel] " Chen, Tiejun
2014-08-19 21:24                     ` Kay, Allen M
2014-08-19 21:24                       ` [Qemu-devel] " Kay, Allen M
2014-08-19 21:51                       ` Michael S. Tsirkin
2014-08-19 21:51                         ` [Qemu-devel] " Michael S. Tsirkin
2014-08-19 22:04                         ` Kay, Allen M
2014-08-19 22:04                           ` [Qemu-devel] " Kay, Allen M
2014-08-20  1:36                         ` Chen, Tiejun
2014-08-20  1:36                           ` [Qemu-devel] " Chen, Tiejun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140818095805.GB26139@redhat.com \
    --to=mst@redhat.com \
    --cc=ddutile@redhat.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=konrad.wilk@oracle.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=tiejun.chen@intel.com \
    --cc=xen-devel@lists.xensource.com \
    --cc=yong.y.wang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.