On Sun, Jul 02, 2023 at 06:46:27PM +0900, Akihiko Odaki wrote:
> The ARI next function number field is undefined for VF so the PF should
> end the linked list formed with the field by specifying 0.
>
> Fixes: 3a977deebe ("Intrdocue igb device emulation")
> Signed-off-by: Akihiko Odaki <akihiko.od...@daynix.com>
> ---
> hw/core/machine.c | 3 ++-
> hw/net/igb.c | 5 ++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/hw/core/machine.c b/hw/core/machine.c
> index f984a767a2..1f5aacd1dc 100644
> --- a/hw/core/machine.c
> +++ b/hw/core/machine.c
> @@ -41,7 +41,8 @@
>
> GlobalProperty hw_compat_8_0[] = {
> { "migration", "multifd-flush-after-each-section", "on"},
> - { "nvme", "ari-nextfn-1", "on"},
> + { "igb", "ari-nextfn-1", "on" },
> + { "nvme", "ari-nextfn-1", "on" },
> };
> const size_t hw_compat_8_0_len = G_N_ELEMENTS(hw_compat_8_0);
>
> diff --git a/hw/net/igb.c b/hw/net/igb.c
> index 1c989d7677..d37d43c155 100644
> --- a/hw/net/igb.c
> +++ b/hw/net/igb.c
> @@ -78,6 +78,8 @@ struct IGBState {
> uint32_t ioaddr;
>
> IGBCore core;
> +
> + bool ari_nextfn_1;
Document this field please, explaining why it's there.
> };
>
> #define IGB_CAP_SRIOV_OFFSET (0x160)
> @@ -431,7 +433,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, s->ari_nextfn_1 ? 1 : 0);
Why don't we move the logic to pci core, and drop code duplication
completely?
>
> 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,
> @@ -582,6 +584,7 @@ static const VMStateDescription igb_vmstate = {
>
> static Property igb_properties[] = {
> DEFINE_NIC_PROPERTIES(IGBState, conf),
> + DEFINE_PROP_BOOL("ari-nextfn-1", IGBState, ari_nextfn_1, false),
> DEFINE_PROP_END_OF_LIST(),
> };
This really needs to be a non stable property (prefix with "x-").
> --
> 2.41.0
