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

Re: [PATCH V2 21/23] xen/arm: Add mapcache invalidation handling




On 11.11.20 21:27, Julien Grall wrote:
Hi Oleksandr,

Hi Julien.



On 11/11/2020 00:03, Oleksandr wrote:

On 16.10.20 11:41, Julien Grall wrote:
On 16/10/2020 07:29, Jan Beulich wrote:
On 15.10.2020 18:44, Oleksandr Tyshchenko wrote:
@@ -1067,7 +1068,14 @@ static int __p2m_set_entry(struct p2m_domain *p2m,
       */
      if ( p2m_is_valid(orig_pte) &&
           !mfn_eq(lpae_get_mfn(*entry), lpae_get_mfn(orig_pte)) )
+    {
+#ifdef CONFIG_IOREQ_SERVER
+        if ( domain_has_ioreq_server(p2m->domain) &&
+             (p2m->domain == current->domain) && p2m_is_ram(orig_pte.p2m.type) )
+            p2m->domain->qemu_mapcache_invalidate = true;
+#endif
          p2m_free_entry(p2m, orig_pte, level);
+    }

For all I have to say here, please bear in mind that I don't know
the internals of Arm memory management.

The first odd thing here the merely MFN-based condition. It may
well be that's sufficient, if there's no way to get a "not present"
entry with an MFN matching any valid MFN. (This isn't just with
your addition, but even before.
Invalid entries are always zeroed. So in theory the problem could arise if MFN 0 used in the guest. It should not be possible on staging, but I agree this should be fixed.


Given how p2m_free_entry() works (or is supposed to work in the
long run), is the new code you add guaranteed to only alter leaf
entries?

This path may also be called with tables. I think we want to move the check in p2m_free_entry() so we can find the correct leaf type.

Well, but inside p2m_free_entry() we don't have a new entry in order to check whether new MFN is actually different. An extra arg only comes to mind...
Aside the recursive call, there are two users for p2m_free_entry():
  - When we fail to shatter a superpage in OOM
  - When the entry is replaced by an entry with a different base

I wouldn't be too concerned to send spurious mapcache invalidation in an error path. So I don't think you need to know the new entry.

Thank you for the clarification, sounds reasonable.



What you need to know if the type of the leaf.

Yes, to check whether it is a RAM page.


--
Regards,

Oleksandr Tyshchenko




 


Rackspace

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