Re: [Xen-devel] [PATCH v8 for-xen-4.5 1/2] dpci: Move from an hvm_irq_dpci (and struct domain) to an hvm_dirq_dpci model.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 21.10.14 at 19:19, <konrad.wilk@xxxxxxxxxx> wrote:
> @@ -130,6 +127,18 @@ int pt_irq_create_bind(
>          return -ENOMEM;
>      }
>      pirq_dpci = pirq_dpci(info);
> +    /*
> +     * The 'pt_irq_create_bind' can be called right after 
> 'pt_irq_destroy_bind'
> +     * was called. The 'pirq_cleanup_check' which would free the structure
> +     * is only called if the event channel for the PIRQ is active. However
> +     * OS-es that use event channels usually bind the PIRQ to an event 
> channel
> +     * and also unbind it before 'pt_irq_destroy_bind' is called which means
> +     * we end up re-using the 'dpci' structure. This can be easily reproduced
> +     * with unloading and loading the driver for the device.
> +     *
> +     * As such on every 'pt_irq_create_bind' call we MUST reset the values.
> +     */
> +    pirq_dpci->dom = d;

I continue to disagree with the placement of this (only needed right
before calling pirq_guest_bind(), as iirc you actually indicated you
agree with), and I can only re-iterate that with it being here two error
paths (hit before/without pirq_guest_bind() getting called) would need
fixing up too (which wouldn't be needed if that assignment got
deferred as much as possible).

> @@ -156,6 +165,7 @@ int pt_irq_create_bind(
>              {
>                  pirq_dpci->gmsi.gflags = 0;
>                  pirq_dpci->gmsi.gvec = 0;
> +                pirq_dpci->dom = NULL;
>                  pirq_dpci->flags = 0;
>                  pirq_cleanup_check(info, d);
>                  spin_unlock(&d->event_lock);

Just like this error path needing adjustment, the other one following
failure of pirq_guest_bind() (after

> @@ -232,7 +242,6 @@ int pt_irq_create_bind(
>          {
>              unsigned int share;
>  
> -            pirq_dpci->dom = d;
>              if ( pt_irq_bind->irq_type == PT_IRQ_TYPE_MSI_TRANSLATE )
>              {
>                  pirq_dpci->flags = HVM_IRQ_DPCI_MAPPED |

) would seem to need adjustment too.

Jan


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