Hi,
Looks like you've sorted out shooting down old users of a p2m table.
hap_write_p2m_entry still isn't right, though:
> @@ -834,38 +864,81 @@ static void
> hap_write_p2m_entry(struct vcpu *v, unsigned long gfn, l1_pgentry_t *p,
> mfn_t table_mfn, l1_pgentry_t new, unsigned int level)
> {
> - uint32_t old_flags;
> + struct domain *d = v->domain;
> + uint32_t old_flags = l1e_get_flags(*p);
You have moved this read outside the hap_lock. Please put it back.
> + p2m_type_t op2mt = p2m_flags_to_type(old_flags);
>
> - hap_lock(v->domain);
> + /* We know always use the host p2m here, regardless if the vcpu
> + * is in host or guest mode. The vcpu can be in guest mode by
> + * a hypercall which passes a domain and chooses mostly the first
> + * vcpu.
> + * XXX This is the reason why this function can not be used re-used
> + * for updating the nestedp2m. Otherwise, hypercalls would randomly
> + * operate on host p2m and nested p2m.
> + */
> + if ( nestedhvm_enabled(d)
> + && p2m_is_valid(op2mt) )
> + {
> + if ( l1e_get_intpte(new) != l1e_get_intpte(*p) ) {
> + p2m_type_t np2mt = p2m_flags_to_type(l1e_get_flags(new));
>
> - old_flags = l1e_get_flags(*p);
> + /* Skip flush on vram tracking or XP mode in Win7 hang
> + * very early in the virtual BIOS (long before the bootloader
> + * runs), otherwise. VRAM tracking happens so often that
> + * flushing and fixing the nestedp2m doesn't let XP mode
> + * proceed to boot.
> + */
> + if ( !((op2mt == p2m_ram_rw && np2mt == p2m_ram_logdirty)
> + || (op2mt == p2m_ram_logdirty && np2mt == p2m_ram_rw)) )
That's not safe. If the MFN has changed, you _have_ to flush, even if
you're replacing a logdirty entry with a r/w one. And if you're
replacing a r/w entry with a logdirty one, you _have_ to flush or
logdirty doesn't work correctly. If that case is too slow then you
should batch the flushes somehow, not just skip them.
Cheers,
Tim.
> + {
> + /* This GFN -> MFN is going to get removed. */
> + /* XXX There is a more efficient way to do that
> + * but it works for now.
> + * Note, p2m_flush_nestedp2m calls hap_lock() internally.
> + */
> + p2m_flush_nestedp2m(d);
> + }
> + }
> + }
> +
> + hap_lock(d);
> +
> safe_write_pte(p, new);
> if ( (old_flags & _PAGE_PRESENT)
> && (level == 1 || (level == 2 && (old_flags & _PAGE_PSE))) )
> - flush_tlb_mask(&v->domain->domain_dirty_cpumask);
> + flush_tlb_mask(&d->domain_dirty_cpumask);
>
> #if CONFIG_PAGING_LEVELS == 3
> /* install P2M in monitor table for PAE Xen */
> if ( level == 3 )
> /* We have written to the p2m l3: need to sync the per-vcpu
> * copies of it in the monitor tables */
> - p2m_install_entry_in_monitors(v->domain, (l3_pgentry_t *)p);
> + p2m_install_entry_in_monitors(d, (l3_pgentry_t *)p);
> #endif
>
> - hap_unlock(v->domain);
> + hap_unlock(d);
> }
--
Tim Deegan <Tim.Deegan@xxxxxxxxxx>
Principal Software Engineer, Xen Platform Team
Citrix Systems UK Ltd. (Company #02937203, SL9 0BG)
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|
Home