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.c...@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. Also it should be pci-assign not pci_assign. > 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