On 09/05/2018 08:15 PM, Andrew Cooper wrote:
> Since its introduction in c/s 8cbb5278e "x86/AMD: Add support for AMD's OSVW
> feature in guests", the OSVW data has been corrected to be per-domain rather
> than per-vcpu, and is initialised during XEN_DOMCTL_createdomain.
>
> Furthermore, because XENPF_microcode_update uses hypercall continuations to
> move between CPUs, it drops the vcpu_alloc_lock mid update, meaning that it
> didn't provided the interlock guarantee that the OSVW patch was looking for in
> the first place.
>
> This interlock serves no purpose, so take the opportunity to drop it and
> remove a global spinlock from the hypervisor.
>
> Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
Reviewed-by: George Dunlap <george.dun...@citrix.com>
> ---
> CC: George Dunlap <george.dun...@eu.citrix.com>
> CC: Jan Beulich <jbeul...@suse.com>
> CC: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>
> CC: Stefano Stabellini <sstabell...@kernel.org>
> CC: Tim Deegan <t...@xen.org>
> CC: Wei Liu <wei.l...@citrix.com>
> CC: Julien Grall <julien.gr...@arm.com>
> ---
> xen/arch/x86/platform_hypercall.c | 15 ---------------
> xen/common/domctl.c | 18 ------------------
> xen/include/xen/domain.h | 1 -
> 3 files changed, 34 deletions(-)
>
> diff --git a/xen/arch/x86/platform_hypercall.c
> b/xen/arch/x86/platform_hypercall.c
> index ea18c32..b19f6ec 100644
> --- a/xen/arch/x86/platform_hypercall.c
> +++ b/xen/arch/x86/platform_hypercall.c
> @@ -280,24 +280,9 @@ ret_t
> do_platform_op(XEN_GUEST_HANDLE_PARAM(xen_platform_op_t) u_xenpf_op)
>
> guest_from_compat_handle(data, op->u.microcode.data);
>
> - /*
> - * alloc_vcpu() will access data which is modified during
> - * microcode update
> - */
> - while ( !spin_trylock(&vcpu_alloc_lock) )
> - {
> - if ( hypercall_preempt_check() )
> - {
> - ret = hypercall_create_continuation(
> - __HYPERVISOR_platform_op, "h", u_xenpf_op);
> - goto out;
> - }
> - }
> -
> ret = microcode_update(
> guest_handle_to_param(data, const_void),
> op->u.microcode.length);
> - spin_unlock(&vcpu_alloc_lock);
> }
> break;
>
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index ee0983d..ed047b7 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -34,7 +34,6 @@
> #include <xsm/xsm.h>
>
> static DEFINE_SPINLOCK(domctl_lock);
> -DEFINE_SPINLOCK(vcpu_alloc_lock);
>
> static int bitmap_to_xenctl_bitmap(struct xenctl_bitmap *xenctl_bitmap,
> const unsigned long *bitmap,
> @@ -560,20 +559,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
> u_domctl)
> /* Needed, for example, to ensure writable p.t. state is synced. */
> domain_pause(d);
>
> - /*
> - * Certain operations (e.g. CPU microcode updates) modify data which
> is
> - * used during VCPU allocation/initialization
> - */
> - while ( !spin_trylock(&vcpu_alloc_lock) )
> - {
> - if ( hypercall_preempt_check() )
> - {
> - ret = hypercall_create_continuation(
> - __HYPERVISOR_domctl, "h", u_domctl);
> - goto maxvcpu_out_novcpulock;
> - }
> - }
> -
> ret = -ENOMEM;
> online = cpupool_domain_cpumask(d);
>
> @@ -593,9 +578,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t)
> u_domctl)
> ret = 0;
>
> maxvcpu_out:
> - spin_unlock(&vcpu_alloc_lock);
> -
> - maxvcpu_out_novcpulock:
> domain_unpause(d);
> break;
> }
> diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
> index ce31999..5593495 100644
> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -87,7 +87,6 @@ void arch_dump_domain_info(struct domain *d);
>
> int arch_vcpu_reset(struct vcpu *);
>
> -extern spinlock_t vcpu_alloc_lock;
> bool_t domctl_lock_acquire(void);
> void domctl_lock_release(void);
>
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
