Re: [Xen-devel] [PATCH 3/5] x86/pt: introduce PT_IRQ_TYPE_GSI to bind GSIs to a PVH Dom0

[Roger Pau Monne <roger.pau@xxxxxxxxxx>]

On Mon, Mar 27, 2017 at 11:44:27AM +0100, Roger Pau Monne wrote:
[...]
> @@ -464,32 +483,56 @@ int pt_irq_create_bind(
>          break;
>      }
>  
> +    case PT_IRQ_TYPE_GSI:
>      case PT_IRQ_TYPE_PCI:
>      case PT_IRQ_TYPE_MSI_TRANSLATE:
>      {
> -        unsigned int bus = pt_irq_bind->u.pci.bus;
> -        unsigned int device = pt_irq_bind->u.pci.device;
> -        unsigned int intx = pt_irq_bind->u.pci.intx;
> -        unsigned int guest_gsi = hvm_pci_intx_gsi(device, intx);
> -        unsigned int link = hvm_pci_intx_link(device, intx);
> -        struct dev_intx_gsi_link *digl = xmalloc(struct dev_intx_gsi_link);
> +        unsigned int bus, device, intx, guest_gsi, link;
> +        struct dev_intx_gsi_link *digl = NULL;
>          struct hvm_girq_dpci_mapping *girq =
>              xmalloc(struct hvm_girq_dpci_mapping);
>  
> -        if ( !digl || !girq )
> +        if ( !girq )
>          {
>              spin_unlock(&d->event_lock);
> -            xfree(girq);
> -            xfree(digl);
>              return -ENOMEM;
>          }
>  
> -        hvm_irq_dpci->link_cnt[link]++;
> +        if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_GSI )
> +        {
> +            digl = xmalloc(struct dev_intx_gsi_link);
> +            if ( !digl )
> +            {
> +                spin_unlock(&d->event_lock);
> +                xfree(girq);
> +                return -ENOMEM;
> +            }
> +
> +            digl->bus = bus = pt_irq_bind->u.pci.bus;
> +            digl->device = device = pt_irq_bind->u.pci.device;
> +            digl->intx = intx = pt_irq_bind->u.pci.intx;
> +            list_add_tail(&digl->list, &pirq_dpci->digl_list);
> +
> +            guest_gsi = hvm_pci_intx_gsi(device, intx);
> +            link = hvm_pci_intx_link(device, intx);
>  
> -        digl->bus = bus;
> -        digl->device = device;
> -        digl->intx = intx;
> -        list_add_tail(&digl->list, &pirq_dpci->digl_list);
> +            hvm_irq_dpci->link_cnt[link]++;
> +        }
> +        else
> +        {
> +            guest_gsi = pt_irq_bind->u.gsi.gsi;
> +
> +            /* PT_IRQ_TYPE_GSI should only be used for identity bindings */
> +            ASSERT(guest_gsi == pirq);
> +            ASSERT(guest_gsi < hvm_domain_irq(d)->nr_gsis);
> +
> +            /*
> +             * The actual PCI device(s) that use this GSI are unknown, Xen
> +             * relies on machine_gsi to be identity bound into the guest.
> +             */
> +            bus = device = intx = 0;

This should be:

link = bus = device = intx = 0;

because...

> +            pirq_dpci->guest_gsi = guest_gsi;
> +        }
>  
>          girq->bus = bus;
>          girq->device = device;
> @@ -535,8 +581,11 @@ int pt_irq_create_bind(
>                   */
>                  pirq_dpci->dom = NULL;
>                  list_del(&girq->list);
> -                list_del(&digl->list);
> -                hvm_irq_dpci->link_cnt[link]--;
> +                if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_GSI)
> +                {
> +                    list_del(&digl->list);
> +                    hvm_irq_dpci->link_cnt[link]--;

... GCC complains that link might be uninitialized here.
> @@ -610,14 +659,26 @@ int pt_irq_destroy_bind(
>  
>      if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_MSI )
>      {
> -        unsigned int bus = pt_irq_bind->u.pci.bus;
> -        unsigned int device = pt_irq_bind->u.pci.device;
> -        unsigned int intx = pt_irq_bind->u.pci.intx;
> -        unsigned int guest_gsi = hvm_pci_intx_gsi(device, intx);
> -        unsigned int link = hvm_pci_intx_link(device, intx);
> +        unsigned int bus, device, intx, guest_gsi, link;
>          struct hvm_girq_dpci_mapping *girq;
>          struct dev_intx_gsi_link *digl, *tmp;
>  
> +        if ( pt_irq_bind->irq_type == PT_IRQ_TYPE_GSI )
> +        {
> +            bus = device = intx = 0;

Same here...

> +            guest_gsi = pt_irq_bind->u.gsi.gsi;
> +        }
> +        else
> +        {
> +             bus = pt_irq_bind->u.pci.bus;
> +             device = pt_irq_bind->u.pci.device;
> +             intx = pt_irq_bind->u.pci.intx;
> +             guest_gsi = hvm_pci_intx_gsi(device, intx);
> +             link = hvm_pci_intx_link(device, intx);
> +        }
> +
> +
> +
>          list_for_each_entry ( girq, &hvm_irq_dpci->girq[guest_gsi], list )
>          {
>              if ( girq->bus         == bus &&
> @@ -638,7 +699,8 @@ int pt_irq_destroy_bind(
>              return -EINVAL;
>          }
>  
> -        hvm_irq_dpci->link_cnt[link]--;
> +        if ( pt_irq_bind->irq_type != PT_IRQ_TYPE_GSI )
> +            hvm_irq_dpci->link_cnt[link]--;

Because GCC complains in the same way here.

Roger.

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