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

[Akihiko Odaki]

On 2023/05/28 19:50, Sriram Yagnaraman wrote:

-----Original Message-----
From: Cédric Le Goater <c...@redhat.com>
Sent: Friday, 26 May 2023 19:31
To: qemu-devel@nongnu.org
Cc: Akihiko Odaki <akihiko.od...@daynix.com>; Sriram Yagnaraman
<sriram.yagnara...@est.tech>; Jason Wang <jasow...@redhat.com>; Cédric
Le Goater <c...@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.od...@daynix.com>
Fixes: 3a977deebe6b ("Intrdocue igb device emulation")
Signed-off-by: Cédric Le Goater <c...@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.

You're right. Advertising FLR capability without implementing it can 
confuse the guest though such probability is quite a low in practice. 
The reset should be implemented first.


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