Message ID | 20240112061317.418658-6-Jiqian.Chen@amd.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Support device passthrough when dom0 is PVH on Xen | expand |
On Fri, Jan 12, 2024 at 02:13:17PM +0800, Jiqian Chen wrote: > diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c > index f2d9d14b4d9f..448ba2c59ae1 100644 > --- a/tools/libs/ctrl/xc_domain.c > +++ b/tools/libs/ctrl/xc_domain.c > @@ -1394,6 +1394,21 @@ int xc_domain_irq_permission(xc_interface *xch, > return do_domctl(xch, &domctl); > } > > +int xc_domain_gsi_permission(xc_interface *xch, > + uint32_t domid, > + uint32_t gsi, > + bool allow_access) > +{ > + struct xen_domctl domctl = {}; > + > + domctl.cmd = XEN_DOMCTL_gsi_permission; > + domctl.domain = domid; > + domctl.u.gsi_permission.gsi = gsi; > + domctl.u.gsi_permission.allow_access = allow_access; This could be written as: struct xen_domctl domctl = { .cmd = XEN_DOMCTL_gsi_permission, .domain = domid, .u.gsi_permission.gsi = gsi, .u.gsi_permission.allow_access = allow_access, }; but your change is fine too. > + > + return do_domctl(xch, &domctl); > +} > + > int xc_domain_iomem_permission(xc_interface *xch, > uint32_t domid, > unsigned long first_mfn, > diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c > index a1c6e82631e9..4136a860a048 100644 > --- a/tools/libs/light/libxl_pci.c > +++ b/tools/libs/light/libxl_pci.c > @@ -1421,6 +1421,8 @@ static void pci_add_dm_done(libxl__egc *egc, > uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED; > uint32_t domainid = domid; > bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid); > + int gsi; > + bool has_gsi = true; Why is this function has "gsi" variable, and "gsi = irq" but the next function pci_remove_detached() does only have "irq" variable? So, gsi can only be positive integer, right? Could we forgo `has_gsi` and just set "gsi = -1" when there's no gsi? > /* Convenience aliases */ > bool starting = pas->starting; > @@ -1482,6 +1484,7 @@ static void pci_add_dm_done(libxl__egc *egc, > pci->bus, pci->dev, pci->func); > > if ( access(sysfs_path, F_OK) != 0 ) { > + has_gsi = false; > if ( errno == ENOENT ) > sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, > pci->bus, pci->dev, pci->func); > @@ -1497,6 +1500,7 @@ static void pci_add_dm_done(libxl__egc *egc, > goto out_no_irq; > } > if ((fscanf(f, "%u", &irq) == 1) && irq) { > + gsi = irq; Why do you do this, that that mean that `gsi` and `irq` are the same? Or does `irq` happen to sometime contain a `gsi` value? > r = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq); > if (r < 0) { > LOGED(ERROR, domainid, "xc_physdev_map_pirq irq=%d (error=%d)", > @@ -1505,7 +1509,10 @@ 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 ( has_gsi ) > + r = xc_domain_gsi_permission(ctx->xch, domid, gsi, 1); > + if ( !has_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); Looks like this error message could be wrong, I think we want to know if which call between xc_domain_irq_permission() and xc_domain_gsi_permission() has failed. > @@ -2185,6 +2192,7 @@ static void pci_remove_detached(libxl__egc *egc, > FILE *f; > uint32_t domainid = prs->domid; > bool isstubdom; > + bool has_gsi = true; > > /* Convenience aliases */ > libxl_device_pci *const pci = &prs->pci; > @@ -2244,6 +2252,7 @@ skip_bar: > pci->bus, pci->dev, pci->func); > > if ( access(sysfs_path, F_OK) != 0 ) { > + has_gsi = false; > if ( errno == ENOENT ) > sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, > pci->bus, pci->dev, pci->func); > @@ -2270,7 +2279,10 @@ skip_bar: > */ > LOGED(ERROR, domid, "xc_physdev_unmap_pirq irq=%d", irq); > } > - rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); > + if ( has_gsi ) > + rc = xc_domain_gsi_permission(ctx->xch, domid, irq, 0); Why do you use the xc_domain_gsi_permission() with "irq" here, but not in pci_add_dm_done() ? > + if ( !has_gsi || rc == -EOPNOTSUPP ) > + rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); > if (rc < 0) { > LOGED(ERROR, domid, "xc_domain_irq_permission irq=%d", irq); > } These changes to libxl are quite confusing, there's a mixed up between "gsi" and "irq", it's hard to follow. Thanks,
Hi Anthony, On 2024/2/21 23:03, Anthony PERARD wrote: > On Fri, Jan 12, 2024 at 02:13:17PM +0800, Jiqian Chen wrote: >> diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c >> index f2d9d14b4d9f..448ba2c59ae1 100644 >> --- a/tools/libs/ctrl/xc_domain.c >> +++ b/tools/libs/ctrl/xc_domain.c >> @@ -1394,6 +1394,21 @@ int xc_domain_irq_permission(xc_interface *xch, >> return do_domctl(xch, &domctl); >> } >> >> +int xc_domain_gsi_permission(xc_interface *xch, >> + uint32_t domid, >> + uint32_t gsi, >> + bool allow_access) >> +{ >> + struct xen_domctl domctl = {}; >> + >> + domctl.cmd = XEN_DOMCTL_gsi_permission; >> + domctl.domain = domid; >> + domctl.u.gsi_permission.gsi = gsi; >> + domctl.u.gsi_permission.allow_access = allow_access; > > This could be written as: > struct xen_domctl domctl = { > .cmd = XEN_DOMCTL_gsi_permission, > .domain = domid, > .u.gsi_permission.gsi = gsi, > .u.gsi_permission.allow_access = allow_access, > }; > That's fine, I will change to this in next version. > but your change is fine too. > > >> + >> + return do_domctl(xch, &domctl); >> +} >> + >> int xc_domain_iomem_permission(xc_interface *xch, >> uint32_t domid, >> unsigned long first_mfn, >> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c >> index a1c6e82631e9..4136a860a048 100644 >> --- a/tools/libs/light/libxl_pci.c >> +++ b/tools/libs/light/libxl_pci.c >> @@ -1421,6 +1421,8 @@ static void pci_add_dm_done(libxl__egc *egc, >> uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED; >> uint32_t domainid = domid; >> bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid); >> + int gsi; >> + bool has_gsi = true; > > Why is this function has "gsi" variable, and "gsi = irq" but the next > function pci_remove_detached() does only have "irq" variable? Because in pci_add_dm_done, it calls " r = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq); ", it passes the pointer of irq in, and then irq will be changed, so I need to use gsi to save the origin value. > > So, gsi can only be positive integer, right? Could we forgo `has_gsi` and > just set "gsi = -1" when there's no gsi? No, we can't forgo 'has_gsi', because we need to set gsi = irq to save the original val. > >> /* Convenience aliases */ >> bool starting = pas->starting; >> @@ -1482,6 +1484,7 @@ static void pci_add_dm_done(libxl__egc *egc, >> pci->bus, pci->dev, pci->func); >> >> if ( access(sysfs_path, F_OK) != 0 ) { >> + has_gsi = false; >> if ( errno == ENOENT ) >> sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, >> pci->bus, pci->dev, pci->func); >> @@ -1497,6 +1500,7 @@ static void pci_add_dm_done(libxl__egc *egc, >> goto out_no_irq; >> } >> if ((fscanf(f, "%u", &irq) == 1) && irq) { >> + gsi = irq; > > Why do you do this, that that mean that `gsi` and `irq` are the same? Or > does `irq` happen to sometime contain a `gsi` value? Above reason. > >> r = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq); >> if (r < 0) { >> LOGED(ERROR, domainid, "xc_physdev_map_pirq irq=%d (error=%d)", >> @@ -1505,7 +1509,10 @@ 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 ( has_gsi ) >> + r = xc_domain_gsi_permission(ctx->xch, domid, gsi, 1); >> + if ( !has_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); > > Looks like this error message could be wrong, I think we want to know if > which call between xc_domain_irq_permission() and > xc_domain_gsi_permission() has failed. You are right, I will separate them in next version. > >> @@ -2185,6 +2192,7 @@ static void pci_remove_detached(libxl__egc *egc, >> FILE *f; >> uint32_t domainid = prs->domid; >> bool isstubdom; >> + bool has_gsi = true; >> >> /* Convenience aliases */ >> libxl_device_pci *const pci = &prs->pci; >> @@ -2244,6 +2252,7 @@ skip_bar: >> pci->bus, pci->dev, pci->func); >> >> if ( access(sysfs_path, F_OK) != 0 ) { >> + has_gsi = false; >> if ( errno == ENOENT ) >> sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, >> pci->bus, pci->dev, pci->func); >> @@ -2270,7 +2279,10 @@ skip_bar: >> */ >> LOGED(ERROR, domid, "xc_physdev_unmap_pirq irq=%d", irq); >> } >> - rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); >> + if ( has_gsi ) >> + rc = xc_domain_gsi_permission(ctx->xch, domid, irq, 0); > > Why do you use the xc_domain_gsi_permission() with "irq" here, but not > in pci_add_dm_done() ? Because xc_physdev_unmap_pirq doesn't change the value of irq, but in pci_add_dm_done, the value of irq is changed by xc_physdev_map_pirq. > >> + if ( !has_gsi || rc == -EOPNOTSUPP ) >> + rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); >> if (rc < 0) { >> LOGED(ERROR, domid, "xc_domain_irq_permission irq=%d", irq); >> } > > > These changes to libxl are quite confusing, there's a mixed up between > "gsi" and "irq", it's hard to follow. > > Thanks, >
On Thu, Feb 22, 2024 at 07:22:45AM +0000, Chen, Jiqian wrote: > On 2024/2/21 23:03, Anthony PERARD wrote: > > On Fri, Jan 12, 2024 at 02:13:17PM +0800, Jiqian Chen wrote: > >> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c > >> index a1c6e82631e9..4136a860a048 100644 > >> --- a/tools/libs/light/libxl_pci.c > >> +++ b/tools/libs/light/libxl_pci.c > >> @@ -1421,6 +1421,8 @@ static void pci_add_dm_done(libxl__egc *egc, > >> uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED; > >> uint32_t domainid = domid; > >> bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid); > >> + int gsi; > >> + bool has_gsi = true; > > > > Why is this function has "gsi" variable, and "gsi = irq" but the next > > function pci_remove_detached() does only have "irq" variable? > Because in pci_add_dm_done, it calls " r = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq); ", it passes the pointer of irq in, and then irq will be changed, so I need to use gsi to save the origin value. I'm sorry but unconditional "gsi = irq" looks very wrong, that line needs to be removed or changed, otherwise we have two functions doing the same thing just after that (xc_domain_irq_permission and xc_domain_gsi_permission). Instead, my guess is that the arguments of xc_physdev_map_pirq() needs to be changes. What does contain `irq` after the map_pirq() call? A "pirq" of some kind? Maybe xc_physdev_map_pirq(ctx->xch, domid, irq, &pirq) would make things more clear about what's going on. And BTW, maybe renaming the variable "has_gsi" to "is_gsi" might make things a bit clearer as well, as in: "the variable 'irq' $is_gsi", instead of a variable that have a meaning of "the system $has_gsi" without necessarily meaning that the code is using it. Maybe using (abusing?) the variable name "irq" might be a bit wrong now? What does "GSI" stand for anyway? And about "PIRQ"? This is just some question to figure out if there's potentially a better name for the variables names. Thanks,
On 2024/2/23 23:59, Anthony PERARD wrote: > On Thu, Feb 22, 2024 at 07:22:45AM +0000, Chen, Jiqian wrote: >> On 2024/2/21 23:03, Anthony PERARD wrote: >>> On Fri, Jan 12, 2024 at 02:13:17PM +0800, Jiqian Chen wrote: >>>> diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c >>>> index a1c6e82631e9..4136a860a048 100644 >>>> --- a/tools/libs/light/libxl_pci.c >>>> +++ b/tools/libs/light/libxl_pci.c >>>> @@ -1421,6 +1421,8 @@ static void pci_add_dm_done(libxl__egc *egc, >>>> uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED; >>>> uint32_t domainid = domid; >>>> bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid); >>>> + int gsi; >>>> + bool has_gsi = true; >>> >>> Why is this function has "gsi" variable, and "gsi = irq" but the next >>> function pci_remove_detached() does only have "irq" variable? >> Because in pci_add_dm_done, it calls " r = xc_physdev_map_pirq(ctx->xch, domid, irq, &irq); ", it passes the pointer of irq in, and then irq will be changed, so I need to use gsi to save the origin value. > > I'm sorry but unconditional "gsi = irq" looks very wrong, that line Make sense, I should add a condition and some comments here. Maybe: /* if has gsi sysfs node, then fscanf() read from the gsi sysfs and store it in irq variable, we need to save the original gsi value, because irq will be changed in xc_physdev_map_pirq */ if (has_gsi) gsi = irq; > needs to be removed or changed, otherwise we have two functions doing the > same thing just after that (xc_domain_irq_permission and > xc_domain_gsi_permission). Instead, my guess is that the > arguments of xc_physdev_map_pirq() needs to be changes. What does > contain `irq` after the map_pirq() call? A "pirq" of some kind? Maybe Yes, pirq. > xc_physdev_map_pirq(ctx->xch, domid, irq, &pirq) would make things more > clear about what's going on. > > > And BTW, maybe renaming the variable "has_gsi" to "is_gsi" might make > things a bit clearer as well, as in: "the variable 'irq' $is_gsi", > instead of a variable that have a meaning of "the system $has_gsi" > without necessarily meaning that the code is using it. Gsi is a new sysfs node added by my kernel patch, and it is still in discussion with PCI Maintainer. And for compatible with old version of kernel that didn't has gsi sysfs node, we still need to use irq here. So, I named this variable to has_gsi. Is it clearer that changing it to has_gsi_sysfs_node? Maybe I need to add some comments in code to make the usage of gsi clear. > > Maybe using (abusing?) the variable name "irq" might be a bit wrong now? > What does "GSI" stand for anyway? And about "PIRQ"? This is just some GSI is x86 specific concept, it is related to IOAPIC-PIN. PIRQ is used to route interrupts in Xen. > question to figure out if there's potentially a better name for the > variables names. > > Thanks, >
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h index 2ef8b4e05422..519c860a00d5 100644 --- a/tools/include/xenctrl.h +++ b/tools/include/xenctrl.h @@ -1382,6 +1382,11 @@ int xc_domain_irq_permission(xc_interface *xch, uint32_t pirq, bool allow_access); +int xc_domain_gsi_permission(xc_interface *xch, + uint32_t domid, + uint32_t gsi, + bool allow_access); + int xc_domain_iomem_permission(xc_interface *xch, uint32_t domid, unsigned long first_mfn, diff --git a/tools/libs/ctrl/xc_domain.c b/tools/libs/ctrl/xc_domain.c index f2d9d14b4d9f..448ba2c59ae1 100644 --- a/tools/libs/ctrl/xc_domain.c +++ b/tools/libs/ctrl/xc_domain.c @@ -1394,6 +1394,21 @@ int xc_domain_irq_permission(xc_interface *xch, return do_domctl(xch, &domctl); } +int xc_domain_gsi_permission(xc_interface *xch, + uint32_t domid, + uint32_t gsi, + bool allow_access) +{ + struct xen_domctl domctl = {}; + + domctl.cmd = XEN_DOMCTL_gsi_permission; + domctl.domain = domid; + domctl.u.gsi_permission.gsi = gsi; + domctl.u.gsi_permission.allow_access = allow_access; + + return do_domctl(xch, &domctl); +} + int xc_domain_iomem_permission(xc_interface *xch, uint32_t domid, unsigned long first_mfn, diff --git a/tools/libs/light/libxl_pci.c b/tools/libs/light/libxl_pci.c index a1c6e82631e9..4136a860a048 100644 --- a/tools/libs/light/libxl_pci.c +++ b/tools/libs/light/libxl_pci.c @@ -1421,6 +1421,8 @@ static void pci_add_dm_done(libxl__egc *egc, uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED; uint32_t domainid = domid; bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid); + int gsi; + bool has_gsi = true; /* Convenience aliases */ bool starting = pas->starting; @@ -1482,6 +1484,7 @@ static void pci_add_dm_done(libxl__egc *egc, pci->bus, pci->dev, pci->func); if ( access(sysfs_path, F_OK) != 0 ) { + has_gsi = false; if ( errno == ENOENT ) sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, pci->bus, pci->dev, pci->func); @@ -1497,6 +1500,7 @@ static void pci_add_dm_done(libxl__egc *egc, goto out_no_irq; } if ((fscanf(f, "%u", &irq) == 1) && irq) { + 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)", @@ -1505,7 +1509,10 @@ 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 ( has_gsi ) + r = xc_domain_gsi_permission(ctx->xch, domid, gsi, 1); + if ( !has_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); @@ -2185,6 +2192,7 @@ static void pci_remove_detached(libxl__egc *egc, FILE *f; uint32_t domainid = prs->domid; bool isstubdom; + bool has_gsi = true; /* Convenience aliases */ libxl_device_pci *const pci = &prs->pci; @@ -2244,6 +2252,7 @@ skip_bar: pci->bus, pci->dev, pci->func); if ( access(sysfs_path, F_OK) != 0 ) { + has_gsi = false; if ( errno == ENOENT ) sysfs_path = GCSPRINTF(SYSFS_PCI_DEV"/"PCI_BDF"/irq", pci->domain, pci->bus, pci->dev, pci->func); @@ -2270,7 +2279,10 @@ skip_bar: */ LOGED(ERROR, domid, "xc_physdev_unmap_pirq irq=%d", irq); } - rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); + if ( has_gsi ) + rc = xc_domain_gsi_permission(ctx->xch, domid, irq, 0); + if ( !has_gsi || rc == -EOPNOTSUPP ) + rc = xc_domain_irq_permission(ctx->xch, domid, irq, 0); if (rc < 0) { LOGED(ERROR, domid, "xc_domain_irq_permission irq=%d", irq); } diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c index 3c0ea8554107..75f23f511ac1 100644 --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -237,6 +237,37 @@ long arch_do_domctl( break; } + case XEN_DOMCTL_gsi_permission: + { + unsigned int gsi = domctl->u.gsi_permission.gsi; + int allow = domctl->u.gsi_permission.allow_access; + + if ( is_pv_domain(current->domain) || has_pirq(current->domain) ) + { + ret = -EOPNOTSUPP; + break; + } + + if ( gsi >= nr_irqs_gsi ) + { + ret = -EINVAL; + break; + } + + if ( !irq_access_permitted(current->domain, gsi) || + xsm_irq_permission(XSM_HOOK, d, gsi, allow) ) + { + ret = -EPERM; + break; + } + + if ( allow ) + ret = irq_permit_access(d, gsi); + else + ret = irq_deny_access(d, gsi); + break; + } + case XEN_DOMCTL_getpageframeinfo3: { unsigned int num = domctl->u.getpageframeinfo3.num; diff --git a/xen/include/public/domctl.h b/xen/include/public/domctl.h index a33f9ec32b08..47e95f9ee824 100644 --- a/xen/include/public/domctl.h +++ b/xen/include/public/domctl.h @@ -447,6 +447,13 @@ struct xen_domctl_irq_permission { }; +/* XEN_DOMCTL_gsi_permission */ +struct xen_domctl_gsi_permission { + uint32_t gsi; + uint8_t allow_access; /* flag to specify enable/disable of x86 gsi access */ +}; + + /* XEN_DOMCTL_iomem_permission */ struct xen_domctl_iomem_permission { uint64_aligned_t first_mfn;/* first page (physical page number) in range */ @@ -1277,6 +1284,7 @@ struct xen_domctl { #define XEN_DOMCTL_vmtrace_op 84 #define XEN_DOMCTL_get_paging_mempool_size 85 #define XEN_DOMCTL_set_paging_mempool_size 86 +#define XEN_DOMCTL_gsi_permission 87 #define XEN_DOMCTL_gdbsx_guestmemio 1000 #define XEN_DOMCTL_gdbsx_pausevcpu 1001 #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 @@ -1299,6 +1307,7 @@ struct xen_domctl { struct xen_domctl_setdomainhandle setdomainhandle; struct xen_domctl_setdebugging setdebugging; struct xen_domctl_irq_permission irq_permission; + struct xen_domctl_gsi_permission gsi_permission; struct xen_domctl_iomem_permission iomem_permission; struct xen_domctl_ioport_permission ioport_permission; struct xen_domctl_hypercall_init hypercall_init; diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c index 78225f68c15c..376076865198 100644 --- a/xen/xsm/flask/hooks.c +++ b/xen/xsm/flask/hooks.c @@ -685,6 +685,7 @@ static int cf_check flask_domctl(struct domain *d, int cmd) case XEN_DOMCTL_shadow_op: case XEN_DOMCTL_ioport_permission: case XEN_DOMCTL_ioport_mapping: + case XEN_DOMCTL_gsi_permission: #endif #ifdef CONFIG_HAS_PASSTHROUGH /*
Some type of domain don't have PIRQ, like PVH, when passthrough a device to guest on PVH dom0, callstack pci_add_dm_done->XEN_DOMCTL_irq_permission will failed at domain_pirq_to_irq. So, add a new hypercall to grant/revoke gsi permission when dom0 is not PV or dom0 has not PIRQ flag. Co-developed-by: Huang Rui <ray.huang@amd.com> Signed-off-by: Jiqian Chen <Jiqian.Chen@amd.com> --- tools/include/xenctrl.h | 5 +++++ tools/libs/ctrl/xc_domain.c | 15 +++++++++++++++ tools/libs/light/libxl_pci.c | 16 ++++++++++++++-- xen/arch/x86/domctl.c | 31 +++++++++++++++++++++++++++++++ xen/include/public/domctl.h | 9 +++++++++ xen/xsm/flask/hooks.c | 1 + 6 files changed, 75 insertions(+), 2 deletions(-)