From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ozlabs.org (ozlabs.org [IPv6:2401:3900:2:1::2]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id F32571A0541 for ; Fri, 7 Aug 2015 12:33:39 +1000 (AEST) Received: from e28smtp01.in.ibm.com (e28smtp01.in.ibm.com [122.248.162.1]) (using TLSv1 with cipher CAMELLIA256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 520891402A8 for ; Fri, 7 Aug 2015 12:33:39 +1000 (AEST) Received: from /spool/local by e28smtp01.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Fri, 7 Aug 2015 08:03:37 +0530 Received: from d28relay04.in.ibm.com (d28relay04.in.ibm.com [9.184.220.61]) by d28dlp01.in.ibm.com (Postfix) with ESMTP id 61A01E005B for ; Fri, 7 Aug 2015 08:07:51 +0530 (IST) Received: from d28av03.in.ibm.com (d28av03.in.ibm.com [9.184.220.65]) by d28relay04.in.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t772XZe428835918 for ; Fri, 7 Aug 2015 08:03:35 +0530 Received: from d28av03.in.ibm.com (localhost [127.0.0.1]) by d28av03.in.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t772XaL8010609 for ; Fri, 7 Aug 2015 08:03:36 +0530 Date: Fri, 7 Aug 2015 10:33:33 +0800 From: Wei Yang To: Gavin Shan Cc: Wei Yang , aik@ozlabs.ru, benh@kernel.crashing.org, linuxppc-dev@ozlabs.org Subject: Re: [PATCH V2 6/6] powerpc/powernv: allocate discrete PE# when using M64 BAR in Single PE mode Message-ID: <20150807023333.GH8292@richard> Reply-To: Wei Yang References: <20150731020148.GA6151@richard> <1438737903-10399-1-git-send-email-weiyang@linux.vnet.ibm.com> <1438737903-10399-7-git-send-email-weiyang@linux.vnet.ibm.com> <20150806053601.GB5636@gwshan> <20150806134141.GA6235@richard> <20150807013656.GB20794@gwshan> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii In-Reply-To: <20150807013656.GB20794@gwshan> List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Fri, Aug 07, 2015 at 11:36:56AM +1000, Gavin Shan wrote: >On Thu, Aug 06, 2015 at 09:41:41PM +0800, Wei Yang wrote: >>On Thu, Aug 06, 2015 at 03:36:01PM +1000, Gavin Shan wrote: >>>On Wed, Aug 05, 2015 at 09:25:03AM +0800, Wei Yang wrote: >>>>When M64 BAR is set to Single PE mode, the PE# assigned to VF could be >>>>discrete. >>>> >>>>This patch restructures the patch to allocate discrete PE# for VFs when M64 >>>>BAR is set to Single PE mode. >>>> >>>>Signed-off-by: Wei Yang >>>>--- >>>> arch/powerpc/include/asm/pci-bridge.h | 2 +- >>>> arch/powerpc/platforms/powernv/pci-ioda.c | 69 +++++++++++++++++++++-------- >>>> 2 files changed, 51 insertions(+), 20 deletions(-) >>>> >>>>diff --git a/arch/powerpc/include/asm/pci-bridge.h b/arch/powerpc/include/asm/pci-bridge.h >>>>index 8aeba4c..72415c7 100644 >>>>--- a/arch/powerpc/include/asm/pci-bridge.h >>>>+++ b/arch/powerpc/include/asm/pci-bridge.h >>>>@@ -213,7 +213,7 @@ struct pci_dn { >>>> #ifdef CONFIG_PCI_IOV >>>> u16 vfs_expanded; /* number of VFs IOV BAR expanded */ >>>> u16 num_vfs; /* number of VFs enabled*/ >>>>- int offset; /* PE# for the first VF PE */ >>>>+ int *offset; /* PE# for the first VF PE or array */ >>>> bool m64_single_mode; /* Use M64 BAR in Single Mode */ >>>> #define IODA_INVALID_M64 (-1) >>>> int (*m64_map)[PCI_SRIOV_NUM_BARS]; >>> >>>how about renaming "offset" to "pe_num_map", or "pe_map" ? Similar to the comments >>>I gave to the "m64_bar_map", num_of_max_vfs entries can be allocated. Though not >>>all of them will be used, not too much memory will be wasted. >>> >> >>Thanks for your comment. >> >>I have thought about change the name to make it more self explain. While >>another fact I want to take in is this field is also used to be reflect the >>shift offset when M64 BAR is used in the Shared Mode. So I maintain the name. >> >>How about use "enum", one maintain the name "offset", and another one rename to >>"pe_num_map". And use the meaningful name at proper place? >> So I suppose you agree with my naming proposal. > >Ok. I'm explaining it with more details. There are two cases: single vs shared >mode. When PHB M64 BARs run in single mode, you need an array to track the >allocated discrete PE#. The VF_index is the index to the array. When PHB M64 >BARs run in shared mode, you need continuous PE#. No array required for this >case. Instead, the starting PE# should be stored to somewhere, which can >be pdn->offset[0] simply. > >So when allocating memory for this array, you just simply allocate (sizeof(*pdn->offset) >*max_vf_num) no matter what mode PHB's M64 BARs will run in. The point is nobody >can enable (max_vf_num + 1) VFs. The max_vf_num is 15? > >With above way, the arrays for PE# and M64 BAR remapping needn't be allocated >when enabling SRIOV capability and releasing on disabling SRIOV capability. >Instead, those two arrays can be allocated during resource fixup time and free'ed >when destroying the pdn. > My same point of view like previous, if the memory is not in the concern, how about define them static? And for the long term, we may support more VFs. Then at that moment, we need to restructure the code to meet it. So I suggest if we want to allocate it dynamically, we allocate the exact number of space. -- Richard Yang Help you, Help me