Re: [PATCH] drm/ttm: set TTM allocated pages as reserved

[Sean Christopherson <seanjc@google.com>]

On Wed, Mar 29, 2023, Christian K�nig wrote:
> Am 29.03.23 um 19:22 schrieb Sean Christopherson:
> > +David
> > 
> > On Wed, Mar 29, 2023, Paolo Bonzini wrote:
> > > On 3/29/23 18:43, Christian K�nig wrote:
> > > > > 
> > > > > 3) other uses of kmap() must switch to MMU-notifier protection.
> > > > I would rather suggest to return the page additionally to the pfn from
> > > > hva_to_pfn() when the function was able to grab a reference to it.
> > > > 
> > > > When the page is then not available you can't call kmap() on that either.
> > > > 
> > > > > If the DRM people are okay with SetPageReserved() as a temporary
> > > > > hack, we can change or remove the WARN in kvm_is_zone_device_page(),
> > > > > since that is what you are referring to in the commit message.
> > > > Adding Daniel for additional comments on this, but essentially we have
> > > > changed quite some infrastructure to make sure that everybody uses
> > > > VM_PFNMAP to prevent stuff like this from happening.
> > > > 
> > > > I would really prefer a proper solution in KVM instead.
> > > Hmm... Now that I think about it that would be
> > > 
> > > https://lore.kernel.org/kvm/Yc4H+dGfK83BaGpC@google.com/t/
> > > 
> > > Time to resurrect that work.
> > Ya.  I had previously asked David to first eliminated the usage that isn't
> > protected by mmu_notifiers, but after seeing the resulting complexity, I had a
> > change of heart[2].  Can you weigh in on the latter thread, specifically my
> > proposal of using a module param to let the admin opt-in to "unsafe" kmap usage.
> 
> I don't think that this is something an admin should be able to decide.

Why not?  Assuming the admin has CAP_SYS_ADMIN, they can reboot the host in a
myriad of ways.  The idea with the KVM module param is to allow curated setups
where the userspace VMM is trusted to a large extent, e.g. AWS' Nitro, to opt-in
to the unsafe behavior.  I.e. by enabling the flag, the admin is acknowledging
that bad things can happen if untrusted/compromised userspace gets ahold of
/dev/kvm.

> x86 system are rather grateful, but if you kmap() pages accessed by GPUs on
> ARM the system might just reboot spontaneously.

FWIW, the dangers of an unsafe kmap() are arguably far worse than spontaneous
reboots, e.g. there's potential for use-after-free and possibly even write access
to arbitrary memory.