Re: [RFC XEN PATCH v3 2/3] x86/pvh: Add (un)map_pirq and setup_gsi for PVH dom0

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Dec 15, 2023 at 07:20:24AM +0000, Chen, Jiqian wrote:
> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c
> index d3507d13a029..f665d17afbf5 100644
> --- a/tools/libs/light/libxl_pci.c
> +++ b/tools/libs/light/libxl_pci.c
> @@ -1486,6 +1486,7 @@ static void pci_add_dm_done(libxl__egc *egc,
>          goto out_no_irq;
>      }
>      if ((fscanf(f, "%u", &irq) == 1) && irq) {
> +        int gsi = irq;
>          r = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq);
>          if (r < 0) {
>              LOGED(ERROR, domainid, "xc_physdev_map_pirq irq=%d (error=%d)",
> @@ -1494,7 +1495,7 @@ static void pci_add_dm_done(libxl__egc *egc,
>              rc = ERROR_FAIL;
>              goto out;
>          }
> -        r = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
> +        r = xc_domain_irq_permission(ctx->xch, domid, gsi, 1);
>          if (r < 0) {
>              LOGED(ERROR, domainid,
>                    "xc_domain_irq_permission irq=%d (error=%d)", irq, r);
> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
> index f5a71ee5f78d..782c4a7a70a4 100644
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -658,7 +658,12 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
> u_domctl)
>              ret = -EINVAL;
>              break;
>          }
> -        irq = pirq_access_permitted(current->domain, pirq);
> +
> +        if ( is_hvm_domain(current->domain) )
> +            irq = pirq;
> +        else
> +            irq = pirq_access_permitted(current->domain, pirq);

You are dropping an irq_access_permitted() check here for the HVM
case, as pirq_access_permitted() translates from pirq to irq and also
checks for permissions.

This would need to be something along the lines of:

irq = 0;
if ( is_hvm_domain(current->domain) &&
     irq_access_permitted(current->domain, pirq) )
    irq = pirq;
else
    irq = pirq_access_permitted(current->domain, pirq);

And then I wonder whether it wouldn't be best to uniformly use a GSI
for both PV and HVM.

Thanks, Roger.