Re: [Xen-devel] [PATCH v2 4/4] xen: enable EPT dirty bit for guest live migration

[Tim Deegan <tim@xxxxxxx>]

At 09:57 +0800 on 20 Jun (1340186267), Xudong Hao wrote:
> @@ -69,6 +70,11 @@ static void ept_p2m_type_to_flags(ept_entry_t *entry, 
> p2m_type_t type, p2m_acces
>                                                      entry->mfn);
>              break;
>          case p2m_ram_logdirty:
> +            entry->w = hap_has_dirty_bit;
> +            entry->r = entry->x = 1;
> +            /* Not necessarily need to clear A bit, but it is safe anyway */
> +            entry->a = entry->d = 0;
> +            break;

I think it's better not to clear the A bit - it will just cause the
hardware to issue extra writes to set it.

> @@ -760,15 +770,33 @@ static void ept_change_entry_type_page(mfn_t 
> ept_page_mfn, int ept_page_level,
>  
>          if ( (ept_page_level > 0) && !is_epte_superpage(epte + i) )
>              ept_change_entry_type_page(_mfn(epte[i].mfn),
> -                                       ept_page_level - 1, ot, nt);
> +                                       ept_page_level - 1, ot, nt, d, mask);
>          else
>          {
>              e = atomic_read_ept_entry(&epte[i]);
>              if ( e.sa_p2mt != ot )
>                  continue;
>  
> +            if ( e.d && (e.sa_p2mt == p2m_ram_logdirty) )
> +            {
> +                int j, nr_pages;
> +                struct p2m_domain *p2m = p2m_get_hostp2m(d);
> +                for ( j = 0, nr_pages = 1; j < ept_page_level;
> +                      j++, nr_pages *= 512 ) {}
> +                for ( j = 0; j < nr_pages; j++ )
> +                    paging_mark_dirty(d, e.mfn + j);
> +
> +                /* split super page to 4k page, so that dirty bitmap can 
> +                 * map the dirty page
> +                 */
> +                if ( !ept_split_super_page(p2m, &e, ept_page_level, 0) )
> +                    continue;

AIUI this code splits log-dirty superpages _after_ they are first seen
to be dirty.  So the first time they're dirtied, the entire superpage is
marked, but OTOH if they're never dirtied you don't need to split.  Is
that right?  If so, I think it needs a comment explaining it.  It took
me a few minutes to understand this code and convince myself it was
safe. :)

> +                atomic_write_ept_entry(&epte[i], e);
> +            }
>              e.sa_p2mt = nt;
>              ept_p2m_type_to_flags(&e, nt, e.access);
> +            if (!mask)
> +                e.a = e.d = 0;

Again, probably best to leave the A-bit set.

Cheers,

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel