On Sun, 2015-10-18 at 14:02 +0300, Marcel Apfelbaum wrote: > > +#define SRIOV_ID(dev) \ > > + (dev)->name, PCI_SLOT((dev)->devfn), PCI_FUNC((dev)->devfn) > > This is a little "hacky" but it is used only for tracing and inside a > C file. > I am OK with it.
For the sake of readability of the trace calls and to avoid the trace lines from disturbing the code overview too much - I would have had to split the line of each trace call. > > +++ b/include/hw/pci/pcie_sriov.h > > @@ -0,0 +1,58 @@ > > +/* > > + * pcie_sriov.h: > > + * > > + * Implementation of SR/IOV emulation support. > > + * > > + * Copyright (c) 2015 Knut Omang <knut.om...@oracle.com> > > + * > > + * This work is licensed under the terms of the GNU GPL, version 2 > > or later. > > + * See the COPYING file in the top-level directory. > > + * > > + */ > > + > > +#ifndef QEMU_PCIE_SRIOV_H > > +#define QEMU_PCIE_SRIOV_H > > + > > +struct PCIESriovPF { > > + uint16_t num_vfs; /* Number of virtual functions > > created */ > > + uint8_t vf_bar_type[PCI_NUM_REGIONS]; /* Store type for each > > VF bar */ > > + const char *vfname; /* Reference to the device type > > used for the VFs */ > > + PCIDevice **vf; /* Pointer to an array of num_vfs > > VF devices */ > > +}; > > + > > +struct PCIESriovVF { > > + PCIDevice *pf; /* Pointer back to owner physical > > function */ > > +}; > > > Regarding naming conventions, maybe you it should be PCIESRIOVPF o Ir > PCIESRIOVPf, > but this would not justify another version (in my opinion) Yes, I did think about that, but didn't like the readability of either of these, still wanted to try to follow conventions - this was the tradeoff I ended at. SR/IOV is kind of complicated compared to e.g. AER in that the '/' has to go anyway :-D > > + > > +/* Optionally add supported page sizes to the mask of supported > > page sizes > > + * Page size values are interpreted as opt_sup_pgsize << 12. > > + */ > > +void pcie_sriov_pf_add_sup_pgsize(PCIDevice *dev, uint16_t > > opt_sup_pgsize); > > + > > +/* SR/IOV capability config write handler */ > > +void pcie_sriov_config_write(PCIDevice *dev, uint32_t address, > > + uint32_t val, int len); > > + > > +/* Reset SR/IOV VF Enable bit to unregister all VFs */ > > +void pcie_sriov_pf_disable_vfs(PCIDevice *dev); > > + > > +#endif /* QEMU_PCIE_SRIOV_H */ > > diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h > > index ee1ce1d..0b89316 100644 > > --- a/include/qemu/typedefs.h > > +++ b/include/qemu/typedefs.h > > @@ -54,6 +54,8 @@ typedef struct PCIDevice PCIDevice; > > typedef struct PCIEAERErr PCIEAERErr; > > typedef struct PCIEAERLog PCIEAERLog; > > typedef struct PCIEAERMsg PCIEAERMsg; > > +typedef struct PCIESriovPF PCIESriovPF; > > +typedef struct PCIESriovVF PCIESriovVF; > > typedef struct PCIEPort PCIEPort; > > typedef struct PCIESlot PCIESlot; > > typedef struct PCIExpressDevice PCIExpressDevice; > > diff --git a/trace-events b/trace-events > > index a0ddc6b..fd51c5f 100644 > > --- a/trace-events > > +++ b/trace-events > > @@ -1531,6 +1531,11 @@ xen_pv_mmio_write(uint64_t addr) "WARNING: > > write to Xen PV Device MMIO space (ad > > pci_cfg_read(const char *dev, unsigned devid, unsigned fnid, > > unsigned offs, unsigned val) "%s %02u:%u @0x%x -> 0x%x" > > pci_cfg_write(const char *dev, unsigned devid, unsigned fnid, > > unsigned offs, unsigned val) "%s %02u:%u @0x%x <- 0x%x" > > > > +# hw/pci/pcie_sriov.c > > +sriov_register_vfs(const char *name, int slot, int function, int > > num_vfs) "%s %02x:%x: creating %d vf devs" > > +sriov_unregister_vfs(const char *name, int slot, int function, int > > num_vfs) "%s %02x:%x: Unregistering %d vf devs" > > +sriov_config_write(const char *name, int slot, int fun, uint32_t > > offset, uint32_t val, uint32_t len) "%s %02x:%x: sriov offset 0x%x > > val 0x%x len %d" > > + > > # hw/vfio/pci.c > > vfio_intx_interrupt(const char *name, char line) " (%s) Pin %c" > > vfio_intx_eoi(const char *name) " (%s) EOI" > > > > Thanks for all the work, besides the questions above, from my point > of view: > > > Reviewed-by: Marcel Apfelbaum <mar...@redhat.com> Thanks, Knut > Thanks, > Marcel