Re: [PATCH V11 03/17] PCI: Add weak pcibios_iov_resource_alignment() interface

[Wei Yang <weiyang@linux.vnet.ibm.com>]

On Tue, Feb 10, 2015 at 11:32:59AM +1100, Benjamin Herrenschmidt wrote:
>On Thu, 2015-01-15 at 10:27 +0800, Wei Yang wrote:
>> The alignment of PF's IOV BAR is designed to be the individual size of a
>> VF's BAR size. This works fine for many platforms, but on PowerNV platform
>> it needs some change.
>> 
>> The original alignment works, since at sizing and assigning stage the
>> requirement is from an individual VF's BAR size instead of the PF's IOV
>> BAR.  This is the reason for the original code to just retrieve the
>> individual VF BAR size as the alignment.
>> 
>> On PowerNV platform, it is required to align the whole PF IOV BAR to a
>> hardware segment. Based on this fact, the alignment of PF's IOV BAR should
>> be calculated seperately.
>> 
>> This patch introduces a weak pcibios_iov_resource_alignment() interface,
>> which gives platform a chance to implement specific method to calculate
>> the PF's IOV BAR alignment.
>
>While the patch is probably fine, I find the above explanation quite
>confusing :)
>

I will try to make it more clear.

>>From my memory (vague now) of the scheme we put in place, we need to
>practically reserve a portion of address space that corresponds to
>VF_size * Number_of_PEs. IE, it's not just the alignment that has
>constraints but also the size that need to be allocated.
>
>Now I suppose if we make the alignment to be the size of the M64
>window and if the core also bounces the allocated size to the
>alignment boundary, then we are fine, but that should be explained.
>

The purpose of this patch is to give a chance to different archs to calculate
the alignment of PF's IOV BAR.

How about I move the detailed explanation on powernv platform in the following
patch? And focus on what this patch does in this log?

>Cheers,
>Ben.
>
>
>> Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>> ---
>>  drivers/pci/iov.c   |   11 ++++++++++-
>>  include/linux/pci.h |    3 +++
>>  2 files changed, 13 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/pci/iov.c b/drivers/pci/iov.c
>> index 933d8cc..5f48201 100644
>> --- a/drivers/pci/iov.c
>> +++ b/drivers/pci/iov.c
>> @@ -556,6 +556,12 @@ int pci_iov_resource_bar(struct pci_dev *dev, int resno)
>>  		4 * (resno - PCI_IOV_RESOURCES);
>>  }
>>  
>> +resource_size_t __weak pcibios_iov_resource_alignment(struct pci_dev *dev,
>> +		int resno, resource_size_t align)
>> +{
>> +	return align;
>> +}
>> +
>>  /**
>>   * pci_sriov_resource_alignment - get resource alignment for VF BAR
>>   * @dev: the PCI device
>> @@ -570,12 +576,15 @@ resource_size_t pci_sriov_resource_alignment(struct pci_dev *dev, int resno)
>>  {
>>  	struct resource tmp;
>>  	int reg = pci_iov_resource_bar(dev, resno);
>> +	resource_size_t align;
>>  
>>  	if (!reg)
>>  		return 0;
>>  
>>  	__pci_read_base(dev, pci_bar_unknown, &tmp, reg);
>> -	return resource_alignment(&tmp);
>> +	align = resource_alignment(&tmp);
>> +
>> +	return pcibios_iov_resource_alignment(dev, resno, align);
>>  }
>>  
>>  /**
>> diff --git a/include/linux/pci.h b/include/linux/pci.h
>> index 74ef944..ae7a7ea 100644
>> --- a/include/linux/pci.h
>> +++ b/include/linux/pci.h
>> @@ -1163,6 +1163,9 @@ unsigned char pci_bus_max_busnr(struct pci_bus *bus);
>>  void pci_setup_bridge(struct pci_bus *bus);
>>  resource_size_t pcibios_window_alignment(struct pci_bus *bus,
>>  					 unsigned long type);
>> +resource_size_t pcibios_iov_resource_alignment(struct pci_dev *dev,
>> +						 int resno,
>> +						 resource_size_t align);
>>  
>>  #define PCI_VGA_STATE_CHANGE_BRIDGE (1 << 0)
>>  #define PCI_VGA_STATE_CHANGE_DECODES (1 << 1)
>

-- 
Richard Yang
Help you, Help me