On 2023/07/02 14:05, Michael S. Tsirkin wrote:
On Sat, Jul 01, 2023 at 04:01:21PM +0900, Akihiko Odaki wrote:
The next function numbers are expected to form a linked list ending with
0.
Fixes: 3a977deebe ("Intrdocue igb device emulation")
Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
---
hw/net/igb_core.h | 3 +++
hw/net/igb.c | 4 +---
hw/net/igbvf.c | 5 ++++-
3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/hw/net/igb_core.h b/hw/net/igb_core.h
index 9cbbfd516b..e1dab76995 100644
--- a/hw/net/igb_core.h
+++ b/hw/net/igb_core.h
@@ -49,6 +49,9 @@
#define IGB_NUM_QUEUES (16)
#define IGB_NUM_VM_POOLS (8)
+#define IGB_VF_OFFSET (0x80)
+#define IGB_VF_STRIDE (2)
+
typedef struct IGBCore IGBCore;
enum { PHY_R = BIT(0),
diff --git a/hw/net/igb.c b/hw/net/igb.c
index 1c989d7677..543ca0114a 100644
--- a/hw/net/igb.c
+++ b/hw/net/igb.c
@@ -81,8 +81,6 @@ struct IGBState {
};
#define IGB_CAP_SRIOV_OFFSET (0x160)
-#define IGB_VF_OFFSET (0x80)
-#define IGB_VF_STRIDE (2)
#define E1000E_MMIO_IDX 0
#define E1000E_FLASH_IDX 1
@@ -431,7 +429,7 @@ static void igb_pci_realize(PCIDevice *pci_dev, Error
**errp)
hw_error("Failed to initialize AER capability");
}
- pcie_ari_init(pci_dev, 0x150, 1);
+ pcie_ari_init(pci_dev, 0x150, IGB_VF_OFFSET);
pcie_sriov_pf_init(pci_dev, IGB_CAP_SRIOV_OFFSET, TYPE_IGBVF,
IGB_82576_VF_DEV_ID, IGB_MAX_VF_FUNCTIONS, IGB_MAX_VF_FUNCTIONS,
I think this change would break migrations from 8.0. No?
Well, I don't have a reason to concern more for this change than other
bug fixes with behavioral changes observable from the guest.
More importantly your commit log says linked list should end
with 0, but you make it point at a VF instead.
diff --git a/hw/net/igbvf.c b/hw/net/igbvf.c
index 284ea61184..bf2f237ab5 100644
--- a/hw/net/igbvf.c
+++ b/hw/net/igbvf.c
@@ -240,6 +240,9 @@ static const MemoryRegionOps mmio_ops = {
static void igbvf_pci_realize(PCIDevice *dev, Error **errp)
{
IgbVfState *s = IGBVF(dev);
+ uint16_t nextvfn = pcie_sriov_vf_number(dev) + 1;
+ uint16_t nextfn = nextvfn < IGB_MAX_VF_FUNCTIONS ?
+ IGB_VF_OFFSET + nextvfn * IGB_VF_STRIDE : 0;
int ret;
int i;
@@ -270,7 +273,7 @@ static void igbvf_pci_realize(PCIDevice *dev, Error **errp)
hw_error("Failed to initialize AER capability");
}
- pcie_ari_init(dev, 0x150, 1);
+ pcie_ari_init(dev, 0x150, nextfn);
For this one I don't see why it matters at all:
The presence of Shadow Functions does not affect this field.
For VFs, this field is undefined since VFs are located using First VF Offset
(see § Section 9.3.3.9 ) and VF
Stride (see § Section 9.3.3.10 ).
I missed the statements saying the field is undefined for VFs. I posted
an alternative series ("[PATCH 0/3] pci: Fix ARI next function numbers")
so please review it.
}
static void igbvf_pci_uninit(PCIDevice *dev)
--
2.41.0
