Re: [PATCH V1 13/16] xen/ioreq: Make x86's invalidate qemu mapcache handling common

[Oleksandr <olekstysh@xxxxxxxxx>]

On 07.10.20 13:38, Julien Grall wrote:
> Hi Oleksandr,
Hi Julien.



> On 02/10/2020 10:55, Oleksandr wrote:
> > If I got it correctly there won't be a suitable common place where to 
> > set qemu_mapcache_invalidate flag anymore
> > as XENMEM_decrease_reservation is not a single place we need to make 
> > a decision whether to set it
> > By principle of analogy, on Arm we probably want to do so in 
> > guest_physmap_remove_page (or maybe better in p2m_remove_mapping).
> > Julien, what do you think?
> At the moment, the Arm code doesn't explicitely remove the existing 
> mapping before inserting the new mapping. Instead, this is done 
> implicitely by p2m_set_entry().
Got it.


> So I think we want to invalidate the QEMU mapcache in p2m_set_entry() 
> if the old entry is a RAM page *and* the new MFN is different.
Thank you. I hope, the following is close to what was suggested (didn't 
test yet):

diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index ae8594f..512eea9 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -1073,7 +1073,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);
+    }

 out:
     unmap_domain_page(table);


But, if I got the review comments correctly [1], the 
qemu_mapcache_invalidate variable should be per-vcpu instead of per-domain?
[1] https://patchwork.kernel.org/patch/11803383/



--
Regards,

Oleksandr Tyshchenko