Re: [Xen-devel] [RFC] Nested Paging Live Migration

[Tim Deegan <Tim.Deegan@xxxxxxxxxxxxx>]

Hi, 

This patch is much nicer.  One or two more nits below, and a
Signed-off-by: line, please. :)

At 16:58 -0500 on 07 Jun (1181235517), Huang2, Wei wrote:
> +int hap_enable_log_dirty(struct domain *d)
> +{
> +    /* turn on PG_log_dirty bit in paging mode */
> +    d->arch.paging.mode |= PG_log_dirty;
> +    p2m_set_flags_global(d, (_PAGE_PRESENT|_PAGE_USER));
> +    flush_tlb_all_pge();
> +
> +    return 0;
> +}
> +
> +int hap_disable_log_dirty(struct domain *d)
> +{
> +    /* log dirty already accquired lock to guard this code */
> +    d->arch.paging.mode &= ~PG_log_dirty;
> +    p2m_set_flags_global(d, __PAGE_HYPERVISOR|_PAGE_USER);
> +    
> +    return 1;
> +}

The log-dirty lock doesn't guard against concurrent updates of
d->arch.paging.mode!  You need the HAP lock here.

>  int paging_domctl(struct domain *d, xen_domctl_shadow_op_t *sc,
>                    XEN_GUEST_HANDLE(void) u_domctl)
>  {
> +    int rc;
> +
> +    if ( unlikely(d == current->domain) )
> +    {
> +        gdprintk(XENLOG_INFO, "Dom %u tried to do a shadow op on itself.\n",

(and subsequently) s/shadow/paging/ here?

> @@ -2565,7 +2568,7 @@ void shadow_teardown(struct domain *d)
>          if (d->arch.paging.shadow.hash_table) 
>              shadow_hash_teardown(d);
>          /* Release the log-dirty bitmap of dirtied pages */
> -        sh_free_log_dirty_bitmap(d);
> +        paging_free_log_dirty_bitmap(d);

Shouldn't this be handled in paging.c?  Otherwise we'd need to acquire
the log-dirty lock with the shadow lock held.

Cheers,

Tim.

-- 
Tim Deegan <Tim.Deegan@xxxxxxxxxxxxx>, XenSource UK Limited
Registered office c/o EC2Y 5EB, UK; company number 05334508

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel