Re: [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics passthrough support

["Chen, Tiejun" <tiejun.chen@xxxxxxxxx>]

Just resend since looks this delivery is delayed to these recipients or groups.

Sorry for any inconveniences.

Thanks
Tiejun

> -----Original Message-----
> From: Chen, Tiejun
> Sent: Tuesday, May 20, 2014 9:30 AM
> To: 'Konrad Rzeszutek Wilk'
> Cc: anthony.perard@xxxxxxxxxx; stefano.stabellini@xxxxxxxxxxxxx;
> mst@xxxxxxxxxx; Kelly.Zytaruk@xxxxxxx; peter.maydell@xxxxxxxxxx;
> xen-devel@xxxxxxxxxxxxxxxxxxx; Kay, Allen M; qemu-devel@xxxxxxxxxx;
> anthony@xxxxxxxxxxxxx; Zhang, Yang Z
> Subject: RE: [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic graphics
> passthrough support
> 
> > -----Original Message-----
> > From: Konrad Rzeszutek Wilk [mailto:konrad.wilk@xxxxxxxxxx]
> > Sent: Monday, May 19, 2014 9:36 PM
> > To: Chen, Tiejun
> > Cc: anthony.perard@xxxxxxxxxx; stefano.stabellini@xxxxxxxxxxxxx;
> > mst@xxxxxxxxxx; Kelly.Zytaruk@xxxxxxx; peter.maydell@xxxxxxxxxx;
> > xen-devel@xxxxxxxxxxxxxxxxxxx; Kay, Allen M; qemu-devel@xxxxxxxxxx;
> > anthony@xxxxxxxxxxxxx; Zhang, Yang Z
> > Subject: Re: [Xen-devel] [v2][PATCH 3/8] xen, gfx passthrough: basic
> > graphics passthrough support
> >
> 
> [snip]
> 
> > > Looks good so what about this based on the original,
> > >
> > > --- a/hw/xen/xen_pt_graphics.c
> > > +++ b/hw/xen/xen_pt_graphics.c
> > > @@ -14,34 +14,73 @@ static int is_vga_passthrough(XenHostPCIDevice
> > *dev)
> > >              && ((dev->class_code >> 0x8) ==
> > PCI_CLASS_DISPLAY_VGA));
> > > }
> > >
> > > +typedef struct VGARegion {
> > > +    int type;           /* Memory or port I/O */
> > > +    uint64_t guest_base_addr;
> > > +    uint64_t machine_base_addr;
> > > +    uint64_t size;    /* size of the region */
> > > +    int rc;
> > > +} VGARegion;
> > > +
> > > +#define IORESOURCE_IO           0x00000100
> > > +#define IORESOURCE_MEM          0x00000200
> > > +
> > > +static struct VGARegion vga_args[] = {
> > > +    {
> > > +        .type = IORESOURCE_IO,
> > > +        .guest_base_addr = 0x3B0,
> > > +        .machine_base_addr = 0x3B0,
> > > +        .size = 0xC,
> > > +        .rc = -1,
> > > +    },
> > > +    {
> > > +        .type = IORESOURCE_IO,
> > > +        .guest_base_addr = 0x3C0,
> > > +        .machine_base_addr = 0x3C0,
> > > +        .size = 0x20,
> > > +        .rc = -1,
> > > +    },
> > > +    {
> > > +        .type = IORESOURCE_MEM,
> > > +        .guest_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
> > > +        .machine_base_addr = 0xa0000 >> XC_PAGE_SHIFT,
> > > +        .size = 0x20,
> > > +        .rc = -1,
> > > +    },
> > > +};
> > > +
> > >  /*
> > >   * register VGA resources for the domain with assigned gfx
> > >   */
> > >  int xen_pt_register_vga_regions(XenHostPCIDevice *dev)  {
> > > -    int ret = 0;
> > > +    int i = 0;
> > >
> > >      if (!is_vga_passthrough(dev)) {
> > > -        return ret;
> > > +        return -1;
> > >      }
> > >
> > > -    ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3B0,
> > > -            0x3B0, 0xA, DPCI_ADD_MAPPING);
> > > -
> > > -    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);
> > > +    for(i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
> > > +        if (vga_args[i].type == IORESOURCE_IO) {
> > > +            vga_args[i].rc = xc_domain_ioport_mapping(xen_xc,
> > xen_domid,
> > > +                            vga_args[i].guest_base_addr,
> > > +                            vga_args[i].machine_base_addr,
> > > +                            vga_args[i].size,
> DPCI_ADD_MAPPING);
> > > +        } else {
> > > +            vga_args[i].rc = xc_domain_memory_mapping(xen_xc,
> > xen_domid,
> > > +                            vga_args[i].guest_base_addr,
> > > +                            vga_args[i].machine_base_addr,
> > > +                            vga_args[i].size,
> DPCI_ADD_MAPPING);
> > > +        }
> > >
> > > -    if (ret) {
> > > -        XEN_PT_ERR(NULL, "VGA region mapping failed\n");
> > > +        if (vga_args[i].rc) {
> > > +            XEN_PT_ERR(NULL, "VGA %s mapping failed! (rc: %i)\n",
> > > +                    vga_args[i].type == IORESOURCE_IO ? "ioport" :
> > "memory",
> > > +                    vga_args[i].rc);
> > > +        }
> > >      }
> > >
> > > -    return ret;
> > > +    return 0;
> > >  }
> > >
> > >  /*
> > > @@ -49,29 +88,33 @@ int
> xen_pt_register_vga_regions(XenHostPCIDevice
> > *dev)
> > >   */
> > >  int xen_pt_unregister_vga_regions(XenHostPCIDevice *dev)  {
> > > -    int ret = 0;
> > > +    int i = 0;
> > >
> > >      if (!is_vga_passthrough(dev)) {
> > > -        return ret;
> > > +        return -1;
> > >      }
> > >
> > > -    ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3B0,
> > > -            0x3B0, 0xC, DPCI_REMOVE_MAPPING);
> > > -
> > > -    ret |= xc_domain_ioport_mapping(xen_xc, xen_domid, 0x3C0,
> > > -            0x3C0, 0x20, DPCI_REMOVE_MAPPING);
> > > -
> > > -    ret |= xc_domain_memory_mapping(xen_xc, xen_domid,
> > > -            0xa0000 >> XC_PAGE_SHIFT,
> > > -            0xa0000 >> XC_PAGE_SHIFT,
> > > -            20,
> > > -            DPCI_REMOVE_MAPPING);
> > > +    for(i = 0 ; i < ARRAY_SIZE(vga_args); i++) {
> > > +        if (vga_args[i].type == IORESOURCE_IO) {
> > > +            vga_args[i].rc = xc_domain_ioport_mapping(xen_xc,
> > xen_domid,
> > > +                            vga_args[i].guest_base_addr,
> > > +                            vga_args[i].machine_base_addr,
> > > +                            vga_args[i].size,
> > DPCI_REMOVE_MAPPING);
> > > +        } else {
> > > +            vga_args[i].rc = xc_domain_memory_mapping(xen_xc,
> > xen_domid,
> > > +                            vga_args[i].guest_base_addr,
> > > +                            vga_args[i].machine_base_addr,
> > > +                            vga_args[i].size,
> > DPCI_REMOVE_MAPPING);
> > > +        }
> > >
> > > -    if (ret) {
> > > -        XEN_PT_ERR(NULL, "VGA region unmapping failed\n");
> > > +        if (vga_args[i].rc) {
> > > +            XEN_PT_ERR(NULL, "VGA %s unmapping failed! (rc: %i)\n",
> > > +                    vga_args[i].type == IORESOURCE_IO ? "ioport" :
> > "memory",
> > > +                    vga_args[i].rc);
> > > +        }
> > >      }
> > >
> > > -    return ret;
> > > +    return 0;
> >
> > I think you still need to return a non-zero value in case of failure.
> >
> 
> Okay I will do this like,
> 
> diff --git a/hw/xen/xen_pt_graphics.c b/hw/xen/xen_pt_graphics.c index
> 5603a8e..1342f4f 100644
> --- a/hw/xen/xen_pt_graphics.c
> +++ b/hw/xen/xen_pt_graphics.c
> @@ -77,6 +77,7 @@ int xen_pt_register_vga_regions(XenHostPCIDevice
> *dev)
>              XEN_PT_ERR(NULL, "VGA %s mapping failed! (rc: %i)\n",
>                      vga_args[i].type == IORESOURCE_IO ? "ioport" :
> "memory",
>                      vga_args[i].rc);
> +            return vga_args[i].rc;
>          }
>      }
> 
> @@ -111,6 +112,7 @@ int xen_pt_unregister_vga_regions(XenHostPCIDevice
> *dev)
>              XEN_PT_ERR(NULL, "VGA %s unmapping failed! (rc: %i)\n",
>                      vga_args[i].type == IORESOURCE_IO ? "ioport" :
> "memory",
>                      vga_args[i].rc);
> +            return vga_args[i].rc;
>          }
>      }
> 
> Thanks
> Tiejun

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel