Re: [Xen-devel] [PATCH] qemu: Don't access /proc/bus/pci unless graphics pass-thru is enabled

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

Sorry, didn't get the CC right...
 -George

On Fri, Feb 10, 2012 at 3:56 PM, George Dunlap
<george.dunlap@xxxxxxxxxxxxx> wrote:
> # HG changeset patch
> # User George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> # Date 1328888874 0
> # Node ID e4a4e4f4156dcdffb8e81fb28bb16b6a9d611af7
> # Parent  6efeff914609a3870e2d07a8d73a26c4615ac60b
> qemu: Don't access /proc/bus/pci unless graphics pass-thru is enabled
>
> A recent changeset introduced a bug whereby an initialization function
> that reads /proc/bus/pci is called from graphics set-up functions even
> if pass-through graphics are not enabled.  If qemu is run without
> permission to this file, this causes qemu to fail during
> initialization.
>
> This patch re-works the functions so that the initialization happens
> only if we actually need to do the pci host read or write.  It also
> makes failures call abort().
>
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>
> diff -r 6efeff914609 -r e4a4e4f4156d hw/pass-through.h
> --- a/hw/pass-through.h Fri Feb 10 11:02:25 2012 +0000
> +++ b/hw/pass-through.h Fri Feb 10 15:47:54 2012 +0000
> @@ -29,6 +29,8 @@
>  /* Log acesss */
>  #define PT_LOGGING_ENABLED
>
> +/* Print errors even if logging is disabled */
> +#define PT_ERR(_f, _a...)   fprintf(logfile, "%s: " _f, __func__, ##_a)
>  #ifdef PT_LOGGING_ENABLED
>  #define PT_LOG(_f, _a...)   fprintf(logfile, "%s: " _f, __func__, ##_a)
>  #define PT_LOG_DEV(_dev, _f, _a...)   fprintf(logfile, "%s: [%02x:%02x:%01x] 
> " _f, __func__,    \
> diff -r 6efeff914609 -r e4a4e4f4156d hw/pt-graphics.c
> --- a/hw/pt-graphics.c  Fri Feb 10 11:02:25 2012 +0000
> +++ b/hw/pt-graphics.c  Fri Feb 10 15:47:54 2012 +0000
> @@ -23,11 +23,17 @@ void intel_pch_init(PCIBus *bus)
>  {
>     uint16_t vid, did;
>     uint8_t  rid;
> -    struct pci_dev *pci_dev_1f = pt_pci_get_dev(0, 0x1f, 0);
> +    struct pci_dev *pci_dev_1f;
>
> -    if ( !gfx_passthru || !pci_dev_1f )
> +    if ( !gfx_passthru )
>         return;
>
> +    if ( !(pci_dev_1f=pt_pci_get_dev(0, 0x1f, 0)) )
> +    {
> +        PT_ERR("Error: Can't get pci_dev_host_bridge\n");
> +        abort();
> +    }
> +
>     vid = pt_pci_host_read(pci_dev_1f, PCI_VENDOR_ID, 2);
>     did = pt_pci_host_read(pci_dev_1f, PCI_DEVICE_ID, 2);
>     rid = pt_pci_host_read(pci_dev_1f, PCI_REVISION, 1);
> @@ -39,36 +45,45 @@ void intel_pch_init(PCIBus *bus)
>
>  void igd_pci_write(PCIDevice *pci_dev, uint32_t config_addr, uint32_t val, 
> int len)
>  {
> -    struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0);
> +    struct pci_dev *pci_dev_host_bridge;
>     assert(pci_dev->devfn == 0x00);
> -    if ( !igd_passthru ) {
> -        pci_default_write_config(pci_dev, config_addr, val, len);
> -        return;
> -    }
> +    if ( !igd_passthru )
> +        goto write_default;
>
>     switch (config_addr)
>     {
>         case 0x58:        // PAVPC Offset
> -            pt_pci_host_write(pci_dev_host_bridge, config_addr, val, len);
> -#ifdef PT_DEBUG_PCI_CONFIG_ACCESS
> -            PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n",
> -                    config_addr, len, val);
> -#endif
>             break;
>         default:
> -            pci_default_write_config(pci_dev, config_addr, val, len);
> +            goto write_default;
>     }
> +
> +    /* Host write */
> +    if ( !(pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0)) )
> +    {
> +        PT_ERR("Error: Can't get pci_dev_host_bridge\n");
> +        abort();
> +    }
> +
> +    pt_pci_host_write(pci_dev_host_bridge, config_addr, val, len);
> +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS
> +    PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n",
> +               config_addr, len, val);
> +#endif
> +    return;
> +
> +write_default:
> +    pci_default_write_config(pci_dev, config_addr, val, len);
>  }
>
>  uint32_t igd_pci_read(PCIDevice *pci_dev, uint32_t config_addr, int len)
>  {
> -    struct pci_dev *pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0);
> +    struct pci_dev *pci_dev_host_bridge;
>     uint32_t val;
>
>     assert(pci_dev->devfn == 0x00);
> -    if ( !igd_passthru ) {
> -        return pci_default_read_config(pci_dev, config_addr, len);
> -    }
> +    if ( !igd_passthru )
> +        goto read_default;
>
>     switch (config_addr)
>     {
> @@ -81,16 +96,28 @@ uint32_t igd_pci_read(PCIDevice *pci_dev
>         case 0x58:        /* SNB: PAVPC Offset */
>         case 0xa4:        /* SNB: graphics base of stolen memory */
>         case 0xa8:        /* SNB: base of GTT stolen memory */
> -            val = pt_pci_host_read(pci_dev_host_bridge, config_addr, len);
> -#ifdef PT_DEBUG_PCI_CONFIG_ACCESS
> -            PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n",
> -                    config_addr, len, val);
> -#endif
>             break;
>         default:
> -            val = pci_default_read_config(pci_dev, config_addr, len);
> +            goto read_default;
>     }
> +
> +    /* Host read */
> +    if ( !(pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0)) )
> +    {
> +        PT_ERR("Error: Can't get pci_dev_host_bridge\n");
> +        abort();
> +    }
> +
> +    val = pt_pci_host_read(pci_dev_host_bridge, config_addr, len);
> +#ifdef PT_DEBUG_PCI_CONFIG_ACCESS
> +    PT_LOG_DEV(pci_dev, "addr=%x len=%x val=%x\n",
> +               config_addr, len, val);
> +#endif
>     return val;
> +
> +read_default:
> +
> +   return pci_default_read_config(pci_dev, config_addr, len);
>  }
>
>  /*
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel