All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Kay, Allen M" <allen.m.kay@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: "igvt-g@ml01.01.org" <igvt-g@ml01.01.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Cao jin <caoj.fnst@cn.fujitsu.com>,
	"vfio-users@redhat.com" <vfio-users@redhat.com>
Subject: Re: [Qemu-devel] [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset	tweaks
Date: Fri, 29 Jan 2016 02:22:57 +0000	[thread overview]
Message-ID: <003AAFE53969E14CB1F09B6FD68C3CD47BB669D2@ORSMSX106.amr.corp.intel.com> (raw)
In-Reply-To: <1454009759.7183.7.camel@redhat.com>



> -----Original Message-----
> From: iGVT-g [mailto:igvt-g-bounces@lists.01.org] On Behalf Of Alex
> Williamson
> Sent: Thursday, January 28, 2016 11:36 AM
> To: Gerd Hoffmann; qemu-devel@nongnu.org
> Cc: igvt-g@ml01.01.org; xen-devel@lists.xensource.com; Eduardo Habkost;
> Stefano Stabellini; Cao jin; vfio-users@redhat.com
> Subject: Re: [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset
> tweaks
> 
> 
> 1) The OpRegion MemoryRegion is mapped into system_memory through
> programming of the 0xFC config space register.
>  a) vfio-pci could pick an address to do this as it is realized.
>  b) SeaBIOS/OVMF could program this.
> 
> Discussion: 1.a) Avoids any BIOS dependency, but vfio-pci would need to pick
> an address and mark it as e820 reserved.  I'm not sure how to pick that
> address.  We'd probably want to make the 0xFC config register read-
> only.  1.b) has the issue you mentioned where in most cases the OpRegion
> will be 8k, but the BIOS won't know how much address space it's mapping
> into system memory when it writes the 0xFC register.  I don't know how
> much of a problem this is since the BIOS can easily determine the size once
> mapped and re-map it somewhere there's sufficient space.
> Practically, it seems like it's always going to be 8K.  This of course requires
> modification to every BIOS.  It also leaves the 0xFC register as a mapping
> control rather than a pointer to the OpRegion in RAM, which doesn't really
> match real hardware.  The BIOS would need to pick an address in this case.
> 
> 2) Read-only mappings version of 1)
> 
> Discussion: Really nothing changes from the issues above, just prevents any
> possibility of the guest modifying anything in the host.  Xen apparently allows
> write access to the host page already.
> 
> 3) Copy OpRegion contents into buffer and do either 1) or 2) above.
> 
> Discussion: No benefit that I can see over above other than maybe allowing
> write access that doesn't affect the host.
> 
> 4) Copy contents into a guest RAM location, mark it reserved, point to it via
> 0xFC config as scratch register.
>  a) Done by QEMU (vfio-pci)
>  b) Done by SeaBIOS/OVMF
> 
> Discussion: This is the most like real hardware.  4.a) has the usual issue of
> how to pick an address, but the benefit of not requiring BIOS changes (simply
> mark the RAM reserved via existing methods).  4.b) would require passing a
> buffer containing the contents of the OpRegion via fw_cfg and letting the
> BIOS do the setup.  The latter of course requires modifying each BIOS for this
> support.
> 
> Of course none of these support hotplug nor really can they since reserved
> memory regions are not dynamic in the architecture.
> 
> In all cases, some piece of software needs to know where it can place the
> OpRegion in guest memory.  It seems like there are advantages or
> disadvantages whether that's done by QEMU or the BIOS, but we only need
> to do it once if it's QEMU.  Suggestions, comments, preferences?
> 

Hi Alex, another thing to consider is how to communicate to the guest driver the address at 0xFC contains a valid GPA address that can be accessed by the driver without causing a EPT fault - since the same driver will be used on other hypervisors and they may not EPT map OpRegion memory.  On idea proposed by display driver team is to set bit0 of the address to 1 for indicating OpRegion memory can be safely accessed by the guest driver.

