From:   Thomas Gleixner <tglx@linutronix.de>
To:     Marc Zyngier <maz@kernel.org>
Cc:     LKML <linux-kernel@vger.kernel.org>,
        Bjorn Helgaas <helgaas@kernel.org>,
        Alex Williamson <alex.williamson@redhat.com>,
        Kevin Tian <kevin.tian@intel.com>,
        Jason Gunthorpe <jgg@nvidia.com>,
        Megha Dey <megha.dey@intel.com>,
        Ashok Raj <ashok.raj@intel.com>,
        Michael Ellerman <mpe@ellerman.id.au>,
        Andrew Cooper <amc96@cam.ac.uk>,
        Juergen Gross <jgross@suse.com>, linux-pci@vger.kernel.org,
        xen-devel@lists.xenproject.org
Subject: Re: [patch 03/10] genirq/msi: Make MSI descriptor alloc/free ready
 for range allocations
In-Reply-To: <8735ngs26o.wl-maz@kernel.org>
References: <20211126233124.618283684@linutronix.de>
 <20211127000918.664542907@linutronix.de> <8735ngs26o.wl-maz@kernel.org>
Date:   Sun, 28 Nov 2021 20:17:56 +0100
Message-ID: <87a6hof5sr.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Sun, Nov 28 2021 at 15:57, Marc Zyngier wrote:
> On Sat, 27 Nov 2021 01:24:34 +0000,
> Thomas Gleixner <tglx@linutronix.de> wrote:
>
> The issue seems to be originating in the previous patch, where the
> following line was added:
>
> +	struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, };
>
> In that context, only 'ndesc' was used, and that was fine.
>
> However, in the current patch, ndesc use is removed, only first/last
> are considered, and UINT_MAX is... a lot of MSIs.
>
> This fixes it:
>
> diff --git a/kernel/irq/msi.c b/kernel/irq/msi.c
> index bef5b74a7268..a520bfd94a56 100644
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -975,7 +975,7 @@ int msi_domain_alloc_irqs_descs_locked(struct irq_domain *domain, struct device
>   */
>  int msi_domain_alloc_irqs(struct irq_domain *domain, struct device *dev, int nvec)
>  {
> -	struct msi_range range = { .first = 0, .last = UINT_MAX, .ndesc = nvec, };
> +	struct msi_range range = { .first = 0, .last = nvec - 1, .ndesc = nvec, };
>  	int ret;
>  
>  	msi_lock_descs(dev);
>
> However, it'd be good to clarify the use of range->ndesc.

Hrm. The stupid search should terminated nevertheless. Let me stare at
it again.

>> -static int msi_add_simple_msi_descs(struct device *dev, unsigned int index, unsigned int ndesc)
>> +static int msi_add_simple_msi_descs(struct device *dev, struct msi_range *range)
>
> nit: most of the functions changed in this patch need to have their
> documentation tidied up.

Duh, yes.

Thanks,

        tglx