Re: [PATCH v2 3/4] igb: Fix ARI next function numbers

["Michael S. Tsirkin" <mst@redhat.com>]

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.odaki@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