On 12/20/18 6:31 PM, George Dunlap wrote: > On 12/5/18 9:18 AM, Razvan Cojocaru 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.dun...@citrix.com> >> Signed-off-by: Razvan Cojocaru <rcojoc...@bitdefender.com> >> Tested-by: Tamas K Lengyel <ta...@tklengyel.com> >> >> --- >> CC: George Dunlap <george.dun...@eu.citrix.com> >> CC: Jan Beulich <jbeul...@suse.com> >> CC: Andrew Cooper <andrew.coop...@citrix.com> >> CC: Wei Liu <wei.l...@citrix.com> >> CC: "Roger Pau Monné" <roger....@citrix.com> >> >> --- >> Changes since V10: >> - Fixed a double-space in the patch description. >> - Fixed a coding style issue for >> "if ( !start && end >= p2m->max_mapped_pfn)" (no space before >> closing ')'). >> - Switched the early return comment back to "/* If the requested >> range is out of scope, return doing nothing. */. >> - Added Tamas' Tested-by. >> --- >> xen/arch/x86/mm/p2m.c | 47 ++++++++++++++++++++++++++++++----------------- >> 1 file changed, 30 insertions(+), 17 deletions(-) >> >> diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c >> index d145850..539ea16 100644 >> --- a/xen/arch/x86/mm/p2m.c >> +++ b/xen/arch/x86/mm/p2m.c >> @@ -1002,30 +1002,43 @@ int p2m_change_type_one(struct domain *d, unsigned >> long gfn_l, >> return rc; >> } >> >> -/* Modify the p2m type of a range of gfns from ot to nt. */ >> +/* Modify the p2m type of [start, end) from ot to nt. */ >> static void change_type_range(struct p2m_domain *p2m, >> unsigned long start, unsigned long end, >> p2m_type_t ot, p2m_type_t nt) >> { >> - unsigned long gfn = start; >> struct domain *d = p2m->domain; >> + const unsigned long host_max_pfn = p2m_get_hostp2m(d)->max_mapped_pfn; >> int rc = 0; >> >> - if ( unlikely(end > p2m->max_mapped_pfn) ) >> - { >> - if ( !gfn ) >> - { >> - p2m->change_entry_type_global(p2m, ot, nt); >> - gfn = end; >> - } >> - end = p2m->max_mapped_pfn + 1; >> - } >> - if ( gfn < end ) >> - rc = p2m->change_entry_type_range(p2m, ot, nt, gfn, end - 1); >> + --end; >> + >> + if ( start >= host_max_pfn ) >> + printk(XENLOG_G_WARNING "Dom%d logdirty rangeset clipped to >> max_mapped_pfn\n", >> + d->domain_id); >> + >> + /* Always clip the rangeset down to the host p2m. */ >> + if ( unlikely(end > host_max_pfn) ) >> + end = host_max_pfn; >> + >> + /* If the requested range is out of scope, return doing nothing. */ >> + if ( start > end ) >> + return; >> + >> + /* >> + * If all valid gfns are in the invalidation range, just do a >> + * global type change. Otherwise, invalidate only the range we >> + * need. >> + */ >> + if ( !start && end >= p2m->max_mapped_pfn ) >> + p2m->change_entry_type_global(p2m, ot, nt); >> + else >> + rc = p2m->change_entry_type_range(p2m, ot, nt, start, end); >> + >> if ( rc ) >> { >> - printk(XENLOG_G_ERR "Error %d changing Dom%d GFNs [%lx,%lx] from %d >> to %d\n", >> - rc, d->domain_id, start, end - 1, ot, nt); >> + printk(XENLOG_G_ERR "Error %d changing Dom%d GFNs [%lx,%lx) from %d >> to %d\n", >> + rc, d->domain_id, start, end, ot, nt); > > Nitpick: This is printk is also wrong ATM: It uses [..), which would > indicate that the last value was exclusive. And it was when we weren't > modifying end; but with the `--end` at the top, the range is now inclusive. > > Whatever we end up doing with `end`, this should match. If we name the > argument as end_exclusive, I'd say leave the string and use > end_exclusive here.
Right, sorry about that. I'll use end_exclusive. Thanks, Razvan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel