[1/1] hw/pci-assign: split pci-assign.c

We will try to reuse assign_dev_load_option_rom in xen side, and
especially its a good beginning to unify pci assign codes both on
kvm and xen in the future.

Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
---
 hw/i386/kvm/pci-assign.c    | 136 +++++++++-----------------------------------
 include/hw/pci/pci-assign.h | 123 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 151 insertions(+), 108 deletions(-)
 create mode 100644 include/hw/pci/pci-assign.h

On Mon, Sep 01, 2014 at 10:07:19AM +0800, Tiejun Chen wrote:
> We will try to reuse assign_dev_load_option_rom in xen side, and
> especially its a good beginning to unify pci assign codes both on
> kvm and xen in the future.
> 
> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> ---
>  hw/i386/kvm/pci-assign.c    | 136 +++++++++-----------------------------------
>  include/hw/pci/pci-assign.h | 123 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 151 insertions(+), 108 deletions(-)
>  create mode 100644 include/hw/pci/pci-assign.h
> 
> diff --git a/hw/i386/kvm/pci-assign.c b/hw/i386/kvm/pci-assign.c
> index 17c7d6dc..4cf24ad 100644
> --- a/hw/i386/kvm/pci-assign.c
> +++ b/hw/i386/kvm/pci-assign.c
> @@ -37,16 +37,7 @@
>  #include "hw/pci/pci.h"
>  #include "hw/pci/msi.h"
>  #include "kvm_i386.h"
> -
> -#define MSIX_PAGE_SIZE 0x1000
> -
> -/* From linux/ioport.h */
> -#define IORESOURCE_IO       0x00000100  /* Resource type */
> -#define IORESOURCE_MEM      0x00000200
> -#define IORESOURCE_IRQ      0x00000400
> -#define IORESOURCE_DMA      0x00000800
> -#define IORESOURCE_PREFETCH 0x00002000  /* No side effects */
> -#define IORESOURCE_MEM_64   0x00100000
> +#include "hw/pci/pci-assign.h"
>  
>  //#define DEVICE_ASSIGNMENT_DEBUG
>  
> @@ -59,88 +50,6 @@
>  #define DEBUG(fmt, ...)
>  #endif
>  
> -typedef struct PCIRegion {
> -    int type;           /* Memory or port I/O */
> -    int valid;
> -    uint64_t base_addr;
> -    uint64_t size;    /* size of the region */
> -    int resource_fd;
> -} PCIRegion;
> -
> -typedef struct PCIDevRegions {
> -    uint8_t bus, dev, func; /* Bus inside domain, device and function */
> -    int irq;                /* IRQ number */
> -    uint16_t region_number; /* number of active regions */
> -
> -    /* Port I/O or MMIO Regions */
> -    PCIRegion regions[PCI_NUM_REGIONS - 1];
> -    int config_fd;
> -} PCIDevRegions;
> -
> -typedef struct AssignedDevRegion {
> -    MemoryRegion container;
> -    MemoryRegion real_iomem;
> -    union {
> -        uint8_t *r_virtbase; /* mmapped access address for memory regions */
> -        uint32_t r_baseport; /* the base guest port for I/O regions */
> -    } u;
> -    pcibus_t e_size;    /* emulated size of region in bytes */
> -    pcibus_t r_size;    /* real size of region in bytes */
> -    PCIRegion *region;
> -} AssignedDevRegion;
> -
> -#define ASSIGNED_DEVICE_PREFER_MSI_BIT  0
> -#define ASSIGNED_DEVICE_SHARE_INTX_BIT  1
> -
> -#define ASSIGNED_DEVICE_PREFER_MSI_MASK (1 << ASSIGNED_DEVICE_PREFER_MSI_BIT)
> -#define ASSIGNED_DEVICE_SHARE_INTX_MASK (1 << ASSIGNED_DEVICE_SHARE_INTX_BIT)
> -
> -typedef struct MSIXTableEntry {
> -    uint32_t addr_lo;
> -    uint32_t addr_hi;
> -    uint32_t data;
> -    uint32_t ctrl;
> -} MSIXTableEntry;
> -
> -typedef enum AssignedIRQType {
> -    ASSIGNED_IRQ_NONE = 0,
> -    ASSIGNED_IRQ_INTX_HOST_INTX,
> -    ASSIGNED_IRQ_INTX_HOST_MSI,
> -    ASSIGNED_IRQ_MSI,
> -    ASSIGNED_IRQ_MSIX
> -} AssignedIRQType;
> -
> -typedef struct AssignedDevice {
> -    PCIDevice dev;
> -    PCIHostDeviceAddress host;
> -    uint32_t dev_id;
> -    uint32_t features;
> -    int intpin;
> -    AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
> -    PCIDevRegions real_device;
> -    PCIINTxRoute intx_route;
> -    AssignedIRQType assigned_irq_type;
> -    struct {
> -#define ASSIGNED_DEVICE_CAP_MSI (1 << 0)
> -#define ASSIGNED_DEVICE_CAP_MSIX (1 << 1)
> -        uint32_t available;
> -#define ASSIGNED_DEVICE_MSI_ENABLED (1 << 0)
> -#define ASSIGNED_DEVICE_MSIX_ENABLED (1 << 1)
> -#define ASSIGNED_DEVICE_MSIX_MASKED (1 << 2)
> -        uint32_t state;
> -    } cap;
> -    uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
> -    uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
> -    int msi_virq_nr;
> -    int *msi_virq;
> -    MSIXTableEntry *msix_table;
> -    hwaddr msix_table_addr;
> -    uint16_t msix_max;
> -    MemoryRegion mmio;
> -    char *configfd_name;
> -    int32_t bootindex;
> -} AssignedDevice;
> -
>  static void assigned_dev_update_irq_routing(PCIDevice *dev);
>  
>  static void assigned_dev_load_option_rom(AssignedDevice *dev);
> @@ -1896,37 +1805,38 @@ type_init(assign_register_types)
>   * load the corresponding ROM data to RAM. If an error occurs while loading an
>   * option ROM, we just ignore that option ROM and continue with the next one.
>   */
> -static void assigned_dev_load_option_rom(AssignedDevice *dev)
> +int dev_load_option_rom(PCIDevice *dev, struct Object *owner, void *ptr,
> +                        unsigned int domain, unsigned int bus,
> +                        unsigned int slot, unsigned int function)
>  {
>      char name[32], rom_file[64];
>      FILE *fp;
>      uint8_t val;
>      struct stat st;
> -    void *ptr;
> +    int size = 0;
>  
>      /* If loading ROM from file, pci handles it */
> -    if (dev->dev.romfile || !dev->dev.rom_bar) {
> -        return;
> +    if (dev->romfile || !dev->rom_bar) {
> +        return -1;
>      }
>  
>      snprintf(rom_file, sizeof(rom_file),
>               "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
> -             dev->host.domain, dev->host.bus, dev->host.slot,
> -             dev->host.function);
> +             domain, bus, slot, function);
>  
>      if (stat(rom_file, &st)) {
> -        return;
> +        return -1;
>      }
>  
>      if (access(rom_file, F_OK)) {
>          error_report("pci-assign: Insufficient privileges for %s", rom_file);
> -        return;
> +        return -1;
>      }
>  
>      /* Write "1" to the ROM file to enable it */
>      fp = fopen(rom_file, "r+");
>      if (fp == NULL) {
> -        return;
> +        return -1;
>      }
>      val = 1;
>      if (fwrite(&val, 1, 1, fp) != 1) {
> @@ -1934,11 +1844,10 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev)
>      }
>      fseek(fp, 0, SEEK_SET);
>  
> -    snprintf(name, sizeof(name), "%s.rom",
> -            object_get_typename(OBJECT(dev)));
> -    memory_region_init_ram(&dev->dev.rom, OBJECT(dev), name, st.st_size);
> -    vmstate_register_ram(&dev->dev.rom, &dev->dev.qdev);
> -    ptr = memory_region_get_ram_ptr(&dev->dev.rom);
> +    snprintf(name, sizeof(name), "%s.rom", object_get_typename(owner));
> +    memory_region_init_ram(&dev->rom, owner, name, st.st_size);
> +    vmstate_register_ram(&dev->rom, &dev->qdev);
> +    ptr = memory_region_get_ram_ptr(&dev->rom);
>      memset(ptr, 0xff, st.st_size);
>  
>      if (!fread(ptr, 1, st.st_size, fp)) {
> @@ -1949,8 +1858,9 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev)
>          goto close_rom;
>      }
>  
> -    pci_register_bar(&dev->dev, PCI_ROM_SLOT, 0, &dev->dev.rom);
> -    dev->dev.has_rom = true;
> +    pci_register_bar(dev, PCI_ROM_SLOT, 0, &dev->rom);
> +    dev->has_rom = true;
> +    size = st.st_size;
>  close_rom:
>      /* Write "0" to disable ROM */
>      fseek(fp, 0, SEEK_SET);
> @@ -1959,4 +1869,14 @@ close_rom:
>          DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
>      }
>      fclose(fp);
> +
> +    return size;
> +}
> +
> +static void assigned_dev_load_option_rom(AssignedDevice *dev)
> +{
> +    void *ptr = NULL;
> +
> +    dev_load_option_rom(&dev->dev, OBJECT(dev), ptr, dev->host.domain,
> +                        dev->host.bus, dev->host.slot, dev->host.function);
>  }
> diff --git a/include/hw/pci/pci-assign.h b/include/hw/pci/pci-assign.h
> new file mode 100644
> index 0000000..2778e8b
> --- /dev/null
> +++ b/include/hw/pci/pci-assign.h
> @@ -0,0 +1,123 @@
> +/*
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + *
> + * Just split from hw/i386/kvm/pci-assign.c.
> + */
> +#ifndef PCI_ASSIGN_H
> +#define PCI_ASSIGN_H
> +
> +#include <stdio.h>
> +#include <unistd.h>
> +#include <sys/io.h>
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include "hw/hw.h"
> +#include "hw/i386/pc.h"
> +#include "qemu/error-report.h"
> +#include "ui/console.h"
> +#include "hw/loader.h"
> +#include "monitor/monitor.h"
> +#include "qemu/range.h"
> +#include "sysemu/sysemu.h"
> +#include "hw/pci/pci.h"
> +#include "hw/pci/msi.h"
> +#include "kvm_i386.h"

Why are you pulling all these headers here?
Please include the minimum required.

> +
> +#define MSIX_PAGE_SIZE 0x1000
> +
> +/* From linux/ioport.h */
> +#define IORESOURCE_IO       0x00000100  /* Resource type */
> +#define IORESOURCE_MEM      0x00000200
> +#define IORESOURCE_IRQ      0x00000400
> +#define IORESOURCE_DMA      0x00000800
> +#define IORESOURCE_PREFETCH 0x00002000  /* No side effects */
> +#define IORESOURCE_MEM_64   0x00100000
> +
> +typedef struct PCIRegion {
> +    int type;           /* Memory or port I/O */
> +    int valid;
> +    uint64_t base_addr;
> +    uint64_t size;    /* size of the region */
> +    int resource_fd;
> +} PCIRegion;
> +
> +typedef struct PCIDevRegions {
> +    uint8_t bus, dev, func; /* Bus inside domain, device and function */
> +    int irq;                /* IRQ number */
> +    uint16_t region_number; /* number of active regions */
> +
> +    /* Port I/O or MMIO Regions */
> +    PCIRegion regions[PCI_NUM_REGIONS - 1];
> +    int config_fd;
> +} PCIDevRegions;
> +
> +typedef struct AssignedDevRegion {
> +    MemoryRegion container;
> +    MemoryRegion real_iomem;
> +    union {
> +        uint8_t *r_virtbase; /* mmapped access address for memory regions */
> +        uint32_t r_baseport; /* the base guest port for I/O regions */
> +    } u;
> +    pcibus_t e_size;    /* emulated size of region in bytes */
> +    pcibus_t r_size;    /* real size of region in bytes */
> +    PCIRegion *region;
> +} AssignedDevRegion;
> +
> +#define ASSIGNED_DEVICE_PREFER_MSI_BIT  0
> +#define ASSIGNED_DEVICE_SHARE_INTX_BIT  1
> +
> +#define ASSIGNED_DEVICE_PREFER_MSI_MASK (1 << ASSIGNED_DEVICE_PREFER_MSI_BIT)
> +#define ASSIGNED_DEVICE_SHARE_INTX_MASK (1 << ASSIGNED_DEVICE_SHARE_INTX_BIT)
> +
> +typedef struct MSIXTableEntry {
> +    uint32_t addr_lo;
> +    uint32_t addr_hi;
> +    uint32_t data;
> +    uint32_t ctrl;
> +} MSIXTableEntry;
> +
> +typedef enum AssignedIRQType {
> +    ASSIGNED_IRQ_NONE = 0,
> +    ASSIGNED_IRQ_INTX_HOST_INTX,
> +    ASSIGNED_IRQ_INTX_HOST_MSI,
> +    ASSIGNED_IRQ_MSI,
> +    ASSIGNED_IRQ_MSIX
> +} AssignedIRQType;
> +
> +typedef struct AssignedDevice {
> +    PCIDevice dev;
> +    PCIHostDeviceAddress host;
> +    uint32_t dev_id;
> +    uint32_t features;
> +    int intpin;
> +    AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
> +    PCIDevRegions real_device;
> +    PCIINTxRoute intx_route;
> +    AssignedIRQType assigned_irq_type;
> +    struct {
> +#define ASSIGNED_DEVICE_CAP_MSI (1 << 0)
> +#define ASSIGNED_DEVICE_CAP_MSIX (1 << 1)
> +        uint32_t available;
> +#define ASSIGNED_DEVICE_MSI_ENABLED (1 << 0)
> +#define ASSIGNED_DEVICE_MSIX_ENABLED (1 << 1)
> +#define ASSIGNED_DEVICE_MSIX_MASKED (1 << 2)
> +        uint32_t state;
> +    } cap;
> +    uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
> +    uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
> +    int msi_virq_nr;
> +    int *msi_virq;
> +    MSIXTableEntry *msix_table;
> +    hwaddr msix_table_addr;
> +    uint16_t msix_max;
> +    MemoryRegion mmio;
> +    char *configfd_name;
> +    int32_t bootindex;
> +} AssignedDevice;
> +

Why are you moving the above here?


> +int dev_load_option_rom(PCIDevice *dev, struct Object *owner, void *ptr,
> +                        unsigned int domain, unsigned int bus,
> +                        unsigned int slot, unsigned int function);

Please use a header-specific prefix to avoid global namespace pollution.
pci_assign_dev_load_option_rom?

> +#endif /* PCI_ASSIGN_H */
> -- 
> 1.9.1

On 2014/9/1 16:27, Michael S. Tsirkin wrote:
> On Mon, Sep 01, 2014 at 10:07:19AM +0800, Tiejun Chen wrote:
>> We will try to reuse assign_dev_load_option_rom in xen side, and
>> especially its a good beginning to unify pci assign codes both on
>> kvm and xen in the future.
>>
>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>> ---

[snip]

>> + */
>> +#ifndef PCI_ASSIGN_H
>> +#define PCI_ASSIGN_H
>> +
>> +#include <stdio.h>
>> +#include <unistd.h>
>> +#include <sys/io.h>
>> +#include <sys/mman.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include "hw/hw.h"
>> +#include "hw/i386/pc.h"
>> +#include "qemu/error-report.h"
>> +#include "ui/console.h"
>> +#include "hw/loader.h"
>> +#include "monitor/monitor.h"
>> +#include "qemu/range.h"
>> +#include "sysemu/sysemu.h"
>> +#include "hw/pci/pci.h"
>> +#include "hw/pci/msi.h"
>> +#include "kvm_i386.h"
>
> Why are you pulling all these headers here?
> Please include the minimum required.

