ACK: [SRU][F][G][PATCH v3 1/1] s390/pci: Fix zpci_alloc_domain() over allocation

[Frank Heimes]

Yes of course - done.
(Sorry, some of the tickets were opened when the dev. of 'groovy' was not
opened, and I forgot to adjust the nominations later ...)

BR, Frank


On Wed, May 13, 2020 at 11:56 AM Kleber Souza <kleber.souza at canonical.com>
wrote:

> On 13.05.20 11:54, Kleber Souza wrote:
> > On 06.05.20 09:21, frank.heimes at canonical.com wrote:
> >> From: Niklas Schnelle <schnelle at linux.ibm.com>
> >>
> >> BugLink: https://bugs.launchpad.net/bugs/1874057
>
> Hi Frank,
>
> I forgot to mention, can you please fix the nominations on the bug report?
>
>
> Thanks,
> Kleber
>
> >>
> >> Until now zpci_alloc_domain() only prevented more than
> >> CONFIG_PCI_NR_FUNCTIONS from being added when using automatic domain
> >> allocation. When explicit UIDs were defined UIDs above
> >> CONFIG_PCI_NR_FUNCTIONS were not counted at all.
> >> When more PCI functions are added this could lead to various errors
> >> including under sized IRQ vectors and similar issues.
> >>
> >> Fix this by explicitly tracking the number of allocated domains.
> >>
> >> Signed-off-by: Niklas Schnelle <schnelle at linux.ibm.com>
> >> Reviewed-by: Pierre Morel <pmorel at linux.ibm.com>
> >> Signed-off-by: Vasily Gorbik <gor at linux.ibm.com>
> >> (backported from commit 969ae01bab2fe938b4c8324836038b5ac1c78fac)
> >> Signed-off-by: Frank Heimes <frank.heimes at canonical.com>
> >
> >
> > Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>
> >
> >> ---
> >>  arch/s390/include/asm/pci.h |  1 +
> >>  arch/s390/pci/pci.c         | 34 ++++++++++++++++++++--------------
> >>  2 files changed, 21 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> >> index 6087a4e9b2bf..7850e8c8c79a 100644
> >> --- a/arch/s390/include/asm/pci.h
> >> +++ b/arch/s390/include/asm/pci.h
> >> @@ -28,6 +28,7 @@ int pci_proc_domain(struct pci_bus *);
> >>
> >>  #define ZPCI_NR_DMA_SPACES          1
> >>  #define ZPCI_NR_DEVICES                     CONFIG_PCI_NR_FUNCTIONS
> >> +#define ZPCI_DOMAIN_BITMAP_SIZE             (1 << 16)
> >>
> >>  /* PCI Function Controls */
> >>  #define ZPCI_FC_FN_ENABLED          0x80
> >> diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> >> index 6105b1b6e49b..0af46683dd66 100644
> >> --- a/arch/s390/pci/pci.c
> >> +++ b/arch/s390/pci/pci.c
> >> @@ -39,8 +39,9 @@
> >>  static LIST_HEAD(zpci_list);
> >>  static DEFINE_SPINLOCK(zpci_list_lock);
> >>
> >> -static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES);
> >> +static DECLARE_BITMAP(zpci_domain, ZPCI_DOMAIN_BITMAP_SIZE);
> >>  static DEFINE_SPINLOCK(zpci_domain_lock);
> >> +static unsigned int zpci_num_domains_allocated;
> >>
> >>  #define ZPCI_IOMAP_ENTRIES                                          \
> >>      min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2),      \
> >> @@ -651,39 +652,44 @@ struct dev_pm_ops pcibios_pm_ops = {
> >>
> >>  static int zpci_alloc_domain(struct zpci_dev *zdev)
> >>  {
> >> +    spin_lock(&zpci_domain_lock);
> >> +    if (zpci_num_domains_allocated > (ZPCI_NR_DEVICES - 1)) {
> >> +            spin_unlock(&zpci_domain_lock);
> >> +            pr_err("Adding PCI function %08x failed because the
> configured limit of %d is reached\n",
> >> +                    zdev->fid, ZPCI_NR_DEVICES);
> >> +            return -ENOSPC;
> >> +    }
> >> +
> >>      if (zpci_unique_uid) {
> >>              zdev->domain = (u16) zdev->uid;
> >> -            if (zdev->domain >= ZPCI_NR_DEVICES)
> >> -                    return 0;
> >> -
> >> -            spin_lock(&zpci_domain_lock);
> >>              if (test_bit(zdev->domain, zpci_domain)) {
> >>                      spin_unlock(&zpci_domain_lock);
> >> +                    pr_err("Adding PCI function %08x failed because
> domain %04x is already assigned\n",
> >> +                            zdev->fid, zdev->domain);
> >>                      return -EEXIST;
> >>              }
> >>              set_bit(zdev->domain, zpci_domain);
> >> +            zpci_num_domains_allocated++;
> >>              spin_unlock(&zpci_domain_lock);
> >>              return 0;
> >>      }
> >> -
> >> -    spin_lock(&zpci_domain_lock);
> >> +    /*
> >> +     * We can always auto allocate domains below ZPCI_NR_DEVICES.
> >> +     * There is either a free domain or we have reached the maximum in
> >> +     * which case we would have bailed earlier.
> >> +     */
> >>      zdev->domain = find_first_zero_bit(zpci_domain, ZPCI_NR_DEVICES);
> >> -    if (zdev->domain == ZPCI_NR_DEVICES) {
> >> -            spin_unlock(&zpci_domain_lock);
> >> -            return -ENOSPC;
> >> -    }
> >>      set_bit(zdev->domain, zpci_domain);
> >> +    zpci_num_domains_allocated++;
> >>      spin_unlock(&zpci_domain_lock);
> >>      return 0;
> >>  }
> >>
> >>  static void zpci_free_domain(struct zpci_dev *zdev)
> >>  {
> >> -    if (zdev->domain >= ZPCI_NR_DEVICES)
> >> -            return;
> >> -
> >>      spin_lock(&zpci_domain_lock);
> >>      clear_bit(zdev->domain, zpci_domain);
> >> +    zpci_num_domains_allocated--;
> >>      spin_unlock(&zpci_domain_lock);
> >>  }
> >>
> >>
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20200513/0f5c040b/attachment.html>