> 
> Another thing I notice in this series is the access to PCI config space of both
> the host bridge and the LPC bridge.  This prevents unprivileged use cases and
> is a barrier to libvirt support since it will need to provide access to the pci-
> sysfs files for the process.  Should vfio add additional device specific regions
> to expose the config space of these other devices?  I don't see that there's
> any write access necessary, so these would be read-only.  The comment in
> the kernel regarding why an unprivileged user can only access standard
> config space indicates that some devices lockup if unimplemented config
> space is accessed.  It seems like that's probably not an issue for recent-ish
> Intel host bridges and LPC devices.  If OpRegion, host bridge config, and LPC
> config were all provided through vfio, would there be any need for igd-
> passthrough switches on the machine type?  It seems like the QEMU vfio-pci
> driver could enable the necessary features and pre-fill the host and LPC
> bridge config items on demand when parsing an IGD device.  Thanks,
> 
> Alex
> 
> __

Allen
_____________________________________________
> iGVT-g mailing list
> iGVT-g@lists.01.org
> https://lists.01.org/mailman/listinfo/igvt-g

WARNING: multiple messages have this Message-ID (diff)
From: "Kay, Allen M" <allen.m.kay@intel.com>
To: Alex Williamson <alex.williamson@redhat.com>,
	Gerd Hoffmann <kraxel@redhat.com>,
	"qemu-devel@nongnu.org" <qemu-devel@nongnu.org>
Cc: "igvt-g@ml01.01.org" <igvt-g@ml01.01.org>,
	"xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	Stefano Stabellini <stefano.stabellini@eu.citrix.com>,
	Cao jin <caoj.fnst@cn.fujitsu.com>,
	"vfio-users@redhat.com" <vfio-users@redhat.com>
Subject: Re: [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset	tweaks
Date: Fri, 29 Jan 2016 02:22:57 +0000	[thread overview]
Message-ID: <003AAFE53969E14CB1F09B6FD68C3CD47BB669D2@ORSMSX106.amr.corp.intel.com> (raw)
In-Reply-To: <1454009759.7183.7.camel@redhat.com>



> -----Original Message-----
> From: iGVT-g [mailto:igvt-g-bounces@lists.01.org] On Behalf Of Alex
> Williamson
> Sent: Thursday, January 28, 2016 11:36 AM
> To: Gerd Hoffmann; qemu-devel@nongnu.org
> Cc: igvt-g@ml01.01.org; xen-devel@lists.xensource.com; Eduardo Habkost;
> Stefano Stabellini; Cao jin; vfio-users@redhat.com
> Subject: Re: [iGVT-g] [vfio-users] [PATCH v3 00/11] igd passthrough chipset
> tweaks
> 
> 
> 1) The OpRegion MemoryRegion is mapped into system_memory through
> programming of the 0xFC config space register.
>  a) vfio-pci could pick an address to do this as it is realized.
>  b) SeaBIOS/OVMF could program this.
> 
> Discussion: 1.a) Avoids any BIOS dependency, but vfio-pci would need to pick
> an address and mark it as e820 reserved.  I'm not sure how to pick that
> address.  We'd probably want to make the 0xFC config register read-
> only.  1.b) has the issue you mentioned where in most cases the OpRegion
> will be 8k, but the BIOS won't know how much address space it's mapping
> into system memory when it writes the 0xFC register.  I don't know how
> much of a problem this is since the BIOS can easily determine the size once
> mapped and re-map it somewhere there's sufficient space.
> Practically, it seems like it's always going to be 8K.  This of course requires
> modification to every BIOS.  It also leaves the 0xFC register as a mapping
> control rather than a pointer to the OpRegion in RAM, which doesn't really
> match real hardware.  The BIOS would need to pick an address in this case.
> 
> 2) Read-only mappings version of 1)
> 
> Discussion: Really nothing changes from the issues above, just prevents any
> possibility of the guest modifying anything in the host.  Xen apparently allows
> write access to the host page already.
> 
> 3) Copy OpRegion contents into buffer and do either 1) or 2) above.
> 
> Discussion: No benefit that I can see over above other than maybe allowing
> write access that doesn't affect the host.
> 
> 4) Copy contents into a guest RAM location, mark it reserved, point to it via
> 0xFC config as scratch register.
>  a) Done by QEMU (vfio-pci)
>  b) Done by SeaBIOS/OVMF
> 
> Discussion: This is the most like real hardware.  4.a) has the usual issue of
> how to pick an address, but the benefit of not requiring BIOS changes (simply
> mark the RAM reserved via existing methods).  4.b) would require passing a
> buffer containing the contents of the OpRegion via fw_cfg and letting the
> BIOS do the setup.  The latter of course requires modifying each BIOS for this
> support.
> 
> Of course none of these support hotplug nor really can they since reserved
> memory regions are not dynamic in the architecture.
> 
> In all cases, some piece of software needs to know where it can place the
> OpRegion in guest memory.  It seems like there are advantages or
> disadvantages whether that's done by QEMU or the BIOS, but we only need
> to do it once if it's QEMU.  Suggestions, comments, preferences?
> 

