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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Fri, 10 Feb 2012, George Dunlap wrote:
> 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 changes the initialization functions to only happen if graphics
> pass-through are enabled.
> 
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

could you please send patches inline?


> # HG changeset patch
> # 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 changes the initialization functions to only happen if graphics
> pass-through are enabled.
> 
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
> 
> diff -r 6efeff914609 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 11:04:01 2012 +0000
> @@ -23,9 +23,9 @@ 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 || !(pci_dev_1f=pt_pci_get_dev(0, 0x1f, 0)) )
>          return;

I would rather have it as a seprate test after if ( !gfx_passthru ),
same for the others below


>      vid = pt_pci_host_read(pci_dev_1f, PCI_VENDOR_ID, 2);
> @@ -39,9 +39,9 @@ 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 ) {
> +    if ( !igd_passthru || !(pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0))) {
>          pci_default_write_config(pci_dev, config_addr, val, len);
>          return;
>      }

Why are you adding this test (!(pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0)) ?

If you are worried that pci_dev_host_bridge could be NULL, shouldn't you
also remove the assert?


> @@ -62,11 +62,11 @@ void igd_pci_write(PCIDevice *pci_dev, u
>  
>  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 ) {
> +    if ( !igd_passthru || !(pci_dev_host_bridge = pt_pci_get_dev(0, 0, 0)) ) 
> {
>          return pci_default_read_config(pci_dev, config_addr, len);
>      }

same here

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