[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [RFC XEN PATCH v6 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi
On Thu, Mar 28, 2024 at 02:34:02PM +0800, Jiqian Chen wrote: > diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c > index 2cec83e0b734..debf6ec6ddc7 100644 > @@ -1500,13 +1510,25 @@ static void pci_add_dm_done(libxl__egc *egc, > rc = ERROR_FAIL; > goto out; > } > - r = xc_domain_irq_permission(ctx->xch, domid, irq, 1); > - if (r < 0) { > - LOGED(ERROR, domainid, > - "xc_domain_irq_permission irq=%d (error=%d)", irq, r); > - fclose(f); > - rc = ERROR_FAIL; > - goto out; > + if (is_gsi) { > + r = xc_domain_gsi_permission(ctx->xch, domid, gsi, 1); > + if (r < 0 && r != -EOPNOTSUPP) { Is it correct to check `r` for "-EOPNOTSUPP" ? Because `man ioctl` and "xenctrl.h:105" says that `r` is 0 on success or -1 on error with `errno` set. > + LOGED(ERROR, domainid, > + "xc_domain_gsi_permission gsi=%d (error=%d)", gsi, r); Is the `r` value useful to print? Because LOGED() already prints strerror(errno). > + fclose(f); > + rc = ERROR_FAIL; > + goto out; > + } > + } > + if (!is_gsi || r == -EOPNOTSUPP) { > + r = xc_domain_irq_permission(ctx->xch, domid, irq, 1); > + if (r < 0) { > + LOGED(ERROR, domainid, > + "xc_domain_irq_permission irq=%d (error=%d)", irq, r); > + fclose(f); > + rc = ERROR_FAIL; > + goto out; > + } > } > } > fclose(f); > @@ -2180,6 +2202,7 @@ static void pci_remove_detached(libxl__egc *egc, > FILE *f; > uint32_t domainid = prs->domid; > bool isstubdom; > + bool is_gsi = true; > > /* Convenience aliases */ > libxl_device_pci *const pci = &prs->pci; > @@ -2243,6 +2266,7 @@ skip_bar: > /* To compitable with old version of kernel, still need to use irq */ > sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, > pci->bus, pci->dev, pci->func); > + is_gsi = false; > } > f = fopen(sysfs_path, "r"); > if (f == NULL) { > @@ -2261,9 +2285,17 @@ skip_bar: > */ > LOGED(ERROR, domid, "xc_physdev_unmap_pirq irq=%d", irq); > } > - rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); > - if (rc < 0) { > - LOGED(ERROR, domid, "xc_domain_irq_permission irq=%d", irq); > + if (is_gsi) { > + rc = xc_domain_gsi_permission(ctx->xch, domid, irq, 0); Could you store the return code of this system call into `r` instead? `rc` is supposed to be use exclusively for libxl error code, so the current code is wrong, but we can partially fixed that in your patch. > + if (rc < 0 && rc != -EOPNOTSUPP) { Shouldn't you check EOPNOTSUPP in `errno` instead? > + LOGED(ERROR, domid, "xc_domain_gsi_permission gsi=%d", irq); > + } > + } > + if (!is_gsi || rc == -EOPNOTSUPP) { > + rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); Same here, use `r` instead of `rc`. > + if (rc < 0) { > + LOGED(ERROR, domid, "xc_domain_irq_permission irq=%d", irq); > + } > } > } > Thanks, -- Anthony PERARD
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |