Re: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 13.07.15 at 11:05, <Paul.Durrant@xxxxxxxxxx> wrote:
> --- a/xen/arch/x86/hvm/stdvga.c
> +++ b/xen/arch/x86/hvm/stdvga.c
> @@ -490,11 +490,18 @@ static bool_t stdvga_mem_accept(const struct 
> hvm_io_handle
>  {
>      struct hvm_hw_stdvga *s = &current->domain->arch.hvm_domain.stdvga;
> 
> +    /*
> +     * The range check must be done without taking any locks, to avoid
> +     * deadlock when hvm_mmio_internal() is called from
> +     * hvm_copy_to/from_guest_phys() in hvm_process_io_intercept().
> +     */
> +    if ( (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
> +         (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
> +        return 0;
> +
>      spin_lock(&s->lock);
> 
> -    if ( !s->stdvga ||
> -         (hvm_mmio_first_byte(p) < VGA_MEM_BASE) ||
> -         (hvm_mmio_last_byte(p) >= (VGA_MEM_BASE + VGA_MEM_SIZE)) )
> +    if ( !s->stdvga )
>          goto reject;
> 
>      if ( p->dir == IOREQ_WRITE && p->count > 1 )

But won't the problem continue to exist if the address falls within the
VGA range? I.e. isn't the problem that the two uses of
hvm_mmio_internal() are quite different - while
hvm_hap_nested_page_fault() immediately afterwards calls a
handle_mmio() variant (which would even seem to call for the lock not
getting dropped between them), __hvm_copy() uses it as just a check.

I.e. perhaps better to convert the lock to a recursive one?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel