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

Re: [Xen-devel] [PATCH v2 14/25] arm/altp2m: Make get_page_from_gva ready for altp2m.



Hello Sergej,

On 01/08/16 18:10, Sergej Proskurin wrote:
The function get_page_from_gva uses ARM's hardware support to translate
gva's to machine addresses. This function is used, among others, for
memory regulation purposes, e.g, within the context of memory ballooning.
To ensure correct behavior while altp2m is in use, we use the host's p2m
table for the associated gva to ma translation. This is required at this
point, as altp2m lazily copies pages from the host's p2m and even might
be flushed because of changes to the host's p2m (as it is done within
the context of memory ballooning).

I was expecting to see some change in p2m_mem_access_check_and_get_page. Is there any reason to not fix it?


Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
---
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
---
 xen/arch/arm/p2m.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index bcad51f..784f8da 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1614,7 +1614,7 @@ struct page_info *get_page_from_gva(struct vcpu *v, 
vaddr_t va,
                                     unsigned long flags)
 {
     struct domain *d = v->domain;
-    struct p2m_domain *p2m = &d->arch.p2m;
+    struct p2m_domain *p2m = p2m_get_hostp2m(d);

This is more a clean-up than necessary. I would prefer to see a patch modifying all "&d->arch.p2m" by p2m_get_hostp2m in one go.

     struct page_info *page = NULL;
     paddr_t maddr = 0;
     int rc;
@@ -1628,7 +1628,34 @@ struct page_info *get_page_from_gva(struct vcpu *v, 
vaddr_t va,

     p2m_read_lock(p2m);

-    rc = gvirt_to_maddr(va, &maddr, flags);
+    /*
+     * If altp2m is active, we still read the gva from the hostp2m, as it

s/still/need to/

+     * contains all valid mappings while the currently active altp2m view might
+     * not have the required gva mapping yet.
+     */
+    if ( unlikely(altp2m_active(d)) )
+    {
+        unsigned long irq_flags = 0;
+        uint64_t ovttbr = READ_SYSREG64(VTTBR_EL2);
+
+        if ( ovttbr != p2m->vttbr.vttbr )
+        {
+            local_irq_save(irq_flags);
+            WRITE_SYSREG64(p2m->vttbr.vttbr, VTTBR_EL2);
+            isb();
+        }
+
+        rc = gvirt_to_maddr(va, &maddr, flags);
+
+        if ( ovttbr != p2m->vttbr.vttbr )
+        {
+            WRITE_SYSREG64(ovttbr, VTTBR_EL2);
+            isb();
+            local_irq_restore(irq_flags);
+        }

The pattern is very similar to what p2m_flush_tlb does. Can we get macro/helpers to avoid duplicate code?

+    }
+    else
+        rc = gvirt_to_maddr(va, &maddr, flags);

     if ( rc )
         goto err;


Regards,

--
Julien Grall

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

 


Rackspace

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