linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: Matthew Garrett <mjg59@google.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: linux-efi <linux-efi@vger.kernel.org>,
	the arch/x86 maintainers <x86@kernel.org>,
	linux-pci <linux-pci@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] [EFI,PCI] Allow disabling PCI busmastering on bridges during boot
Date: Wed, 4 Dec 2019 08:11:36 +0100	[thread overview]
Message-ID: <41cecdd8-f411-00c4-be82-be5d4d13fcb1@redhat.com> (raw)
In-Reply-To: <CACdnJusMeC+G3wq_oDGTYi1CBMWDiuq4NdANTBmhNBTDu5zCug@mail.gmail.com>

On 12/03/19 20:40, Matthew Garrett wrote:
> On Tue, Dec 3, 2019 at 3:54 AM Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> 
>> There is no reason this shouldn't apply to ARM, but disabling bus
>> mastering like that before the drivers themselves get a chance to do
>> so is likely to cause trouble. Network devices or storage controllers
>> that are still running and have live descriptor rings in DMA memory
>> shouldn't get the rug pulled from under their feet like that by
>> blindly disabling the BM attribute on all root ports before their
>> drivers have had the opportunity to do this cleanly.
> 
> Yes, whether this causes problems is going to be influenced by the
> behaviour of the hardware on the system. That's why I'm not defaulting
> it to being enabled :)
> 
>> One trick we implemented in EDK2 for memory encryption was to do the
>> following (Laszlo, mind correcting me here if I am remembering this
>> wrong?)
>> - create an event X
>> - register an AtExitBootServices event that signals event X in its handler
>> - in the handler of event X, iterate over all PPBs to clear the bus
>> master attribute
>> - for bonus points, do the same for the PCIe devices themselves,
>> because root ports are known to exist that entirely ignore the BM
>> attribute
>>
>> This way, event X should get handled after all the drivers' EBS event
>> handlers have been called.
> 
> Can we guarantee that this happens before IOMMU state teardown?

In OVMF, the handler of "event X" is in the IOMMU driver itself, so it's
the IOMMU driver that takes care of blacklisting everything *after*
other drivers had a chance to clean up.

But in this case, we'd have to insert the PPB clearing *before* the
(platform's) IOMMU driver's EBS handler (because the latter is going to
deny, not permit, everything); and we can't modify the IOMMU driver.

I guess we could install an EBS handler with TPL_NOTIFY (PciIo usage
appears permitted at TPL_NOTIFY, from "Table 27. TPL Restrictions"). But:
- if the IOMMU driver's EBS handler is also to be enqueued at
TPL_NOTIFY, then the order will be unspecified
- if a PCI driver sets up an EBS handler at TPL_CALLBACK, then in our
handler we could shut down a PPB in front of a device bound by that
driver too early.

Handling dependencies between event notification functions is a
never-ending struggle in UEFI, AFAICT.

> I don't think there's a benefit to clearing the bit on endpoint devices,
> if they're malicious they're just going to turn it back on anyway.
> 

Thanks
Laszlo


  reply	other threads:[~2019-12-04  7:12 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03  0:40 [PATCH] [EFI,PCI] Allow disabling PCI busmastering on bridges during boot Matthew Garrett
2019-12-03  0:42 ` Matthew Garrett
2019-12-03 11:54   ` Ard Biesheuvel
2019-12-03 13:38     ` Laszlo Ersek
2019-12-03 19:36       ` Matthew Garrett
2019-12-03 19:40     ` Matthew Garrett
2019-12-04  7:11       ` Laszlo Ersek [this message]
2019-12-04 19:29         ` Matthew Garrett
2019-12-03 15:30 ` Andy Lutomirski
2019-12-03 16:33   ` Ard Biesheuvel
2019-12-03 19:41   ` Matthew Garrett
2019-12-04 19:50     ` Andy Lutomirski
2019-12-04 19:56       ` Matthew Garrett
2019-12-12 15:46         ` Ard Biesheuvel
2019-12-13 21:24           ` Matthew Garrett
2019-12-03 18:23 ` kbuild test robot
2019-12-05 13:04 ` kbuild test robot

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=41cecdd8-f411-00c4-be82-be5d4d13fcb1@redhat.com \
    --to=lersek@redhat.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pci@vger.kernel.org \
    --cc=mjg59@google.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).