linux-kernel - Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL bit is cleared

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y3YzY0aUc42cQGk/@mail-itl>
Date:   Thu, 17 Nov 2022 14:13:04 +0100
From:   Marek Marczykowski-Górecki 
        <marmarek@...isiblethingslab.com>
To:     David Vrabel <dvrabel@...tab.net>
Cc:     linux-kernel@...r.kernel.org, Juergen Gross <jgross@...e.com>,
        Stefano Stabellini <sstabellini@...nel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
        Jan Beulich <jbeulich@...e.com>,
        "moderated list:XEN HYPERVISOR INTERFACE" 
        <xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH] xen-pciback: Consider MSI-X enabled only when MASKALL
 bit is cleared

On Thu, Nov 17, 2022 at 12:54:51PM +0000, David Vrabel wrote:
> On 17/11/2022 11:41, Marek Marczykowski-Górecki 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.
> > Allow setting PCI_MSIX_FLAGS_ENABLE while INTx is still enabled as long
> > as PCI_MSIX_FLAGS_MASKALL is set too.
> 
> The use of MSI-X interrupts is conditional on only the MSI-X Enable bit.
> Setting MSI-X Enable effectively overrides the Interrupt Disable bit in the
> Command register.

That means the second chunk of the patch may even drop the '(new_value &
PCI_MSIX_FLAGS_MASKALL)' part, right? 

> PCIe 6.0.1 section 7.7.2.2. "MSI-X Enable ... is prohibited from using INTx
> interrupts (if implemented)." And there is similar wording for MSI Enable.

And this would mean the 'field_config->int_type == INTERRUPT_TYPE_MSIX'
part isn't necessary either.

Jan in another thread pointed out that disabling INTx explicitly is
still a useful workaround for a flawed hardware. But if that isn't
mandated by the spec, maybe it doesn't need to be enforced by pciback
either?

> I think you need to shuffle the checks for MSI/MSI-X in
> xen_pcibk_get_interrupt_type() so they are _before_ the check of the
> Interrupt Disable bit in the Command register.

Note the xen_pcibk_get_interrupt_type() returns a bitmask of enabled
types. It seems it should consider INTx only if both MSI and MSI-X are
disabled. I'll make the adjustment. But this alone, won't cover enabling
part, as INTx is the only one active at the time.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Download attachment "signature.asc" of type "application/pgp-signature" (489 bytes)