So just leave #include "hw/pci/pci.h".

>
>> +
>> +#define MSIX_PAGE_SIZE 0x1000
>> +
>> +/* From linux/ioport.h */
>> +#define IORESOURCE_IO       0x00000100  /* Resource type */
>> +#define IORESOURCE_MEM      0x00000200

[snip]

>> +    uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
>> +    uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
>> +    int msi_virq_nr;
>> +    int *msi_virq;
>> +    MSIXTableEntry *msix_table;
>> +    hwaddr msix_table_addr;
>> +    uint16_t msix_max;
>> +    MemoryRegion mmio;
>> +    char *configfd_name;
>> +    int32_t bootindex;
>> +} AssignedDevice;
>> +
>
> Why are you moving the above here?

As I said in the patch head description, I think this is a good 
beginning to unify pci-assign in both KVM and XEN. So I tried to move 
these common stuffs. Although we mightn't use them directly in the 
future, but I guess we still need to move them into this head file.

If you think we should do this on-demand exactly, I can move back them 
to pci-assign.c.

>
>
>> +int dev_load_option_rom(PCIDevice *dev, struct Object *owner, void *ptr,
>> +                        unsigned int domain, unsigned int bus,
>> +                        unsigned int slot, unsigned int function);
>
> Please use a header-specific prefix to avoid global namespace pollution.
> pci_assign_dev_load_option_rom?

