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

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 13 July 2015 10:28
> To: Paul Durrant
> Cc: Razvan Cojocaru; Andrew Cooper; xen-devel@xxxxxxxxxxxxx; Keir
> (Xen.org)
> Subject: RE: [Xen-devel] Deadlock in stdvga_mem_accept() with emulation
> 
> >>> 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?
> 

I think we are ok because the stdvga model will never actually accept the I/O 
since MMIO <-> MMIO rep mov is explicitly disallowed.

  Paul

> Jan


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