Re: [RFC PATCH 20/21] pci: Allow encrypted MMIO mapping via sysfs

Bjorn Helgaas
2024-09-03T21:46:06Z

On Mon, Sep 02, 2024 at 06:22:00PM +1000, Alexey Kardashevskiy wrote:
> On 24/8/24 08:37, Bjorn Helgaas wrote:
> > On Fri, Aug 23, 2024 at 11:21:34PM +1000, Alexey Kardashevskiy wrote:
> > > Add another resource#d_enc to allow mapping MMIO as
> > > an encrypted/private region.
> > >
> > > Unlike resourceN_wc, the node is added always as ability to
> > > map MMIO as private depends on negotiation with the TSM which
> > > happens quite late.

> > > @@ -46,6 +46,15 @@ int pci_mmap_resource_range(struct pci_dev *pdev, int bar,
> > > vma->vm_ops = &pci_phys_vm_ops;
> > > + /*
> > > + * Calling remap_pfn_range() directly as io_remap_pfn_range()
> > > + * enforces shared mapping.
> >
> > s/Calling/Call/
> >
> > Needs some additional context about why io_remap_pfn_range() can't be
> > used here.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f8f6ae5d077a9bdaf5cbf2ac960a5d1a04b47482
> added this.
>
> "IO devices do not understand encryption, so this memory must always be
> decrypted" it says.

Thanks for the pointer. Given that hint, the pgprot_decrypted()
inside io_remap_pfn_range() is ... at least *there*, if not obvious.
io_remap_pfn_range() probably could benefit from a simple comment to
highlight that.

> But devices do understand encryption so forcing decryption is not wanted.
> What additional context is missing here, that "shared" means
> "non-encrypted"? Thanks,

If "shared" means "non-encrypted", that would be useful. That wasn't
obvious to me.

IIUC, in the "enc" case, you *want* the mapping to remain encrypted?
In that case, it would be helpful to say something like
"io_remap_pfn_range() always produces decrypted mappings, so use
remap_pfn_range() directly to avoid the decryption".

Renaming "enc" to "encrypted" would also be a nice hint.

> > > + */
> > > + if (enc)
> > > + return remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> > > + vma->vm_end - vma->vm_start,
> > > + vma->vm_page_prot);
> > > +
> > > return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> > > vma->vm_end - vma->vm_start,
> > > vma->vm_page_prot);
>
> --
> Alexey
>