diff mbox

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

Message ID 53FE8961.3050608@intel.com
State New
Headers show

Commit Message

Tiejun Chen Aug. 28, 2014, 1:44 a.m. UTC
On 2014/8/27 21:03, Michael S. Tsirkin wrote:
> On Wed, Aug 27, 2014 at 05:13:07PM +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    | 170 +-----------------------------------
>>   include/hw/pci/pci_assign.h | 204 ++++++++++++++++++++++++++++++++++++++++++++
>
> Why are you making so much code inline?
> Please move it to an out of line file.

So just leave these structs and macros definition here,

+                        unsigned int domain, unsigned int bus,
+                        unsigned int slot, unsigned int function);
+#endif /* PCI_ASSIGN_H */

> Also it should be pci-assign not pci_assign.

Okay, I can rename this as pci-assign.h.

But,

$ ls include/hw/pci/
msi.h   pci_bus.h   pcie.h       pcie_port.h  pci.h       pci_ids.h   shpc.h
msix.h  pci_bridge.h  pcie_aer.h  pcie_host.h  pcie_regs.h  pci_host.h 
pci_regs.h  slotid_cap.h

And you also can see some files are named with xxx_xxx.c/.h in many places.

So what is the rule to name a file in qemu?

Thanks
Tiejun

>
>
>>   2 files changed, 207 insertions(+), 167 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..a4fe2d6 100644
>> --- a/hw/i386/kvm/pci-assign.c
>> +++ b/hw/i386/kvm/pci-assign.c
>> @@ -37,109 +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
>> -
>> -//#define DEVICE_ASSIGNMENT_DEBUG
>> -
>> -#ifdef DEVICE_ASSIGNMENT_DEBUG
>> -#define DEBUG(fmt, ...)                                       \
>> -    do {                                                      \
>> -        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
>> -    } while (0)
>> -#else
>> -#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;
>> +#include "hw/pci/pci_assign.h"
>>
>>   static void assigned_dev_update_irq_routing(PCIDevice *dev);
>>
>> @@ -1891,72 +1789,10 @@ static void assign_register_types(void)
>>
>>   type_init(assign_register_types)
>>
>> -/*
>> - * Scan the assigned devices for the devices that have an option ROM, and then
>> - * 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)
>>   {
>> -    char name[32], rom_file[64];
>> -    FILE *fp;
>> -    uint8_t val;
>> -    struct stat st;
>>       void *ptr;
>>
>> -    /* If loading ROM from file, pci handles it */
>> -    if (dev->dev.romfile || !dev->dev.rom_bar) {
>> -        return;
>> -    }
>> -
>> -    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);
>> -
>> -    if (stat(rom_file, &st)) {
>> -        return;
>> -    }
>> -
>> -    if (access(rom_file, F_OK)) {
>> -        error_report("pci-assign: Insufficient privileges for %s", rom_file);
>> -        return;
>> -    }
>> -
>> -    /* Write "1" to the ROM file to enable it */
>> -    fp = fopen(rom_file, "r+");
>> -    if (fp == NULL) {
>> -        return;
>> -    }
>> -    val = 1;
>> -    if (fwrite(&val, 1, 1, fp) != 1) {
>> -        goto close_rom;
>> -    }
>> -    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);
>> -    memset(ptr, 0xff, st.st_size);
>> -
>> -    if (!fread(ptr, 1, st.st_size, fp)) {
>> -        error_report("pci-assign: Cannot read from host %s", rom_file);
>> -        error_printf("Device option ROM contents are probably invalid "
>> -                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
>> -                     "or load from file with romfile=\n");
>> -        goto close_rom;
>> -    }
>> -
>> -    pci_register_bar(&dev->dev, PCI_ROM_SLOT, 0, &dev->dev.rom);
>> -    dev->dev.has_rom = true;
>> -close_rom:
>> -    /* Write "0" to disable ROM */
>> -    fseek(fp, 0, SEEK_SET);
>> -    val = 0;
>> -    if (!fwrite(&val, 1, 1, fp)) {
>> -        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
>> -    }
>> -    fclose(fp);
>> +    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..b146fcd
>> --- /dev/null
>> +++ b/include/hw/pci/pci_assign.h
>> @@ -0,0 +1,204 @@
>> +/*
>> + * 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"
>> +
>> +#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
>> +
>> +//#define DEVICE_ASSIGNMENT_DEBUG
>> +
>> +#ifdef DEVICE_ASSIGNMENT_DEBUG
>> +#define DEBUG(fmt, ...)                                       \
>> +    do {                                                      \
>> +        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
>> +    } while (0)
>> +#else
>> +#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;
>> +
>> +/*
>> + * Scan the assigned devices for the devices that have an option ROM, and then
>> + * 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 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;
>> +    int size = 0;
>> +
>> +    /* If loading ROM from file, pci handles it */
>> +    if (dev->romfile || !dev->rom_bar) {
>> +        return -1;
>> +    }
>> +
>> +    snprintf(rom_file, sizeof(rom_file),
>> +             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
>> +             domain, bus, slot, function);
>> +
>> +    if (stat(rom_file, &st)) {
>> +        return -1;
>> +    }
>> +
>> +    if (access(rom_file, F_OK)) {
>> +        error_report("pci-assign: Insufficient privileges for %s", rom_file);
>> +        return -1;
>> +    }
>> +
>> +    /* Write "1" to the ROM file to enable it */
>> +    fp = fopen(rom_file, "r+");
>> +    if (fp == NULL) {
>> +        return -1;
>> +    }
>> +    val = 1;
>> +    if (fwrite(&val, 1, 1, fp) != 1) {
>> +        goto close_rom;
>> +    }
>> +    fseek(fp, 0, SEEK_SET);
>> +
>> +    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)) {
>> +        error_report("pci-assign: Cannot read from host %s", rom_file);
>> +        error_printf("Device option ROM contents are probably invalid "
>> +                     "(check dmesg).\nSkip option ROM probe with rombar=0, "
>> +                     "or load from file with romfile=\n");
>> +        goto close_rom;
>> +    }
>> +
>> +    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);
>> +    val = 0;
>> +    if (!fwrite(&val, 1, 1, fp)) {
>> +        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
>> +    }
>> +    fclose(fp);
>> +
>> +    return size;
>> +}
>> +#endif /* PCI_ASSIGN_H */
>> --
>> 1.9.1
>

