RE: [PATCH] igb: Add Function Level Reset to PF and VF

Sriram Yagnaraman posted 1 patch 11 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/DBBP189MB14335F641546F8C47ECC951095459@DBBP189MB1433.EURP189.PROD.OUTLOOK.COM
Maintainers: Akihiko Odaki <akihiko.odaki@daynix.com>, Sriram Yagnaraman <sriram.yagnaraman@est.tech>, Jason Wang <jasowang@redhat.com>
RE: [PATCH] igb: Add Function Level Reset to PF and VF
Posted by Sriram Yagnaraman 11 months, 2 weeks ago
> -----Original Message-----
> From: qemu-devel-bounces+sriram.yagnaraman=est.tech@nongnu.org
> <qemu-devel-bounces+sriram.yagnaraman=est.tech@nongnu.org> On Behalf
> Of Sriram Yagnaraman
> Sent: Sunday, 28 May 2023 12:51
> To: Cédric Le Goater <clg@redhat.com>; qemu-devel@nongnu.org
> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>; Jason Wang
> <jasowang@redhat.com>
> Subject: RE: [PATCH] igb: Add Function Level Reset to PF and VF
> 
> 
> > -----Original Message-----
> > From: Cédric Le Goater <clg@redhat.com>
> > Sent: Friday, 26 May 2023 19:31
> > To: qemu-devel@nongnu.org
> > Cc: Akihiko Odaki <akihiko.odaki@daynix.com>; Sriram Yagnaraman
> > <sriram.yagnaraman@est.tech>; Jason Wang <jasowang@redhat.com>;
> Cédric
> > Le Goater <clg@redhat.com>
> > Subject: [PATCH] igb: Add Function Level Reset to PF and VF
> >
> > The Intel 82576EB GbE Controller say that the Physical and Virtual
> > Functions support Function Level Reset. Add the capability to each device
> model.
> >
> > Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
> > Fixes: 3a977deebe6b ("Intrdocue igb device emulation")
> > Signed-off-by: Cédric Le Goater <clg@redhat.com>
> > ---
> >  hw/net/igb.c   | 3 +++
> >  hw/net/igbvf.c | 3 +++
> >  2 files changed, 6 insertions(+)
> >
> > diff --git a/hw/net/igb.c b/hw/net/igb.c index
> > 1c989d767725..08e389338dca
> > 100644
> > --- a/hw/net/igb.c
> > +++ b/hw/net/igb.c
> > @@ -101,6 +101,7 @@ static void igb_write_config(PCIDevice *dev,
> > uint32_t addr,
> >
> >      trace_igb_write_config(addr, val, len);
> >      pci_default_write_config(dev, addr, val, len);
> > +    pcie_cap_flr_write_config(dev, addr, val, len);
> >
> >      if (range_covers_byte(addr, len, PCI_COMMAND) &&
> >          (dev->config[PCI_COMMAND] & PCI_COMMAND_MASTER)) { @@ -
> 427,6
> > +428,8 @@ static void igb_pci_realize(PCIDevice *pci_dev, Error
> > +**errp)
> >      }
> >
> >      /* PCIe extended capabilities (in order) */
> > +    pcie_cap_flr_init(pci_dev);
> > +
> >      if (pcie_aer_init(pci_dev, 1, 0x100, 0x40, errp) < 0) {
> >          hw_error("Failed to initialize AER capability");
> >      }
> > diff --git a/hw/net/igbvf.c b/hw/net/igbvf.c index
> > 284ea611848b..0a58dad06802 100644
> > --- a/hw/net/igbvf.c
> > +++ b/hw/net/igbvf.c
> > @@ -204,6 +204,7 @@ static void igbvf_write_config(PCIDevice *dev,
> > uint32_t addr, uint32_t val,  {
> >      trace_igbvf_write_config(addr, val, len);
> >      pci_default_write_config(dev, addr, val, len);
> > +    pcie_cap_flr_write_config(dev, addr, val, len);
> >  }
> >
> >  static uint64_t igbvf_mmio_read(void *opaque, hwaddr addr, unsigned
> > size) @@ -266,6 +267,8 @@ static void igbvf_pci_realize(PCIDevice
> > *dev, Error
> > **errp)
> >          hw_error("Failed to initialize PCIe capability");
> >      }
> >
> > +    pcie_cap_flr_init(dev);
> 
> Sorry for my naive question, and perhaps not related to your patch, IGBVF
> device class doesn't seem to have any reset functions registered via
> igbvf_class_init(). So, I am guessing an FLR will not trigger igb_vf_reset(), which
> is probably what we want.
> 

Something like this perhaps? Not compile tested, just an idea.
diff --git a/hw/net/igbvf.c b/hw/net/igbvf.c
index 284ea61184..9f07983bc9 100644
--- a/hw/net/igbvf.c
+++ b/hw/net/igbvf.c
@@ -283,9 +283,17 @@ static void igbvf_pci_uninit(PCIDevice *dev)
     msix_uninit(dev, &s->msix, &s->msix);
 }
 
+static void igbvf_qdev_reset_hold(Object *obj)
+{
+    trace_e1000e_cb_qdev_reset_hold();
+
+    igbvf_mmio_write(obj, E1000_CTRL, E1000_CTRL_RST, 0x4); /* Write to VTCTRL to trigger VF reset */
+}
+
 static void igbvf_class_init(ObjectClass *class, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(class);
+    ResettableClass *rc = RESETTABLE_CLASS(class);
     PCIDeviceClass *c = PCI_DEVICE_CLASS(class);
 
     c->realize = igbvf_pci_realize;
@@ -295,6 +303,8 @@ static void igbvf_class_init(ObjectClass *class, void *data)
     c->revision = 1;
     c->class_id = PCI_CLASS_NETWORK_ETHERNET;
 
+    rc->phases.hold = igbvf_qdev_reset_hold;
+
     dc->desc = "Intel 82576 Virtual Function";
     dc->user_creatable = false;

> > +
> >      if (pcie_aer_init(dev, 1, 0x100, 0x40, errp) < 0) {
> >          hw_error("Failed to initialize AER capability");
> >      }
> > --
> > 2.40.1