Re: [Xen-devel] [PATCH v2] xen/x86: vpmu: Unmap per-vCPU PMU page when the domain is destroyed

["Durrant, Paul" <pdurrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Julien Grall <jgrall@xxxxxxxxxx>
> Sent: 27 November 2019 19:42
> To: Durrant, Paul <pdurrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Jan Beulich <jbeulich@xxxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Wei Liu <wl@xxxxxxx>; Roger Pau Monné
> <roger.pau@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin Tian
> <kevin.tian@xxxxxxxxx>
> Subject: Re: [PATCH v2] xen/x86: vpmu: Unmap per-vCPU PMU page when the
> domain is destroyed
> 
> Hi Paul,
> 
> On 27/11/2019 12:00, Paul Durrant wrote:
> > From: Julien Grall <jgrall@xxxxxxxxxx>
> >
> > 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.
> >        Also, whils it appears that vpmu_arch_destroy() is idempotent it
> is
> >        by no means obvious. Hence move manipulation of the
> >        VPMU_CONTEXT_ALLOCATED flag out of implementation specific code
> and
> >        make sure it is cleared at the end of vpmu_arch_destroy().
> 
> If you resend the patch, it might be worth to add a line about the lack
> of XSA. Something like:
> 
> There is no associated XSA because vPMU  is not security supported (see
> XSA-163).

Sure, I'll add another note.

  Paul

> 
> Cheers,
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel