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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.04.19 at 17:48, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> 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.

I guess I'm missing something here: Why "rightfully"? If the guest
runs on this altp2m, it'll die a quick death then, won't it? Yet if
this is intended to be demand-populate, then why would there be
automatic propagation for existing altp2m-s (as you explain
further up, unless I'm getting this wrong)?

> 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.

Then I still don't see why you would want to force propagation
in these cases: If it's generally demand-populate, it can't be right
to _fully_ populate that slot. You ought to be setting the access
type or the "suppress #VE" bit alone, without propagating the
MFN and alike. The later, when the entry actually gets propagated,
the entered values should be used as overrides to what comes
from the host p2m.

Jan



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