On 28.11.2019 10:38, Paul Durrant wrote: > From: Julien Grall <jgr...@amazon.com> > > A guest will setup a shared page with the hypervisor for each vCPU via > XENPMU_init. The page will then get mapped in the hypervisor and only > released when XENPMU_finish is called. > > This means that if the guest fails to invoke XENPMU_finish, e.g if it is > destroyed rather than cleanly shut down, the page will stay mapped in the > hypervisor. One of the consequences is the domain can never be fully > destroyed as a page reference is still held. > > As Xen should never rely on the guest to correctly clean-up any > allocation in the hypervisor, we should also unmap such pages during the > domain destruction if there are any left. > > We can re-use the same logic as in pvpmu_finish(). To avoid > duplication, move the logic in a new function that can also be called > from vpmu_destroy(). > > NOTE: - The call to vpmu_destroy() must also be moved from > arch_vcpu_destroy() into domain_relinquish_resources() such that > the reference on the mapped page does not prevent domain_destroy() > (which calls arch_vcpu_destroy()) from being called. > - Whilst it appears that vpmu_arch_destroy() is idempotent it is > by no means obvious. Hence make sure the VPMU_CONTEXT_ALLOCATED > flag is cleared at the end of vpmu_arch_destroy(). > - This is not an XSA because vPMU is not security supported (see > XSA-163). > > Signed-off-by: Julien Grall <jgr...@amazon.com> > Signed-off-by: Paul Durrant <pdurr...@amazon.com> > --- > Cc: Jan Beulich <jbeul...@suse.com> > Cc: Andrew Cooper <andrew.coop...@citrix.com> > Cc: Wei Liu <w...@xen.org> > Cc: "Roger Pau Monné" <roger....@citrix.com> > Cc: Jun Nakajima <jun.nakaj...@intel.com> > Cc: Kevin Tian <kevin.t...@intel.com> > > v2: > - Re-word commit comment slightly > - Re-enforce idempotency of vmpu_arch_destroy() > - Move invocation of vpmu_destroy() earlier in > domain_relinquish_resources()
What about v3? > --- a/xen/arch/x86/cpu/vpmu.c > +++ b/xen/arch/x86/cpu/vpmu.c > @@ -576,11 +576,36 @@ static void vpmu_arch_destroy(struct vcpu *v) > > vpmu->arch_vpmu_ops->arch_vpmu_destroy(v); > } > + > + vpmu_reset(vpmu, VPMU_CONTEXT_ALLOCATED); > } Boris, to be on the safe side - are you in agreement with this change, now that the setting of the flag is being left untouched? Jan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel