Re: [PATCH V2 3/6] powerpc/powernv: use one M64 BAR in Single PE mode for one VF BAR

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

On Thu, Aug 06, 2015 at 03:20:25PM +1000, Gavin Shan wrote:
>On Wed, Aug 05, 2015 at 09:25:00AM +0800, Wei Yang wrote:
>>In current implementation, when VF BAR is bigger than 64MB, it uses 4 M64
>>BAR in Single PE mode to cover the number of VFs required to be enabled.
>>By doing so, several VFs would be in one VF Group and leads to interference
>>between VFs in the same group.
>>
>>This patch changes the design by using one M64 BAR in Single PE mode for
>>one VF BAR. This gives absolute isolation for VFs.
>>
>>Signed-off-by: Wei Yang <weiyang@linux.vnet.ibm.com>
>>---
>> arch/powerpc/include/asm/pci-bridge.h     |    5 +-
>> arch/powerpc/platforms/powernv/pci-ioda.c |  180 ++++++++++++-----------------
>> 2 files changed, 76 insertions(+), 109 deletions(-)
>>
>>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h
>>index 712add5..8aeba4c 100644
>>--- a/arch/powerpc/include/asm/pci-bridge.h
>>+++ b/arch/powerpc/include/asm/pci-bridge.h
>>@@ -214,10 +214,9 @@ struct pci_dn {
>> 	u16     vfs_expanded;		/* number of VFs IOV BAR expanded */
>> 	u16     num_vfs;		/* number of VFs enabled*/
>> 	int     offset;			/* PE# for the first VF PE */
>>-#define M64_PER_IOV 4
>>-	int     m64_per_iov;
>>+	bool    m64_single_mode;	/* Use M64 BAR in Single Mode */
>> #define IODA_INVALID_M64        (-1)
>>-	int     m64_wins[PCI_SRIOV_NUM_BARS][M64_PER_IOV];
>>+	int     (*m64_map)[PCI_SRIOV_NUM_BARS];
>
>It can be explicit? For example:
>
>	int	*m64_map;
>
>	/* Initialization */
>	size_t size = sizeof(*pdn->m64_map) * PCI_SRIOV_NUM_BARS * num_of_max_VFs;
>	pdn->m64_map = kmalloc(size, GFP_KERNEL);
>	for (i = 0; i < PCI_SRIOV_NUM_BARS; i++)
>		for (j = 0; j < num_of_max_VFs; j++)
>			pdn->m64_map[i * PCI_SRIOV_NUM_BARS + j] = PNV_INVALID_M64;
>
>	/* Destroy */
>	int step = 1;
>
>	if (!pdn->m64_single_mode)
>		step = phb->ioda.total_pe;
>	for (i = 0; i < PCI_SRIOV_NUM_BARS * num_of_max_VFs; i += step)
>		if (pdn->m64_map[i] == PNV_INVALID_M64)
>			continue;
>
>		/* Unmap the window */
>	

The m64_map is a pointer to an array with 6 elements, which represents the 6
M64 BAR index for the 6 VF BARs.

    When we use Shared Mode, one array is allocated. The six elements
    represents the six M64 BAR(at most) used to map the whole IOV BAR.
    
    When we use Single Mode, num_vfs array is allocate. Each array represents
    the map between one VF's BAR and M64 BAR index.

During the map and un-map, M64 BAR is assigned one by one in VF BAR's order.
So I think the code is explicit.

In your code, you allocate a big one dimension array to hold the M64 BAR
index. It works, while I don't think this is more explicit than original code.

-- 
Richard Yang
Help you, Help me