Comments

Tiejun Chen Aug. 29, 2014, 1:23 a.m. UTC | #1
On 2014/8/28 9:44, Chen, Tiejun wrote:
> On 2014/8/27 21:03, Michael S. Tsirkin wrote:
>> On Wed, Aug 27, 2014 at 05:13:07PM +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    | 170 +-----------------------------------
>>>   include/hw/pci/pci_assign.h | 204
>>> ++++++++++++++++++++++++++++++++++++++++++++
>>
>> Why are you making so much code inline?
>> Please move it to an out of line file.
>
> So just leave these structs and macros definition here,
>

Michel,

Is the following change inline fine to you?

Your comments are appreciated in advance.

Thanks
Tiejun

> 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"
> +
> +#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;
> +
> +int dev_load_option_rom(PCIDevice *dev, struct Object *owner, void *ptr,
> +                        unsigned int domain, unsigned int bus,
> +                        unsigned int slot, unsigned int function);
> +#endif /* PCI_ASSIGN_H */
>
>> Also it should be pci-assign not pci_assign.
>
> Okay, I can rename this as pci-assign.h.
>
> But,
>
> $ ls include/hw/pci/
> msi.h   pci_bus.h   pcie.h       pcie_port.h  pci.h       pci_ids.h
> shpc.h
> msix.h  pci_bridge.h  pcie_aer.h  pcie_host.h  pcie_regs.h  pci_host.h
> pci_regs.h  slotid_cap.h
>
> And you also can see some files are named with xxx_xxx.c/.h in many places.
>
> So what is the rule to name a file in qemu?
>
> Thanks
> Tiejun
>
>>
>>
>>>   2 files changed, 207 insertions(+), 167 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..a4fe2d6 100644
>>> --- a/hw/i386/kvm/pci-assign.c
>>> +++ b/hw/i386/kvm/pci-assign.c
>>> @@ -37,109 +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
>>> -
>>> -//#define DEVICE_ASSIGNMENT_DEBUG
>>> -
>>> -#ifdef DEVICE_ASSIGNMENT_DEBUG
>>> -#define DEBUG(fmt, ...)                                       \
>>> -    do {                                                      \
>>> -        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
>>> -    } while (0)
>>> -#else
>>> -#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;
>>> +#include "hw/pci/pci_assign.h"
>>>
>>>   static void assigned_dev_update_irq_routing(PCIDevice *dev);
>>>
>>> @@ -1891,72 +1789,10 @@ static void assign_register_types(void)
>>>
>>>   type_init(assign_register_types)
>>>
>>> -/*
>>> - * Scan the assigned devices for the devices that have an option
>>> ROM, and then
>>> - * 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)
>>>   {
>>> -    char name[32], rom_file[64];
>>> -    FILE *fp;
>>> -    uint8_t val;
>>> -    struct stat st;
>>>       void *ptr;
>>>
>>> -    /* If loading ROM from file, pci handles it */
>>> -    if (dev->dev.romfile || !dev->dev.rom_bar) {
>>> -        return;
>>> -    }
>>> -
>>> -    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);
>>> -
>>> -    if (stat(rom_file, &st)) {
>>> -        return;
>>> -    }
>>> -
>>> -    if (access(rom_file, F_OK)) {
>>> -        error_report("pci-assign: Insufficient privileges for %s",
>>> rom_file);
>>> -        return;
>>> -    }
>>> -
>>> -    /* Write "1" to the ROM file to enable it */
>>> -    fp = fopen(rom_file, "r+");
>>> -    if (fp == NULL) {
>>> -        return;
>>> -    }
>>> -    val = 1;
>>> -    if (fwrite(&val, 1, 1, fp) != 1) {
>>> -        goto close_rom;
>>> -    }
>>> -    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);
>>> -    memset(ptr, 0xff, st.st_size);
>>> -
>>> -    if (!fread(ptr, 1, st.st_size, fp)) {
>>> -        error_report("pci-assign: Cannot read from host %s", rom_file);
>>> -        error_printf("Device option ROM contents are probably invalid "
>>> -                     "(check dmesg).\nSkip option ROM probe with
>>> rombar=0, "
>>> -                     "or load from file with romfile=\n");
>>> -        goto close_rom;
>>> -    }
>>> -
>>> -    pci_register_bar(&dev->dev, PCI_ROM_SLOT, 0, &dev->dev.rom);
>>> -    dev->dev.has_rom = true;
>>> -close_rom:
>>> -    /* Write "0" to disable ROM */
>>> -    fseek(fp, 0, SEEK_SET);
>>> -    val = 0;
>>> -    if (!fwrite(&val, 1, 1, fp)) {
>>> -        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
>>> -    }
>>> -    fclose(fp);
>>> +    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..b146fcd
>>> --- /dev/null
>>> +++ b/include/hw/pci/pci_assign.h
>>> @@ -0,0 +1,204 @@
>>> +/*
>>> + * 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"
>>> +
>>> +#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
>>> +
>>> +//#define DEVICE_ASSIGNMENT_DEBUG
>>> +
>>> +#ifdef DEVICE_ASSIGNMENT_DEBUG
>>> +#define DEBUG(fmt, ...)                                       \
>>> +    do {                                                      \
>>> +        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
>>> +    } while (0)
>>> +#else
>>> +#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;
>>> +
>>> +/*
>>> + * Scan the assigned devices for the devices that have an option
>>> ROM, and then
>>> + * 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 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;
>>> +    int size = 0;
>>> +
>>> +    /* If loading ROM from file, pci handles it */
>>> +    if (dev->romfile || !dev->rom_bar) {
>>> +        return -1;
>>> +    }
>>> +
>>> +    snprintf(rom_file, sizeof(rom_file),
>>> +             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
>>> +             domain, bus, slot, function);
>>> +
>>> +    if (stat(rom_file, &st)) {
>>> +        return -1;
>>> +    }
>>> +
>>> +    if (access(rom_file, F_OK)) {
>>> +        error_report("pci-assign: Insufficient privileges for %s",
>>> rom_file);
>>> +        return -1;
>>> +    }
>>> +
>>> +    /* Write "1" to the ROM file to enable it */
>>> +    fp = fopen(rom_file, "r+");
>>> +    if (fp == NULL) {
>>> +        return -1;
>>> +    }
>>> +    val = 1;
>>> +    if (fwrite(&val, 1, 1, fp) != 1) {
>>> +        goto close_rom;
>>> +    }
>>> +    fseek(fp, 0, SEEK_SET);
>>> +
>>> +    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)) {
>>> +        error_report("pci-assign: Cannot read from host %s", rom_file);
>>> +        error_printf("Device option ROM contents are probably invalid "
>>> +                     "(check dmesg).\nSkip option ROM probe with
>>> rombar=0, "
>>> +                     "or load from file with romfile=\n");
>>> +        goto close_rom;
>>> +    }
>>> +
>>> +    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);
>>> +    val = 0;
>>> +    if (!fwrite(&val, 1, 1, fp)) {
>>> +        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
>>> +    }
>>> +    fclose(fp);
>>> +
>>> +    return size;
>>> +}
>>> +#endif /* PCI_ASSIGN_H */
>>> --
>>> 1.9.1
>>
>
>
>
Michael S. Tsirkin Aug. 31, 2014, 8:48 a.m. UTC | #2
On Fri, Aug 29, 2014 at 09:23:48AM +0800, Chen, Tiejun wrote:
> On 2014/8/28 9:44, Chen, Tiejun wrote:
> >On 2014/8/27 21:03, Michael S. Tsirkin wrote:
> >>On Wed, Aug 27, 2014 at 05:13:07PM +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    | 170 +-----------------------------------
> >>>  include/hw/pci/pci_assign.h | 204
> >>>++++++++++++++++++++++++++++++++++++++++++++
> >>
> >>Why are you making so much code inline?
> >>Please move it to an out of line file.
> >
> >So just leave these structs and macros definition here,
> >
> 
> Michel,
> 
> Is the following change inline fine to you?
> 
> Your comments are appreciated in advance.
> 
> Thanks
> Tiejun

