On Wed, Feb 15, 2023 at 6:03 AM Jan Beulich <[email protected]> wrote: > > On 14.02.2023 06:07, Tamas K Lengyel wrote: > > When VM forking is initiated a VM is not supposed to try to perform mem_sharing > > yet as the fork process hasn't completed all required steps. However, the vCPU > > bring-up paths trigger guest memory accesses that can lead to such premature > > sharing ops. However, the gating check to see whether a VM is a fork is set > > already (ie. the domain has a parent). > > I find this confusing, and I had to read the patch to understand what's > meant. Since there are a total of three VMs involved here (the one > driving the fork, the one being forked, and the new clone), "a VM" is > insufficient. The sentence further reads as if that VM is performing > sharing operations on itself, which according to my understanding is > impossible. > > Furthermore "the vCPU bringup paths" is also not specific enough imo. > The forked VM as well as the new clone are both paused if I'm not > mistaken, so neither can be in the process of bringing up any vCPU-s. > I assume you refer to bring_up_vcpus(), but I'm afraid I view this > function as misnamed. vCPU-s are being _created_ there, not brought up. > Furthermore there are no guest memory accesses from underneath > bring_up_vcpus(), so with those accesses you may be referring to > copy_settings() instead? Which in turn - I'm sorry for my ignorance - > raises the question why (implicit) hypervisor side accesses to guest > memory might trigger sharing: So far I was working from the assumption > that it's only control tool requests which do. Otoh you talk of > "sharing ops", which suggests it may indeed be requests coming from > the control tool. Yet that's also what invoked the request to fork, > so shouldn't it coordinate with itself and avoid issuing sharing ops > prematurely?
I went back to double-check and here is the memory access during vcpu_create: (XEN) Xen call trace: (XEN) [<ffff82d0402ebf38>] R mem_sharing.c#mem_sharing_gfn_alloc+0x5c/0x5e (XEN) [<ffff82d0402ecb12>] F mem_sharing.c#add_to_physmap+0x175/0x233 (XEN) [<ffff82d0402ee81d>] F mem_sharing_fork_page+0x4ee/0x51e (XEN) [<ffff82d0402f244f>] F p2m_get_gfn_type_access+0x119/0x1a7 (XEN) [<ffff82d0402e67ef>] F hap.c#hap_update_paging_modes+0xbe/0x2ee (XEN) [<ffff82d0402942a0>] F vmx_create_vmcs+0x981/0xb71 (XEN) [<ffff82d040298884>] F vmx.c#vmx_vcpu_initialise+0x64/0x1a0 (XEN) [<ffff82d0402acc59>] F hvm_vcpu_initialise+0x97/0x19e (XEN) [<ffff82d0403168db>] F arch_vcpu_create+0xf3/0x1db (XEN) [<ffff82d040206c69>] F vcpu_create+0x211/0x35d (XEN) [<ffff82d0402f00b7>] F mem_sharing_memop+0x16a9/0x1869 (XEN) [<ffff82d0403317c5>] F subarch_memory_op+0x298/0x2c4 (XEN) [<ffff82d04032ca26>] F arch_memory_op+0xa9f/0xaa4 (XEN) [<ffff82d040221e66>] F do_memory_op+0x2150/0x2297 (XEN) [<ffff82d04030bcfb>] F pv_do_multicall_call+0x22c/0x4c7 (XEN) [<ffff82d040319727>] F arch_do_multicall_call+0x23/0x2c (XEN) [<ffff82d04022231f>] F do_multicall+0xd3/0x417 (XEN) [<ffff82d04030c0e4>] F pv_hypercall+0xea/0x3a0 (XEN) [<ffff82d040201292>] F lstar_enter+0x122/0x130 Any time any page is requested in a fork that's not present it gets populated from the parent (if the parent has it). What I was worried about is nominate_page being called on the fork but I was mistaken, it was called on the parent before add_to_physmap is called on the fork - which is fine and the expected behavior. We can drop this patch, sorry for the noise. Tamas
