Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221118154931.1928298-1-marmarek@invisiblethingslab.com>
In-Reply-To: <20221118154931.1928298-1-marmarek@invisiblethingslab.com>
From:   Jason Andryuk <jandryuk@gmail.com>
Date:   Fri, 18 Nov 2022 15:46:47 -0500
Message-ID: <CAKf6xpuCxftyQ+PKN_ffJ0onsSxcT8kVSwkM7Z10pfjqf0XFgA@mail.gmail.com>
Subject: Re: [PATCH v3] xen-pciback: Consider INTx disabled when MSI/MSI-X is enabled
To:     =?UTF-8?Q?Marek_Marczykowski=2DG=C3=B3recki?= 
        <marmarek@invisiblethingslab.com>
Cc:     linux-kernel@vger.kernel.org, Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        Jan Beulich <jbeulich@suse.com>,
        "moderated list:XEN HYPERVISOR INTERFACE" 
        <xen-devel@lists.xenproject.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Nov 18, 2022 at 10:50 AM Marek Marczykowski-G=C3=B3recki
<marmarek@invisiblethingslab.com> wrote:
>
> Linux enables MSI-X before disabling INTx, but keeps MSI-X masked until
> the table is filled. Then it disables INTx just before clearing MASKALL
> bit. Currently this approach is rejected by xen-pciback.
> According to the PCIe spec, device cannot use INTx when MSI/MSI-X is
> enabled (in other words: enabling MSI/MSI-X implicitly disables INTx).
>
> Change the logic to consider INTx disabled if MSI/MSI-X is enabled. This
> applies to three places:
>  - checking currently enabled interrupts type,
>  - transition to MSI/MSI-X - where INTx would be implicitly disabled,
>  - clearing INTx disable bit - which can be allowed even if MSI/MSI-X is
>    enabled, as device should consider INTx disabled anyway in that case
>
> Fixes: 5e29500eba2a ("xen-pciback: Allow setting PCI_MSIX_FLAGS_MASKALL t=
oo")
> Signed-off-by: Marek Marczykowski-G=C3=B3recki <marmarek@invisiblethingsl=
ab.com>
> ---
> Changes in v3:
>  - allow clearing INTx regardless of MSI/MSI-X state, to be consistent
>    with enabling MSI/MSI-X
> Changes in v2:
>  - restructure the patch to consider not only MASKALL bit, but enabling
>    MSI/MSI-X generally, without explicitly disabling INTx first
> ---

I was trying to test your xen-pciback v3 patch, and I am having
assignment fail consistently now.  It is actually failing to
quarantine to domIO in the first place, which matches the failure from
the other day (when I more carefully read through the logs).  It now
consistently fails to quarantine on every boot unlike the other day
where it happened once.

I added some printks and it 's getting -EBUSY from pdev_msix_assign()
which means pci_reset_msix_state() is failing:
    if ( pci_conf_read16(pdev->sbdf, msix_control_reg(pos)) &
         PCI_MSIX_FLAGS_MASKALL )
        return -EBUSY;

# lspci -vv -s 14.3
...
    Capabilities: [80] MSI-X: Enable- Count=3D16 Masked+
        Vector table: BAR=3D0 offset=3D00002000
        PBA: BAR=3D0 offset=3D00003000

So it looks like MASKALL is set and prevents assignment.

setpci -s 00:14.3 82.W=3Df
cleared that out for me and I could assign the device.

My dom0 boots, it runs flask-label-pci for a set of PCI devices
(including iwlwifi), then xl pci-assignable-add for all PCI devices
which will be passed through, then a little later it boots the
associated domains.  Dom0 does not have a driver for iwlwifi.

I'll have to investigate more to see how MASKALL is getting set.  This
had not been an issue before your recent patches.

Regards,
Jason