Looks good so I will follow-up yours.

Thanks
Tiejun

>
>> +#endif /* PCI_ASSIGN_H */
>> --
>> 1.9.1
>
>

On Mon, Sep 01, 2014 at 05:26:24PM +0800, Chen, Tiejun wrote:
> On 2014/9/1 16:27, Michael S. Tsirkin wrote:
> >On Mon, Sep 01, 2014 at 10:07:19AM +0800, Tiejun Chen wrote:
> >>We will try to reuse assign_dev_load_option_rom in xen side, and
> >>especially its a good beginning to unify pci assign codes both on
> >>kvm and xen in the future.
> >>
> >>Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
> >>---
> 
> [snip]
> 
> >>+ */
> >>+#ifndef PCI_ASSIGN_H
> >>+#define PCI_ASSIGN_H
> >>+
> >>+#include <stdio.h>
> >>+#include <unistd.h>
> >>+#include <sys/io.h>
> >>+#include <sys/mman.h>
> >>+#include <sys/types.h>
> >>+#include <sys/stat.h>
> >>+#include "hw/hw.h"
> >>+#include "hw/i386/pc.h"
> >>+#include "qemu/error-report.h"
> >>+#include "ui/console.h"
> >>+#include "hw/loader.h"
> >>+#include "monitor/monitor.h"
> >>+#include "qemu/range.h"
> >>+#include "sysemu/sysemu.h"
> >>+#include "hw/pci/pci.h"
> >>+#include "hw/pci/msi.h"
> >>+#include "kvm_i386.h"
> >
> >Why are you pulling all these headers here?
> >Please include the minimum required.
> 
> So just leave #include "hw/pci/pci.h".
> 
> >
> >>+
> >>+#define MSIX_PAGE_SIZE 0x1000
> >>+
> >>+/* From linux/ioport.h */
> >>+#define IORESOURCE_IO       0x00000100  /* Resource type */
> >>+#define IORESOURCE_MEM      0x00000200
> 
> [snip]
> 
> >>+    uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
> >>+    uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
> >>+    int msi_virq_nr;
> >>+    int *msi_virq;
> >>+    MSIXTableEntry *msix_table;
> >>+    hwaddr msix_table_addr;
> >>+    uint16_t msix_max;
> >>+    MemoryRegion mmio;
> >>+    char *configfd_name;
> >>+    int32_t bootindex;
> >>+} AssignedDevice;
> >>+
> >
> >Why are you moving the above here?
> 
> As I said in the patch head description, I think this is a good beginning to
> unify pci-assign in both KVM and XEN. So I tried to move these common
> stuffs. Although we mightn't use them directly in the future, but I guess we
> still need to move them into this head file.
> 
> If you think we should do this on-demand exactly, I can move back them to
> pci-assign.c.

Yes, I think this is better on demand.

