lkml.org 
[lkml]   [2023]   [Mar]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [patch 05/39] genirq/msi: Remove filter from msi_free_descs_free_range()
    Date
    Miquel!

    On Wed, Mar 01 2023 at 11:55, Miquel Raynal wrote:
    > tglx@linutronix.de wrote on Fri, 11 Nov 2022 14:54:22 +0100 (CET):
    >
    >> When a range of descriptors is freed then all of them are not associated to
    >> a linux interrupt. Remove the filter and add a warning to the free function.
    >> + /* Leak the descriptor when it is still referenced */
    >> + if (WARN_ON_ONCE(msi_desc_match(desc, MSI_DESC_ASSOCIATED)))
    >> + continue;
    >> + msi_free_desc(desc);
    >> }
    >> }
    >
    > It looks like since this commit I am getting warnings upon EPROBE_DEFER
    > errors in the mvpp2 Marvell Ethernet driver. I looked a bit at the
    > internals to understand why this warning was shown, and it seems that
    > nothing "de-references" the descriptors, which would mean here:
    > resetting desc->irq to 0.

    Correct. This platform-msi ^(*&!@&^ hack really needs to die ASAP.

    Marc, where are we on that? Is this still in limbo?

    > I am wondering how useful thisd WARN_ON() is, or otherwise where the

    It is useful as it caught bugs already.

    > desc->irq entry should be zeroed (if I understand that correctly), any
    > help will be appreciated.

    Untested workaround below. I hate it with a passion, but *shrug*.

    Thanks,

    tglx
    ---
    drivers/base/platform-msi.c | 1 +
    include/linux/msi.h | 2 ++
    kernel/irq/msi.c | 23 ++++++++++++++++++++++-
    3 files changed, 25 insertions(+), 1 deletion(-)

    --- a/drivers/base/platform-msi.c
    +++ b/drivers/base/platform-msi.c
    @@ -324,6 +324,7 @@ void platform_msi_device_domain_free(str
    struct platform_msi_priv_data *data = domain->host_data;

    msi_lock_descs(data->dev);
    + msi_domain_depopulate_descs(data->dev, virq, nr_irqs);
    irq_domain_free_irqs_common(domain, virq, nr_irqs);
    msi_free_msi_descs_range(data->dev, virq, virq + nr_irqs - 1);
    msi_unlock_descs(data->dev);
    --- a/include/linux/msi.h
    +++ b/include/linux/msi.h
    @@ -631,6 +631,8 @@ int msi_domain_prepare_irqs(struct irq_d
    int nvec, msi_alloc_info_t *args);
    int msi_domain_populate_irqs(struct irq_domain *domain, struct device *dev,
    int virq, int nvec, msi_alloc_info_t *args);
    +void msi_domain_depopulate_descs(struct device *dev, int virq, int nvec);
    +
    struct irq_domain *
    __platform_msi_create_device_domain(struct device *dev,
    unsigned int nvec,
    --- a/kernel/irq/msi.c
    +++ b/kernel/irq/msi.c
    @@ -1109,14 +1109,35 @@ int msi_domain_populate_irqs(struct irq_
    return 0;

    fail:
    - for (--virq; virq >= virq_base; virq--)
    + for (--virq; virq >= virq_base; virq--) {
    + msi_domain_depopulate_descs(dev, virq, 1);
    irq_domain_free_irqs_common(domain, virq, 1);
    + }
    msi_domain_free_descs(dev, &ctrl);
    unlock:
    msi_unlock_descs(dev);
    return ret;
    }

    +void msi_domain_depopulate_descs(struct device *dev, int virq_base, int nvec)
    +{
    + struct msi_ctrl ctrl = {
    + .domid = MSI_DEFAULT_DOMAIN,
    + .first = virq_base,
    + .last = virq_base + nvec - 1,
    + };
    + struct msi_desc *desc;
    + struct xarray *xa;
    + unsigned long idx;
    +
    + if (!msi_ctrl_valid(dev, &ctrl))
    + return;
    +
    + xa = &dev->msi.data->__domains[ctrl.domid].store;
    + xa_for_each_range(xa, idx, desc, ctrl.first, ctrl.last)
    + desc->irq = 0;
    +}
    +
    /*
    * Carefully check whether the device can use reservation mode. If
    * reservation mode is enabled then the early activation will assign a
    \
     
     \ /
      Last update: 2023-03-27 00:41    [W:2.215 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site