Re: [PATCH] x86/altp2m: p2m_altp2m_get_or_propagate() should honor ap2m->default_access

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Thu, Feb 8, 2024 at 9:00 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 08.02.2024 14:45, Tamas K Lengyel wrote:
> > On Thu, Feb 8, 2024 at 2:46 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>
> >> On 08.02.2024 05:32, George Dunlap wrote:
> >>> Er, ok, just one more comment: this could allow an altp2m to have more
> >>> permissions than the host; for example, the host p2m entry could be
> >>> p2m_access_r, but if the altp2m's default_access were p2m_access_rw,
> >>> it would override that.  Is that the behavior we want?  Or do we want
> >>> to do some sort of intersection of permissions?
> >>>
> >>> If the former, I'd propose the comment be adjusted thus:
> >
> > No intersection of permissions please, that needlessly complicates
> > things and makes it hard to reason about the state of a view where
> > default permissions are used. No need to force a specific type of
> > usecase here where the hostp2m's permissions are special just cause we
> > say so. No, the permissions in the hostp2m should not have more weight
> > then the specifically requested default permission.
> >
> >>>
> >>>  * If the entry is invalid, and the host entry was valid, propagate
> >>>  * the host's entry to the altp2m, retaining page order but using the
> >>>  * altp2m's default_access, and indicate that the caller should re-try
> >>>  * the faulting instruction.
> >>
> >> I find it highly questionable that such blind overriding should be taking
> >> place.
> >
> > It's not blind overriding, it's the requested default permission set
> > for a view where no entry was present before. It is the expected
> > behavior. It would be way harder to design applications with this
> > feature if it was special cased and it would take different
> > permissions based on what permission is set in another view.
>
> But the default can be only one specific value: How do you make sure that
> R/O, R/X, and R/W mappings all retain their respective restrictions in the
> alternative view? To not over-restrict permissions, the default would then
> need to be RWX, yet then all mappings will have full permission. What am I
> missing?

Why do you assume you need to retain the permissions that were set in
the hostp2m across all altp2ms? It is entirely reasonable to set
permissions for a couple entries manually in an altp2m and set the
default to _n, which may be totally different then what the hostp2m
has. When you get the event for entries you didn't manually specify
you can decide which view to switch to, which may be the hostp2m, but
may be some other view.

If you wanted to have a view with a default permission that inherits
permissions that are always at least as restrictive as the hostp2m,
you could do that, but the way to do that would be to introduce a
special mem_access permission to signify that that's the expected
behavior (similar to permissions like n2rwx). I don't think anyone is
asking for that.

Tamas