linux-kernel - Re: iommu/vt-d: Cure VF irqdomain hickup

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Fri, 13 Nov 2020 09:19:10 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Thomas Gleixner <tglx@...utronix.de>
Cc:     Jason Gunthorpe <jgg@...dia.com>,
        Ziyad Atiyyeh <ziyadat@...dia.com>,
        Itay Aveksis <itayav@...dia.com>,
        Moshe Shemesh <moshe@...dia.com>,
        LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Joerg Roedel <joro@...tes.org>,
        iommu@...ts.linux-foundation.org, linux-pci@...r.kernel.org,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        David Woodhouse <dwmw2@...radead.org>
Subject: Re: iommu/vt-d: Cure VF irqdomain hickup

On 2020-11-12 21:34, Thomas Gleixner wrote:
> On Thu, Nov 12 2020 at 20:15, Thomas Gleixner wrote:
>> The recent changes to store the MSI irqdomain pointer in struct device
>> missed that Intel DMAR does not register virtual function devices.  
>> Due to
>> that a VF device gets the plain PCI-MSI domain assigned and then 
>> issues
>> compat MSI messages which get caught by the interrupt remapping unit.
>> 
>> Cure that by inheriting the irq domain from the physical function
>> device.
>> 
>> That's a temporary workaround. The correct fix is to inherit the irq 
>> domain
>> from the bus, but that's a larger effort which needs quite some other
>> changes to the way how x86 manages PCI and MSI domains.
> 
> Bah, that's not really going to work with the way how irq remapping
> works on x86 because at least Intel/DMAR can have more than one DMAR
> unit on a bus.
> 
> So the alternative solution would be to assign the domain per device,
> but the current ordering creates a hen and egg problem. Looking the
> domain up in pci_set_msi_domain() does not work because at that point
> the device is not registered in the IOMMU. That happens from
> device_add().
> 
> Marc, is there any problem to reorder the calls in pci_device_add():
> 
>       device_add();
>       pci_set_msi_domain();

I *think* it works as long as we keep the "match_driver = false" hack.
Otherwise, we risk binding to a driver early, and game over.

> That would allow to add a irq_find_matching_fwspec() based lookup to
> pci_msi_get_device_domain().

Just so that I understand the issue: is the core of the problem that
there is no 1:1 mapping between a PCI bus and a DMAR unit, and no
firmware topology information to indicate which one to pick?

> Though I'm not yet convinced that the outcome would be less horrible
> than the hack in the DMAR driver when I'm taking all the other horrors
> of x86 (including XEN) into account :)

I tried to follow the notifier into the DMAR driver, ended up in the
IRQ remapping code, and lost the will to live. I have a question though:

In the bus notifier callback, you end-up in dmar_pci_bus_add_dev(),
which calls intel_irq_remap_add_device(), which tries to set the
MSI domain. Why isn't that enough? Are we still missing any information
at that stage?

Thanks,

         M.
-- 
Jazz is not dead. It just smells funny...