Re: [PATCH 1/3] x86/HVM: properly propagate errors from HVMOP_inject_msi

[Andrew Cooper <andrew.cooper3@citrix.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2984 bytes --]

On 03/06/14 14:58, Jan Beulich wrote:
> There are a number of ways this operation can go wrong, all of which
> got ignored so far.
>
> In the context of this I wonder whether map_domain_emuirq_pirq()
> returning 0 in the "already mapped" case is really intended to be that
> way (this is why the subsequent NULL check here can't be an ASSERT()).
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -5096,7 +5096,7 @@ static int hvmop_inject_msi(
>      if ( rc )
>          goto out;
>  
> -    hvm_inject_msi(d, op.addr, op.data);
> +    rc = hvm_inject_msi(d, op.addr, op.data);
>  
>   out:
>      rcu_unlock_domain(d);
> --- a/xen/arch/x86/hvm/irq.c
> +++ b/xen/arch/x86/hvm/irq.c
> @@ -270,7 +270,7 @@ void hvm_set_pci_link_route(struct domai
>              d->domain_id, link, old_isa_irq, isa_irq);
>  }
>  
> -void hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data)
> +int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data)
>  {
>      uint32_t tmp = (uint32_t) addr;
>      uint8_t  dest = (tmp & MSI_ADDR_DEST_ID_MASK) >> MSI_ADDR_DEST_ID_SHIFT;
> @@ -292,20 +292,28 @@ void hvm_inject_msi(struct domain *d, ui
>              /* if it is the first time, allocate the pirq */
>              if ( !info || info->arch.hvm.emuirq == IRQ_UNBOUND )
>              {
> +                int rc;
> +
>                  spin_lock(&d->event_lock);
> -                map_domain_emuirq_pirq(d, pirq, IRQ_MSI_EMU);
> +                rc = map_domain_emuirq_pirq(d, pirq, IRQ_MSI_EMU);
>                  spin_unlock(&d->event_lock);
> +                if ( rc )
> +                    return rc;
>                  info = pirq_info(d, pirq);
>                  if ( !info )
> -                    return;
> -            } else if (info->arch.hvm.emuirq != IRQ_MSI_EMU)
> -                return;
> +                    return -EBUSY;
> +            }
> +            else if ( info->arch.hvm.emuirq != IRQ_MSI_EMU )
> +                return -EINVAL;
>              send_guest_pirq(d, info);
> -            return;
> +            return 0;
>          }
> +        return -ERANGE;
>      }
>  
>      vmsi_deliver(d, vector, dest, dest_mode, delivery_mode, trig_mode);
> +
> +    return 0;
>  }
>  
>  void hvm_set_callback_via(struct domain *d, uint64_t via)
> --- a/xen/include/xen/hvm/irq.h
> +++ b/xen/include/xen/hvm/irq.h
> @@ -123,7 +123,7 @@ void hvm_isa_irq_deassert(
>  
>  void hvm_set_pci_link_route(struct domain *d, u8 link, u8 isa_irq);
>  
> -void hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data);
> +int hvm_inject_msi(struct domain *d, uint64_t addr, uint32_t data);
>  
>  void hvm_maybe_deassert_evtchn_irq(void);
>  void hvm_assert_evtchn_irq(struct vcpu *v);
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 3793 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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