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
|