Re: [PATCH v5 12/16] PCI: Add pci_iomap_host_shared(), pci_iomap_host_shared_range()

From: Andi Kleen
Date: Tue Oct 12 2021 - 17:28:33 EST



But that was due to performance problems in hot paths. Nothing of this
applies here.
It applies because a new API that individual driver authors is being
proposed and that's an ongoing maintenance burden that might be
mitigated by hiding that implementation detail from leaf drivers.

Right now we're only talking about 2 places to change, and none of those are actually in individual drivers, but in the virtio generic code and in the MSI code.

While there might be drivers in the future that do it directly it will be always the exception, normal drivers don't have to deal with this.



For me it both seems very straight forward and simple (but then I'm biased)
You seem to be having a difficult time iterating this proposal toward
consensus. I don't think the base principles are being contested as
much as the semantics, scope, and need for the proposed API that is in
the purview of all leaf driver developers.
Right now there is no leaf driver changed at all.

I'd rather see more concerted efforts focused/limited core changes
rather than leaf driver changes until there is a clearer definition of
hardened.
A hardened driver is a driver that

- Had similar security (not API) oriented review of its IO operations
(mainly MMIO access, but also PCI config space) as a non privileged user
interface (like a ioctl). That review should be focused on memory safety.

- Had some fuzzing on these IO interfaces using to be released tools.
What is the intersection of ioremap() users that are outside of the
proposed probe authorization regime AND want confidential computing
support?


Right now it's zero I believe.

That is there is other low level code that sets memory shared, but it's not using ioremap, but some other mechanisms.


are needed
for booting. For example in TDX we can't print something to the console
with this mechanism, so you would never get any output before the
initrd. Just seems like a nightmare for debugging anything. There really
needs to be an authorization mechanism that works reasonably early.

I can see a point of having user space overrides though, but we need to
have a sane kernel default that works early.
Right, as I suggested [1], just enough early authorization to
bootstrap/debug initramfs and then that can authorize the remainder.
But how do you debug the kernel then? Making early undebuggable seems
just bad policy to me.
I am not proposing making the early undebuggable.


That's the implication of moving the policy into initrd.


If only initrd can authorize then it won't be possible to authorize before initrd, thus the early console won't work.

-Andi