Re: [Xen-devel] [PATCH v6 for-xen-4.5 1/3] dpci: Move from domain centric model to hvm_dirq_dpci model.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 23.09.14 at 04:10, <konrad.wilk@xxxxxxxxxx> wrote:
> @@ -130,6 +146,7 @@ int pt_irq_create_bind(
>          return -ENOMEM;
>      }
>      pirq_dpci = pirq_dpci(info);
> +    pt_pirq_reset(d, pirq_dpci);
>  
>      switch ( pt_irq_bind->irq_type )
>      {
> @@ -232,7 +249,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 |
> @@ -258,7 +274,6 @@ int pt_irq_create_bind(
>              {
>                  if ( pt_irq_need_timer(pirq_dpci->flags) )
>                      kill_timer(&pirq_dpci->timer);
> -                pirq_dpci->dom = NULL;
>                  list_del(&girq->list);
>                  list_del(&digl->list);
>                  hvm_irq_dpci->link_cnt[link]--;
> @@ -391,7 +406,6 @@ int pt_irq_destroy_bind(
>          msixtbl_pt_unregister(d, pirq);
>          if ( pt_irq_need_timer(pirq_dpci->flags) )
>              kill_timer(&pirq_dpci->timer);
> -        pirq_dpci->dom   = NULL;
>          pirq_dpci->flags = 0;
>          pirq_cleanup_check(pirq, d);
>      }

Is all of the above really necessary? I.e. I can neither see why setting
->dom earlier is needed, nor why clearing it on the error paths should
be dropped.

> @@ -459,7 +480,7 @@ int hvm_do_IRQ_dpci(struct domain *d, struct pirq *pirq)
>          return 0;
>  
>      pirq_dpci->masked = 1;
> -    tasklet_schedule(&dpci->dirq_tasklet);
> +    tasklet_schedule(&pirq_dpci->tasklet);

Please also drop the effectively no longer used local variable "dpci"
here (the NULL check of it needs to stay though, but you can simply
check the return value of domain_get_irq_dpci() without using a local
variable now).

> @@ -564,7 +585,7 @@ static int _hvm_dirq_assist(struct domain *d, struct 
> hvm_pirq_dpci *pirq_dpci,
>  
>  static void hvm_dirq_assist(unsigned long _d)
>  {
> -    struct domain *d = (struct domain *)_d;
> +    struct domain *d = ((struct hvm_pirq_dpci *)_d)->dom;
>  
>      ASSERT(d->arch.hvm_domain.irq.dpci);
>  

This seems too little of a change - there's no point in calling
pt_pirq_iterate() here anymore, as you already have the struct
hvm_pirq_dpci instance that needs acting on (any others will get
dealt with in their own tasklets). I.e. the current hvm_dirq_assist()
can go away, and what right now is _hvm_dirq_assist() should
become hvm_dirq_assist().

Oh - peeking ahead I see this is patch 2 of your series. Not sure
this should be separate patches...

Jan


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