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