linux-kernel - RE: Virtualizing MSI-X on IMS via VFIO

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: <87im22uncn.ffs@nanos.tec.linutronix.de>
Date:   Fri, 25 Jun 2021 14:42:48 +0200
From:   Thomas Gleixner <tglx@...utronix.de>
To:     "Tian\, Kevin" <kevin.tian@...el.com>,
        Alex Williamson <alex.williamson@...hat.com>
Cc:     Jason Gunthorpe <jgg@...dia.com>,
        "Dey\, Megha" <megha.dey@...el.com>,
        "Raj\, Ashok" <ashok.raj@...el.com>,
        "Pan\, Jacob jun" <jacob.jun.pan@...el.com>,
        "Jiang\, Dave" <dave.jiang@...el.com>,
        "Liu\, Yi L" <yi.l.liu@...el.com>,
        "Lu\, Baolu" <baolu.lu@...el.com>,
        "Williams\, Dan J" <dan.j.williams@...el.com>,
        "Luck\, Tony" <tony.luck@...el.com>,
        "Kumar\, Sanjay K" <sanjay.k.kumar@...el.com>,
        LKML <linux-kernel@...r.kernel.org>, KVM <kvm@...r.kernel.org>,
        Kirti Wankhede <kwankhede@...dia.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Marc Zyngier <maz@...nel.org>,
        Bjorn Helgaas <helgaas@...nel.org>
Subject: RE: Virtualizing MSI-X on IMS via VFIO

On Fri, Jun 25 2021 at 10:43, Thomas Gleixner wrote:
> On Fri, Jun 25 2021 at 05:21, Kevin Tian wrote:
>> p.s. one question to Thomas. As Alex cited above, software must 
>> not modify the Address, Data, or Steering Tag fields of an MSI-X
>> entry while it is unmasked. However this rule might be violated
>> today in below flow:
>>
>> request_irq()
>>     __setup_irq()
>>         irq_startup()
>>             __irq_startup()
>>                 irq_enable()
>>                     unmask_irq() <<<<<<<<<<<<<
>>         irq_setup_affinity()
>>             irq_do_set_affinity()
>>                 msi_set_affinity() // when IR is disabled
>>                     irq_msi_update_msg()
>>                         pci_msi_domain_write_msg() <<<<<<<<<<<<<<
>>
>> Isn't above have msi-x entry updated after it's unmasked? 
>
> Dammit, I could swear that we had masking at the core or PCI level at
> some point. Let me dig into this.

Indeed, that code path does not check irq_can_move_pcntxt(). It doesn't
blow up in our face by chance because of this:

     __setup_irq()
        irq_activate()
        unmask()
        irq_setup_affinity()

irq_activate() assigns a vector based on the affinity mask so
irq_setup_affinity() ends up writing the same data again pointlessly.

For some stupid reason the ordering of startup/setup_affinity is the way
it is for historical reasons. I tried to reorder it at some point but
that caused failure on !x86 so I went back to the status quo.

All other affinity settings happen with the interrupt masked because we
do that from actual interrupt context via irq_move_masked_irq() which
does the right thing.

Let me fix that proper for the startup case.

Thanks,

        tglx