Sorry I don't really understand. Please send a patch and
I can review.

> >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"
> >+
> >+#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;
> >+
> >+int dev_load_option_rom(PCIDevice *dev, struct Object *owner, void *ptr,
> >+                        unsigned int domain, unsigned int bus,
> >+                        unsigned int slot, unsigned int function);
> >+#endif /* PCI_ASSIGN_H */
> >
> >>Also it should be pci-assign not pci_assign.
> >
> >Okay, I can rename this as pci-assign.h.
> >
> >But,
> >
> >$ ls include/hw/pci/
> >msi.h   pci_bus.h   pcie.h       pcie_port.h  pci.h       pci_ids.h
> >shpc.h
> >msix.h  pci_bridge.h  pcie_aer.h  pcie_host.h  pcie_regs.h  pci_host.h
> >pci_regs.h  slotid_cap.h
> >
> >And you also can see some files are named with xxx_xxx.c/.h in many places.
> >
> >So what is the rule to name a file in qemu?
> >
> >Thanks
> >Tiejun
> >
> >>
> >>
> >>>  2 files changed, 207 insertions(+), 167 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..a4fe2d6 100644
> >>>--- a/hw/i386/kvm/pci-assign.c
> >>>+++ b/hw/i386/kvm/pci-assign.c
> >>>@@ -37,109 +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
> >>>-
> >>>-//#define DEVICE_ASSIGNMENT_DEBUG
> >>>-
> >>>-#ifdef DEVICE_ASSIGNMENT_DEBUG
> >>>-#define DEBUG(fmt, ...)                                       \
> >>>-    do {                                                      \
> >>>-        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
> >>>-    } while (0)
> >>>-#else
> >>>-#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;
> >>>+#include "hw/pci/pci_assign.h"
> >>>
> >>>  static void assigned_dev_update_irq_routing(PCIDevice *dev);
> >>>
> >>>@@ -1891,72 +1789,10 @@ static void assign_register_types(void)
> >>>
> >>>  type_init(assign_register_types)
> >>>
> >>>-/*
> >>>- * Scan the assigned devices for the devices that have an option
> >>>ROM, and then
> >>>- * 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)
> >>>  {
> >>>-    char name[32], rom_file[64];
> >>>-    FILE *fp;
> >>>-    uint8_t val;
> >>>-    struct stat st;
> >>>      void *ptr;
> >>>
> >>>-    /* If loading ROM from file, pci handles it */
> >>>-    if (dev->dev.romfile || !dev->dev.rom_bar) {
> >>>-        return;
> >>>-    }
> >>>-
> >>>-    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);
> >>>-
> >>>-    if (stat(rom_file, &st)) {
> >>>-        return;
> >>>-    }
> >>>-
> >>>-    if (access(rom_file, F_OK)) {
> >>>-        error_report("pci-assign: Insufficient privileges for %s",
> >>>rom_file);
> >>>-        return;
> >>>-    }
> >>>-
> >>>-    /* Write "1" to the ROM file to enable it */
> >>>-    fp = fopen(rom_file, "r+");
> >>>-    if (fp == NULL) {
> >>>-        return;
> >>>-    }
> >>>-    val = 1;
> >>>-    if (fwrite(&val, 1, 1, fp) != 1) {
> >>>-        goto close_rom;
> >>>-    }
> >>>-    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);
> >>>-    memset(ptr, 0xff, st.st_size);
> >>>-
> >>>-    if (!fread(ptr, 1, st.st_size, fp)) {
> >>>-        error_report("pci-assign: Cannot read from host %s", rom_file);
> >>>-        error_printf("Device option ROM contents are probably invalid "
> >>>-                     "(check dmesg).\nSkip option ROM probe with
> >>>rombar=0, "
> >>>-                     "or load from file with romfile=\n");
> >>>-        goto close_rom;
> >>>-    }
> >>>-
> >>>-    pci_register_bar(&dev->dev, PCI_ROM_SLOT, 0, &dev->dev.rom);
> >>>-    dev->dev.has_rom = true;
> >>>-close_rom:
> >>>-    /* Write "0" to disable ROM */
> >>>-    fseek(fp, 0, SEEK_SET);
> >>>-    val = 0;
> >>>-    if (!fwrite(&val, 1, 1, fp)) {
> >>>-        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
> >>>-    }
> >>>-    fclose(fp);
> >>>+    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..b146fcd
> >>>--- /dev/null
> >>>+++ b/include/hw/pci/pci_assign.h
> >>>@@ -0,0 +1,204 @@
> >>>+/*
> >>>+ * 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"
> >>>+
> >>>+#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
> >>>+
> >>>+//#define DEVICE_ASSIGNMENT_DEBUG
> >>>+
> >>>+#ifdef DEVICE_ASSIGNMENT_DEBUG
> >>>+#define DEBUG(fmt, ...)                                       \
> >>>+    do {                                                      \
> >>>+        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
> >>>+    } while (0)
> >>>+#else
> >>>+#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;
> >>>+
> >>>+/*
> >>>+ * Scan the assigned devices for the devices that have an option
> >>>ROM, and then
> >>>+ * 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 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;
> >>>+    int size = 0;
> >>>+
> >>>+    /* If loading ROM from file, pci handles it */
> >>>+    if (dev->romfile || !dev->rom_bar) {
> >>>+        return -1;
> >>>+    }
> >>>+
> >>>+    snprintf(rom_file, sizeof(rom_file),
> >>>+             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
> >>>+             domain, bus, slot, function);
> >>>+
> >>>+    if (stat(rom_file, &st)) {
> >>>+        return -1;
> >>>+    }
> >>>+
> >>>+    if (access(rom_file, F_OK)) {
> >>>+        error_report("pci-assign: Insufficient privileges for %s",
> >>>rom_file);
> >>>+        return -1;
> >>>+    }
> >>>+
> >>>+    /* Write "1" to the ROM file to enable it */
> >>>+    fp = fopen(rom_file, "r+");
> >>>+    if (fp == NULL) {
> >>>+        return -1;
> >>>+    }
> >>>+    val = 1;
> >>>+    if (fwrite(&val, 1, 1, fp) != 1) {
> >>>+        goto close_rom;
> >>>+    }
> >>>+    fseek(fp, 0, SEEK_SET);
> >>>+
> >>>+    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)) {
> >>>+        error_report("pci-assign: Cannot read from host %s", rom_file);
> >>>+        error_printf("Device option ROM contents are probably invalid "
> >>>+                     "(check dmesg).\nSkip option ROM probe with
> >>>rombar=0, "
> >>>+                     "or load from file with romfile=\n");
> >>>+        goto close_rom;
> >>>+    }
> >>>+
> >>>+    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);
> >>>+    val = 0;
> >>>+    if (!fwrite(&val, 1, 1, fp)) {
> >>>+        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
> >>>+    }
> >>>+    fclose(fp);
> >>>+
> >>>+    return size;
> >>>+}
> >>>+#endif /* PCI_ASSIGN_H */
> >>>--
> >>>1.9.1
> >>
> >
> >
> >
Tiejun Chen Sept. 1, 2014, 2:08 a.m. UTC | #3
On 2014/8/31 16:48, Michael S. Tsirkin wrote:
> On Fri, Aug 29, 2014 at 09:23:48AM +0800, Chen, Tiejun wrote:
>> On 2014/8/28 9:44, Chen, Tiejun wrote:
>>> On 2014/8/27 21:03, Michael S. Tsirkin wrote:
>>>> On Wed, Aug 27, 2014 at 05:13:07PM +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    | 170 +-----------------------------------
>>>>>   include/hw/pci/pci_assign.h | 204
>>>>> ++++++++++++++++++++++++++++++++++++++++++++
>>>>
>>>> Why are you making so much code inline?
>>>> Please move it to an out of line file.
>>>
>>> So just leave these structs and macros definition here,
>>>
>>
>> Michel,
>>
>> Is the following change inline fine to you?
>>
>> Your comments are appreciated in advance.
>>
>> Thanks
>> Tiejun
>
> Sorry I don't really understand. Please send a patch and
> I can review.

