[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v1] Fix p2m_set_suppress_ve



On 4/3/19 6:30 PM, Jan Beulich wrote:
On 03.04.19 at 17:17, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
Changes to the hostp2m (also known as altp2m view 0) propagate to all
existing altp2ms, but they do so in a lazy manner, and also that won't
happen for altp2ms created after a while. So altp2ms will not
necessarily know about a page that the hostp2m knows about, which should
not stop us from setting mem access restrictions or the value of the SVE
bit.

But even if the propagation is lazy, a "get" should then return the
propagated value, shouldn't it? Or else callers (like is the case here)
can be mislead. I do recognize that there may be callers who
intentionally _don't_ want the propagated value, but I'd expect this
to be the exception, not the rule. I wonder therefore whether there
shouldn't be a wrapper around ->get_entry() to take care of this for
callers which want the propagated value. Calling the bare hook would
remain as is.

But I believe that some hostp2m entries will never be propagated. Sorry that I've not been clear about this. This is what I meant by "won't happen for altp2ms created after a while".

Take, for example, the case where there's only the hostp2m for the first 30 minutes of a guest's life, and in this time somebody calls ept_set_entry(). This, in turn, calls p2m_altp2m_propagate_change() if ( entry_written && p2m_is_hostp2m(p2m) ).

But at this point there are no altp2ms, so there's nowhere to propagate these changes to.

Now, at minute 31 we create a new altp2m. The changes will never be propagated here, so altp2m->get_entry() will always (rightfully) return an invalid MFN.

We only want to make an exception for setting the SVE bit or mem access restrictions on an entry in the altp2m, but having get_entry() (or a wrapper) return the hostp2m values and MFN might not necessarily be what current callers expect.

Whether the described scenario _should_ work the way it does is debatable, but it appears to do so by design.

Hopefully I'm not missing anything.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.