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

[Kleber Souza]

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);
>>  }
>>  
>>
>