Hi Alex, another thing to consider is how to communicate to the guest driver the address at 0xFC contains a valid GPA address that can be accessed by the driver without causing a EPT fault - since the same driver will be used on other hypervisors and they may not EPT map OpRegion memory.  On idea proposed by display driver team is to set bit0 of the address to 1 for indicating OpRegion memory can be safely accessed by the guest driver.

> 
> Another thing I notice in this series is the access to PCI config space of both
> the host bridge and the LPC bridge.  This prevents unprivileged use cases and
> is a barrier to libvirt support since it will need to provide access to the pci-
> sysfs files for the process.  Should vfio add additional device specific regions
> to expose the config space of these other devices?  I don't see that there's
> any write access necessary, so these would be read-only.  The comment in
> the kernel regarding why an unprivileged user can only access standard
> config space indicates that some devices lockup if unimplemented config
> space is accessed.  It seems like that's probably not an issue for recent-ish
> Intel host bridges and LPC devices.  If OpRegion, host bridge config, and LPC
> config were all provided through vfio, would there be any need for igd-
> passthrough switches on the machine type?  It seems like the QEMU vfio-pci
> driver could enable the necessary features and pre-fill the host and LPC
> bridge config items on demand when parsing an IGD device.  Thanks,
> 
> Alex
> 
> __

Allen
_____________________________________________
> iGVT-g mailing list
> iGVT-g@lists.01.org
> https://lists.01.org/mailman/listinfo/igvt-g

  reply	other threads:[~2016-01-29  2:23 UTC|newest]

