[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic
From: |
Chen, Tiejun |
Subject: |
Re: [Qemu-devel] [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support |
Date: |
Tue, 20 May 2014 05:09:53 +0000 |
> -----Original Message-----
> From: Stefano Stabellini [mailto:address@hidden
> Sent: Monday, May 19, 2014 8:10 PM
> To: Konrad Rzeszutek Wilk
> Cc: Chen, Tiejun; address@hidden; address@hidden;
> address@hidden; address@hidden; address@hidden;
> address@hidden; address@hidden; Kay, Allen M;
> address@hidden; address@hidden;
> address@hidden; Zhang, Yang Z
> Subject: Re: [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics
> passthrough support
>
> On Fri, 16 May 2014, Konrad Rzeszutek Wilk wrote:
> > On Fri, May 16, 2014 at 06:53:39PM +0800, Tiejun Chen wrote:
> > > basic gfx passthrough support:
> > > - add a vga type for gfx passthrough
> > > - retrieve VGA bios from sysfs, then load it to guest 0xC0000
> > > - register/unregister legacy VGA I/O ports and MMIOs for
> > > passthroughed gfx
> > >
> > > The original patch is from Weidong Han <address@hidden>
> > >
> > > Signed-off-by: Yang Zhang <address@hidden>
> > > Signed-off-by: Tiejun Chen <address@hidden>
> > > Cc: Weidong Han <address@hidden>
> > > ---
> > > v2:
> > >
> > > * retrieve VGA bios from sysfs properly.
> > > * redefine some function name.
> > >
> > > hw/xen/Makefile.objs | 2 +-
> > > hw/xen/xen-host-pci-device.c | 5 ++
> > > hw/xen/xen-host-pci-device.h | 1 +
> > > hw/xen/xen_pt.c | 10 +++
> > > hw/xen/xen_pt.h | 4 +
> > > hw/xen/xen_pt_graphics.c | 177
> +++++++++++++++++++++++++++++++++++++++++++
> > > qemu-options.hx | 9 +++
> > > vl.c | 11 ++-
> > > 8 files changed, 217 insertions(+), 2 deletions(-) create mode
> > > 100644 hw/xen/xen_pt_graphics.c
> > >
> > > diff --git a/hw/xen/Makefile.objs b/hw/xen/Makefile.objs index
> > > a0ca0aa..77b7dae 100644
> > > --- a/hw/xen/Makefile.objs
> > > +++ b/hw/xen/Makefile.objs
> > > @@ -2,4 +2,4 @@
> > > common-obj-$(CONFIG_XEN_BACKEND) += xen_backend.o
> xen_devconfig.o
> > >
> > > obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen-host-pci-device.o
> > > -obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
> > > xen_pt_msi.o
> > > +obj-$(CONFIG_XEN_PCI_PASSTHROUGH) += xen_pt.o xen_pt_config_init.o
> > > +xen_pt_msi.o xen_pt_graphics.o
> > > diff --git a/hw/xen/xen-host-pci-device.c
> > > b/hw/xen/xen-host-pci-device.c index 743b37b..a54b7de 100644
> > > --- a/hw/xen/xen-host-pci-device.c
> > > +++ b/hw/xen/xen-host-pci-device.c
> > > @@ -376,6 +376,11 @@ int xen_host_pci_device_get(XenHostPCIDevice
> *d, uint16_t domain,
> > > goto error;
> > > }
> > > d->irq = v;
> > > + rc = xen_host_pci_get_hex_value(d, "class", &v);
> > > + if (rc) {
> > > + goto error;
> > > + }
> > > + d->class_code = v;
> > > d->is_virtfn = xen_host_pci_dev_is_virtfn(d);
> > >
> > > return 0;
> > > diff --git a/hw/xen/xen-host-pci-device.h
> > > b/hw/xen/xen-host-pci-device.h index c2486f0..f1e1c30 100644
> > > --- a/hw/xen/xen-host-pci-device.h
> > > +++ b/hw/xen/xen-host-pci-device.h
> > > @@ -25,6 +25,7 @@ typedef struct XenHostPCIDevice {
> > >
> > > uint16_t vendor_id;
> > > uint16_t device_id;
> > > + uint32_t class_code;
> > > int irq;
> > >
> > > XenHostPCIIORegion io_regions[PCI_NUM_REGIONS - 1]; diff --git
> > > a/hw/xen/xen_pt.c b/hw/xen/xen_pt.c index be4220b..a0113ea 100644
> > > --- a/hw/xen/xen_pt.c
> > > +++ b/hw/xen/xen_pt.c
> > > @@ -450,6 +450,7 @@ static int
> xen_pt_register_regions(XenPCIPassthroughState *s)
> > > d->rom.size, d->rom.base_addr);
> > > }
> > >
> > > + xen_pt_register_vga_regions(d);
> > > return 0;
> > > }
> > >
> > > @@ -470,6 +471,8 @@ static void
> xen_pt_unregister_regions(XenPCIPassthroughState *s)
> > > if (d->rom.base_addr && d->rom.size) {
> > > memory_region_destroy(&s->rom);
> > > }
> > > +
> > > + xen_pt_unregister_vga_regions(d);
> > > }
> > >
> > > /* region mapping */
> > > @@ -693,6 +696,13 @@ static int xen_pt_initfn(PCIDevice *d)
> > > /* Handle real device's MMIO/PIO BARs */
> > > xen_pt_register_regions(s);
> > >
> > > + /* Setup VGA bios for passthroughed gfx */
> > > + if (xen_pt_setup_vga(&s->real_device) < 0) {
> > > + XEN_PT_ERR(d, "Setup VGA BIOS of passthroughed gfx
> failed!\n");
> > > + xen_host_pci_device_put(&s->real_device);
> > > + return -1;
> > > + }
> > > +
> > > /* reinitialize each config register to be emulated */
> > > if (xen_pt_config_init(s)) {
> > > XEN_PT_ERR(d, "PCI Config space initialisation failed.\n");
> > > diff --git a/hw/xen/xen_pt.h b/hw/xen/xen_pt.h index
> > > 942dc60..4d3a18d 100644
> > > --- a/hw/xen/xen_pt.h
> > > +++ b/hw/xen/xen_pt.h
> > > @@ -298,5 +298,9 @@ static inline bool
> xen_pt_has_msix_mapping(XenPCIPassthroughState *s, int bar)
> > > return s->msix && s->msix->bar_index == bar; }
> > >
> > > +extern int xen_has_gfx_passthru;
> > > +int xen_pt_register_vga_regions(XenHostPCIDevice *dev); int
> > > +xen_pt_unregister_vga_regions(XenHostPCIDevice *dev); int
> > > +xen_pt_setup_vga(XenHostPCIDevice *dev);
> > >
> > > #endif /* !XEN_PT_H */
> > > diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c new
> > > file mode 100644 index 0000000..e1f0724
> > > --- /dev/null
> > > +++ b/hw/xen/xen_pt_graphics.c
> > > @@ -0,0 +1,177 @@
> > > +/*
> > > + * graphics passthrough
> > > + */
> > > +#include "xen_pt.h"
> > > +#include "xen-host-pci-device.h"
> > > +#include "hw/xen/xen_backend.h"
> > > +
> > > +static int is_vga_passthrough(XenHostPCIDevice *dev) {
> > > + return (xen_has_gfx_passthru
> > > + && ((dev->class_code >> 0x8) ==
> > > +PCI_CLASS_DISPLAY_VGA)); }
> > > +
> > > +/*
> > > + * register VGA resources for the domain with assigned gfx */ int
> > > +xen_pt_register_vga_regions(XenHostPCIDevice *dev) {
> > > + int ret = 0;
> > > +
> > > + if (!is_vga_passthrough(dev)) {
> > > + return ret;
> > > + }
> > > +
> > > + ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3B0,
> > > + 0x3B0, 0xA, DPCI_ADD_MAPPING);
>
> The original code does:
>
> ret |= xc_domain_ioport_mapping(xc_handle, domid, 0x3B0,
> 0x3B0, 0xC, DPCI_ADD_MAPPING);
>
> why are we remapping fewer ports now?
>
>
> > > + ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3C0,
> > > + 0x3C0, 0x20, DPCI_ADD_MAPPING);
> > > +
> > > + ret |= xc_domain_memory_mapping(xen_xc, xen_domid,
> > > + 0xa0000 >> XC_PAGE_SHIFT,
> > > + 0xa0000 >> XC_PAGE_SHIFT,
> > > + 0x20,
> > > + DPCI_ADD_MAPPING);
> > > +
> > > + if (ret) {
> > > + XEN_PT_ERR(NULL, "VGA region mapping failed\n");
> >
> > It would be actually useful to know _which_ of them failed. Perhaps
> > you could structure this a bit differently and do:
> >
> >
> > struct _args {
> > uint32_t gport;
> > uint32_t mport;
> > uint32_t nport;
> > };
> >
> > struct _args args[3] = {{ 0x3B0, 0x3B0, 0xA }, {...}};
> > unsigned int i;
> >
> > for (i = 0; i < ARRAY_SIZE(args); i++) {
> > ret = xc_domain_ioport_mapping(xen_xc, xen_domid, args[i].gport,
> args[i]..)
> > if (ret) {
> > XEN_PT_ERR_(NULL, "VGA region mapping of 0x%lx for 0x%x
> pages failed with rc:%d\n",
> > ... fill in the values)
> > return ret;
> > }
> >
> >
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +/*
> > > + * unregister VGA resources for the domain with assigned gfx */
> > > +int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev) {
> > > + int ret = 0;
> > > +
> > > + if (!is_vga_passthrough(dev)) {
> > > + return ret;
> > > + }
> > > +
> > > + ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3B0,
> > > + 0x3B0, 0xC, DPCI_REMOVE_MAPPING);
>
> But here we are unmapping 0xC ioports....
>
They're typos and they should be introduced by the original v1.
Actually I also notice this yesterday when I try to improve this part with your
comments, you can see I already unify these places when I reply to you online.
Thanks
Tiejun
- [Qemu-devel] [v2][PATCH 1/8] pci: use bitmap to manage registe/runregister pci device, (continued)
- [Qemu-devel] [v2][PATCH 1/8] pci: use bitmap to manage registe/runregister pci device, Tiejun Chen, 2014/05/16
- [Qemu-devel] [v2][PATCH 2/8] pci: provide a way to reserve some specific devfn, Tiejun Chen, 2014/05/16
- [Qemu-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support, Tiejun Chen, 2014/05/16
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support, Konrad Rzeszutek Wilk, 2014/05/16
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support, Chen, Tiejun, 2014/05/19
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support, Konrad Rzeszutek Wilk, 2014/05/19
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support, Chen, Tiejun, 2014/05/20
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support, Stefano Stabellini, 2014/05/19
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support,
Chen, Tiejun <=
[Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Tiejun Chen, 2014/05/16
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Konrad Rzeszutek Wilk, 2014/05/16
- Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Gerd Hoffmann, 2014/05/19
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Fabio Fantoni, 2014/05/19
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Zhang, Yang Z, 2014/05/19
- Re: [Qemu-devel] [Xen-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Chen, Tiejun, 2014/05/19
- Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Chen, Tiejun, 2014/05/19
- Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Michael S. Tsirkin, 2014/05/19
- Re: [Qemu-devel] [v2][PATCH 4/8] xen, gfx passthrough: reserve 00:02.0 for INTEL IGD, Chen, Tiejun, 2014/05/20