Re: [Xen-devel] [PATCH V12 4/5] p2m: Always use hostp2m when clipping rangesets

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 12/21/18 8:24 PM, George Dunlap wrote:
> 
>> On Dec 20, 2018, at 5:38 PM, Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx> 
>> wrote:
>>
>> The logdirty rangesets of the altp2ms need to be kept in sync with the
>> hostp2m. This means when iterating through the altp2ms, we need to
>> use the host p2m to clip the rangeset, not the indiviual altp2m's
>> value.
>>
>> This change also:
>>
>> - Documents that the end is non-inclusive
>>
>> - Calculates an "inclusive" value for the end once, rather than
>>  open-coding the modification, and (worse) back-modifying updates so
>>  that the calculation ends up correct
>>
>> - Clarifies the logic deciding whether to call
>>  change_entry_type_global() or change_entry_type_range()
>>
>> - Handles the case where start >= hostp2m->max_mapped_pfn
>>
>> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>> Tested-by: Tamas K Lengyel <tamas@xxxxxxxxxxxxx>
>>
>> ---
>> CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>
>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
>> CC: "Roger Pau Monné" <roger.pau@xxxxxxxxxx>
>>
>> ---
>> Changes since V11:
>> - Added end_exclusive, to avoid modifying function arguments.
>> - Moved the if ( start >= host_max_pfn ) warning below the code
>>   doing the clipping.
> 
> I’m afraid you misunderstood what I meant.
> 
> Why would we want to give a warning only if start >= host_max_pfn?  And if 
> the *start* was above host_max_pfn, why would we tell people we were 
> “clipping” the rangeset, when in fact we drop it entirely in that case?
> 
> Rather, I meant this:
> 
> —
> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
> index f552fd5199..2af50af2bd 100644
> --- a/xen/arch/x86/mm/p2m.c
> +++ b/xen/arch/x86/mm/p2m.c
> @@ -1018,11 +1018,11 @@ static void change_type_range(struct p2m_domain *p2m,
>       * warning.
>       */
>      if ( unlikely(end > host_max_pfn) )
> -        end = host_max_pfn;
> -
> -    if ( start >= host_max_pfn )
> +    {
>          printk(XENLOG_G_WARNING "Dom%d logdirty rangeset clipped to 
> max_mapped_pfn\n",
>                 d->domain_id);
> +        end = host_max_pfn;
> +    }
> 
>      /* If the requested range is out of scope, return doing nothing. */
>      if ( start > end )
> —
> 
> Everything else looks good.

Apologies, I'll resend.


Thanks,
Razvan

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