> >
> >
> >>+int dev_load_option_rom(PCIDevice *dev, struct Object *owner, void *ptr,
> >>+                        unsigned int domain, unsigned int bus,
> >>+                        unsigned int slot, unsigned int function);
> >
> >Please use a header-specific prefix to avoid global namespace pollution.
> >pci_assign_dev_load_option_rom?
> 
> Looks good so I will follow-up yours.
> 
> Thanks
> Tiejun
> 
> >
> >>+#endif /* PCI_ASSIGN_H */
> >>--
> >>1.9.1
> >
> >

On 2014/9/1 17:53, Michael S. Tsirkin wrote:
> On Mon, Sep 01, 2014 at 05:26:24PM +0800, Chen, Tiejun wrote:
>> On 2014/9/1 16:27, Michael S. Tsirkin wrote:
>>> On Mon, Sep 01, 2014 at 10:07:19AM +0800, Tiejun Chen wrote:
>>>> We will try to reuse assign_dev_load_option_rom in xen side, and
>>>> especially its a good beginning to unify pci assign codes both on
>>>> kvm and xen in the future.
>>>>
>>>> Signed-off-by: Tiejun Chen <tiejun.chen@intel.com>
>>>> ---
>>
>> [snip]
>>
>>>> + */
>>>> +#ifndef PCI_ASSIGN_H
>>>> +#define PCI_ASSIGN_H
>>>> +
>>>> +#include <stdio.h>
>>>> +#include <unistd.h>
>>>> +#include <sys/io.h>
>>>> +#include <sys/mman.h>
>>>> +#include <sys/types.h>
>>>> +#include <sys/stat.h>
>>>> +#include "hw/hw.h"
>>>> +#include "hw/i386/pc.h"
>>>> +#include "qemu/error-report.h"
>>>> +#include "ui/console.h"
>>>> +#include "hw/loader.h"
>>>> +#include "monitor/monitor.h"
>>>> +#include "qemu/range.h"
>>>> +#include "sysemu/sysemu.h"
>>>> +#include "hw/pci/pci.h"
>>>> +#include "hw/pci/msi.h"
>>>> +#include "kvm_i386.h"
>>>
>>> Why are you pulling all these headers here?
>>> Please include the minimum required.
>>
>> So just leave #include "hw/pci/pci.h".
>>
>>>
>>>> +
>>>> +#define MSIX_PAGE_SIZE 0x1000
>>>> +
>>>> +/* From linux/ioport.h */
>>>> +#define IORESOURCE_IO       0x00000100  /* Resource type */
>>>> +#define IORESOURCE_MEM      0x00000200
>>
>> [snip]
>>
>>>> +    uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
>>>> +    uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
>>>> +    int msi_virq_nr;
>>>> +    int *msi_virq;
>>>> +    MSIXTableEntry *msix_table;
>>>> +    hwaddr msix_table_addr;
>>>> +    uint16_t msix_max;
>>>> +    MemoryRegion mmio;
>>>> +    char *configfd_name;
>>>> +    int32_t bootindex;
>>>> +} AssignedDevice;
>>>> +
>>>
>>> Why are you moving the above here?
>>
>> As I said in the patch head description, I think this is a good beginning to
>> unify pci-assign in both KVM and XEN. So I tried to move these common
>> stuffs. Although we mightn't use them directly in the future, but I guess we
>> still need to move them into this head file.
>>
>> If you think we should do this on-demand exactly, I can move back them to
>> pci-assign.c.
>
> Yes, I think this is better on demand.

Okay, I'll restore this in next revision.

Thanks
Tiejun

>
>>>
>>>
>>>> +int dev_load_option_rom(PCIDevice *dev, struct Object *owner, void *ptr,
>>>> +                        unsigned int domain, unsigned int bus,
>>>> +                        unsigned int slot, unsigned int function);
>>>
>>> Please use a header-specific prefix to avoid global namespace pollution.
>>> pci_assign_dev_load_option_rom?
>>
>> Looks good so I will follow-up yours.
>>
>> Thanks
>> Tiejun
>>
>>>
>>>> +#endif /* PCI_ASSIGN_H */
>>>> --
>>>> 1.9.1
>>>
>>>
>