Stubdomains need to be given sufficient privilege over the guest which it
provides emulation for in order for PCI passthrough to work correctly.
When a HVM domain try to enable MSI, QEMU in stubdomain calls
PHYSDEVOP_map_pirq, but later it needs to call XEN_DOMCTL_bind_pt_irq as
part of xc_domain_update_msi_irq. Give the stubdomain enough permissions
over the mapped interrupt in order to bind it successfully to it's
target domain.
This is not needed for PCI INTx, because IRQ in that case is known
beforehand and the stubdomain is given permissions over this IRQ by
libxl__device_pci_add (there's a do_pci_add against the stubdomain).
create_irq() already grant IRQ access to hardware_domain, with
assumption the device model lives there.
Modify create_irq() to take additional parameter, whether to grant
permissions to the domain creating the IRQ, which may be dom0 or a
stubdomain. Do this instead of granting access always to
hardware_domain. Save ID of the domain given permission, to revoke it in
destroy_irq() - easier and cleaner than replaying logic of create_irq()
parameter. Use domid instead of actual reference to the domain,
because it might get destroyed before destroying IRQ (stubdomain is
destroyed before its target domain). And it is not an issue,
because IRQ permissions live within domain structure, so destroying
a domain also implicitly revoke the permission. Potential domid
reuse is detected by checking if that domain does have permission
over the IRQ being destroyed.
Then, adjust all callers to provide the parameter. In case of Xen
internal allocations, set it to false, but for domain accessible
interrupt set it to true.
Inspired by
https://github.com/OpenXT/xenclient-oe/blob/5e0e7304a5a3c75ef01240a1e3673665b2aaf05e/recipes-extended/xen/files/stubdomain-msi-irq-access.patch
by Eric Chanudet .
Signed-off-by: Simon Gaiser
Signed-off-by: Marek Marczykowski-Górecki
Reviewed-by: Roger Pau Monné
---
Changes in v3:
- extend commit message
Changes in v4:
- add missing destroy_irq on error path
Changes in v5:
- move irq_{grant,revoke}_access() to {create,destroy}_irq(), which
basically make it a different patch
- add get_dm_domain() helper
- do not give hardware_domain permission over IRQs used in Xen
internally
- rename create_irq argument to just 'd', to avoid confusion
when it's called by hardware domain
- verify that device is de-assigned before pci_remove_device call
- save ID of domain given permission in create_irq(), to revoke it in
destroy_irq()
- drop domain parameter from destroy_irq() and msi_free_irq()
- do not give hardware domain permission over IRQ created in
iommu_set_interrupt()
Changes in v6:
- do not give permission over hpet irq to hardware_domain
- move creator_domid to arch_irq_desc
- fix creator_domid initialization
- always give current->domain permission instead of using
get_dm_domain() helper. Analysis of all its use cases tells that it is
the only value it returns.
- drop unrelated change
Changes in v7:
- Code style improvements (spaces, use %pd etc)
- use bool parameter to create_irq, as it's only getting
current->domain or NULL
- remove redundant irq_access_permitted()
Changes in v7.1:
- adjust comments, merge if
- update commit message
---
xen/arch/x86/hpet.c | 3 +-
xen/arch/x86/irq.c | 42 +
xen/drivers/char/ns16550.c | 2 +-
xen/drivers/passthrough/amd/iommu_init.c | 2 +-
xen/drivers/passthrough/vtd/iommu.c | 3 +-
xen/include/asm-x86/irq.h| 11 ++-
6 files changed, 45 insertions(+), 18 deletions(-)
diff --git a/xen/arch/x86/hpet.c b/xen/arch/x86/hpet.c
index 4b08488..57f68fa 100644
--- a/xen/arch/x86/hpet.c
+++ b/xen/arch/x86/hpet.c
@@ -11,6 +11,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -368,7 +369,7 @@ static int __init hpet_assign_irq(struct hpet_event_channel
*ch)
{
int irq;
-if ( (irq = create_irq(NUMA_NO_NODE)) < 0 )
+if ( (irq = create_irq(NUMA_NO_NODE, false)) < 0 )
return irq;
ch->msi.irq = irq;
diff --git a/xen/arch/x86/irq.c b/xen/arch/x86/irq.c
index 0ee3346..4304896 100644
--- a/xen/arch/x86/irq.c
+++ b/xen/arch/x86/irq.c
@@ -254,7 +254,8 @@ void __init clear_irq_vector(int irq)
/*
* Dynamic irq allocate and deallocation for MSI
*/
-int create_irq(nodeid_t node)
+
+int create_irq(nodeid_t node, bool grant_access)
{
int irq, ret;
struct irq_desc *desc;
@@ -282,18 +283,23 @@ int create_irq(nodeid_t node)
}
ret = assign_irq_vector(irq, mask);
}
+
+ASSERT(desc->arch.creator_domid == DOMID_INVALID);
+
if (ret < 0)
{
desc->arch.used = IRQ_UNUSED;
irq = ret;
}
-else if ( hardware_domain )
+else if ( grant_access )
{
-ret = irq_permit_access(hardware_domain, irq);
+ret = irq_permit_access(current->domain, irq);
if ( ret )