Okay, will send v2 to address your two comments:

* v1 is making so much code inline, so try to move it to an out of line 
file.
* rename pci-assign not pci_assign.

>
>>> diff --git a/include/hw/pci/pci-assign.h b/include/hw/pci/pci-assign.h

[snip]

>>>
>>>> Also it should be pci-assign not pci_assign.
>>>
>>> Okay, I can rename this as pci-assign.h.
>>>
>>> But,
>>>
>>> $ ls include/hw/pci/
>>> msi.h   pci_bus.h   pcie.h       pcie_port.h  pci.h       pci_ids.h
>>> shpc.h
>>> msix.h  pci_bridge.h  pcie_aer.h  pcie_host.h  pcie_regs.h  pci_host.h
>>> pci_regs.h  slotid_cap.h
>>>
>>> And you also can see some files are named with xxx_xxx.c/.h in many places.
>>>
>>> So what is the rule to name a file in qemu?

Anyway, if you can provide something about this, I'd like to unify these 
files as well.

Thanks
Tiejun

>>>
>>> Thanks
>>> Tiejun
>>>
>>>>
>>>>
>>>>>   2 files changed, 207 insertions(+), 167 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..a4fe2d6 100644
>>>>> --- a/hw/i386/kvm/pci-assign.c
>>>>> +++ b/hw/i386/kvm/pci-assign.c
>>>>> @@ -37,109 +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
>>>>> -
>>>>> -//#define DEVICE_ASSIGNMENT_DEBUG
>>>>> -
>>>>> -#ifdef DEVICE_ASSIGNMENT_DEBUG
>>>>> -#define DEBUG(fmt, ...)                                       \
>>>>> -    do {                                                      \
>>>>> -        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
>>>>> -    } while (0)
>>>>> -#else
>>>>> -#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;
>>>>> +#include "hw/pci/pci_assign.h"
>>>>>
>>>>>   static void assigned_dev_update_irq_routing(PCIDevice *dev);
>>>>>
>>>>> @@ -1891,72 +1789,10 @@ static void assign_register_types(void)
>>>>>
>>>>>   type_init(assign_register_types)
>>>>>
>>>>> -/*
>>>>> - * Scan the assigned devices for the devices that have an option
>>>>> ROM, and then
>>>>> - * 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)
>>>>>   {
>>>>> -    char name[32], rom_file[64];
>>>>> -    FILE *fp;
>>>>> -    uint8_t val;
>>>>> -    struct stat st;
>>>>>       void *ptr;
>>>>>
>>>>> -    /* If loading ROM from file, pci handles it */
>>>>> -    if (dev->dev.romfile || !dev->dev.rom_bar) {
>>>>> -        return;
>>>>> -    }
>>>>> -
>>>>> -    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);
>>>>> -
>>>>> -    if (stat(rom_file, &st)) {
>>>>> -        return;
>>>>> -    }
>>>>> -
>>>>> -    if (access(rom_file, F_OK)) {
>>>>> -        error_report("pci-assign: Insufficient privileges for %s",
>>>>> rom_file);
>>>>> -        return;
>>>>> -    }
>>>>> -
>>>>> -    /* Write "1" to the ROM file to enable it */
>>>>> -    fp = fopen(rom_file, "r+");
>>>>> -    if (fp == NULL) {
>>>>> -        return;
>>>>> -    }
>>>>> -    val = 1;
>>>>> -    if (fwrite(&val, 1, 1, fp) != 1) {
>>>>> -        goto close_rom;
>>>>> -    }
>>>>> -    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);
>>>>> -    memset(ptr, 0xff, st.st_size);
>>>>> -
>>>>> -    if (!fread(ptr, 1, st.st_size, fp)) {
>>>>> -        error_report("pci-assign: Cannot read from host %s", rom_file);
>>>>> -        error_printf("Device option ROM contents are probably invalid "
>>>>> -                     "(check dmesg).\nSkip option ROM probe with
>>>>> rombar=0, "
>>>>> -                     "or load from file with romfile=\n");
>>>>> -        goto close_rom;
>>>>> -    }
>>>>> -
>>>>> -    pci_register_bar(&dev->dev, PCI_ROM_SLOT, 0, &dev->dev.rom);
>>>>> -    dev->dev.has_rom = true;
>>>>> -close_rom:
>>>>> -    /* Write "0" to disable ROM */
>>>>> -    fseek(fp, 0, SEEK_SET);
>>>>> -    val = 0;
>>>>> -    if (!fwrite(&val, 1, 1, fp)) {
>>>>> -        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
>>>>> -    }
>>>>> -    fclose(fp);
>>>>> +    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..b146fcd
>>>>> --- /dev/null
>>>>> +++ b/include/hw/pci/pci_assign.h
>>>>> @@ -0,0 +1,204 @@
>>>>> +/*
>>>>> + * 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"
>>>>> +
>>>>> +#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
>>>>> +
>>>>> +//#define DEVICE_ASSIGNMENT_DEBUG
>>>>> +
>>>>> +#ifdef DEVICE_ASSIGNMENT_DEBUG
>>>>> +#define DEBUG(fmt, ...)                                       \
>>>>> +    do {                                                      \
>>>>> +        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
>>>>> +    } while (0)
>>>>> +#else
>>>>> +#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;
>>>>> +
>>>>> +/*
>>>>> + * Scan the assigned devices for the devices that have an option
>>>>> ROM, and then
>>>>> + * 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 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;
>>>>> +    int size = 0;
>>>>> +
>>>>> +    /* If loading ROM from file, pci handles it */
>>>>> +    if (dev->romfile || !dev->rom_bar) {
>>>>> +        return -1;
>>>>> +    }
>>>>> +
>>>>> +    snprintf(rom_file, sizeof(rom_file),
>>>>> +             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
>>>>> +             domain, bus, slot, function);
>>>>> +
>>>>> +    if (stat(rom_file, &st)) {
>>>>> +        return -1;
>>>>> +    }
>>>>> +
>>>>> +    if (access(rom_file, F_OK)) {
>>>>> +        error_report("pci-assign: Insufficient privileges for %s",
>>>>> rom_file);
>>>>> +        return -1;
>>>>> +    }
>>>>> +
>>>>> +    /* Write "1" to the ROM file to enable it */
>>>>> +    fp = fopen(rom_file, "r+");
>>>>> +    if (fp == NULL) {
>>>>> +        return -1;
>>>>> +    }
>>>>> +    val = 1;
>>>>> +    if (fwrite(&val, 1, 1, fp) != 1) {
>>>>> +        goto close_rom;
>>>>> +    }
>>>>> +    fseek(fp, 0, SEEK_SET);
>>>>> +
>>>>> +    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)) {
>>>>> +        error_report("pci-assign: Cannot read from host %s", rom_file);
>>>>> +        error_printf("Device option ROM contents are probably invalid "
>>>>> +                     "(check dmesg).\nSkip option ROM probe with
>>>>> rombar=0, "
>>>>> +                     "or load from file with romfile=\n");
>>>>> +        goto close_rom;
>>>>> +    }
>>>>> +
>>>>> +    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);
>>>>> +    val = 0;
>>>>> +    if (!fwrite(&val, 1, 1, fp)) {
>>>>> +        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
>>>>> +    }
>>>>> +    fclose(fp);
>>>>> +
>>>>> +    return size;
>>>>> +}
>>>>> +#endif /* PCI_ASSIGN_H */
>>>>> --
>>>>> 1.9.1
>>>>
>>>
>>>
>>>
>
Michael S. Tsirkin Sept. 1, 2014, 6:44 a.m. UTC | #4
On Mon, Sep 01, 2014 at 10:08:34AM +0800, Chen, Tiejun wrote:
> On 2014/8/31 16:48, Michael S. Tsirkin wrote:
> >On Fri, Aug 29, 2014 at 09:23:48AM +0800, Chen, Tiejun wrote:
> >>On 2014/8/28 9:44, Chen, Tiejun wrote:
> >>>On 2014/8/27 21:03, Michael S. Tsirkin wrote:
> >>>>On Wed, Aug 27, 2014 at 05:13:07PM +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    | 170 +-----------------------------------
> >>>>>  include/hw/pci/pci_assign.h | 204
> >>>>>++++++++++++++++++++++++++++++++++++++++++++
> >>>>
> >>>>Why are you making so much code inline?
> >>>>Please move it to an out of line file.
> >>>
> >>>So just leave these structs and macros definition here,
> >>>
> >>
> >>Michel,
> >>
> >>Is the following change inline fine to you?
> >>
> >>Your comments are appreciated in advance.
> >>
> >>Thanks
> >>Tiejun
> >
> >Sorry I don't really understand. Please send a patch and
> >I can review.
> 
> Okay, will send v2 to address your two comments:
> 
> * v1 is making so much code inline, so try to move it to an out of line
> file.
> * rename pci-assign not pci_assign.
> 
> >
> >>>diff --git a/include/hw/pci/pci-assign.h b/include/hw/pci/pci-assign.h
> 
> [snip]
> 
> >>>
> >>>>Also it should be pci-assign not pci_assign.
> >>>
> >>>Okay, I can rename this as pci-assign.h.
> >>>
> >>>But,
> >>>
> >>>$ ls include/hw/pci/
> >>>msi.h   pci_bus.h   pcie.h       pcie_port.h  pci.h       pci_ids.h
> >>>shpc.h
> >>>msix.h  pci_bridge.h  pcie_aer.h  pcie_host.h  pcie_regs.h  pci_host.h
> >>>pci_regs.h  slotid_cap.h
> >>>
> >>>And you also can see some files are named with xxx_xxx.c/.h in many places.
> >>>
> >>>So what is the rule to name a file in qemu?

