From: YASUOKA Masahiko <yasuoka@openbsd.org>
Subject: Re: iavf: panic on QEMU with i440fx chipset emulation
Date: Mon, 07 Oct 2024 10:53:56 +0900 (JST)

> On Mon, 07 Oct 2024 10:22:00 +0900 (JST)
> Yuichiro NAITO <naito.yuichiro@gmail.com> wrote:
>> From: YASUOKA Masahiko <yasuoka@openbsd.org>
>> Subject: Re: iavf: panic on QEMU with i440fx chipset emulation
>> Date: Fri, 04 Oct 2024 17:19:28 +0900 (JST)
>> 
>>> On Wed, 25 Sep 2024 11:42:02 +0900 (JST)
>>> Yuichiro NAITO <naito.yuichiro@gmail.com> wrote:
>>>> Hi, While I'm testing iavf(4) on Linux QEMU with i440fx chipset emulation,
>>>> I had the following panic message on my OpenBSD current build.
>>>> I440fx is the default chipset emulation of QEMU. I know q35 chipset
>>>> emulation is required for msix interrupts. But if users forget to select it,
>>>> I tested what happens on the default chipset emulation. 
>>> 
>>> I think the root problem is this i440fx situation.
>> 
>> No, it's a separate issue.
>> 
>> I mean the iavf(4) panic occurs if an error happens between
>> 'iavf_get_vf_resources' and 'if_atatch' in the 'iavf_attach' function.
>> Looking at the 'iavf_attach' function, there are at least 3 functions
>> that may cause an error, 'iavf_config_irq_map', 'pci_intr_map_msix',
>> and 'pci_intr_establish' functions.
>> The 'pci_intr_map_msix' case is 100% reproducible on the QEMU i440fx
>> chipset emulation. The other cases are very rare but should be fixed for
>> stability.
> 
> Yes, it might be true if there is a broken hardware which has the same
> problem of qemu.  We are discussing to have a quirk for qemu.  For the
> other hardwares than qemu, I think
> 
> diff --git a/sys/dev/pci/if_iavf.c b/sys/dev/pci/if_iavf.c
> index d573d6725f4..e3eadf23904 100644
> --- a/sys/dev/pci/if_iavf.c
> +++ b/sys/dev/pci/if_iavf.c
> @@ -766,6 +766,11 @@ iavf_attach(struct device *parent, struct device *self, void *aux)
>  	pcireg_t memtype;
>  	int tries;
>  
> +	if (pci_intr_msix_count(pa) <= 0) {
> +		printf(": no msix interrupt\n");
> +		return;
> +	}
> +
>  	rw_init(&sc->sc_cfg_lock, "iavfcfg");
>  
>  	sc->sc_pc = pa->pa_pc;
> 
> this kind of workaround is enough.

Why don't you imagine another kind of PCI bus error?

-- 
Yuichiro NAITO (naito.yuichiro@gmail.com)