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

Re: [Xen-devel] [PATCH 14/18] arm/altp2m: Add HVMOP_altp2m_set_mem_access.





On 06/07/16 17:12, Tamas K Lengyel wrote:
On Wed, Jul 6, 2016 at 8:32 AM, Julien Grall <julien.grall@xxxxxxx> wrote:


On 05/07/16 22:55, Sergej Proskurin wrote:

Hello Julien,

On 07/05/2016 02:49 PM, Julien Grall wrote:

Hello Sergej,

On 04/07/16 12:45, Sergej Proskurin wrote:

+static inline
+int p2m_set_altp2m_mem_access(struct domain *d, struct p2m_domain
*hp2m,
+                              struct p2m_domain *ap2m, p2m_access_t a,
+                              gfn_t gfn)
+{


[...]

+    /* Set mem access attributes - currently supporting only one (4K)
page. */
+    mask = level_masks[3];
+    return apply_p2m_changes(d, ap2m, INSERT,
+                             gpa & mask,
+                             (gpa + level_sizes[level]) & mask,
+                             maddr & mask, mattr, 0, p2mt, a);


The operation INSERT will remove the previous mapping and decrease page
reference for foreign mapping (see p2m_put_l3_page). So if you set the
memory access for this kind of page, the reference count will be wrong
afterward.


I see your point. As far as I understand, the purpose of the function
p2m_put_l3_page to simply decrement the ref count of the particular pte
and free the page if its' ref count reaches zero. Since p2m_put_l3_page
is called only twice in p2m.c, we could insert a check
(p2m_is_hostp2m/altp2m) making sure that the ref count is decremented
only if the p2m in question is the hostp2m. This will make sure that the
ref count is maintained correctly.


Why don't you use the operation MEMACCESS?

I believe it would require duplicating some parts of the INSERT
routine as MEMACCESS assumes the pte is already present in the p2m,
which may not be the case for ap2m.

Hmmm right. The p2m code is already complex so extending MEMACCESS will be worst. I don't much like the idea of a check which I think is very fragile, although it would be my preference for now.

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®.