Re: [Xen-devel] [PATCH V2] x86/p2m: fixed p2m_change_type_range() start / end check

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.04.18 at 09:23, <rcojocaru@xxxxxxxxxxxxxxx> wrote:
> On 04/18/2018 04:12 PM, Razvan Cojocaru wrote:
>> p2m_change_type_range() handles end > max_mapped_pfn, but not
>> start > max_mapped_pfn. Check the latter just after grabbing the
>> lock and bail if true.
>> 
>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>> Suggested-by: George Dunlap <george.dunlap@xxxxxxxxxx>
>> 
>> ---
>> Changes since V1:
>>  - Added ASSERT()s.
>>  - Wrapped the new condition in an unlikely().
>> ---
>>  xen/arch/x86/mm/p2m.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>> 
>> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
>> index c53cab4..e09b256 100644
>> --- a/xen/arch/x86/mm/p2m.c
>> +++ b/xen/arch/x86/mm/p2m.c
>> @@ -978,8 +978,19 @@ void p2m_change_type_range(struct domain *d,
>>      p2m_lock(p2m);
>>      p2m->defer_nested_flush = 1;
>>  
>> +    ASSERT(start < end);
>> +
>> +    if ( unlikely(start > p2m->max_mapped_pfn) )
>> +    {
>> +        ASSERT(!p2m_is_hostp2m(p2m));
>> +        p2m_unlock(p2m);
>> +        return;
>> +    }
>> +
>>      if ( unlikely(end > p2m->max_mapped_pfn) )
>>      {
>> +        ASSERT(end == ~0UL || !p2m_is_hostp2m(p2m));
>> +
>>          if ( !gfn )
>>          {
>>              p2m->change_entry_type_global(p2m, ot, nt);
>> 
> 
> I think I've addressed George's comments on the altp2m VGA logdirty
> issue and I'm ready to send the current version of the patch, however it
> depends on this one.
> 
> Is the above patch uncontroversial enough to find its way into staging
> soon (in which case I should probably wait until it's in to send out the
> other patch), or should I perhaps create a series of two patches and
> send that out?

Well, you'll need George's ack and (for the tree being frozen) you'd need
to justify why this is important to fix for 4.11 (to convince Jürgen to give
a release ack).

> Or is it maybe customary to proceed somehow else in a situation like this?

A series is one way. Adding a post-commit-message note to clarify the
dependency (textual and/or functional) is another.

Jan


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