[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 2/4] pcie: Add support for Single Root I/O Vi
From: |
Knut Omang |
Subject: |
Re: [Qemu-devel] [PATCH v5 2/4] pcie: Add support for Single Root I/O Virtualization (SR/IOV) |
Date: |
Sun, 18 Oct 2015 17:57:04 +0200 |
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 <address@hidden>
> > + *
> > + * 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 <address@hidden>
Thanks,
Knut
> Thanks,
> Marcel
- Re: [Qemu-devel] [PATCH v5 3/4] pcie: Add some SR/IOV API documentation in docs/pcie_sriov.txt, (continued)
Re: [Qemu-devel] [PATCH v5 3/4] pcie: Add some SR/IOV API documentation in docs/pcie_sriov.txt, Marcel Apfelbaum, 2015/10/18
[Qemu-devel] [PATCH v5 4/4] pcie: A few minor fixes (type+code simplify), Knut Omang, 2015/10/14
[Qemu-devel] [PATCH v5 2/4] pcie: Add support for Single Root I/O Virtualization (SR/IOV), Knut Omang, 2015/10/14
Re: [Qemu-devel] [PATCH v5 0/4] pcie: Add support for Single Root I/O Virtualization, Marcel Apfelbaum, 2015/10/18