[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v5 14/18] x86/mem_sharing: use default_access in add_to_physmap
On Thu, Jan 23, 2020 at 9:44 AM George Dunlap <george.dunlap@xxxxxxxxxx> wrote: > > On 1/22/20 5:08 PM, Tamas K Lengyel wrote: > > On Wed, Jan 22, 2020 at 8:35 AM Jan Beulich <jbeulich@xxxxxxxx> wrote: > >> > >> On 21.01.2020 18:49, Tamas K Lengyel wrote: > >>> When plugging a hole in the target physmap don't use the access permission > >>> returned by __get_gfn_type_access as it can be non-sensical, > >> > >> "can be" is too vague for my taste - it suggests there may also be cases > >> where a sensible value is returned, and hence it should be used. Could > >> you clarify this please? (The code change itself of course is simple and > >> mechanical enough to look okay.) > > > > Well, I can only speak of what I observed. The case seems to be that > > most of the time the function actually returns p2m_access_rwx (which > > is sensible), but occasionally something else. I didn't investigate > > where that value actually comes from, but when populating a physmap > > like this only the default_access is sane. > > It would be good to get to the bottom of this. Is it possible that your > dom0 agent (or whatever it's called) is calling add_to_physmap() on gfns > that have already been populated? Is that something you want to catch? > OK, I went back and deciphered why sometimes I saw different permissions here. In the context I ran into this issue there is no dom0 agent calling add_to_physmap. We wind up in this path with a VM fork where the MMU faulted. The fault handler is trying to determine whether the page needs to be forked from its parent: populated with a shared entry if it's R/X access, or deduplicated if it's a W. This forking only actually happens if the page type that is returned by ept_get_entry is of a hole type. When it's a hole type, ept_get_entry always returns p2m_access_n as the access permission. Copying that access permission to the newly populated entry is bad - that's what this patch fixes. But this path also gets hit when the MMU faults for other reasons. In those cases will get permissions other then p2m_access_n since the type is not a hole. But when it's not a hole, this function bails as that's a clear signal that the page doesn't need forking. So I was seeing p2m_access_rwx permission for page accesses that triggered the MMU fault for reasons other then mem_access. For example, when a previously shared entry needs unsharing. So there is no mystery after all, I was just printing my debug lines with the mem_access permissions irrespective of the page type before the path bails due to the type check. Tamas _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/xen-devel
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |