Re: [Xen-devel] [PATCH v2] xen: make sure stop_machine_run() is always called in a tasklet

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.02.2020 08:19, Juergen Gross wrote:
> With core scheduling active it is mandatory for stop_machine_run() to
> be called in idle context only (so either during boot or in a tasklet),
> as otherwise a scheduling deadlock would occur: stop_machine_run()
> does a cpu rendezvous by activating a tasklet on all other cpus. In
> case stop_machine_run() was not called in an idle vcpu it would block
> scheduling the idle vcpu on its siblings with core scheduling being
> active, resulting in a hang.
> 
> Put a BUG_ON() into stop_machine_run() to test for being called in an
> idle vcpu only and adapt the missing call site (ucode loading) to use a
> tasklet for calling stop_machine_run().
> 
> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> ---
> V2:
> - rephrase commit message (Julien Grall)
> ---
>  xen/arch/x86/microcode.c  | 54 
> +++++++++++++++++++++++++++++------------------
>  xen/common/stop_machine.c |  1 +
>  2 files changed, 35 insertions(+), 20 deletions(-)

There's no mention anywhere of a connection to your RCU series,
nor to rcu_barrier(). Yet the change puts a new restriction also
on its use, and iirc this was mentioned in prior discussion. Did
I miss anything?

> @@ -700,6 +688,32 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
> buf, unsigned long len)
>      return ret;
>  }
>  
> +int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long 
> len)
> +{
> +    int ret;
> +    struct ucode_buf *buffer;
> +
> +    if ( len != (uint32_t)len )
> +        return -E2BIG;
> +
> +    if ( microcode_ops == NULL )
> +        return -EINVAL;
> +
> +    buffer = xmalloc_flex_struct(struct ucode_buf, buffer, len);
> +    if ( !buffer )
> +        return -ENOMEM;
> +
> +    ret = copy_from_guest(buffer->buffer, buf, len);
> +    if ( ret )
> +    {
> +        xfree(buffer);
> +        return -EFAULT;
> +    }
> +    buffer->len = len;
> +
> +    return continue_hypercall_on_cpu(0, microcode_update_helper, buffer);
> +}

Andrew, just to clarify - were you okay with Jürgen's response regarding
this re-introduction of continue_hypercall_on_cpu() here?

Jan

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