On 23.01.2018 22:15, David Hildenbrand wrote:
> On 13.12.2017 13:53, Janosch Frank wrote:
> Please correct me if I'm wrong (this stuff is complicated):
> 
> 
> Right now we have to split huge pages under the following condition:
> 
> a) We are write protecting (prot != PROT_WRITE) ...
> b) ... and we are doing it during shadow page table creation
> (GMAP_NOTIFY_SHADOW)
> 
> -> gmap_protect_pmd()

Yes

> 
> 
> This is to work around issues (RW vs. RO) when
> a) G2 puts G2->G3 DAT tables on same huge page as a G2 prefix
> b) Guest G2->G3 DAT tables on same huge page as G2->G3 pages referenced
> in such a table
> 
> "we cannot have RO and RW at the same time if things depend on each other".

Yes

> 
> 
> Now, the interesting thing is, for shadow page tables
> (GMAP_NOTIFY_SHADOW), we only protect RO: via gmap_protect_rmap() and
> gmap_protect_range().
> 
> So basically for all shadow page table housekeeping, we never protect on
> pmds but only on ptes. -> We always split huge pages
> 
> This implies and important insight: _SEGMENT_ENTRY_GMAP_VSIE is never
> used. (and I will prepare a cleanup patch to make PROT_READ implicit on
> e.g. gmap_protect_rmap(), because this clarifies this a lot)

Yes, I guess _SEGMENT_ENTRY_GMAP_VSIE is a leftover from before the
splitting.

> 
> 
> We only ever protect right now on huge pages without splitting it up for
> the prefix, as I already mentioned. And as discussed, I doubt this is
> really worth it. And we can get rid of a lot of code this way.

See next answer

> 
> 
> Long story short:
> 
> If we simply split up huge pages when protecting the prefix, we don't
> need gmap_protect_pmd() anymore, and therefore also (at least) not

We need it for the dirty tracking, no?

> 
> - s390/mm: Abstract gmap notify bit setting

Yes, that's not needed then.

> - s390/mm: add gmap PMD invalidation notification

We need that one (in parts) because of the protection transfer to user
space. We will be notified on mm pmds. Even if we split a pmd, we will
be notified on a pmd, not on a pte. So we need at least a skeleton that
calls pmdp_notify_split.

I'm currently preparing a patch that rips out pmd protection with
software bits. I'll attach it when finished, so we can have a look what
can go.

> 
> 
> So I think doing proper sub-hugepage protection right from the beginning
> makes perfect sense.
> 
> @Martin, Christian, am I missing something? What's your take on this?
>