Re: [PATCH 04/15] pcie: Add callback preceding SR-IOV VFs update

2021-10-20 Thread Lukasz Maniak 
On Fri, Oct 15, 2021 at 01:30:32PM -0400, Michael S. Tsirkin wrote:
> On Fri, Oct 15, 2021 at 06:24:14PM +0200, Lukasz Maniak wrote:
> > On Wed, Oct 13, 2021 at 05:10:35AM -0400, Michael S. Tsirkin wrote:
> > > On Tue, Oct 12, 2021 at 06:06:46PM +0200, Lukasz Maniak wrote:
> > > > On Tue, Oct 12, 2021 at 03:25:12AM -0400, Michael S. Tsirkin wrote:
> > > > > On Thu, Oct 07, 2021 at 06:23:55PM +0200, Lukasz Maniak wrote:
> > > > > > PCIe devices implementing SR-IOV may need to perform certain actions
> > > > > > before the VFs are unrealized or vice versa.
> > > > > > 
> > > > > > Signed-off-by: Lukasz Maniak 
> > > > > 
> > > > > Callbacks are annoying and easy to misuse though.
> > > > > VFs are enabled through a config cycle, we generally just
> > > > > have devices invoke the capability handler.
> > > > > E.g.
> > > > > 
> > > > > static void pci_bridge_dev_write_config(PCIDevice *d,
> > > > > uint32_t address, uint32_t 
> > > > > val, int len)
> > > > > {
> > > > > pci_bridge_write_config(d, address, val, len);
> > > > > if (msi_present(d)) {
> > > > > msi_write_config(d, address, val, len);
> > > > > }
> > > > > }
> > > > > 
> > > > > this makes it easy to do whatever you want before/after
> > > > > the write. You can also add a helper to check
> > > > > that SRIOV is being enabled/disabled if necessary.
> > > > > 
> > > > > > ---
> > > > > >  docs/pcie_sriov.txt |  2 +-
> > > > > >  hw/pci/pcie_sriov.c | 14 +-
> > > > > >  include/hw/pci/pcie_sriov.h |  8 +++-
> > > > > >  3 files changed, 21 insertions(+), 3 deletions(-)
> > > > > > 
> > > > > > diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
> > > > > > index f5e891e1d4..63ca1a7b8e 100644
> > > > > > --- a/docs/pcie_sriov.txt
> > > > > > +++ b/docs/pcie_sriov.txt
> > > > > > @@ -57,7 +57,7 @@ setting up a BAR for a VF.
> > > > > >/* Add and initialize the SR/IOV capability */
> > > > > >pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
> > > > > > vf_devid, initial_vfs, total_vfs,
> > > > > > -   fun_offset, stride);
> > > > > > +   fun_offset, stride, pre_vfs_update_cb);
> > > > > >  
> > > > > >/* Set up individual VF BARs (parameters as for normal BARs) 
> > > > > > */
> > > > > >pcie_sriov_pf_init_vf_bar( ... )
> > > > > > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> > > > > > index 501a1ff433..cac2aee061 100644
> > > > > > --- a/hw/pci/pcie_sriov.c
> > > > > > +++ b/hw/pci/pcie_sriov.c
> > > > > > @@ -30,7 +30,8 @@ static void unregister_vfs(PCIDevice *dev);
> > > > > >  void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> > > > > >  const char *vfname, uint16_t vf_dev_id,
> > > > > >  uint16_t init_vfs, uint16_t total_vfs,
> > > > > > -uint16_t vf_offset, uint16_t vf_stride)
> > > > > > +uint16_t vf_offset, uint16_t vf_stride,
> > > > > > +SriovVfsUpdate pre_vfs_update)
> > > > > >  {
> > > > > >  uint8_t *cfg = dev->config + offset;
> > > > > >  uint8_t *wmask;
> > > > > > @@ -41,6 +42,7 @@ void pcie_sriov_pf_init(PCIDevice *dev, uint16_t 
> > > > > > offset,
> > > > > >  dev->exp.sriov_pf.num_vfs = 0;
> > > > > >  dev->exp.sriov_pf.vfname = g_strdup(vfname);
> > > > > >  dev->exp.sriov_pf.vf = NULL;
> > > > > > +dev->exp.sriov_pf.pre_vfs_update = pre_vfs_update;
> > > > > >  
> > > > > >  pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
> > > > > >  pci_set_word(cfg + PCI_SRIOV_VF_STRIDE, vf_stride);
> > > > > > @@ -180,6 +182,11 @@ static void register_vfs(PCIDevice *dev)
> > > > > >  assert(dev->exp.sriov_pf.vf);
> > > > > >  
> > > > > >  trace_sriov_register_vfs(SRIOV_ID(dev), num_vfs);
> > > > > > +
> > > > > > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > > > > > +dev->exp.sriov_pf.pre_vfs_update(dev, 
> > > > > > dev->exp.sriov_pf.num_vfs, num_vfs);
> > > > > > +}
> > > > > > +
> > > > > >  for (i = 0; i < num_vfs; i++) {
> > > > > >  dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn, 
> > > > > > dev->exp.sriov_pf.vfname, i);
> > > > > >  if (!dev->exp.sriov_pf.vf[i]) {
> > > > > > @@ -198,6 +205,11 @@ static void unregister_vfs(PCIDevice *dev)
> > > > > >  uint16_t i;
> > > > > >  
> > > > > >  trace_sriov_unregister_vfs(SRIOV_ID(dev), num_vfs);
> > > > > > +
> > > > > > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > > > > > +dev->exp.sriov_pf.pre_vfs_update(dev, 
> > > > > > dev->exp.sriov_pf.num_vfs, 0);
> > > > > > +}
> > > > > > +
> > > > > >  for (i = 0; i < num_vfs; i++) {
> > > > > >  PCIDevice *vf = dev->exp.sriov_pf.vf[i];
> > > > > >  object_property_set_bool(OBJECT(vf), "realized", false, 
> > > > > > _err);
> > > > > > diff --git a/include/hw/pci/pcie_sriov.h 
> > > > > >

Re: [PATCH 04/15] pcie: Add callback preceding SR-IOV VFs update

2021-10-15 Thread Michael S. Tsirkin 
On Fri, Oct 15, 2021 at 06:24:14PM +0200, Lukasz Maniak wrote:
> On Wed, Oct 13, 2021 at 05:10:35AM -0400, Michael S. Tsirkin wrote:
> > On Tue, Oct 12, 2021 at 06:06:46PM +0200, Lukasz Maniak wrote:
> > > On Tue, Oct 12, 2021 at 03:25:12AM -0400, Michael S. Tsirkin wrote:
> > > > On Thu, Oct 07, 2021 at 06:23:55PM +0200, Lukasz Maniak wrote:
> > > > > PCIe devices implementing SR-IOV may need to perform certain actions
> > > > > before the VFs are unrealized or vice versa.
> > > > > 
> > > > > Signed-off-by: Lukasz Maniak 
> > > > 
> > > > Callbacks are annoying and easy to misuse though.
> > > > VFs are enabled through a config cycle, we generally just
> > > > have devices invoke the capability handler.
> > > > E.g.
> > > > 
> > > > static void pci_bridge_dev_write_config(PCIDevice *d,
> > > > uint32_t address, uint32_t val, 
> > > > int len)
> > > > {
> > > > pci_bridge_write_config(d, address, val, len);
> > > > if (msi_present(d)) {
> > > > msi_write_config(d, address, val, len);
> > > > }
> > > > }
> > > > 
> > > > this makes it easy to do whatever you want before/after
> > > > the write. You can also add a helper to check
> > > > that SRIOV is being enabled/disabled if necessary.
> > > > 
> > > > > ---
> > > > >  docs/pcie_sriov.txt |  2 +-
> > > > >  hw/pci/pcie_sriov.c | 14 +-
> > > > >  include/hw/pci/pcie_sriov.h |  8 +++-
> > > > >  3 files changed, 21 insertions(+), 3 deletions(-)
> > > > > 
> > > > > diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
> > > > > index f5e891e1d4..63ca1a7b8e 100644
> > > > > --- a/docs/pcie_sriov.txt
> > > > > +++ b/docs/pcie_sriov.txt
> > > > > @@ -57,7 +57,7 @@ setting up a BAR for a VF.
> > > > >/* Add and initialize the SR/IOV capability */
> > > > >pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
> > > > > vf_devid, initial_vfs, total_vfs,
> > > > > -   fun_offset, stride);
> > > > > +   fun_offset, stride, pre_vfs_update_cb);
> > > > >  
> > > > >/* Set up individual VF BARs (parameters as for normal BARs) */
> > > > >pcie_sriov_pf_init_vf_bar( ... )
> > > > > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> > > > > index 501a1ff433..cac2aee061 100644
> > > > > --- a/hw/pci/pcie_sriov.c
> > > > > +++ b/hw/pci/pcie_sriov.c
> > > > > @@ -30,7 +30,8 @@ static void unregister_vfs(PCIDevice *dev);
> > > > >  void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> > > > >  const char *vfname, uint16_t vf_dev_id,
> > > > >  uint16_t init_vfs, uint16_t total_vfs,
> > > > > -uint16_t vf_offset, uint16_t vf_stride)
> > > > > +uint16_t vf_offset, uint16_t vf_stride,
> > > > > +SriovVfsUpdate pre_vfs_update)
> > > > >  {
> > > > >  uint8_t *cfg = dev->config + offset;
> > > > >  uint8_t *wmask;
> > > > > @@ -41,6 +42,7 @@ void pcie_sriov_pf_init(PCIDevice *dev, uint16_t 
> > > > > offset,
> > > > >  dev->exp.sriov_pf.num_vfs = 0;
> > > > >  dev->exp.sriov_pf.vfname = g_strdup(vfname);
> > > > >  dev->exp.sriov_pf.vf = NULL;
> > > > > +dev->exp.sriov_pf.pre_vfs_update = pre_vfs_update;
> > > > >  
> > > > >  pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
> > > > >  pci_set_word(cfg + PCI_SRIOV_VF_STRIDE, vf_stride);
> > > > > @@ -180,6 +182,11 @@ static void register_vfs(PCIDevice *dev)
> > > > >  assert(dev->exp.sriov_pf.vf);
> > > > >  
> > > > >  trace_sriov_register_vfs(SRIOV_ID(dev), num_vfs);
> > > > > +
> > > > > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > > > > +dev->exp.sriov_pf.pre_vfs_update(dev, 
> > > > > dev->exp.sriov_pf.num_vfs, num_vfs);
> > > > > +}
> > > > > +
> > > > >  for (i = 0; i < num_vfs; i++) {
> > > > >  dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn, 
> > > > > dev->exp.sriov_pf.vfname, i);
> > > > >  if (!dev->exp.sriov_pf.vf[i]) {
> > > > > @@ -198,6 +205,11 @@ static void unregister_vfs(PCIDevice *dev)
> > > > >  uint16_t i;
> > > > >  
> > > > >  trace_sriov_unregister_vfs(SRIOV_ID(dev), num_vfs);
> > > > > +
> > > > > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > > > > +dev->exp.sriov_pf.pre_vfs_update(dev, 
> > > > > dev->exp.sriov_pf.num_vfs, 0);
> > > > > +}
> > > > > +
> > > > >  for (i = 0; i < num_vfs; i++) {
> > > > >  PCIDevice *vf = dev->exp.sriov_pf.vf[i];
> > > > >  object_property_set_bool(OBJECT(vf), "realized", false, 
> > > > > _err);
> > > > > diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
> > > > > index 0974f00054..9ab48b79c0 100644
> > > > > --- a/include/hw/pci/pcie_sriov.h
> > > > > +++ b/include/hw/pci/pcie_sriov.h
> > > > > @@ -13,11 +13,16 @@
> > > > >  #ifndef QEMU_PCIE_SRIOV_H
> > > > >  #define QEMU_PCIE_SRIOV_H
> > > > >  
> > > > > +typedef

Re: [PATCH 04/15] pcie: Add callback preceding SR-IOV VFs update

2021-10-15 Thread Lukasz Maniak 
On Wed, Oct 13, 2021 at 05:10:35AM -0400, Michael S. Tsirkin wrote:
> On Tue, Oct 12, 2021 at 06:06:46PM +0200, Lukasz Maniak wrote:
> > On Tue, Oct 12, 2021 at 03:25:12AM -0400, Michael S. Tsirkin wrote:
> > > On Thu, Oct 07, 2021 at 06:23:55PM +0200, Lukasz Maniak wrote:
> > > > PCIe devices implementing SR-IOV may need to perform certain actions
> > > > before the VFs are unrealized or vice versa.
> > > > 
> > > > Signed-off-by: Lukasz Maniak 
> > > 
> > > Callbacks are annoying and easy to misuse though.
> > > VFs are enabled through a config cycle, we generally just
> > > have devices invoke the capability handler.
> > > E.g.
> > > 
> > > static void pci_bridge_dev_write_config(PCIDevice *d,
> > > uint32_t address, uint32_t val, 
> > > int len)
> > > {
> > > pci_bridge_write_config(d, address, val, len);
> > > if (msi_present(d)) {
> > > msi_write_config(d, address, val, len);
> > > }
> > > }
> > > 
> > > this makes it easy to do whatever you want before/after
> > > the write. You can also add a helper to check
> > > that SRIOV is being enabled/disabled if necessary.
> > > 
> > > > ---
> > > >  docs/pcie_sriov.txt |  2 +-
> > > >  hw/pci/pcie_sriov.c | 14 +-
> > > >  include/hw/pci/pcie_sriov.h |  8 +++-
> > > >  3 files changed, 21 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
> > > > index f5e891e1d4..63ca1a7b8e 100644
> > > > --- a/docs/pcie_sriov.txt
> > > > +++ b/docs/pcie_sriov.txt
> > > > @@ -57,7 +57,7 @@ setting up a BAR for a VF.
> > > >/* Add and initialize the SR/IOV capability */
> > > >pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
> > > > vf_devid, initial_vfs, total_vfs,
> > > > -   fun_offset, stride);
> > > > +   fun_offset, stride, pre_vfs_update_cb);
> > > >  
> > > >/* Set up individual VF BARs (parameters as for normal BARs) */
> > > >pcie_sriov_pf_init_vf_bar( ... )
> > > > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> > > > index 501a1ff433..cac2aee061 100644
> > > > --- a/hw/pci/pcie_sriov.c
> > > > +++ b/hw/pci/pcie_sriov.c
> > > > @@ -30,7 +30,8 @@ static void unregister_vfs(PCIDevice *dev);
> > > >  void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> > > >  const char *vfname, uint16_t vf_dev_id,
> > > >  uint16_t init_vfs, uint16_t total_vfs,
> > > > -uint16_t vf_offset, uint16_t vf_stride)
> > > > +uint16_t vf_offset, uint16_t vf_stride,
> > > > +SriovVfsUpdate pre_vfs_update)
> > > >  {
> > > >  uint8_t *cfg = dev->config + offset;
> > > >  uint8_t *wmask;
> > > > @@ -41,6 +42,7 @@ void pcie_sriov_pf_init(PCIDevice *dev, uint16_t 
> > > > offset,
> > > >  dev->exp.sriov_pf.num_vfs = 0;
> > > >  dev->exp.sriov_pf.vfname = g_strdup(vfname);
> > > >  dev->exp.sriov_pf.vf = NULL;
> > > > +dev->exp.sriov_pf.pre_vfs_update = pre_vfs_update;
> > > >  
> > > >  pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
> > > >  pci_set_word(cfg + PCI_SRIOV_VF_STRIDE, vf_stride);
> > > > @@ -180,6 +182,11 @@ static void register_vfs(PCIDevice *dev)
> > > >  assert(dev->exp.sriov_pf.vf);
> > > >  
> > > >  trace_sriov_register_vfs(SRIOV_ID(dev), num_vfs);
> > > > +
> > > > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > > > +dev->exp.sriov_pf.pre_vfs_update(dev, 
> > > > dev->exp.sriov_pf.num_vfs, num_vfs);
> > > > +}
> > > > +
> > > >  for (i = 0; i < num_vfs; i++) {
> > > >  dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn, 
> > > > dev->exp.sriov_pf.vfname, i);
> > > >  if (!dev->exp.sriov_pf.vf[i]) {
> > > > @@ -198,6 +205,11 @@ static void unregister_vfs(PCIDevice *dev)
> > > >  uint16_t i;
> > > >  
> > > >  trace_sriov_unregister_vfs(SRIOV_ID(dev), num_vfs);
> > > > +
> > > > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > > > +dev->exp.sriov_pf.pre_vfs_update(dev, 
> > > > dev->exp.sriov_pf.num_vfs, 0);
> > > > +}
> > > > +
> > > >  for (i = 0; i < num_vfs; i++) {
> > > >  PCIDevice *vf = dev->exp.sriov_pf.vf[i];
> > > >  object_property_set_bool(OBJECT(vf), "realized", false, 
> > > > _err);
> > > > diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
> > > > index 0974f00054..9ab48b79c0 100644
> > > > --- a/include/hw/pci/pcie_sriov.h
> > > > +++ b/include/hw/pci/pcie_sriov.h
> > > > @@ -13,11 +13,16 @@
> > > >  #ifndef QEMU_PCIE_SRIOV_H
> > > >  #define QEMU_PCIE_SRIOV_H
> > > >  
> > > > +typedef void (*SriovVfsUpdate)(PCIDevice *dev, uint16_t prev_num_vfs,
> > > > +   uint16_t num_vfs);
> > > > +
> > > >  struct PCIESriovPF {
> > > >  uint16_t num_vfs;   /* Number of virtual functions created 
> > > > */
> > > >  uint8_t

Re: [PATCH 04/15] pcie: Add callback preceding SR-IOV VFs update

2021-10-13 Thread Michael S. Tsirkin 
On Tue, Oct 12, 2021 at 06:06:46PM +0200, Lukasz Maniak wrote:
> On Tue, Oct 12, 2021 at 03:25:12AM -0400, Michael S. Tsirkin wrote:
> > On Thu, Oct 07, 2021 at 06:23:55PM +0200, Lukasz Maniak wrote:
> > > PCIe devices implementing SR-IOV may need to perform certain actions
> > > before the VFs are unrealized or vice versa.
> > > 
> > > Signed-off-by: Lukasz Maniak 
> > 
> > Callbacks are annoying and easy to misuse though.
> > VFs are enabled through a config cycle, we generally just
> > have devices invoke the capability handler.
> > E.g.
> > 
> > static void pci_bridge_dev_write_config(PCIDevice *d,
> > uint32_t address, uint32_t val, int 
> > len)
> > {
> > pci_bridge_write_config(d, address, val, len);
> > if (msi_present(d)) {
> > msi_write_config(d, address, val, len);
> > }
> > }
> > 
> > this makes it easy to do whatever you want before/after
> > the write. You can also add a helper to check
> > that SRIOV is being enabled/disabled if necessary.
> > 
> > > ---
> > >  docs/pcie_sriov.txt |  2 +-
> > >  hw/pci/pcie_sriov.c | 14 +-
> > >  include/hw/pci/pcie_sriov.h |  8 +++-
> > >  3 files changed, 21 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
> > > index f5e891e1d4..63ca1a7b8e 100644
> > > --- a/docs/pcie_sriov.txt
> > > +++ b/docs/pcie_sriov.txt
> > > @@ -57,7 +57,7 @@ setting up a BAR for a VF.
> > >/* Add and initialize the SR/IOV capability */
> > >pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
> > > vf_devid, initial_vfs, total_vfs,
> > > -   fun_offset, stride);
> > > +   fun_offset, stride, pre_vfs_update_cb);
> > >  
> > >/* Set up individual VF BARs (parameters as for normal BARs) */
> > >pcie_sriov_pf_init_vf_bar( ... )
> > > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> > > index 501a1ff433..cac2aee061 100644
> > > --- a/hw/pci/pcie_sriov.c
> > > +++ b/hw/pci/pcie_sriov.c
> > > @@ -30,7 +30,8 @@ static void unregister_vfs(PCIDevice *dev);
> > >  void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> > >  const char *vfname, uint16_t vf_dev_id,
> > >  uint16_t init_vfs, uint16_t total_vfs,
> > > -uint16_t vf_offset, uint16_t vf_stride)
> > > +uint16_t vf_offset, uint16_t vf_stride,
> > > +SriovVfsUpdate pre_vfs_update)
> > >  {
> > >  uint8_t *cfg = dev->config + offset;
> > >  uint8_t *wmask;
> > > @@ -41,6 +42,7 @@ void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> > >  dev->exp.sriov_pf.num_vfs = 0;
> > >  dev->exp.sriov_pf.vfname = g_strdup(vfname);
> > >  dev->exp.sriov_pf.vf = NULL;
> > > +dev->exp.sriov_pf.pre_vfs_update = pre_vfs_update;
> > >  
> > >  pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
> > >  pci_set_word(cfg + PCI_SRIOV_VF_STRIDE, vf_stride);
> > > @@ -180,6 +182,11 @@ static void register_vfs(PCIDevice *dev)
> > >  assert(dev->exp.sriov_pf.vf);
> > >  
> > >  trace_sriov_register_vfs(SRIOV_ID(dev), num_vfs);
> > > +
> > > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > > +dev->exp.sriov_pf.pre_vfs_update(dev, dev->exp.sriov_pf.num_vfs, 
> > > num_vfs);
> > > +}
> > > +
> > >  for (i = 0; i < num_vfs; i++) {
> > >  dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn, 
> > > dev->exp.sriov_pf.vfname, i);
> > >  if (!dev->exp.sriov_pf.vf[i]) {
> > > @@ -198,6 +205,11 @@ static void unregister_vfs(PCIDevice *dev)
> > >  uint16_t i;
> > >  
> > >  trace_sriov_unregister_vfs(SRIOV_ID(dev), num_vfs);
> > > +
> > > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > > +dev->exp.sriov_pf.pre_vfs_update(dev, dev->exp.sriov_pf.num_vfs, 
> > > 0);
> > > +}
> > > +
> > >  for (i = 0; i < num_vfs; i++) {
> > >  PCIDevice *vf = dev->exp.sriov_pf.vf[i];
> > >  object_property_set_bool(OBJECT(vf), "realized", false, 
> > > _err);
> > > diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
> > > index 0974f00054..9ab48b79c0 100644
> > > --- a/include/hw/pci/pcie_sriov.h
> > > +++ b/include/hw/pci/pcie_sriov.h
> > > @@ -13,11 +13,16 @@
> > >  #ifndef QEMU_PCIE_SRIOV_H
> > >  #define QEMU_PCIE_SRIOV_H
> > >  
> > > +typedef void (*SriovVfsUpdate)(PCIDevice *dev, uint16_t prev_num_vfs,
> > > +   uint16_t num_vfs);
> > > +
> > >  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 */
> > > +
> > > +SriovVfsUpdate

Re: [PATCH 04/15] pcie: Add callback preceding SR-IOV VFs update

2021-10-12 Thread Lukasz Maniak 
On Tue, Oct 12, 2021 at 03:25:12AM -0400, Michael S. Tsirkin wrote:
> On Thu, Oct 07, 2021 at 06:23:55PM +0200, Lukasz Maniak wrote:
> > PCIe devices implementing SR-IOV may need to perform certain actions
> > before the VFs are unrealized or vice versa.
> > 
> > Signed-off-by: Lukasz Maniak 
> 
> Callbacks are annoying and easy to misuse though.
> VFs are enabled through a config cycle, we generally just
> have devices invoke the capability handler.
> E.g.
> 
> static void pci_bridge_dev_write_config(PCIDevice *d,
> uint32_t address, uint32_t val, int 
> len)
> {
> pci_bridge_write_config(d, address, val, len);
> if (msi_present(d)) {
> msi_write_config(d, address, val, len);
> }
> }
> 
> this makes it easy to do whatever you want before/after
> the write. You can also add a helper to check
> that SRIOV is being enabled/disabled if necessary.
> 
> > ---
> >  docs/pcie_sriov.txt |  2 +-
> >  hw/pci/pcie_sriov.c | 14 +-
> >  include/hw/pci/pcie_sriov.h |  8 +++-
> >  3 files changed, 21 insertions(+), 3 deletions(-)
> > 
> > diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
> > index f5e891e1d4..63ca1a7b8e 100644
> > --- a/docs/pcie_sriov.txt
> > +++ b/docs/pcie_sriov.txt
> > @@ -57,7 +57,7 @@ setting up a BAR for a VF.
> >/* Add and initialize the SR/IOV capability */
> >pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
> > vf_devid, initial_vfs, total_vfs,
> > -   fun_offset, stride);
> > +   fun_offset, stride, pre_vfs_update_cb);
> >  
> >/* Set up individual VF BARs (parameters as for normal BARs) */
> >pcie_sriov_pf_init_vf_bar( ... )
> > diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> > index 501a1ff433..cac2aee061 100644
> > --- a/hw/pci/pcie_sriov.c
> > +++ b/hw/pci/pcie_sriov.c
> > @@ -30,7 +30,8 @@ static void unregister_vfs(PCIDevice *dev);
> >  void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> >  const char *vfname, uint16_t vf_dev_id,
> >  uint16_t init_vfs, uint16_t total_vfs,
> > -uint16_t vf_offset, uint16_t vf_stride)
> > +uint16_t vf_offset, uint16_t vf_stride,
> > +SriovVfsUpdate pre_vfs_update)
> >  {
> >  uint8_t *cfg = dev->config + offset;
> >  uint8_t *wmask;
> > @@ -41,6 +42,7 @@ void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> >  dev->exp.sriov_pf.num_vfs = 0;
> >  dev->exp.sriov_pf.vfname = g_strdup(vfname);
> >  dev->exp.sriov_pf.vf = NULL;
> > +dev->exp.sriov_pf.pre_vfs_update = pre_vfs_update;
> >  
> >  pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
> >  pci_set_word(cfg + PCI_SRIOV_VF_STRIDE, vf_stride);
> > @@ -180,6 +182,11 @@ static void register_vfs(PCIDevice *dev)
> >  assert(dev->exp.sriov_pf.vf);
> >  
> >  trace_sriov_register_vfs(SRIOV_ID(dev), num_vfs);
> > +
> > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > +dev->exp.sriov_pf.pre_vfs_update(dev, dev->exp.sriov_pf.num_vfs, 
> > num_vfs);
> > +}
> > +
> >  for (i = 0; i < num_vfs; i++) {
> >  dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn, 
> > dev->exp.sriov_pf.vfname, i);
> >  if (!dev->exp.sriov_pf.vf[i]) {
> > @@ -198,6 +205,11 @@ static void unregister_vfs(PCIDevice *dev)
> >  uint16_t i;
> >  
> >  trace_sriov_unregister_vfs(SRIOV_ID(dev), num_vfs);
> > +
> > +if (dev->exp.sriov_pf.pre_vfs_update) {
> > +dev->exp.sriov_pf.pre_vfs_update(dev, dev->exp.sriov_pf.num_vfs, 
> > 0);
> > +}
> > +
> >  for (i = 0; i < num_vfs; i++) {
> >  PCIDevice *vf = dev->exp.sriov_pf.vf[i];
> >  object_property_set_bool(OBJECT(vf), "realized", false, 
> > _err);
> > diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
> > index 0974f00054..9ab48b79c0 100644
> > --- a/include/hw/pci/pcie_sriov.h
> > +++ b/include/hw/pci/pcie_sriov.h
> > @@ -13,11 +13,16 @@
> >  #ifndef QEMU_PCIE_SRIOV_H
> >  #define QEMU_PCIE_SRIOV_H
> >  
> > +typedef void (*SriovVfsUpdate)(PCIDevice *dev, uint16_t prev_num_vfs,
> > +   uint16_t num_vfs);
> > +
> >  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 */
> > +
> > +SriovVfsUpdate pre_vfs_update;  /* Callback preceding VFs count change 
> > */
> >  };
> >  
> >  struct PCIESriovVF {
> > @@ -28,7 +33,8 @@ struct PCIESriovVF {
> >  void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
> >  const char *vfname, uint16_t vf_dev_id,
> >

Re: [PATCH 04/15] pcie: Add callback preceding SR-IOV VFs update

2021-10-12 Thread Michael S. Tsirkin 
On Thu, Oct 07, 2021 at 06:23:55PM +0200, Lukasz Maniak wrote:
> PCIe devices implementing SR-IOV may need to perform certain actions
> before the VFs are unrealized or vice versa.
> 
> Signed-off-by: Lukasz Maniak 

Callbacks are annoying and easy to misuse though.
VFs are enabled through a config cycle, we generally just
have devices invoke the capability handler.
E.g.

static void pci_bridge_dev_write_config(PCIDevice *d,
uint32_t address, uint32_t val, int len)
{
pci_bridge_write_config(d, address, val, len);
if (msi_present(d)) {
msi_write_config(d, address, val, len);
}
}

this makes it easy to do whatever you want before/after
the write. You can also add a helper to check
that SRIOV is being enabled/disabled if necessary.

> ---
>  docs/pcie_sriov.txt |  2 +-
>  hw/pci/pcie_sriov.c | 14 +-
>  include/hw/pci/pcie_sriov.h |  8 +++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
> index f5e891e1d4..63ca1a7b8e 100644
> --- a/docs/pcie_sriov.txt
> +++ b/docs/pcie_sriov.txt
> @@ -57,7 +57,7 @@ setting up a BAR for a VF.
>/* Add and initialize the SR/IOV capability */
>pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
> vf_devid, initial_vfs, total_vfs,
> -   fun_offset, stride);
> +   fun_offset, stride, pre_vfs_update_cb);
>  
>/* Set up individual VF BARs (parameters as for normal BARs) */
>pcie_sriov_pf_init_vf_bar( ... )
> diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
> index 501a1ff433..cac2aee061 100644
> --- a/hw/pci/pcie_sriov.c
> +++ b/hw/pci/pcie_sriov.c
> @@ -30,7 +30,8 @@ static void unregister_vfs(PCIDevice *dev);
>  void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
>  const char *vfname, uint16_t vf_dev_id,
>  uint16_t init_vfs, uint16_t total_vfs,
> -uint16_t vf_offset, uint16_t vf_stride)
> +uint16_t vf_offset, uint16_t vf_stride,
> +SriovVfsUpdate pre_vfs_update)
>  {
>  uint8_t *cfg = dev->config + offset;
>  uint8_t *wmask;
> @@ -41,6 +42,7 @@ void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
>  dev->exp.sriov_pf.num_vfs = 0;
>  dev->exp.sriov_pf.vfname = g_strdup(vfname);
>  dev->exp.sriov_pf.vf = NULL;
> +dev->exp.sriov_pf.pre_vfs_update = pre_vfs_update;
>  
>  pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
>  pci_set_word(cfg + PCI_SRIOV_VF_STRIDE, vf_stride);
> @@ -180,6 +182,11 @@ static void register_vfs(PCIDevice *dev)
>  assert(dev->exp.sriov_pf.vf);
>  
>  trace_sriov_register_vfs(SRIOV_ID(dev), num_vfs);
> +
> +if (dev->exp.sriov_pf.pre_vfs_update) {
> +dev->exp.sriov_pf.pre_vfs_update(dev, dev->exp.sriov_pf.num_vfs, 
> num_vfs);
> +}
> +
>  for (i = 0; i < num_vfs; i++) {
>  dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn, 
> dev->exp.sriov_pf.vfname, i);
>  if (!dev->exp.sriov_pf.vf[i]) {
> @@ -198,6 +205,11 @@ static void unregister_vfs(PCIDevice *dev)
>  uint16_t i;
>  
>  trace_sriov_unregister_vfs(SRIOV_ID(dev), num_vfs);
> +
> +if (dev->exp.sriov_pf.pre_vfs_update) {
> +dev->exp.sriov_pf.pre_vfs_update(dev, dev->exp.sriov_pf.num_vfs, 0);
> +}
> +
>  for (i = 0; i < num_vfs; i++) {
>  PCIDevice *vf = dev->exp.sriov_pf.vf[i];
>  object_property_set_bool(OBJECT(vf), "realized", false, _err);
> diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
> index 0974f00054..9ab48b79c0 100644
> --- a/include/hw/pci/pcie_sriov.h
> +++ b/include/hw/pci/pcie_sriov.h
> @@ -13,11 +13,16 @@
>  #ifndef QEMU_PCIE_SRIOV_H
>  #define QEMU_PCIE_SRIOV_H
>  
> +typedef void (*SriovVfsUpdate)(PCIDevice *dev, uint16_t prev_num_vfs,
> +   uint16_t num_vfs);
> +
>  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 
> */
> +
> +SriovVfsUpdate pre_vfs_update;  /* Callback preceding VFs count change */
>  };
>  
>  struct PCIESriovVF {
> @@ -28,7 +33,8 @@ struct PCIESriovVF {
>  void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
>  const char *vfname, uint16_t vf_dev_id,
>  uint16_t init_vfs, uint16_t total_vfs,
> -uint16_t vf_offset, uint16_t vf_stride);
> +uint16_t vf_offset, uint16_t vf_stride,
> +SriovVfsUpdate pre_vfs_update);
>  void pcie_sriov_pf_exit(PCIDevice *dev);
>  
>  /* Set up a VF bar in the SR/IOV bar area

[PATCH 04/15] pcie: Add callback preceding SR-IOV VFs update

2021-10-07 Thread Lukasz Maniak 
PCIe devices implementing SR-IOV may need to perform certain actions
before the VFs are unrealized or vice versa.

Signed-off-by: Lukasz Maniak 
---
 docs/pcie_sriov.txt |  2 +-
 hw/pci/pcie_sriov.c | 14 +-
 include/hw/pci/pcie_sriov.h |  8 +++-
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/docs/pcie_sriov.txt b/docs/pcie_sriov.txt
index f5e891e1d4..63ca1a7b8e 100644
--- a/docs/pcie_sriov.txt
+++ b/docs/pcie_sriov.txt
@@ -57,7 +57,7 @@ setting up a BAR for a VF.
   /* Add and initialize the SR/IOV capability */
   pcie_sriov_pf_init(d, 0x200, "your_virtual_dev",
vf_devid, initial_vfs, total_vfs,
-   fun_offset, stride);
+   fun_offset, stride, pre_vfs_update_cb);
 
   /* Set up individual VF BARs (parameters as for normal BARs) */
   pcie_sriov_pf_init_vf_bar( ... )
diff --git a/hw/pci/pcie_sriov.c b/hw/pci/pcie_sriov.c
index 501a1ff433..cac2aee061 100644
--- a/hw/pci/pcie_sriov.c
+++ b/hw/pci/pcie_sriov.c
@@ -30,7 +30,8 @@ static void unregister_vfs(PCIDevice *dev);
 void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
 const char *vfname, uint16_t vf_dev_id,
 uint16_t init_vfs, uint16_t total_vfs,
-uint16_t vf_offset, uint16_t vf_stride)
+uint16_t vf_offset, uint16_t vf_stride,
+SriovVfsUpdate pre_vfs_update)
 {
 uint8_t *cfg = dev->config + offset;
 uint8_t *wmask;
@@ -41,6 +42,7 @@ void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
 dev->exp.sriov_pf.num_vfs = 0;
 dev->exp.sriov_pf.vfname = g_strdup(vfname);
 dev->exp.sriov_pf.vf = NULL;
+dev->exp.sriov_pf.pre_vfs_update = pre_vfs_update;
 
 pci_set_word(cfg + PCI_SRIOV_VF_OFFSET, vf_offset);
 pci_set_word(cfg + PCI_SRIOV_VF_STRIDE, vf_stride);
@@ -180,6 +182,11 @@ static void register_vfs(PCIDevice *dev)
 assert(dev->exp.sriov_pf.vf);
 
 trace_sriov_register_vfs(SRIOV_ID(dev), num_vfs);
+
+if (dev->exp.sriov_pf.pre_vfs_update) {
+dev->exp.sriov_pf.pre_vfs_update(dev, dev->exp.sriov_pf.num_vfs, 
num_vfs);
+}
+
 for (i = 0; i < num_vfs; i++) {
 dev->exp.sriov_pf.vf[i] = register_vf(dev, devfn, 
dev->exp.sriov_pf.vfname, i);
 if (!dev->exp.sriov_pf.vf[i]) {
@@ -198,6 +205,11 @@ static void unregister_vfs(PCIDevice *dev)
 uint16_t i;
 
 trace_sriov_unregister_vfs(SRIOV_ID(dev), num_vfs);
+
+if (dev->exp.sriov_pf.pre_vfs_update) {
+dev->exp.sriov_pf.pre_vfs_update(dev, dev->exp.sriov_pf.num_vfs, 0);
+}
+
 for (i = 0; i < num_vfs; i++) {
 PCIDevice *vf = dev->exp.sriov_pf.vf[i];
 object_property_set_bool(OBJECT(vf), "realized", false, _err);
diff --git a/include/hw/pci/pcie_sriov.h b/include/hw/pci/pcie_sriov.h
index 0974f00054..9ab48b79c0 100644
--- a/include/hw/pci/pcie_sriov.h
+++ b/include/hw/pci/pcie_sriov.h
@@ -13,11 +13,16 @@
 #ifndef QEMU_PCIE_SRIOV_H
 #define QEMU_PCIE_SRIOV_H
 
+typedef void (*SriovVfsUpdate)(PCIDevice *dev, uint16_t prev_num_vfs,
+   uint16_t num_vfs);
+
 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 */
+
+SriovVfsUpdate pre_vfs_update;  /* Callback preceding VFs count change */
 };
 
 struct PCIESriovVF {
@@ -28,7 +33,8 @@ struct PCIESriovVF {
 void pcie_sriov_pf_init(PCIDevice *dev, uint16_t offset,
 const char *vfname, uint16_t vf_dev_id,
 uint16_t init_vfs, uint16_t total_vfs,
-uint16_t vf_offset, uint16_t vf_stride);
+uint16_t vf_offset, uint16_t vf_stride,
+SriovVfsUpdate pre_vfs_update);
 void pcie_sriov_pf_exit(PCIDevice *dev);
 
 /* Set up a VF bar in the SR/IOV bar area */
-- 
2.25.1