[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v2 8/9] vpci/msi: add MSI handlers



Hi Roger,

On 20/04/17 16:17, Roger Pau Monne wrote:
diff --git a/xen/drivers/vpci/msi.c b/xen/drivers/vpci/msi.c
new file mode 100644
index 0000000000..aea6c68907
--- /dev/null
+++ b/xen/drivers/vpci/msi.c
@@ -0,0 +1,469 @@
+/*
+ * Handlers for accesses to the MSI capability structure.
+ *
+ * Copyright (C) 2017 Citrix Systems R&D
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms and conditions of the GNU General Public
+ * License, version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program; If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/sched.h>
+#include <xen/vpci.h>
+#include <asm/msi.h>
+#include <xen/keyhandler.h>
+
+static void vpci_msi_mask_pirq(int pirq, bool mask)
+{
+        struct pirq *pinfo = pirq_info(current->domain, pirq);

We don't have pirq on ARM and don't plan to introduce it for MSI as interrupt will be handled directly by a virtual interrupt controller (see the vITS series [1]).

It would be nice if you can get the vPCI architecture agnostic. We would be to help here.

+        struct irq_desc *desc;
+        unsigned long flags;
+        int irq;
+
+        ASSERT(pinfo);
+        irq = pinfo->arch.irq;
+        ASSERT(irq < nr_irqs);
+
+        desc = irq_to_desc(irq);

Similarly we don't have irq_desc for MSI.

+        ASSERT(desc);
+
+        spin_lock_irqsave(&desc->lock, flags);
+        guest_mask_msi_irq(desc, mask);
+        spin_unlock_irqrestore(&desc->lock, flags);
+}
+

[...]

+static int vpci_init_msi(struct pci_dev *pdev)
+{
+    uint8_t seg = pdev->seg, bus = pdev->bus;
+    uint8_t slot = PCI_SLOT(pdev->devfn), func = PCI_FUNC(pdev->devfn);
+    struct vpci_msi *msi = NULL;
+    unsigned int msi_offset;
+    uint16_t control;
+    int rc;
+
+    msi_offset = pci_find_cap_offset(seg, bus, slot, func, PCI_CAP_ID_MSI);
+    if ( !msi_offset )
+        return 0;
+
+    if ( !dom0_msi )

I would introduce an helper to allow per-architecture decision. Likely on ARM MSI will be enabled by default.

[...]

diff --git a/xen/include/asm-x86/hvm/io.h b/xen/include/asm-x86/hvm/io.h
index 0434aca706..899e37ae0f 100644
--- a/xen/include/asm-x86/hvm/io.h
+++ b/xen/include/asm-x86/hvm/io.h
@@ -126,6 +126,10 @@ void hvm_dpci_eoi(struct domain *d, unsigned int guest_irq,
 void msix_write_completion(struct vcpu *);
 void msixtbl_init(struct domain *d);

+/* Get the vector/flags from a MSI address/data fields. */
+unsigned int msi_vector(uint16_t data);
+unsigned int msi_flags(uint16_t data, uint64_t addr);

Should not those 2 helpers go in msi.h?

+
 enum stdvga_cache_state {
     STDVGA_CACHE_UNINITIALIZED,
     STDVGA_CACHE_ENABLED,
diff --git a/xen/include/asm-x86/msi.h b/xen/include/asm-x86/msi.h
index a5de6a1328..dcbec8cf04 100644
--- a/xen/include/asm-x86/msi.h
+++ b/xen/include/asm-x86/msi.h
@@ -251,4 +251,6 @@ void ack_nonmaskable_msi_irq(struct irq_desc *);
 void end_nonmaskable_msi_irq(struct irq_desc *, u8 vector);
 void set_msi_affinity(struct irq_desc *, const cpumask_t *);

+extern bool dom0_msi;
+
 #endif /* __ASM_MSI_H */

Cheers,

[1] https://lists.xenproject.org/archives/html/xen-devel/2017-04/msg01672.html

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.