Re: [Xen-devel] [PATCH V8 4/7] x86/p2m: refactor p2m_reset_altp2m()

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.11.18 at 11:57, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 11/21/18 12:49 PM, Jan Beulich wrote:
>>>>> On 21.11.18 at 11:19, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>>> Refactor p2m_reset_altp2m() so that it can be used to remove
>>> redundant codepaths, fixing the locking while we're at it.
>> 
>> Still no word about ...
>> 
>>> +static void p2m_reset_altp2m(struct domain *d, unsigned int idx,
>>> +                             enum altp2m_reset_type reset_type)
>>> +{
>>> +    struct p2m_domain *p2m;
>>> +
>>> +    ASSERT(idx < MAX_ALTP2M);
>>> +    p2m = d->arch.altp2m_p2m[idx];
>>> +
>>> +    p2m_lock(p2m);
>>> +
>>> +    p2m_flush_table_locked(p2m);
>>> +
>>> +    if ( reset_type == ALTP2M_DEACTIVATE )
>>> +        p2m_free_logdirty(p2m);
>>> +
>>> +    /* Uninit and reinit ept to force TLB shootdown */
>>> +    ept_p2m_uninit(p2m);
>>> +    ept_p2m_init(p2m);
>>> +
>>> +    p2m->min_remapped_gfn = gfn_x(INVALID_GFN);
>>> +    p2m->max_remapped_gfn = 0;
>> 
>> ... these now done even when previously they weren't? In fact,
>> having looked again, it seems as if their omission from e.g.
>> p2m_flush_altp2m() was a mistake before, which you now fix.
> 
> Right, that's an omission I'll rectify in the next version.
> 
> According to a previous review from George, I'm not now fixing a problem
> since the assignment was not required, however it is harmless (and
> perhaps preferrable). AFAICT it does not fix a correctness issue.

Not a correctness one, agreed, but an efficiency one?

Jan



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