From: "Koenig, Christian" <Christian.Koenig@amd.com>
To: "Thomas Hellström (VMware)" <thomas_os@shipmail.org>,
"Andy Lutomirski" <luto@amacapital.net>,
"Christoph Hellwig" <hch@infradead.org>
Cc: Dave Hansen <dave.hansen@intel.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
"x86@kernel.org" <x86@kernel.org>,
"pv-drivers@vmware.com" <pv-drivers@vmware.com>,
Thomas Hellstrom <thellstrom@vmware.com>,
Dave Hansen <dave.hansen@linux.intel.com>,
Andy Lutomirski <luto@kernel.org>,
Peter Zijlstra <peterz@infradead.org>,
Thomas Gleixner <tglx@linutronix.de>,
Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
"H. Peter Anvin" <hpa@zytor.com>,
Marek Szyprowski <m.szyprowski@samsung.com>,
"Lendacky, Thomas" <Thomas.Lendacky@amd.com>
Subject: Re: [RFC PATCH 1/2] x86: Don't let pgprot_modify() change the page encryption bit
Date: Wed, 11 Sep 2019 09:08:01 +0000 [thread overview]
Message-ID: <d6da6e46-d283-9efc-52cb-9f2a6b0b7188@amd.com> (raw)
In-Reply-To: <92171412-eed7-40e9-2554-adb358e65767@shipmail.org>
Am 10.09.19 um 21:26 schrieb Thomas Hellström (VMware):
> On 9/10/19 6:11 PM, Andy Lutomirski wrote:
>>
>>> On Sep 5, 2019, at 8:24 AM, Christoph Hellwig <hch@infradead.org>
>>> wrote:
>>>
>>>> On Thu, Sep 05, 2019 at 05:21:24PM +0200, Thomas Hellström (VMware)
>>>> wrote:
>>>>> On 9/5/19 4:15 PM, Dave Hansen wrote:
>>>>> Hi Thomas,
>>>>>
>>>>> Thanks for the second batch of patches! These look much improved
>>>>> on all
>>>>> fronts.
>>>> Yes, although the TTM functionality isn't in yet. Hopefully we
>>>> won't have to
>>>> bother you with those though, since this assumes TTM will be using
>>>> the dma
>>>> API.
>>> Please take a look at dma_mmap_prepare and dma_mmap_fault in this
>>> branch:
>>>
>>> http://git.infradead.org/users/hch/misc.git/shortlog/refs/heads/dma-mmap-improvements
>>>
>>> they should allow to fault dma api pages in the page fault handler.
>>> But
>>> this is totally hot off the press and not actually tested for the last
>>> few patches. Note that I've also included your two patches from this
>>> series to handle SEV.
>> I read that patch, and it seems like you’ve built in the assumption
>> that all pages in the mapping use identical protection or, if not,
>> that the same fake vma hack that TTM already has is used to fudge
>> around it. Could it be reworked slightly to avoid this?
>>
>> I wonder if it’s a mistake to put the encryption bits in vm_page_prot
>> at all.
>
> From my POW, the encryption bits behave quite similar in behaviour to
> the caching mode bits, and they're also in vm_page_prot. They're the
> reason TTM needs to modify the page protection in the fault handler in
> the first place.
>
> The problem seen in TTM is that we want to be able to change the
> vm_page_prot from the fault handler, but it's problematic since we
> have the mmap_sem typically only in read mode. Hence the fake vma
> hack. From what I can tell it's reasonably well-behaved, since
> pte_modify() skips the bits TTM updates, so mprotect() and mremap()
> works OK. I think split_huge_pmd may run into trouble, but we don't
> support it (yet) with TTM.
Ah! I actually ran into this while implementing huge page support for
TTM and never figured out why that doesn't work. Dropped CPU huge page
support because of this.
>
> We could probably get away with a WRITE_ONCE() update of the
> vm_page_prot before taking the page table lock since
>
> a) We're locking out all other writers.
> b) We can't race with another fault to the same vma since we hold an
> address space lock ("buffer object reservation")
> c) When we need to update there are no valid page table entries in the
> vma, since it only happens directly after mmap(), or after an
> unmap_mapping_range() with the same address space lock. When another
> reader (for example split_huge_pmd()) sees a valid page table entry,
> it also sees the new page protection and things are fine.
Yeah, that's exactly why I always wondered why we need this hack with
the vma copy on the stack.
>
> But that would really be a special case. To solve this properly we'd
> probably need an additional lock to protect the vm_flags and
> vm_page_prot, taken after mmap_sem and i_mmap_lock.
Well we already have a special lock for this: The reservation object. So
memory barriers etc should be in place and I also think we can just
update the vm_page_prot on the fly.
Christian.
>
> /Thomas
>
>
>
>
next prev parent reply other threads:[~2019-09-11 9:08 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-05 10:35 [RFC PATCH 0/2] Fix SEV user-space mapping of unencrypted coherent memory Thomas Hellström (VMware)
2019-09-05 10:35 ` [RFC PATCH 1/2] x86: Don't let pgprot_modify() change the page encryption bit Thomas Hellström (VMware)
2019-09-05 14:15 ` Dave Hansen
2019-09-05 15:21 ` Thomas Hellström (VMware)
2019-09-05 15:24 ` Christoph Hellwig
2019-09-05 16:40 ` Thomas Hellström (VMware)
2019-09-05 17:05 ` dma_mmap_fault discussion Thomas Hellström (VMware)
2019-09-06 6:32 ` Christoph Hellwig
2019-09-06 7:10 ` Thomas Hellström (VMware)
2019-09-06 7:20 ` Christoph Hellwig
2019-09-10 8:37 ` Thomas Hellström (VMware)
2019-09-10 16:11 ` [RFC PATCH 1/2] x86: Don't let pgprot_modify() change the page encryption bit Andy Lutomirski
2019-09-10 19:26 ` Thomas Hellström (VMware)
2019-09-11 4:18 ` Andy Lutomirski
2019-09-11 7:49 ` Thomas Hellström (VMware)
2019-09-11 18:03 ` Andy Lutomirski
2019-09-12 8:29 ` Thomas Hellström (VMware)
2019-09-11 9:08 ` Koenig, Christian [this message]
2019-09-11 10:10 ` TTM huge page-faults WAS: " Thomas Hellström (VMware)
2019-09-11 14:06 ` Koenig, Christian
2019-09-11 15:08 ` Thomas Hellström (VMware)
2019-09-24 12:03 ` Koenig, Christian
2019-09-05 15:59 ` Dave Hansen
2019-09-05 16:29 ` Thomas Hellström (VMware)
2019-09-05 10:35 ` [RFC PATCH 2/2] dma-mapping: Fix dma_pgprot() for unencrypted coherent pages Thomas Hellström (VMware)
2019-09-05 11:23 ` [RFC PATCH 0/2] Fix SEV user-space mapping of unencrypted coherent memory Christoph Hellwig
2019-09-10 6:11 ` Christoph Hellwig
2019-09-10 6:25 ` Thomas Hellström (VMware)
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=d6da6e46-d283-9efc-52cb-9f2a6b0b7188@amd.com \
--to=christian.koenig@amd.com \
--cc=Thomas.Lendacky@amd.com \
--cc=bp@alien8.de \
--cc=dave.hansen@intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=hch@infradead.org \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@amacapital.net \
--cc=luto@kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=pv-drivers@vmware.com \
--cc=tglx@linutronix.de \
--cc=thellstrom@vmware.com \
--cc=thomas_os@shipmail.org \
--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).