Move away from the legacy MSI domain setup, switch to use
msi_create_parent_irq_domain().
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
Cc: Nirmal Patel <nirmal.patel@linux.intel.com>
Cc: Jonathan Derrick <jonathan.derrick@linux.dev>
---
drivers/pci/controller/Kconfig | 1 +
drivers/pci/controller/vmd.c | 160 +++++++++++++++++----------------
2 files changed, 82 insertions(+), 79 deletions(-)
diff --git a/drivers/pci/controller/Kconfig b/drivers/pci/controller/Kconfig
index 8f56ffd029ba2..41748d083b933 100644
--- a/drivers/pci/controller/Kconfig
+++ b/drivers/pci/controller/Kconfig
@@ -156,6 +156,7 @@ config PCI_IXP4XX
config VMD
depends on PCI_MSI && X86_64 && !UML
tristate "Intel Volume Management Device Driver"
+ select IRQ_MSI_LIB
help
Adds support for the Intel Volume Management Device (VMD). VMD is a
secondary PCI host bridge that allows PCI Express root ports,
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index d9b893bf4e456..38693a9487d9b 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -7,6 +7,7 @@
#include <linux/device.h>
#include <linux/interrupt.h>
#include <linux/irq.h>
+#include <linux/irqchip/irq-msi-lib.h>
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/msi.h>
@@ -174,9 +175,6 @@ static void vmd_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
msg->arch_addr_lo.destid_0_7 = index_from_irqs(vmd, irq);
}
-/*
- * We rely on MSI_FLAG_USE_DEF_CHIP_OPS to set the IRQ mask/unmask ops.
- */
static void vmd_irq_enable(struct irq_data *data)
{
struct vmd_irq *vmdirq = data->chip_data;
@@ -186,7 +184,11 @@ static void vmd_irq_enable(struct irq_data *data)
list_add_tail_rcu(&vmdirq->node, &vmdirq->irq->irq_list);
vmdirq->enabled = true;
}
+}
+static void vmd_pci_msi_enable(struct irq_data *data)
+{
+ vmd_irq_enable(data->parent_data);
data->chip->irq_unmask(data);
}
@@ -194,8 +196,6 @@ static void vmd_irq_disable(struct irq_data *data)
{
struct vmd_irq *vmdirq = data->chip_data;
- data->chip->irq_mask(data);
-
scoped_guard(raw_spinlock_irqsave, &list_lock) {
if (vmdirq->enabled) {
list_del_rcu(&vmdirq->node);
@@ -204,19 +204,17 @@ static void vmd_irq_disable(struct irq_data *data)
}
}
+static void vmd_pci_msi_disable(struct irq_data *data)
+{
+ data->chip->irq_mask(data);
+ vmd_irq_disable(data->parent_data);
+}
+
static struct irq_chip vmd_msi_controller = {
.name = "VMD-MSI",
- .irq_enable = vmd_irq_enable,
- .irq_disable = vmd_irq_disable,
.irq_compose_msi_msg = vmd_compose_msi_msg,
};
-static irq_hw_number_t vmd_get_hwirq(struct msi_domain_info *info,
- msi_alloc_info_t *arg)
-{
- return 0;
-}
-
/*
* XXX: We can be even smarter selecting the best IRQ once we solve the
* affinity problem.
@@ -250,100 +248,110 @@ static struct vmd_irq_list *vmd_next_irq(struct vmd_dev *vmd, struct msi_desc *d
return &vmd->irqs[best];
}
-static int vmd_msi_init(struct irq_domain *domain, struct msi_domain_info *info,
- unsigned int virq, irq_hw_number_t hwirq,
- msi_alloc_info_t *arg)
+static void vmd_msi_free(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs);
+
+static int vmd_msi_alloc(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs,
+ void *arg)
{
- struct msi_desc *desc = arg->desc;
- struct vmd_dev *vmd = vmd_from_bus(msi_desc_to_pci_dev(desc)->bus);
- struct vmd_irq *vmdirq = kzalloc(sizeof(*vmdirq), GFP_KERNEL);
+ struct msi_desc *desc = ((msi_alloc_info_t *)arg)->desc;
+ struct vmd_dev *vmd = domain->host_data;
+ struct vmd_irq *vmdirq;
- if (!vmdirq)
- return -ENOMEM;
+ for (int i = 0; i < nr_irqs; ++i) {
+ vmdirq = kzalloc(sizeof(*vmdirq), GFP_KERNEL);
+ if (!vmdirq) {
+ vmd_msi_free(domain, virq, i);
+ return -ENOMEM;
+ }
- INIT_LIST_HEAD(&vmdirq->node);
- vmdirq->irq = vmd_next_irq(vmd, desc);
- vmdirq->virq = virq;
+ INIT_LIST_HEAD(&vmdirq->node);
+ vmdirq->irq = vmd_next_irq(vmd, desc);
+ vmdirq->virq = virq + i;
+
+ irq_domain_set_info(domain, virq + i, vmdirq->irq->virq, &vmd_msi_controller,
+ vmdirq, handle_untracked_irq, vmd, NULL);
+ }
- irq_domain_set_info(domain, virq, vmdirq->irq->virq, info->chip, vmdirq,
- handle_untracked_irq, vmd, NULL);
return 0;
}
-static void vmd_msi_free(struct irq_domain *domain,
- struct msi_domain_info *info, unsigned int virq)
+static void vmd_msi_free(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs)
{
struct vmd_irq *vmdirq = irq_get_chip_data(virq);
- synchronize_srcu(&vmdirq->irq->srcu);
+ for (int i = 0; i < nr_irqs; ++i) {
+ synchronize_srcu(&vmdirq->irq->srcu);
- /* XXX: Potential optimization to rebalance */
- scoped_guard(raw_spinlock_irq, &list_lock)
- vmdirq->irq->count--;
+ /* XXX: Potential optimization to rebalance */
+ scoped_guard(raw_spinlock_irq, &list_lock)
+ vmdirq->irq->count--;
- kfree(vmdirq);
+ kfree(vmdirq);
+ }
}
-static int vmd_msi_prepare(struct irq_domain *domain, struct device *dev,
- int nvec, msi_alloc_info_t *arg)
+static const struct irq_domain_ops vmd_msi_domain_ops = {
+ .alloc = vmd_msi_alloc,
+ .free = vmd_msi_free,
+};
+
+static bool vmd_init_dev_msi_info(struct device *dev, struct irq_domain *domain,
+ struct irq_domain *real_parent, struct msi_domain_info *info)
{
- struct pci_dev *pdev = to_pci_dev(dev);
- struct vmd_dev *vmd = vmd_from_bus(pdev->bus);
+ if (WARN_ON_ONCE(info->bus_token != DOMAIN_BUS_PCI_DEVICE_MSIX))
+ return false;
- if (nvec > vmd->msix_count)
- return vmd->msix_count;
+ if (!msi_lib_init_dev_msi_info(dev, domain, real_parent, info))
+ return false;
- memset(arg, 0, sizeof(*arg));
- return 0;
+ info->chip->irq_enable = vmd_pci_msi_enable;
+ info->chip->irq_disable = vmd_pci_msi_disable;
+ return true;
}
-static void vmd_set_desc(msi_alloc_info_t *arg, struct msi_desc *desc)
-{
- arg->desc = desc;
-}
+#define VMD_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK | MSI_FLAG_PCI_MSIX)
+#define VMD_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_NO_AFFINITY)
-static struct msi_domain_ops vmd_msi_domain_ops = {
- .get_hwirq = vmd_get_hwirq,
- .msi_init = vmd_msi_init,
- .msi_free = vmd_msi_free,
- .msi_prepare = vmd_msi_prepare,
- .set_desc = vmd_set_desc,
+static const struct msi_parent_ops vmd_msi_parent_ops = {
+ .supported_flags = VMD_MSI_FLAGS_SUPPORTED,
+ .required_flags = VMD_MSI_FLAGS_REQUIRED,
+ .bus_select_token = DOMAIN_BUS_VMD_MSI,
+ .bus_select_mask = MATCH_PCI_MSI,
+ .prefix = "VMD-",
+ .init_dev_msi_info = vmd_init_dev_msi_info,
};
-static struct msi_domain_info vmd_msi_domain_info = {
- .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS |
- MSI_FLAG_NO_AFFINITY | MSI_FLAG_PCI_MSIX,
- .ops = &vmd_msi_domain_ops,
- .chip = &vmd_msi_controller,
-};
-
-static void vmd_set_msi_remapping(struct vmd_dev *vmd, bool enable)
-{
- u16 reg;
-
- pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, ®);
- reg = enable ? (reg & ~VMCONFIG_MSI_REMAP) :
- (reg | VMCONFIG_MSI_REMAP);
- pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
-}
-
static int vmd_create_irq_domain(struct vmd_dev *vmd)
{
- struct fwnode_handle *fn;
+ struct irq_domain_info info = {
+ .size = vmd->msix_count,
+ .ops = &vmd_msi_domain_ops,
+ .host_data = vmd,
+ };
- fn = irq_domain_alloc_named_id_fwnode("VMD-MSI", vmd->sysdata.domain);
- if (!fn)
+ info.fwnode = irq_domain_alloc_named_id_fwnode("VMD-MSI", vmd->sysdata.domain);
+ if (!info.fwnode)
return -ENODEV;
- vmd->irq_domain = pci_msi_create_irq_domain(fn, &vmd_msi_domain_info, NULL);
+ vmd->irq_domain = msi_create_parent_irq_domain(&info, &vmd_msi_parent_ops);
if (!vmd->irq_domain) {
- irq_domain_free_fwnode(fn);
+ irq_domain_free_fwnode(info.fwnode);
return -ENODEV;
}
return 0;
}
+static void vmd_set_msi_remapping(struct vmd_dev *vmd, bool enable)
+{
+ u16 reg;
+
+ pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, ®);
+ reg = enable ? (reg & ~VMCONFIG_MSI_REMAP) :
+ (reg | VMCONFIG_MSI_REMAP);
+ pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
+}
+
static void vmd_remove_irq_domain(struct vmd_dev *vmd)
{
/*
@@ -874,12 +882,6 @@ static int vmd_enable_domain(struct vmd_dev *vmd, unsigned long features)
ret = vmd_create_irq_domain(vmd);
if (ret)
return ret;
-
- /*
- * Override the IRQ domain bus token so the domain can be
- * distinguished from a regular PCI/MSI domain.
- */
- irq_domain_update_bus_token(vmd->irq_domain, DOMAIN_BUS_VMD_MSI);
} else {
vmd_set_msi_remapping(vmd, false);
}
--
2.39.5Hi Nam,
On 26/06/2025 16:48, Nam Cao wrote:
[...]
> -static void vmd_msi_free(struct irq_domain *domain,
> - struct msi_domain_info *info, unsigned int virq)
> +static void vmd_msi_free(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs)
> {
> struct vmd_irq *vmdirq = irq_get_chip_data(virq);
>
> - synchronize_srcu(&vmdirq->irq->srcu);
> + for (int i = 0; i < nr_irqs; ++i) {
> + synchronize_srcu(&vmdirq->irq->srcu);
>
> - /* XXX: Potential optimization to rebalance */
> - scoped_guard(raw_spinlock_irq, &list_lock)
> - vmdirq->irq->count--;
> + /* XXX: Potential optimization to rebalance */
> + scoped_guard(raw_spinlock_irq, &list_lock)
> + vmdirq->irq->count--;
>
> - kfree(vmdirq);
> + kfree(vmdirq);
> + }
By introducing a for loop in this function, you are re-using vmdirq
after free'ing it.
I can't send a patch because I am not faimliar with this API and I don't
know how to fix it.
However, the issue was reported today by Coverity.
Any idea? :-)
Regards,
--
Antonio Quartulli
CEO and Co-Founder
Mandelbit Srl
https://www.mandelbit.com
On Wed, Jul 16, 2025 at 09:52:05PM +0200, Antonio Quartulli wrote:
> Hi Nam,
Hi Antonio,
> On 26/06/2025 16:48, Nam Cao wrote:
> [...]
> > -static void vmd_msi_free(struct irq_domain *domain,
> > - struct msi_domain_info *info, unsigned int virq)
> > +static void vmd_msi_free(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs)
> > {
> > struct vmd_irq *vmdirq = irq_get_chip_data(virq);
> > - synchronize_srcu(&vmdirq->irq->srcu);
> > + for (int i = 0; i < nr_irqs; ++i) {
> > + synchronize_srcu(&vmdirq->irq->srcu);
> > - /* XXX: Potential optimization to rebalance */
> > - scoped_guard(raw_spinlock_irq, &list_lock)
> > - vmdirq->irq->count--;
> > + /* XXX: Potential optimization to rebalance */
> > + scoped_guard(raw_spinlock_irq, &list_lock)
> > + vmdirq->irq->count--;
> > - kfree(vmdirq);
> > + kfree(vmdirq);
> > + }
>
> By introducing a for loop in this function, you are re-using vmdirq after
> free'ing it.
>
> I can't send a patch because I am not faimliar with this API and I don't
> know how to fix it.
>
> However, the issue was reported today by Coverity.
>
> Any idea? :-)
Thanks for the report. That was indeed a mistake from my side.
I hope PCI maintainers don't mind squashing the below diff.
Sorry for the troubles so far,
Nam
diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
index 48a6096cbbc0..50f0c91d561c 100644
--- a/drivers/pci/controller/vmd.c
+++ b/drivers/pci/controller/vmd.c
@@ -280,9 +280,11 @@ static int vmd_msi_alloc(struct irq_domain *domain, unsigned int virq,
static void vmd_msi_free(struct irq_domain *domain, unsigned int virq,
unsigned int nr_irqs)
{
- struct vmd_irq *vmdirq = irq_get_chip_data(virq);
+ struct vmd_irq *vmdirq;
for (int i = 0; i < nr_irqs; ++i) {
+ vmdirq = irq_get_chip_data(virq + i);
+
synchronize_srcu(&vmdirq->irq->srcu);
/* XXX: Potential optimization to rebalance */
On Wed, Jul 16, 2025 at 10:12:16PM +0200, Nam Cao wrote:
> On Wed, Jul 16, 2025 at 09:52:05PM +0200, Antonio Quartulli wrote:
> > Hi Nam,
> Hi Antonio,
>
> > On 26/06/2025 16:48, Nam Cao wrote:
> > [...]
> > > -static void vmd_msi_free(struct irq_domain *domain,
> > > - struct msi_domain_info *info, unsigned int virq)
> > > +static void vmd_msi_free(struct irq_domain *domain, unsigned int virq, unsigned int nr_irqs)
> > > {
> > > struct vmd_irq *vmdirq = irq_get_chip_data(virq);
> > > - synchronize_srcu(&vmdirq->irq->srcu);
> > > + for (int i = 0; i < nr_irqs; ++i) {
> > > + synchronize_srcu(&vmdirq->irq->srcu);
> > > - /* XXX: Potential optimization to rebalance */
> > > - scoped_guard(raw_spinlock_irq, &list_lock)
> > > - vmdirq->irq->count--;
> > > + /* XXX: Potential optimization to rebalance */
> > > + scoped_guard(raw_spinlock_irq, &list_lock)
> > > + vmdirq->irq->count--;
> > > - kfree(vmdirq);
> > > + kfree(vmdirq);
> > > + }
> >
> > By introducing a for loop in this function, you are re-using vmdirq after
> > free'ing it.
> >
> > I can't send a patch because I am not faimliar with this API and I don't
> > know how to fix it.
> >
> > However, the issue was reported today by Coverity.
> >
> > Any idea? :-)
>
> Thanks for the report. That was indeed a mistake from my side.
>
> I hope PCI maintainers don't mind squashing the below diff.
Squashed, thanks! Updated commit:
https://git.kernel.org/pub/scm/linux/kernel/git/pci/pci.git/commit/?id=4246b7fccf26
> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 48a6096cbbc0..50f0c91d561c 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -280,9 +280,11 @@ static int vmd_msi_alloc(struct irq_domain *domain, unsigned int virq,
> static void vmd_msi_free(struct irq_domain *domain, unsigned int virq,
> unsigned int nr_irqs)
> {
> - struct vmd_irq *vmdirq = irq_get_chip_data(virq);
> + struct vmd_irq *vmdirq;
>
> for (int i = 0; i < nr_irqs; ++i) {
> + vmdirq = irq_get_chip_data(virq + i);
> +
> synchronize_srcu(&vmdirq->irq->srcu);
>
> /* XXX: Potential optimization to rebalance */
>
On Thu, 26 Jun 2025 16:48:06 +0200
Nam Cao <namcao@linutronix.de> wrote:
> Move away from the legacy MSI domain setup, switch to use
> msi_create_parent_irq_domain().
>
> Signed-off-by: Nam Cao <namcao@linutronix.de>
> ---
> Cc: Nirmal Patel <nirmal.patel@linux.intel.com>
> Cc: Jonathan Derrick <jonathan.derrick@linux.dev>
> ---
> drivers/pci/controller/Kconfig | 1 +
> drivers/pci/controller/vmd.c | 160
> +++++++++++++++++---------------- 2 files changed, 82 insertions(+),
> 79 deletions(-)
>
> diff --git a/drivers/pci/controller/Kconfig
> b/drivers/pci/controller/Kconfig index 8f56ffd029ba2..41748d083b933
> 100644 --- a/drivers/pci/controller/Kconfig
> +++ b/drivers/pci/controller/Kconfig
> @@ -156,6 +156,7 @@ config PCI_IXP4XX
> config VMD
> depends on PCI_MSI && X86_64 && !UML
> tristate "Intel Volume Management Device Driver"
> + select IRQ_MSI_LIB
> help
> Adds support for the Intel Volume Management Device (VMD).
> VMD is a secondary PCI host bridge that allows PCI Express root ports,
> diff --git a/drivers/pci/controller/vmd.c
> b/drivers/pci/controller/vmd.c index d9b893bf4e456..38693a9487d9b
> 100644 --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
> @@ -7,6 +7,7 @@
> #include <linux/device.h>
> #include <linux/interrupt.h>
> #include <linux/irq.h>
> +#include <linux/irqchip/irq-msi-lib.h>
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/msi.h>
> @@ -174,9 +175,6 @@ static void vmd_compose_msi_msg(struct irq_data
> *data, struct msi_msg *msg) msg->arch_addr_lo.destid_0_7 =
> index_from_irqs(vmd, irq); }
>
> -/*
> - * We rely on MSI_FLAG_USE_DEF_CHIP_OPS to set the IRQ mask/unmask
> ops.
> - */
> static void vmd_irq_enable(struct irq_data *data)
> {
> struct vmd_irq *vmdirq = data->chip_data;
> @@ -186,7 +184,11 @@ static void vmd_irq_enable(struct irq_data *data)
> list_add_tail_rcu(&vmdirq->node,
> &vmdirq->irq->irq_list); vmdirq->enabled = true;
> }
> +}
>
> +static void vmd_pci_msi_enable(struct irq_data *data)
> +{
> + vmd_irq_enable(data->parent_data);
> data->chip->irq_unmask(data);
> }
>
> @@ -194,8 +196,6 @@ static void vmd_irq_disable(struct irq_data *data)
> {
> struct vmd_irq *vmdirq = data->chip_data;
>
> - data->chip->irq_mask(data);
> -
> scoped_guard(raw_spinlock_irqsave, &list_lock) {
> if (vmdirq->enabled) {
> list_del_rcu(&vmdirq->node);
> @@ -204,19 +204,17 @@ static void vmd_irq_disable(struct irq_data
> *data) }
> }
>
> +static void vmd_pci_msi_disable(struct irq_data *data)
> +{
> + data->chip->irq_mask(data);
> + vmd_irq_disable(data->parent_data);
> +}
> +
> static struct irq_chip vmd_msi_controller = {
> .name = "VMD-MSI",
> - .irq_enable = vmd_irq_enable,
> - .irq_disable = vmd_irq_disable,
> .irq_compose_msi_msg = vmd_compose_msi_msg,
> };
>
> -static irq_hw_number_t vmd_get_hwirq(struct msi_domain_info *info,
> - msi_alloc_info_t *arg)
> -{
> - return 0;
> -}
> -
> /*
> * XXX: We can be even smarter selecting the best IRQ once we solve
> the
> * affinity problem.
> @@ -250,100 +248,110 @@ static struct vmd_irq_list
> *vmd_next_irq(struct vmd_dev *vmd, struct msi_desc *d return
> &vmd->irqs[best]; }
>
> -static int vmd_msi_init(struct irq_domain *domain, struct
> msi_domain_info *info,
> - unsigned int virq, irq_hw_number_t hwirq,
> - msi_alloc_info_t *arg)
> +static void vmd_msi_free(struct irq_domain *domain, unsigned int
> virq, unsigned int nr_irqs); +
> +static int vmd_msi_alloc(struct irq_domain *domain, unsigned int
> virq, unsigned int nr_irqs,
> + void *arg)
Is this wrapped in 80 columns? I can see few lines are more than 80.
Disregard this if it is wrapped and it can be my claws mail client
issue.
> {
> - struct msi_desc *desc = arg->desc;
> - struct vmd_dev *vmd =
> vmd_from_bus(msi_desc_to_pci_dev(desc)->bus);
> - struct vmd_irq *vmdirq = kzalloc(sizeof(*vmdirq),
> GFP_KERNEL);
> + struct msi_desc *desc = ((msi_alloc_info_t *)arg)->desc;
> + struct vmd_dev *vmd = domain->host_data;
> + struct vmd_irq *vmdirq;
>
> - if (!vmdirq)
> - return -ENOMEM;
> + for (int i = 0; i < nr_irqs; ++i) {
> + vmdirq = kzalloc(sizeof(*vmdirq), GFP_KERNEL);
> + if (!vmdirq) {
> + vmd_msi_free(domain, virq, i);
> + return -ENOMEM;
> + }
>
> - INIT_LIST_HEAD(&vmdirq->node);
> - vmdirq->irq = vmd_next_irq(vmd, desc);
> - vmdirq->virq = virq;
> + INIT_LIST_HEAD(&vmdirq->node);
> + vmdirq->irq = vmd_next_irq(vmd, desc);
> + vmdirq->virq = virq + i;
> +
> + irq_domain_set_info(domain, virq + i,
> vmdirq->irq->virq, &vmd_msi_controller,
> + vmdirq, handle_untracked_irq,
> vmd, NULL);
> + }
>
> - irq_domain_set_info(domain, virq, vmdirq->irq->virq,
> info->chip, vmdirq,
> - handle_untracked_irq, vmd, NULL);
> return 0;
> }
>
> -static void vmd_msi_free(struct irq_domain *domain,
> - struct msi_domain_info *info, unsigned int
> virq) +static void vmd_msi_free(struct irq_domain *domain, unsigned
> int virq, unsigned int nr_irqs) {
> struct vmd_irq *vmdirq = irq_get_chip_data(virq);
>
> - synchronize_srcu(&vmdirq->irq->srcu);
> + for (int i = 0; i < nr_irqs; ++i) {
> + synchronize_srcu(&vmdirq->irq->srcu);
>
> - /* XXX: Potential optimization to rebalance */
> - scoped_guard(raw_spinlock_irq, &list_lock)
> - vmdirq->irq->count--;
> + /* XXX: Potential optimization to rebalance */
> + scoped_guard(raw_spinlock_irq, &list_lock)
> + vmdirq->irq->count--;
>
> - kfree(vmdirq);
> + kfree(vmdirq);
> + }
> }
>
> -static int vmd_msi_prepare(struct irq_domain *domain, struct device
> *dev,
> - int nvec, msi_alloc_info_t *arg)
> +static const struct irq_domain_ops vmd_msi_domain_ops = {
> + .alloc = vmd_msi_alloc,
> + .free = vmd_msi_free,
> +};
> +
> +static bool vmd_init_dev_msi_info(struct device *dev, struct
> irq_domain *domain,
> + struct irq_domain *real_parent,
> struct msi_domain_info *info) {
> - struct pci_dev *pdev = to_pci_dev(dev);
> - struct vmd_dev *vmd = vmd_from_bus(pdev->bus);
> + if (WARN_ON_ONCE(info->bus_token !=
> DOMAIN_BUS_PCI_DEVICE_MSIX))
> + return false;
>
> - if (nvec > vmd->msix_count)
> - return vmd->msix_count;
> + if (!msi_lib_init_dev_msi_info(dev, domain, real_parent,
> info))
> + return false;
>
> - memset(arg, 0, sizeof(*arg));
> - return 0;
> + info->chip->irq_enable = vmd_pci_msi_enable;
> + info->chip->irq_disable = vmd_pci_msi_disable;
> + return true;
> }
>
> -static void vmd_set_desc(msi_alloc_info_t *arg, struct msi_desc
> *desc) -{
> - arg->desc = desc;
> -}
> +#define VMD_MSI_FLAGS_SUPPORTED
> (MSI_GENERIC_FLAGS_MASK | MSI_FLAG_PCI_MSIX) +#define
> VMD_MSI_FLAGS_REQUIRED (MSI_FLAG_USE_DEF_DOM_OPS |
> MSI_FLAG_NO_AFFINITY) -static struct msi_domain_ops
> vmd_msi_domain_ops = {
> - .get_hwirq = vmd_get_hwirq,
> - .msi_init = vmd_msi_init,
> - .msi_free = vmd_msi_free,
> - .msi_prepare = vmd_msi_prepare,
> - .set_desc = vmd_set_desc,
> +static const struct msi_parent_ops vmd_msi_parent_ops = {
> + .supported_flags = VMD_MSI_FLAGS_SUPPORTED,
> + .required_flags = VMD_MSI_FLAGS_REQUIRED,
> + .bus_select_token = DOMAIN_BUS_VMD_MSI,
> + .bus_select_mask = MATCH_PCI_MSI,
> + .prefix = "VMD-",
> + .init_dev_msi_info = vmd_init_dev_msi_info,
> };
>
> -static struct msi_domain_info vmd_msi_domain_info = {
> - .flags = MSI_FLAG_USE_DEF_DOM_OPS |
> MSI_FLAG_USE_DEF_CHIP_OPS |
> - MSI_FLAG_NO_AFFINITY | MSI_FLAG_PCI_MSIX,
> - .ops = &vmd_msi_domain_ops,
> - .chip = &vmd_msi_controller,
> -};
> -
> -static void vmd_set_msi_remapping(struct vmd_dev *vmd, bool enable)
> -{
> - u16 reg;
> -
> - pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, ®);
> - reg = enable ? (reg & ~VMCONFIG_MSI_REMAP) :
> - (reg | VMCONFIG_MSI_REMAP);
> - pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
> -}
> -
> static int vmd_create_irq_domain(struct vmd_dev *vmd)
> {
> - struct fwnode_handle *fn;
> + struct irq_domain_info info = {
> + .size = vmd->msix_count,
> + .ops = &vmd_msi_domain_ops,
> + .host_data = vmd,
> + };
>
> - fn = irq_domain_alloc_named_id_fwnode("VMD-MSI",
> vmd->sysdata.domain);
> - if (!fn)
> + info.fwnode = irq_domain_alloc_named_id_fwnode("VMD-MSI",
> vmd->sysdata.domain);
> + if (!info.fwnode)
> return -ENODEV;
>
> - vmd->irq_domain = pci_msi_create_irq_domain(fn,
> &vmd_msi_domain_info, NULL);
> + vmd->irq_domain = msi_create_parent_irq_domain(&info,
> &vmd_msi_parent_ops); if (!vmd->irq_domain) {
> - irq_domain_free_fwnode(fn);
> + irq_domain_free_fwnode(info.fwnode);
> return -ENODEV;
> }
>
> return 0;
> }
>
> +static void vmd_set_msi_remapping(struct vmd_dev *vmd, bool enable)
> +{
> + u16 reg;
> +
> + pci_read_config_word(vmd->dev, PCI_REG_VMCONFIG, ®);
> + reg = enable ? (reg & ~VMCONFIG_MSI_REMAP) :
> + (reg | VMCONFIG_MSI_REMAP);
> + pci_write_config_word(vmd->dev, PCI_REG_VMCONFIG, reg);
> +}
> +
> static void vmd_remove_irq_domain(struct vmd_dev *vmd)
> {
> /*
> @@ -874,12 +882,6 @@ static int vmd_enable_domain(struct vmd_dev
> *vmd, unsigned long features) ret = vmd_create_irq_domain(vmd);
> if (ret)
> return ret;
> -
> - /*
> - * Override the IRQ domain bus token so the domain
> can be
> - * distinguished from a regular PCI/MSI domain.
> - */
> - irq_domain_update_bus_token(vmd->irq_domain,
> DOMAIN_BUS_VMD_MSI); } else {
> vmd_set_msi_remapping(vmd, false);
> }
On Wed, Jul 16, 2025 at 11:10:09AM -0700, Nirmal Patel wrote: > On Thu, 26 Jun 2025 16:48:06 +0200 > Nam Cao <namcao@linutronix.de> wrote: > > > Move away from the legacy MSI domain setup, switch to use > > msi_create_parent_irq_domain(). > > - unsigned int virq, irq_hw_number_t hwirq, > > - msi_alloc_info_t *arg) > > +static void vmd_msi_free(struct irq_domain *domain, unsigned int > > virq, unsigned int nr_irqs); + > > +static int vmd_msi_alloc(struct irq_domain *domain, unsigned int > > virq, unsigned int nr_irqs, > > + void *arg) > > Is this wrapped in 80 columns? I can see few lines are more than 80. > Disregard this if it is wrapped and it can be my claws mail client > issue. Wrapped locally, thanks Nirmal. Bjorn
On Thu, Jun 26 2025 at 16:48, Nam Cao wrote: > Move away from the legacy MSI domain setup, switch to use > msi_create_parent_irq_domain(). > > Signed-off-by: Nam Cao <namcao@linutronix.de> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
© 2016 - 2025 Red Hat, Inc.