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

[George Dunlap <George.Dunlap@xxxxxxxxxx>]

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

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