In new code, - is prefered to _.

> Anyway, if you can provide something about this, I'd like to unify these
> files as well.

So far we avoided pure coding style cleanups. We only touch what we
have to about the file, this makes life easier for downstreams.

> Thanks
> Tiejun
> 
> >>>
> >>>Thanks
> >>>Tiejun
> >>>
> >>>>
> >>>>
> >>>>>  2 files changed, 207 insertions(+), 167 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..a4fe2d6 100644
> >>>>>--- a/hw/i386/kvm/pci-assign.c
> >>>>>+++ b/hw/i386/kvm/pci-assign.c
> >>>>>@@ -37,109 +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
> >>>>>-
> >>>>>-//#define DEVICE_ASSIGNMENT_DEBUG
> >>>>>-
> >>>>>-#ifdef DEVICE_ASSIGNMENT_DEBUG
> >>>>>-#define DEBUG(fmt, ...)                                       \
> >>>>>-    do {                                                      \
> >>>>>-        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
> >>>>>-    } while (0)
> >>>>>-#else
> >>>>>-#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;
> >>>>>+#include "hw/pci/pci_assign.h"
> >>>>>
> >>>>>  static void assigned_dev_update_irq_routing(PCIDevice *dev);
> >>>>>
> >>>>>@@ -1891,72 +1789,10 @@ static void assign_register_types(void)
> >>>>>
> >>>>>  type_init(assign_register_types)
> >>>>>
> >>>>>-/*
> >>>>>- * Scan the assigned devices for the devices that have an option
> >>>>>ROM, and then
> >>>>>- * 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)
> >>>>>  {
> >>>>>-    char name[32], rom_file[64];
> >>>>>-    FILE *fp;
> >>>>>-    uint8_t val;
> >>>>>-    struct stat st;
> >>>>>      void *ptr;
> >>>>>
> >>>>>-    /* If loading ROM from file, pci handles it */
> >>>>>-    if (dev->dev.romfile || !dev->dev.rom_bar) {
> >>>>>-        return;
> >>>>>-    }
> >>>>>-
> >>>>>-    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);
> >>>>>-
> >>>>>-    if (stat(rom_file, &st)) {
> >>>>>-        return;
> >>>>>-    }
> >>>>>-
> >>>>>-    if (access(rom_file, F_OK)) {
> >>>>>-        error_report("pci-assign: Insufficient privileges for %s",
> >>>>>rom_file);
> >>>>>-        return;
> >>>>>-    }
> >>>>>-
> >>>>>-    /* Write "1" to the ROM file to enable it */
> >>>>>-    fp = fopen(rom_file, "r+");
> >>>>>-    if (fp == NULL) {
> >>>>>-        return;
> >>>>>-    }
> >>>>>-    val = 1;
> >>>>>-    if (fwrite(&val, 1, 1, fp) != 1) {
> >>>>>-        goto close_rom;
> >>>>>-    }
> >>>>>-    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);
> >>>>>-    memset(ptr, 0xff, st.st_size);
> >>>>>-
> >>>>>-    if (!fread(ptr, 1, st.st_size, fp)) {
> >>>>>-        error_report("pci-assign: Cannot read from host %s", rom_file);
> >>>>>-        error_printf("Device option ROM contents are probably invalid "
> >>>>>-                     "(check dmesg).\nSkip option ROM probe with
> >>>>>rombar=0, "
> >>>>>-                     "or load from file with romfile=\n");
> >>>>>-        goto close_rom;
> >>>>>-    }
> >>>>>-
> >>>>>-    pci_register_bar(&dev->dev, PCI_ROM_SLOT, 0, &dev->dev.rom);
> >>>>>-    dev->dev.has_rom = true;
> >>>>>-close_rom:
> >>>>>-    /* Write "0" to disable ROM */
> >>>>>-    fseek(fp, 0, SEEK_SET);
> >>>>>-    val = 0;
> >>>>>-    if (!fwrite(&val, 1, 1, fp)) {
> >>>>>-        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
> >>>>>-    }
> >>>>>-    fclose(fp);
> >>>>>+    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..b146fcd
> >>>>>--- /dev/null
> >>>>>+++ b/include/hw/pci/pci_assign.h
> >>>>>@@ -0,0 +1,204 @@
> >>>>>+/*
> >>>>>+ * 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"
> >>>>>+
> >>>>>+#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
> >>>>>+
> >>>>>+//#define DEVICE_ASSIGNMENT_DEBUG
> >>>>>+
> >>>>>+#ifdef DEVICE_ASSIGNMENT_DEBUG
> >>>>>+#define DEBUG(fmt, ...)                                       \
> >>>>>+    do {                                                      \
> >>>>>+        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
> >>>>>+    } while (0)
> >>>>>+#else
> >>>>>+#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;
> >>>>>+
> >>>>>+/*
> >>>>>+ * Scan the assigned devices for the devices that have an option
> >>>>>ROM, and then
> >>>>>+ * 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 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;
> >>>>>+    int size = 0;
> >>>>>+
> >>>>>+    /* If loading ROM from file, pci handles it */
> >>>>>+    if (dev->romfile || !dev->rom_bar) {
> >>>>>+        return -1;
> >>>>>+    }
> >>>>>+
> >>>>>+    snprintf(rom_file, sizeof(rom_file),
> >>>>>+             "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
> >>>>>+             domain, bus, slot, function);
> >>>>>+
> >>>>>+    if (stat(rom_file, &st)) {
> >>>>>+        return -1;
> >>>>>+    }
> >>>>>+
> >>>>>+    if (access(rom_file, F_OK)) {
> >>>>>+        error_report("pci-assign: Insufficient privileges for %s",
> >>>>>rom_file);
> >>>>>+        return -1;
> >>>>>+    }
> >>>>>+
> >>>>>+    /* Write "1" to the ROM file to enable it */
> >>>>>+    fp = fopen(rom_file, "r+");
> >>>>>+    if (fp == NULL) {
> >>>>>+        return -1;
> >>>>>+    }
> >>>>>+    val = 1;
> >>>>>+    if (fwrite(&val, 1, 1, fp) != 1) {
> >>>>>+        goto close_rom;
> >>>>>+    }
> >>>>>+    fseek(fp, 0, SEEK_SET);
> >>>>>+
> >>>>>+    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)) {
> >>>>>+        error_report("pci-assign: Cannot read from host %s", rom_file);
> >>>>>+        error_printf("Device option ROM contents are probably invalid "
> >>>>>+                     "(check dmesg).\nSkip option ROM probe with
> >>>>>rombar=0, "
> >>>>>+                     "or load from file with romfile=\n");
> >>>>>+        goto close_rom;
> >>>>>+    }
> >>>>>+
> >>>>>+    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);
> >>>>>+    val = 0;
> >>>>>+    if (!fwrite(&val, 1, 1, fp)) {
> >>>>>+        DEBUG("%s\n", "Failed to disable pci-sysfs rom file");
> >>>>>+    }
> >>>>>+    fclose(fp);
> >>>>>+
> >>>>>+    return size;
> >>>>>+}
> >>>>>+#endif /* PCI_ASSIGN_H */
> >>>>>--
> >>>>>1.9.1
> >>>>
> >>>
> >>>
> >>>
> >
diff mbox

Patch

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"
+
+#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;
+
+int dev_load_option_rom(PCIDevice *dev, struct Object *owner, void *ptr,