Re: [PATCH v2 7/8] x86/dpci: move hvm_dpci_isairq_eoi() to generic HVM code

[Jan Beulich <jbeulich@xxxxxxxx>]

On 04.01.2023 09:45, Xenia Ragiadakou wrote:
> The function hvm_dpci_isairq_eoi() has no dependencies on VT-d driver code
> and can be moved from xen/drivers/passthrough/vtd/x86/hvm.c to
> xen/drivers/passthrough/x86/hvm.c, along with the corresponding copyrights.
> 
> Remove the now empty xen/drivers/passthrough/vtd/x86/hvm.c.
> 
> Since the funcion is used only in this file, declare it static.
> 
> No functional change intended.
> 
> Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>

Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
with a couple of cosmetic suggestions since you're touching this code
anyway:

> @@ -924,6 +925,48 @@ static void hvm_gsi_eoi(struct domain *d, unsigned int 
> gsi)
>      hvm_pirq_eoi(pirq);
>  }
>  
> +static int cf_check _hvm_dpci_isairq_eoi(
> +    struct domain *d, struct hvm_pirq_dpci *pirq_dpci, void *arg)
> +{
> +    struct hvm_irq *hvm_irq = hvm_domain_irq(d);

I think this could become pointer-to-const.

> +    unsigned int isairq = (long)arg;
> +    const struct dev_intx_gsi_link *digl;
> +
> +    list_for_each_entry ( digl, &pirq_dpci->digl_list, list )
> +    {
> +        unsigned int link = hvm_pci_intx_link(digl->device, digl->intx);
> +
> +        if ( hvm_irq->pci_link.route[link] == isairq )
> +        {
> +            hvm_pci_intx_deassert(d, digl->device, digl->intx);
> +            if ( --pirq_dpci->pending == 0 )
> +                pirq_guest_eoi(dpci_pirq(pirq_dpci));
> +        }
> +    }
> +
> +    return 0;
> +}
> +
> +static void hvm_dpci_isairq_eoi(struct domain *d, unsigned int isairq)
> +{
> +    struct hvm_irq_dpci *dpci = NULL;

And this too.

> +    ASSERT(isairq < NR_ISAIRQS);
> +    if ( !is_iommu_enabled(d) )

A blank line between the above two would be nice.

> +        return;
> +
> +    write_lock(&d->event_lock);
> +
> +    dpci = domain_get_irq_dpci(d);
> +
> +    if ( dpci && test_bit(isairq, dpci->isairq_map) )
> +    {
> +        /* Multiple mirq may be mapped to one isa irq */
> +        pt_pirq_iterate(d, _hvm_dpci_isairq_eoi, (void *)(long)isairq);
> +    }
> +    write_unlock(&d->event_lock);

For symmetry with code above this could to with a blank line ahead of it.

Jan