On Arm, the initial plan is to only support GICv3 ITS which doesn't
require us to manage the MSIs because the HW will protect against
spoofing. Move the code under CONFIG_HAS_PCI_MSI flag to gate the code
for ARM.
No functional change intended.
Signed-off-by: Rahul Singh <rahul.singh@arm.com>
Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
Change in v3: none
Change in v2: Fixed minor comments
---
xen/arch/x86/Kconfig | 1 +
xen/drivers/passthrough/Makefile | 1 +
xen/drivers/passthrough/msi.c | 83 ++++++++++++++++++++++++++++++++
xen/drivers/passthrough/pci.c | 54 +++++----------------
xen/drivers/pci/Kconfig | 4 ++
xen/include/xen/msi.h | 43 +++++++++++++++++
xen/xsm/flask/hooks.c | 8 +--
7 files changed, 149 insertions(+), 45 deletions(-)
create mode 100644 xen/drivers/passthrough/msi.c
create mode 100644 xen/include/xen/msi.h
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 1f83518ee0..b4abfca46f 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -20,6 +20,7 @@ config X86
select HAS_NS16550
select HAS_PASSTHROUGH
select HAS_PCI
+ select HAS_PCI_MSI
select HAS_PDX
select HAS_SCHED_GRANULARITY
select HAS_UBSAN
diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
index 445690e3e5..a5efa22714 100644
--- a/xen/drivers/passthrough/Makefile
+++ b/xen/drivers/passthrough/Makefile
@@ -7,3 +7,4 @@ obj-y += iommu.o
obj-$(CONFIG_HAS_PCI) += pci.o
obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
obj-$(CONFIG_HAS_PCI) += ats.o
+obj-$(CONFIG_HAS_PCI_MSI) += msi.o
diff --git a/xen/drivers/passthrough/msi.c b/xen/drivers/passthrough/msi.c
new file mode 100644
index 0000000000..ce1a450f6f
--- /dev/null
+++ b/xen/drivers/passthrough/msi.c
@@ -0,0 +1,83 @@
+#include <xen/init.h>
+#include <xen/pci.h>
+#include <xen/msi.h>
+#include <asm/hvm/io.h>
+
+int pdev_msix_assign(struct domain *d, struct pci_dev *pdev)
+{
+ int rc;
+
+ if ( pdev->msix )
+ {
+ rc = pci_reset_msix_state(pdev);
+ if ( rc )
+ return rc;
+ msixtbl_init(d);
+ }
+
+ return 0;
+}
+
+int pdev_msi_init(struct pci_dev *pdev)
+{
+ unsigned int pos;
+
+ INIT_LIST_HEAD(&pdev->msi_list);
+
+ pos = pci_find_cap_offset(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
+ PCI_FUNC(pdev->devfn), PCI_CAP_ID_MSI);
+ if ( pos )
+ {
+ uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
+
+ pdev->msi_maxvec = multi_msi_capable(ctrl);
+ }
+
+ pos = pci_find_cap_offset(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
+ PCI_FUNC(pdev->devfn), PCI_CAP_ID_MSIX);
+ if ( pos )
+ {
+ struct arch_msix *msix = xzalloc(struct arch_msix);
+ uint16_t ctrl;
+
+ if ( !msix )
+ return -ENOMEM;
+
+ spin_lock_init(&msix->table_lock);
+
+ ctrl = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
+ msix->nr_entries = msix_table_size(ctrl);
+
+ pdev->msix = msix;
+ }
+
+ return 0;
+}
+
+void pdev_msi_deinit(struct pci_dev *pdev)
+{
+ XFREE(pdev->msix);
+}
+
+void pdev_dump_msi(const struct pci_dev *pdev)
+{
+ const struct msi_desc *msi;
+
+ if ( list_empty(&pdev->msi_list) )
+ return;
+
+ printk(" - MSIs < ");
+ list_for_each_entry ( msi, &pdev->msi_list, list )
+ printk("%d ", msi->irq);
+ printk(">");
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
index fc4fa2e5c3..8996403161 100644
--- a/xen/drivers/passthrough/pci.c
+++ b/xen/drivers/passthrough/pci.c
@@ -32,8 +32,8 @@
#include <xen/softirq.h>
#include <xen/tasklet.h>
#include <xen/vpci.h>
+#include <xen/msi.h>
#include <xsm/xsm.h>
-#include <asm/msi.h>
#include "ats.h"
struct pci_seg {
@@ -314,6 +314,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
{
struct pci_dev *pdev;
unsigned int pos;
+ int rc;
list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
if ( pdev->bus == bus && pdev->devfn == devfn )
@@ -327,35 +328,12 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
*((u8*) &pdev->bus) = bus;
*((u8*) &pdev->devfn) = devfn;
pdev->domain = NULL;
- INIT_LIST_HEAD(&pdev->msi_list);
-
- pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
- PCI_CAP_ID_MSI);
- if ( pos )
- {
- uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
-
- pdev->msi_maxvec = multi_msi_capable(ctrl);
- }
- pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
- PCI_CAP_ID_MSIX);
- if ( pos )
+ rc = pdev_msi_init(pdev);
+ if ( rc )
{
- struct arch_msix *msix = xzalloc(struct arch_msix);
- uint16_t ctrl;
-
- if ( !msix )
- {
- xfree(pdev);
- return NULL;
- }
- spin_lock_init(&msix->table_lock);
-
- ctrl = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
- msix->nr_entries = msix_table_size(ctrl);
-
- pdev->msix = msix;
+ xfree(pdev);
+ return NULL;
}
list_add(&pdev->alldevs_list, &pseg->alldevs_list);
@@ -449,7 +427,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
}
list_del(&pdev->alldevs_list);
- xfree(pdev->msix);
+ pdev_msi_deinit(pdev);
xfree(pdev);
}
@@ -1271,18 +1249,16 @@ bool_t pcie_aer_get_firmware_first(const struct pci_dev *pdev)
static int _dump_pci_devices(struct pci_seg *pseg, void *arg)
{
struct pci_dev *pdev;
- struct msi_desc *msi;
printk("==== segment %04x ====\n", pseg->nr);
list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
{
- printk("%pp - %pd - node %-3d - MSIs < ",
+ printk("%pp - %pd - node %-3d",
&pdev->sbdf, pdev->domain,
(pdev->node != NUMA_NO_NODE) ? pdev->node : -1);
- list_for_each_entry ( msi, &pdev->msi_list, list )
- printk("%d ", msi->irq);
- printk(">\n");
+ pdev_dump_msi(pdev);
+ printk("\n");
}
return 0;
@@ -1422,13 +1398,9 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
ASSERT(pdev && (pdev->domain == hardware_domain ||
pdev->domain == dom_io));
- if ( pdev->msix )
- {
- rc = pci_reset_msix_state(pdev);
- if ( rc )
- goto done;
- msixtbl_init(d);
- }
+ rc = pdev_msix_assign(d, pdev);
+ if ( rc )
+ goto done;
pdev->fault.count = 0;
diff --git a/xen/drivers/pci/Kconfig b/xen/drivers/pci/Kconfig
index 7da03fa13b..c6a7bc8007 100644
--- a/xen/drivers/pci/Kconfig
+++ b/xen/drivers/pci/Kconfig
@@ -1,3 +1,7 @@
config HAS_PCI
bool
+
+config HAS_PCI_MSI
+ bool
+ depends on HAS_PCI
diff --git a/xen/include/xen/msi.h b/xen/include/xen/msi.h
new file mode 100644
index 0000000000..c903d0050c
--- /dev/null
+++ b/xen/include/xen/msi.h
@@ -0,0 +1,43 @@
+#ifndef __XEN_MSI_H_
+#define __XEN_MSI_H_
+
+#include <xen/pci.h>
+
+#ifdef CONFIG_HAS_PCI_MSI
+
+#include <asm/msi.h>
+
+int pdev_msix_assign(struct domain *d, struct pci_dev *pdev);
+int pdev_msi_init(struct pci_dev *pdev);
+void pdev_msi_deinit(struct pci_dev *pdev);
+void pdev_dump_msi(const struct pci_dev *pdev);
+
+#else /* !CONFIG_HAS_PCI_MSI */
+
+static inline int pdev_msix_assign(struct domain *d, struct pci_dev *pdev)
+{
+ return 0;
+}
+
+static inline int pdev_msi_init(struct pci_dev *pdev)
+{
+ return 0;
+}
+
+static inline void pdev_msi_deinit(struct pci_dev *pdev) {}
+static inline void pci_cleanup_msi(struct pci_dev *pdev) {}
+static inline void pdev_dump_msi(const struct pci_dev *pdev) {}
+
+#endif /* CONFIG_HAS_PCI_MSI */
+
+#endif /* __XEN_MSI_H */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
index 25e87180b4..ea9a12bd71 100644
--- a/xen/xsm/flask/hooks.c
+++ b/xen/xsm/flask/hooks.c
@@ -21,7 +21,7 @@
#include <xen/guest_access.h>
#include <xen/xenoprof.h>
#include <xen/iommu.h>
-#ifdef CONFIG_HAS_PCI
+#ifdef CONFIG_HAS_PCI_MSI
#include <asm/msi.h>
#endif
#include <public/xen.h>
@@ -114,7 +114,7 @@ static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad)
}
return security_irq_sid(irq, sid);
}
-#ifdef CONFIG_HAS_PCI
+#ifdef CONFIG_HAS_PCI_MSI
{
struct irq_desc *desc = irq_to_desc(irq);
if ( desc->msi_desc && desc->msi_desc->dev ) {
@@ -874,7 +874,7 @@ static int flask_map_domain_pirq (struct domain *d)
static int flask_map_domain_msi (struct domain *d, int irq, const void *data,
u32 *sid, struct avc_audit_data *ad)
{
-#ifdef CONFIG_HAS_PCI
+#ifdef CONFIG_HAS_PCI_MSI
const struct msi_info *msi = data;
u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn;
@@ -940,7 +940,7 @@ static int flask_unmap_domain_pirq (struct domain *d)
static int flask_unmap_domain_msi (struct domain *d, int irq, const void *data,
u32 *sid, struct avc_audit_data *ad)
{
-#ifdef CONFIG_HAS_PCI
+#ifdef CONFIG_HAS_PCI_MSI
const struct pci_dev *pdev = data;
u32 machine_bdf = (pdev->seg << 16) | (pdev->bus << 8) | pdev->devfn;
--
2.17.1
Hi Rahul,
> On 28 Sep 2021, at 19:18, Rahul Singh <rahul.singh@arm.com> wrote:
>
> On Arm, the initial plan is to only support GICv3 ITS which doesn't
> require us to manage the MSIs because the HW will protect against
> spoofing. Move the code under CONFIG_HAS_PCI_MSI flag to gate the code
> for ARM.
>
> No functional change intended.
>
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Bertrand Marquis <bertrand.marquis@arm.com>
Cheers
Bertrand
> ---
> Change in v3: none
> Change in v2: Fixed minor comments
> ---
> xen/arch/x86/Kconfig | 1 +
> xen/drivers/passthrough/Makefile | 1 +
> xen/drivers/passthrough/msi.c | 83 ++++++++++++++++++++++++++++++++
> xen/drivers/passthrough/pci.c | 54 +++++----------------
> xen/drivers/pci/Kconfig | 4 ++
> xen/include/xen/msi.h | 43 +++++++++++++++++
> xen/xsm/flask/hooks.c | 8 +--
> 7 files changed, 149 insertions(+), 45 deletions(-)
> create mode 100644 xen/drivers/passthrough/msi.c
> create mode 100644 xen/include/xen/msi.h
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 1f83518ee0..b4abfca46f 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -20,6 +20,7 @@ config X86
> select HAS_NS16550
> select HAS_PASSTHROUGH
> select HAS_PCI
> + select HAS_PCI_MSI
> select HAS_PDX
> select HAS_SCHED_GRANULARITY
> select HAS_UBSAN
> diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
> index 445690e3e5..a5efa22714 100644
> --- a/xen/drivers/passthrough/Makefile
> +++ b/xen/drivers/passthrough/Makefile
> @@ -7,3 +7,4 @@ obj-y += iommu.o
> obj-$(CONFIG_HAS_PCI) += pci.o
> obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
> obj-$(CONFIG_HAS_PCI) += ats.o
> +obj-$(CONFIG_HAS_PCI_MSI) += msi.o
> diff --git a/xen/drivers/passthrough/msi.c b/xen/drivers/passthrough/msi.c
> new file mode 100644
> index 0000000000..ce1a450f6f
> --- /dev/null
> +++ b/xen/drivers/passthrough/msi.c
> @@ -0,0 +1,83 @@
> +#include <xen/init.h>
> +#include <xen/pci.h>
> +#include <xen/msi.h>
> +#include <asm/hvm/io.h>
> +
> +int pdev_msix_assign(struct domain *d, struct pci_dev *pdev)
> +{
> + int rc;
> +
> + if ( pdev->msix )
> + {
> + rc = pci_reset_msix_state(pdev);
> + if ( rc )
> + return rc;
> + msixtbl_init(d);
> + }
> +
> + return 0;
> +}
> +
> +int pdev_msi_init(struct pci_dev *pdev)
> +{
> + unsigned int pos;
> +
> + INIT_LIST_HEAD(&pdev->msi_list);
> +
> + pos = pci_find_cap_offset(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> + PCI_FUNC(pdev->devfn), PCI_CAP_ID_MSI);
> + if ( pos )
> + {
> + uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
> +
> + pdev->msi_maxvec = multi_msi_capable(ctrl);
> + }
> +
> + pos = pci_find_cap_offset(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> + PCI_FUNC(pdev->devfn), PCI_CAP_ID_MSIX);
> + if ( pos )
> + {
> + struct arch_msix *msix = xzalloc(struct arch_msix);
> + uint16_t ctrl;
> +
> + if ( !msix )
> + return -ENOMEM;
> +
> + spin_lock_init(&msix->table_lock);
> +
> + ctrl = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
> + msix->nr_entries = msix_table_size(ctrl);
> +
> + pdev->msix = msix;
> + }
> +
> + return 0;
> +}
> +
> +void pdev_msi_deinit(struct pci_dev *pdev)
> +{
> + XFREE(pdev->msix);
> +}
> +
> +void pdev_dump_msi(const struct pci_dev *pdev)
> +{
> + const struct msi_desc *msi;
> +
> + if ( list_empty(&pdev->msi_list) )
> + return;
> +
> + printk(" - MSIs < ");
> + list_for_each_entry ( msi, &pdev->msi_list, list )
> + printk("%d ", msi->irq);
> + printk(">");
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index fc4fa2e5c3..8996403161 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -32,8 +32,8 @@
> #include <xen/softirq.h>
> #include <xen/tasklet.h>
> #include <xen/vpci.h>
> +#include <xen/msi.h>
> #include <xsm/xsm.h>
> -#include <asm/msi.h>
> #include "ats.h"
>
> struct pci_seg {
> @@ -314,6 +314,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
> {
> struct pci_dev *pdev;
> unsigned int pos;
> + int rc;
>
> list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
> if ( pdev->bus == bus && pdev->devfn == devfn )
> @@ -327,35 +328,12 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
> *((u8*) &pdev->bus) = bus;
> *((u8*) &pdev->devfn) = devfn;
> pdev->domain = NULL;
> - INIT_LIST_HEAD(&pdev->msi_list);
> -
> - pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> - PCI_CAP_ID_MSI);
> - if ( pos )
> - {
> - uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
> -
> - pdev->msi_maxvec = multi_msi_capable(ctrl);
> - }
>
> - pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> - PCI_CAP_ID_MSIX);
> - if ( pos )
> + rc = pdev_msi_init(pdev);
> + if ( rc )
> {
> - struct arch_msix *msix = xzalloc(struct arch_msix);
> - uint16_t ctrl;
> -
> - if ( !msix )
> - {
> - xfree(pdev);
> - return NULL;
> - }
> - spin_lock_init(&msix->table_lock);
> -
> - ctrl = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
> - msix->nr_entries = msix_table_size(ctrl);
> -
> - pdev->msix = msix;
> + xfree(pdev);
> + return NULL;
> }
>
> list_add(&pdev->alldevs_list, &pseg->alldevs_list);
> @@ -449,7 +427,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
> }
>
> list_del(&pdev->alldevs_list);
> - xfree(pdev->msix);
> + pdev_msi_deinit(pdev);
> xfree(pdev);
> }
>
> @@ -1271,18 +1249,16 @@ bool_t pcie_aer_get_firmware_first(const struct pci_dev *pdev)
> static int _dump_pci_devices(struct pci_seg *pseg, void *arg)
> {
> struct pci_dev *pdev;
> - struct msi_desc *msi;
>
> printk("==== segment %04x ====\n", pseg->nr);
>
> list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
> {
> - printk("%pp - %pd - node %-3d - MSIs < ",
> + printk("%pp - %pd - node %-3d",
> &pdev->sbdf, pdev->domain,
> (pdev->node != NUMA_NO_NODE) ? pdev->node : -1);
> - list_for_each_entry ( msi, &pdev->msi_list, list )
> - printk("%d ", msi->irq);
> - printk(">\n");
> + pdev_dump_msi(pdev);
> + printk("\n");
> }
>
> return 0;
> @@ -1422,13 +1398,9 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
> ASSERT(pdev && (pdev->domain == hardware_domain ||
> pdev->domain == dom_io));
>
> - if ( pdev->msix )
> - {
> - rc = pci_reset_msix_state(pdev);
> - if ( rc )
> - goto done;
> - msixtbl_init(d);
> - }
> + rc = pdev_msix_assign(d, pdev);
> + if ( rc )
> + goto done;
>
> pdev->fault.count = 0;
>
> diff --git a/xen/drivers/pci/Kconfig b/xen/drivers/pci/Kconfig
> index 7da03fa13b..c6a7bc8007 100644
> --- a/xen/drivers/pci/Kconfig
> +++ b/xen/drivers/pci/Kconfig
> @@ -1,3 +1,7 @@
>
> config HAS_PCI
> bool
> +
> +config HAS_PCI_MSI
> + bool
> + depends on HAS_PCI
> diff --git a/xen/include/xen/msi.h b/xen/include/xen/msi.h
> new file mode 100644
> index 0000000000..c903d0050c
> --- /dev/null
> +++ b/xen/include/xen/msi.h
> @@ -0,0 +1,43 @@
> +#ifndef __XEN_MSI_H_
> +#define __XEN_MSI_H_
> +
> +#include <xen/pci.h>
> +
> +#ifdef CONFIG_HAS_PCI_MSI
> +
> +#include <asm/msi.h>
> +
> +int pdev_msix_assign(struct domain *d, struct pci_dev *pdev);
> +int pdev_msi_init(struct pci_dev *pdev);
> +void pdev_msi_deinit(struct pci_dev *pdev);
> +void pdev_dump_msi(const struct pci_dev *pdev);
> +
> +#else /* !CONFIG_HAS_PCI_MSI */
> +
> +static inline int pdev_msix_assign(struct domain *d, struct pci_dev *pdev)
> +{
> + return 0;
> +}
> +
> +static inline int pdev_msi_init(struct pci_dev *pdev)
> +{
> + return 0;
> +}
> +
> +static inline void pdev_msi_deinit(struct pci_dev *pdev) {}
> +static inline void pci_cleanup_msi(struct pci_dev *pdev) {}
> +static inline void pdev_dump_msi(const struct pci_dev *pdev) {}
> +
> +#endif /* CONFIG_HAS_PCI_MSI */
> +
> +#endif /* __XEN_MSI_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 25e87180b4..ea9a12bd71 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -21,7 +21,7 @@
> #include <xen/guest_access.h>
> #include <xen/xenoprof.h>
> #include <xen/iommu.h>
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_PCI_MSI
> #include <asm/msi.h>
> #endif
> #include <public/xen.h>
> @@ -114,7 +114,7 @@ static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad)
> }
> return security_irq_sid(irq, sid);
> }
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_PCI_MSI
> {
> struct irq_desc *desc = irq_to_desc(irq);
> if ( desc->msi_desc && desc->msi_desc->dev ) {
> @@ -874,7 +874,7 @@ static int flask_map_domain_pirq (struct domain *d)
> static int flask_map_domain_msi (struct domain *d, int irq, const void *data,
> u32 *sid, struct avc_audit_data *ad)
> {
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_PCI_MSI
> const struct msi_info *msi = data;
> u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn;
>
> @@ -940,7 +940,7 @@ static int flask_unmap_domain_pirq (struct domain *d)
> static int flask_unmap_domain_msi (struct domain *d, int irq, const void *data,
> u32 *sid, struct avc_audit_data *ad)
> {
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_PCI_MSI
> const struct pci_dev *pdev = data;
> u32 machine_bdf = (pdev->seg << 16) | (pdev->bus << 8) | pdev->devfn;
>
> --
> 2.17.1
>
On Tue, 28 Sep 2021, Rahul Singh wrote:
> On Arm, the initial plan is to only support GICv3 ITS which doesn't
> require us to manage the MSIs because the HW will protect against
> spoofing. Move the code under CONFIG_HAS_PCI_MSI flag to gate the code
> for ARM.
>
> No functional change intended.
>
> Signed-off-by: Rahul Singh <rahul.singh@arm.com>
> Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
> ---
> Change in v3: none
> Change in v2: Fixed minor comments
> ---
> xen/arch/x86/Kconfig | 1 +
> xen/drivers/passthrough/Makefile | 1 +
> xen/drivers/passthrough/msi.c | 83 ++++++++++++++++++++++++++++++++
> xen/drivers/passthrough/pci.c | 54 +++++----------------
> xen/drivers/pci/Kconfig | 4 ++
> xen/include/xen/msi.h | 43 +++++++++++++++++
> xen/xsm/flask/hooks.c | 8 +--
> 7 files changed, 149 insertions(+), 45 deletions(-)
> create mode 100644 xen/drivers/passthrough/msi.c
> create mode 100644 xen/include/xen/msi.h
>
> diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
> index 1f83518ee0..b4abfca46f 100644
> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -20,6 +20,7 @@ config X86
> select HAS_NS16550
> select HAS_PASSTHROUGH
> select HAS_PCI
> + select HAS_PCI_MSI
> select HAS_PDX
> select HAS_SCHED_GRANULARITY
> select HAS_UBSAN
> diff --git a/xen/drivers/passthrough/Makefile b/xen/drivers/passthrough/Makefile
> index 445690e3e5..a5efa22714 100644
> --- a/xen/drivers/passthrough/Makefile
> +++ b/xen/drivers/passthrough/Makefile
> @@ -7,3 +7,4 @@ obj-y += iommu.o
> obj-$(CONFIG_HAS_PCI) += pci.o
> obj-$(CONFIG_HAS_DEVICE_TREE) += device_tree.o
> obj-$(CONFIG_HAS_PCI) += ats.o
> +obj-$(CONFIG_HAS_PCI_MSI) += msi.o
> diff --git a/xen/drivers/passthrough/msi.c b/xen/drivers/passthrough/msi.c
> new file mode 100644
> index 0000000000..ce1a450f6f
> --- /dev/null
> +++ b/xen/drivers/passthrough/msi.c
> @@ -0,0 +1,83 @@
> +#include <xen/init.h>
> +#include <xen/pci.h>
> +#include <xen/msi.h>
> +#include <asm/hvm/io.h>
> +
> +int pdev_msix_assign(struct domain *d, struct pci_dev *pdev)
> +{
> + int rc;
> +
> + if ( pdev->msix )
> + {
> + rc = pci_reset_msix_state(pdev);
> + if ( rc )
> + return rc;
> + msixtbl_init(d);
> + }
> +
> + return 0;
> +}
> +
> +int pdev_msi_init(struct pci_dev *pdev)
> +{
> + unsigned int pos;
> +
> + INIT_LIST_HEAD(&pdev->msi_list);
> +
> + pos = pci_find_cap_offset(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> + PCI_FUNC(pdev->devfn), PCI_CAP_ID_MSI);
> + if ( pos )
> + {
> + uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
> +
> + pdev->msi_maxvec = multi_msi_capable(ctrl);
> + }
> +
> + pos = pci_find_cap_offset(pdev->seg, pdev->bus, PCI_SLOT(pdev->devfn),
> + PCI_FUNC(pdev->devfn), PCI_CAP_ID_MSIX);
> + if ( pos )
> + {
> + struct arch_msix *msix = xzalloc(struct arch_msix);
> + uint16_t ctrl;
> +
> + if ( !msix )
> + return -ENOMEM;
> +
> + spin_lock_init(&msix->table_lock);
> +
> + ctrl = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
> + msix->nr_entries = msix_table_size(ctrl);
> +
> + pdev->msix = msix;
> + }
> +
> + return 0;
> +}
> +
> +void pdev_msi_deinit(struct pci_dev *pdev)
> +{
> + XFREE(pdev->msix);
> +}
> +
> +void pdev_dump_msi(const struct pci_dev *pdev)
> +{
> + const struct msi_desc *msi;
> +
> + if ( list_empty(&pdev->msi_list) )
> + return;
> +
> + printk(" - MSIs < ");
> + list_for_each_entry ( msi, &pdev->msi_list, list )
> + printk("%d ", msi->irq);
> + printk(">");
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/drivers/passthrough/pci.c b/xen/drivers/passthrough/pci.c
> index fc4fa2e5c3..8996403161 100644
> --- a/xen/drivers/passthrough/pci.c
> +++ b/xen/drivers/passthrough/pci.c
> @@ -32,8 +32,8 @@
> #include <xen/softirq.h>
> #include <xen/tasklet.h>
> #include <xen/vpci.h>
> +#include <xen/msi.h>
> #include <xsm/xsm.h>
> -#include <asm/msi.h>
> #include "ats.h"
>
> struct pci_seg {
> @@ -314,6 +314,7 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
> {
> struct pci_dev *pdev;
> unsigned int pos;
> + int rc;
>
> list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
> if ( pdev->bus == bus && pdev->devfn == devfn )
> @@ -327,35 +328,12 @@ static struct pci_dev *alloc_pdev(struct pci_seg *pseg, u8 bus, u8 devfn)
> *((u8*) &pdev->bus) = bus;
> *((u8*) &pdev->devfn) = devfn;
> pdev->domain = NULL;
> - INIT_LIST_HEAD(&pdev->msi_list);
> -
> - pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> - PCI_CAP_ID_MSI);
> - if ( pos )
> - {
> - uint16_t ctrl = pci_conf_read16(pdev->sbdf, msi_control_reg(pos));
> -
> - pdev->msi_maxvec = multi_msi_capable(ctrl);
> - }
>
> - pos = pci_find_cap_offset(pseg->nr, bus, PCI_SLOT(devfn), PCI_FUNC(devfn),
> - PCI_CAP_ID_MSIX);
> - if ( pos )
> + rc = pdev_msi_init(pdev);
> + if ( rc )
> {
> - struct arch_msix *msix = xzalloc(struct arch_msix);
> - uint16_t ctrl;
> -
> - if ( !msix )
> - {
> - xfree(pdev);
> - return NULL;
> - }
> - spin_lock_init(&msix->table_lock);
> -
> - ctrl = pci_conf_read16(pdev->sbdf, msix_control_reg(pos));
> - msix->nr_entries = msix_table_size(ctrl);
> -
> - pdev->msix = msix;
> + xfree(pdev);
> + return NULL;
> }
>
> list_add(&pdev->alldevs_list, &pseg->alldevs_list);
> @@ -449,7 +427,7 @@ static void free_pdev(struct pci_seg *pseg, struct pci_dev *pdev)
> }
>
> list_del(&pdev->alldevs_list);
> - xfree(pdev->msix);
> + pdev_msi_deinit(pdev);
> xfree(pdev);
> }
>
> @@ -1271,18 +1249,16 @@ bool_t pcie_aer_get_firmware_first(const struct pci_dev *pdev)
> static int _dump_pci_devices(struct pci_seg *pseg, void *arg)
> {
> struct pci_dev *pdev;
> - struct msi_desc *msi;
>
> printk("==== segment %04x ====\n", pseg->nr);
>
> list_for_each_entry ( pdev, &pseg->alldevs_list, alldevs_list )
> {
> - printk("%pp - %pd - node %-3d - MSIs < ",
> + printk("%pp - %pd - node %-3d",
> &pdev->sbdf, pdev->domain,
> (pdev->node != NUMA_NO_NODE) ? pdev->node : -1);
> - list_for_each_entry ( msi, &pdev->msi_list, list )
> - printk("%d ", msi->irq);
> - printk(">\n");
> + pdev_dump_msi(pdev);
> + printk("\n");
> }
>
> return 0;
> @@ -1422,13 +1398,9 @@ static int assign_device(struct domain *d, u16 seg, u8 bus, u8 devfn, u32 flag)
> ASSERT(pdev && (pdev->domain == hardware_domain ||
> pdev->domain == dom_io));
>
> - if ( pdev->msix )
> - {
> - rc = pci_reset_msix_state(pdev);
> - if ( rc )
> - goto done;
> - msixtbl_init(d);
> - }
> + rc = pdev_msix_assign(d, pdev);
> + if ( rc )
> + goto done;
>
> pdev->fault.count = 0;
>
> diff --git a/xen/drivers/pci/Kconfig b/xen/drivers/pci/Kconfig
> index 7da03fa13b..c6a7bc8007 100644
> --- a/xen/drivers/pci/Kconfig
> +++ b/xen/drivers/pci/Kconfig
> @@ -1,3 +1,7 @@
>
> config HAS_PCI
> bool
> +
> +config HAS_PCI_MSI
> + bool
> + depends on HAS_PCI
> diff --git a/xen/include/xen/msi.h b/xen/include/xen/msi.h
> new file mode 100644
> index 0000000000..c903d0050c
> --- /dev/null
> +++ b/xen/include/xen/msi.h
> @@ -0,0 +1,43 @@
> +#ifndef __XEN_MSI_H_
> +#define __XEN_MSI_H_
> +
> +#include <xen/pci.h>
> +
> +#ifdef CONFIG_HAS_PCI_MSI
> +
> +#include <asm/msi.h>
> +
> +int pdev_msix_assign(struct domain *d, struct pci_dev *pdev);
> +int pdev_msi_init(struct pci_dev *pdev);
> +void pdev_msi_deinit(struct pci_dev *pdev);
> +void pdev_dump_msi(const struct pci_dev *pdev);
> +
> +#else /* !CONFIG_HAS_PCI_MSI */
> +
> +static inline int pdev_msix_assign(struct domain *d, struct pci_dev *pdev)
> +{
> + return 0;
> +}
> +
> +static inline int pdev_msi_init(struct pci_dev *pdev)
> +{
> + return 0;
> +}
> +
> +static inline void pdev_msi_deinit(struct pci_dev *pdev) {}
> +static inline void pci_cleanup_msi(struct pci_dev *pdev) {}
> +static inline void pdev_dump_msi(const struct pci_dev *pdev) {}
> +
> +#endif /* CONFIG_HAS_PCI_MSI */
> +
> +#endif /* __XEN_MSI_H */
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * tab-width: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/xsm/flask/hooks.c b/xen/xsm/flask/hooks.c
> index 25e87180b4..ea9a12bd71 100644
> --- a/xen/xsm/flask/hooks.c
> +++ b/xen/xsm/flask/hooks.c
> @@ -21,7 +21,7 @@
> #include <xen/guest_access.h>
> #include <xen/xenoprof.h>
> #include <xen/iommu.h>
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_PCI_MSI
> #include <asm/msi.h>
> #endif
> #include <public/xen.h>
> @@ -114,7 +114,7 @@ static int get_irq_sid(int irq, u32 *sid, struct avc_audit_data *ad)
> }
> return security_irq_sid(irq, sid);
> }
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_PCI_MSI
> {
> struct irq_desc *desc = irq_to_desc(irq);
> if ( desc->msi_desc && desc->msi_desc->dev ) {
> @@ -874,7 +874,7 @@ static int flask_map_domain_pirq (struct domain *d)
> static int flask_map_domain_msi (struct domain *d, int irq, const void *data,
> u32 *sid, struct avc_audit_data *ad)
> {
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_PCI_MSI
> const struct msi_info *msi = data;
> u32 machine_bdf = (msi->seg << 16) | (msi->bus << 8) | msi->devfn;
>
> @@ -940,7 +940,7 @@ static int flask_unmap_domain_pirq (struct domain *d)
> static int flask_unmap_domain_msi (struct domain *d, int irq, const void *data,
> u32 *sid, struct avc_audit_data *ad)
> {
> -#ifdef CONFIG_HAS_PCI
> +#ifdef CONFIG_HAS_PCI_MSI
> const struct pci_dev *pdev = data;
> u32 machine_bdf = (pdev->seg << 16) | (pdev->bus << 8) | pdev->devfn;
>
> --
> 2.17.1
>
On 29.09.2021 18:56, Stefano Stabellini wrote: > On Tue, 28 Sep 2021, Rahul Singh wrote: >> On Arm, the initial plan is to only support GICv3 ITS which doesn't >> require us to manage the MSIs because the HW will protect against >> spoofing. Move the code under CONFIG_HAS_PCI_MSI flag to gate the code >> for ARM. >> >> No functional change intended. >> >> Signed-off-by: Rahul Singh <rahul.singh@arm.com> >> Reviewed-by: Daniel P. Smith <dpsmith@apertussolutions.com> > > Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> Acked-by: Jan Beulich <jbeulich@suse.com> Nevertheless I'd like to express that ... >> --- a/xen/drivers/pci/Kconfig >> +++ b/xen/drivers/pci/Kconfig >> @@ -1,3 +1,7 @@ >> >> config HAS_PCI >> bool >> + >> +config HAS_PCI_MSI >> + bool >> + depends on HAS_PCI ... personally I think this should be "select HAS_PCI", and the then redundant one in x86 might then want dropping. Jan
© 2016 - 2024 Red Hat, Inc.