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

[Cédric Le Goater]

On 5/29/23 04:45, Akihiko Odaki wrote:

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.

It does through the VTCTRL registers.

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.

I was looking at an issue from a VFIO perspective which does a FLR
on a device when pass through. Software and FLR are equivalent for
a VF.

I am not sure a VF needs more really, since it is all controlled
by the PF.

C.

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