Thread overview: 132+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-05 11:41 [Qemu-devel] [PATCH v3 00/11] igd passthrough chipset tweaks Gerd Hoffmann
2016-01-05 11:41 ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 01/11] pc: wire up TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE for !xen Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 02/11] pc: remove has_igd_gfx_passthru global Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 14:32   ` [Qemu-devel] [Xen-devel] " Stefano Stabellini
2016-01-06 14:32     ` Stefano Stabellini
2016-01-19 15:09   ` [Qemu-devel] " Eduardo Habkost
2016-01-19 15:09     ` Eduardo Habkost
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 03/11] pc: move igd support code to igd.c Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 04/11] igd: switch TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE to realize Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 14:32   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 14:32     ` Stefano Stabellini
2016-01-23 14:51   ` [Qemu-devel] " Eduardo Habkost
2016-01-23 14:51     ` Eduardo Habkost
2016-01-25  8:59     ` [Qemu-devel] " Gerd Hoffmann
2016-01-25  8:59       ` Gerd Hoffmann
2016-01-25 11:53       ` [Qemu-devel] " Stefano Stabellini
2016-01-25 11:53         ` Stefano Stabellini
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 05/11] igd: TYPE_IGD_PASSTHROUGH_I440FX_PCI_DEVICE: call parent realize Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 14:41   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 14:41     ` Stefano Stabellini
2016-01-06 15:45     ` [Qemu-devel] " Gerd Hoffmann
2016-01-06 15:45       ` Gerd Hoffmann
2016-01-19 15:13       ` [Qemu-devel] " Eduardo Habkost
2016-01-19 15:13         ` Eduardo Habkost
2016-01-20  9:10         ` [Qemu-devel] " Gerd Hoffmann
2016-01-20  9:10           ` Gerd Hoffmann
2016-01-23 14:52           ` [Qemu-devel] " Eduardo Habkost
2016-01-23 14:52             ` Eduardo Habkost
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 06/11] igd: use defines for standard pci config space offsets Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 14:43   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 14:43     ` Stefano Stabellini
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 07/11] igd: revamp host config read Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 15:02   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 15:02     ` Stefano Stabellini
2016-01-06 15:51     ` [Qemu-devel] " Gerd Hoffmann
2016-01-06 15:51       ` Gerd Hoffmann
2016-01-06 16:23       ` [Qemu-devel] [Xen-devel] " Stefano Stabellini
2016-01-06 16:23         ` Stefano Stabellini
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 08/11] igd: add q35 support Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 09/11] igd: move igd-passthrough-isa-bridge to igd.c too Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 10/11] igd: handle igd-passthrough-isa-bridge setup in realize() Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 15:29   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 15:29     ` Stefano Stabellini
2016-01-06 15:52     ` [Qemu-devel] " Gerd Hoffmann
2016-01-06 15:52       ` Gerd Hoffmann
2016-01-05 11:41 ` [Qemu-devel] [PATCH v3 11/11] igd: move igd-passthrough-isa-bridge creation to machine init Gerd Hoffmann
2016-01-05 11:41   ` Gerd Hoffmann
2016-01-06 15:36   ` [Qemu-devel] " Stefano Stabellini
2016-01-06 15:36     ` Stefano Stabellini
2016-01-07  7:38     ` [Qemu-devel] " Gerd Hoffmann
2016-01-07  7:38       ` Gerd Hoffmann
2016-01-07 13:10       ` [Qemu-devel] " Stefano Stabellini
2016-01-07 13:10         ` Stefano Stabellini
2016-01-07 15:50         ` [Qemu-devel] " Gerd Hoffmann
2016-01-07 15:50           ` Gerd Hoffmann
2016-01-08 11:20           ` Stefano Stabellini
2016-01-08 11:20             ` Stefano Stabellini
2016-01-08 12:12             ` [Qemu-devel] " Stefano Stabellini
2016-01-08 12:12               ` Stefano Stabellini
2016-01-08 12:32               ` Gerd Hoffmann
2016-01-08 12:32                 ` Gerd Hoffmann
2016-01-08 12:38                 ` [Qemu-devel] " Stefano Stabellini
2016-01-08 12:38                   ` Stefano Stabellini
2016-01-05 13:07 ` [Qemu-devel] [PATCH v3 00/11] igd passthrough chipset tweaks Michael S. Tsirkin
2016-01-05 13:07   ` Michael S. Tsirkin
2016-01-28 19:35 ` [Qemu-devel] [vfio-users] " Alex Williamson
2016-01-28 19:35   ` Alex Williamson
2016-01-29  2:22   ` Kay, Allen M [this message]
2016-01-29  2:22     ` [iGVT-g] " Kay, Allen M
2016-01-29  2:54     ` [Qemu-devel] " Alex Williamson
2016-01-29  2:54       ` Alex Williamson
2016-01-29  6:21       ` [Qemu-devel] " Jike Song
2016-01-29  6:21         ` Jike Song
2016-01-29 21:58       ` [Qemu-devel] " Kay, Allen M
2016-01-29 21:58         ` Kay, Allen M
2016-02-02  7:07         ` [Qemu-devel] " Tian, Kevin
2016-02-02  7:07           ` Tian, Kevin
2016-02-02 19:10           ` [Qemu-devel] " Kay, Allen M
2016-02-02 19:10             ` [iGVT-g] " Kay, Allen M
2016-02-02 19:37             ` [Qemu-devel] [iGVT-g] [vfio-users] " Alex Williamson
2016-02-02 19:37               ` Alex Williamson
2016-02-02 23:32               ` [Qemu-devel] " Kay, Allen M
2016-02-02 23:32                 ` Kay, Allen M
2016-01-29  7:09   ` [Qemu-devel] " Gerd Hoffmann
2016-01-29  7:09     ` Gerd Hoffmann
2016-01-29 17:59     ` [Qemu-devel] " Alex Williamson
2016-01-29 17:59       ` Alex Williamson
2016-01-30  1:18       ` [Qemu-devel] [iGVT-g] " Kay, Allen M
2016-01-30  1:18         ` Kay, Allen M
2016-01-31 17:42         ` [Qemu-devel] " Alex Williamson
2016-01-31 17:42           ` Alex Williamson
2016-02-02  0:04           ` [Qemu-devel] " Kay, Allen M
2016-02-02  0:04             ` Kay, Allen M
2016-02-02  6:42             ` [Qemu-devel] [Xen-devel] " Tian, Kevin
2016-02-02  6:42               ` Tian, Kevin
2016-02-02 11:50               ` [Qemu-devel] " David Woodhouse
2016-02-02 11:50                 ` David Woodhouse
2016-02-02 14:54                 ` [Qemu-devel] " Alex Williamson
2016-02-02 14:54                   ` Alex Williamson
2016-02-02 15:06                   ` [Qemu-devel] " David Woodhouse
2016-02-02 15:06                     ` David Woodhouse
2016-02-02 14:38             ` [Qemu-devel] " Alex Williamson
2016-02-02 14:38               ` Alex Williamson
2016-02-01 12:49       ` [Qemu-devel] " Gerd Hoffmann
2016-02-01 12:49         ` Gerd Hoffmann
2016-02-01 22:16         ` [Qemu-devel] " Alex Williamson
2016-02-01 22:16           ` Alex Williamson
2016-02-02  7:43           ` [Qemu-devel] [vfio-users] " Gerd Hoffmann
2016-02-02  7:43             ` Gerd Hoffmann
2016-02-02  7:01         ` [Qemu-devel] [iGVT-g] " Tian, Kevin
2016-02-02  7:01           ` [iGVT-g] " Tian, Kevin
2016-02-02  8:56           ` [Qemu-devel] [iGVT-g] [vfio-users] " Gerd Hoffmann
2016-02-02  8:56             ` Gerd Hoffmann
2016-02-02 16:31             ` [Qemu-devel] " Kevin O'Connor
2016-02-02 16:31               ` Kevin O'Connor
2016-02-02 16:49               ` [Qemu-devel] " Laszlo Ersek
2016-02-02 16:49                 ` Laszlo Ersek
2016-02-02 20:18               ` [Qemu-devel] " Alex Williamson
2016-02-02 20:18                 ` Alex Williamson
2016-02-03  6:08             ` [Qemu-devel] " Tian, Kevin
2016-02-03  6:08               ` Tian, Kevin

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=003AAFE53969E14CB1F09B6FD68C3CD47BB669D2@ORSMSX106.amr.corp.intel.com \
    --to=allen.m.kay@intel.com \
    --cc=alex.williamson@redhat.com \
    --cc=caoj.fnst@cn.fujitsu.com \
    --cc=ehabkost@redhat.com \
    --cc=igvt-g@ml01.01.org \
    --cc=kraxel@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=stefano.stabellini@eu.citrix.com \
    --cc=vfio-users@redhat.com \
    --cc=xen-devel@lists.xensource.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.