[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 05/10] VMX: add help functions to support PML





On 04/09/2015 08:00 PM, Tim Deegan wrote:
Hi,

At 10:35 +0800 on 27 Mar (1427452549), Kai Huang wrote:
+void vmx_vcpu_flush_pml_buffer(struct vcpu *v)
+{
+    uint64_t *pml_buf;
+    unsigned long pml_idx;
+
+    ASSERT(vmx_vcpu_pml_enabled(v));
+
+    vmx_vmcs_enter(v);
+
+    __vmread(GUEST_PML_INDEX, &pml_idx);
+
+    /* Do nothing if PML buffer is empty */
+    if ( pml_idx == (PML_ENTITY_NUM - 1) )
+        goto out;
+
+    pml_buf = map_domain_page(page_to_mfn(v->arch.hvm_vmx.pml_pg));
+
+    /*
+     * PML index can be either 2^16-1 (buffer is full), or 0~511 (buffer is not
+     * full), and in latter case PML index always points to next available
+     * entity.
+     */
+    if (pml_idx >= PML_ENTITY_NUM)
+        pml_idx = 0;
+    else
+        pml_idx++;
+
+    for ( ; pml_idx < PML_ENTITY_NUM; pml_idx++ )
+    {
+        struct p2m_domain *p2m = p2m_get_hostp2m(v->domain);
+        unsigned long gfn;
+        mfn_t mfn;
+        p2m_type_t t;
+        p2m_access_t a;
+
+        gfn = pml_buf[pml_idx] >> PAGE_SHIFT;
+        mfn = p2m->get_entry(p2m, gfn, &t, &a, 0, NULL);
Please don't call p2m->get_entry() directly -- that interface should
only be used inside the p2m code.  As it happens, I don't think this
lookup is correct anyway: the logging only sees races (which are not
interesting) or buggy hardware (which is not worth the extra lookup to
detect).

So you only need this to get 'mfn' to pass to paging_mark_dirty().
True.
That's also buggy, because there's no locking here to make sure
gfn->mfn->gfn ends up in the right place. :(
Yes you are right. Thanks for pointing out.

Just curious, looks if I use get_gfn_type_access and put_gfn here, the gfn->mfn->gfn can be guaranteed, and it's safe to use them here, right?


I think the right thing to do is:

  - split paging_park_dirty() into paging_mark_gfn_dirty() (the bulk of
    the current function) and a paging_mark_dirty() wrapper that does
    get_gpfn_from_mfn(mfn_x(gmfn)) and calls paging_mark_gfn_dirty().

  - call paging_mark_gfn_dirty() from vmx_vcpu_flush_pml_buffer().

That will avoid _two_ p2m lookups in this function. :)
Yours is indeed much better! Will do in this way.

Thanks,
-Kai

Cheers,

Tim.


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.