| Message ID | 17e7ad65ff5.d9de88b4962.1109678039880040918@zohomail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v4] PCI: endpoint: Add prefetch BAR support | expand |
ping. PS: this patch can be git am on 5.17-rc3 without confliction. ---- On Thu, 20 Jan 2022 23:12:56 -0500 Li Chen <lchen.firstlove@zohomail.com> wrote ---- > From: Li Chen <lchen@ambarella.com> > > Before this commit, epf cannot set BAR to be prefetchable. > Prefetchable BAR can also help epf device to use bridge's > prefetch memory window. > > Signed-off-by: Li Chen <lchen@ambarella.com> > --- > Changes in v2: > Remove Gerrit Change-id > Changes in v3: > capitalize "BAR" in the subject and commit log as suggested by Bjorn. > Changes in v4: > This patch context doesn't change but resend with my Zoho mail account in that previous > company mail will contain un-removeable proprietary messages. > Add "From:" to the first line of the message body. > > drivers/pci/endpoint/functions/pci-epf-test.c | 4 ++++ > include/linux/pci-epc.h | 2 ++ > 2 files changed, 6 insertions(+) > > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c > index 90d84d3bc868..96489cfdf58d 100644 > --- a/drivers/pci/endpoint/functions/pci-epf-test.c > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c > @@ -817,15 +817,19 @@ static void pci_epf_configure_bar(struct pci_epf *epf, > { > struct pci_epf_bar *epf_bar; > bool bar_fixed_64bit; > + bool bar_prefetch; > int i; > > for (i = 0; i < PCI_STD_NUM_BARS; i++) { > epf_bar = &epf->bar[i]; > bar_fixed_64bit = !!(epc_features->bar_fixed_64bit & (1 << i)); > + bar_prefetch = !!(epc_features->bar_prefetch & (1 << i)); > if (bar_fixed_64bit) > epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; > if (epc_features->bar_fixed_size[i]) > bar_size[i] = epc_features->bar_fixed_size[i]; > + if (bar_prefetch) > + epf_bar->flags |= PCI_BASE_ADDRESS_MEM_PREFETCH; > } > } > > diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h > index a48778e1a4ee..825632d581d0 100644 > --- a/include/linux/pci-epc.h > +++ b/include/linux/pci-epc.h > @@ -161,6 +161,7 @@ struct pci_epc { > * @msix_capable: indicate if the endpoint function has MSI-X capability > * @reserved_bar: bitmap to indicate reserved BAR unavailable to function driver > * @bar_fixed_64bit: bitmap to indicate fixed 64bit BARs > + * @bar_prefetch: bitmap to indicate prefetchable BARs > * @bar_fixed_size: Array specifying the size supported by each BAR > * @align: alignment size required for BAR buffer allocation > */ > @@ -171,6 +172,7 @@ struct pci_epc_features { > unsigned int msix_capable : 1; > u8 reserved_bar; > u8 bar_fixed_64bit; > + u8 bar_prefetch; > u64 bar_fixed_size[PCI_STD_NUM_BARS]; > size_t align; > }; > -- > 2.34.1 > > >
Reviewed-by: Shradha Todi <shradha.t@samsung.com> This patch looks useful. Can we revisit this and get it merged? >Date: Fri, 21 Jan 2022 12:12:56 +0800 >From: Li Chen <lchen.firstlove@zohomail.com> >To: "Bjorn Helgaas" <helgaas@kernel.org> >Cc: "Kishon Vijay Abraham I" <kishon@ti.com>, > "Lorenzo Pieralisi" <lorenzo.pieralisi@arm.com>, > "Bjorn Helgaas" <bhelgaas@google.com>, > "linux-pci" <linux-pci@vger.kernel.org>, > "linux-kernel" <linux-kernel@vger.kernel.org> >Message-ID: <17e7ad65ff5.d9de88b4962.1109678039880040918@zohomail.com> >In-Reply-To: >Subject: [PATCH v4] PCI: endpoint: Add prefetch BAR support >MIME-Version: 1.0 >Content-Type: text/plain; charset="UTF-8" >Content-Transfer-Encoding: 7bit >Importance: Medium >User-Agent: Zoho Mail >X-Mailer: Zoho Mail >Precedence: bulk >List-ID: <linux-pci.vger.kernel.org> >X-Mailing-List: linux-pci@vger.kernel.org > >From: Li Chen <lchen@ambarella.com> > >Before this commit, epf cannot set BAR to be prefetchable. >Prefetchable BAR can also help epf device to use bridge's >prefetch memory window. > >Signed-off-by: Li Chen <lchen@ambarella.com> >--- >Changes in v2: >Remove Gerrit Change-id >Changes in v3: >capitalize "BAR" in the subject and commit log as suggested by Bjorn. >Changes in v4: >This patch context doesn't change but resend with my Zoho mail account in that previous >company mail will contain un-removeable proprietary messages. >Add "From:" to the first line of the message body. > > drivers/pci/endpoint/functions/pci-epf-test.c | 4 ++++ > include/linux/pci-epc.h | 2 ++ > 2 files changed, 6 insertions(+) > >diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c >index 90d84d3bc868..96489cfdf58d 100644 >--- a/drivers/pci/endpoint/functions/pci-epf-test.c >+++ b/drivers/pci/endpoint/functions/pci-epf-test.c >@@ -817,15 +817,19 @@ static void pci_epf_configure_bar(struct pci_epf *epf, > { > struct pci_epf_bar *epf_bar; > bool bar_fixed_64bit; >+ bool bar_prefetch; > int i; > > for (i = 0; i < PCI_STD_NUM_BARS; i++) { > epf_bar = &epf->bar[i]; > bar_fixed_64bit = !!(epc_features->bar_fixed_64bit & (1 << i)); >+ bar_prefetch = !!(epc_features->bar_prefetch & (1 << i)); > if (bar_fixed_64bit) > epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; > if (epc_features->bar_fixed_size[i]) > bar_size[i] = epc_features->bar_fixed_size[i]; >+ if (bar_prefetch) >+ epf_bar->flags |= PCI_BASE_ADDRESS_MEM_PREFETCH; > } > } > >diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h >index a48778e1a4ee..825632d581d0 100644 >--- a/include/linux/pci-epc.h >+++ b/include/linux/pci-epc.h >@@ -161,6 +161,7 @@ struct pci_epc { > * @msix_capable: indicate if the endpoint function has MSI-X capability > * @reserved_bar: bitmap to indicate reserved BAR unavailable to function driver > * @bar_fixed_64bit: bitmap to indicate fixed 64bit BARs >+ * @bar_prefetch: bitmap to indicate prefetchable BARs > * @bar_fixed_size: Array specifying the size supported by each BAR > * @align: alignment size required for BAR buffer allocation > */ >@@ -171,6 +172,7 @@ struct pci_epc_features { > unsigned int msix_capable : 1; > u8 reserved_bar; > u8 bar_fixed_64bit; >+ u8 bar_prefetch; > u64 bar_fixed_size[PCI_STD_NUM_BARS]; > size_t align; > }; >-- >2.34.1 > > >
On Wed, Feb 28, 2024 at 07:14:48PM +0530, Shradha Todi wrote: > Reviewed-by: Shradha Todi <shradha.t@samsung.com> > > This patch looks useful. Can we revisit this and get it merged? Hello Shradha, This patch is two years old, and no longer applies to pci-next. However: Usually, fixed hardware requirements are specified in struct pci_epc_features (more specifically struct pci_epc_bar_desc). A requested BAR configuration by an EPF is specified in struct epf_bar. I don't think that Prefetch is a fixed hardware requirement, so I do not think that we should put it in struct pci_epc_features. It seems more like something that an endpoint function driver can chose to request (or not to request), just like MEM_TYPE_64. From the PCIe base spec: "Generally only 64-bit BARs are good candidates, since only Legacy Endpoints are permitted to set the Prefetchable bit in 32-bit BARs, and most scalable platforms map all 32-bit Memory BARs into non-prefetchable Memory Space regardless of the Prefetchable bit value." "For a PCI Express Endpoint, 64-bit addressing must be supported for all BARs that have the Prefetchable bit Set. 32-bit addressing is permitted for all BARs that do not have the Prefetchable bit Set." "Any device that has a range that behaves like normal memory should mark the range as prefetchable. A linear frame buffer in a graphics device is an example of a range that should be marked prefetchable." We are not a legacy endpoint, so we should never set Prefetch for 32-bit BARs. For 64-bit BARs, we should always set it, if the EPF-core allocated the memory (regular memory) for that BAR. Thus, I think the best solution is to do: diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index cd4ffb39dcdc..186c8cd87bb3 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -879,7 +879,8 @@ static void pci_epf_configure_bar(struct pci_epf *epf, for (i = 0; i < PCI_STD_NUM_BARS; i++) { epf_bar = &epf->bar[i]; if (epc_features->bar[i].only_64bit) - epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; + epf_bar->flags |= (PCI_BASE_ADDRESS_MEM_TYPE_64 | + PCI_BASE_ADDRESS_MEM_PREFETCH); } } diff --git a/drivers/pci/endpoint/pci-epf-core.c b/drivers/pci/endpoint/pci-epf-core.c index 0a28a0b0911b..acb93055181b 100644 --- a/drivers/pci/endpoint/pci-epf-core.c +++ b/drivers/pci/endpoint/pci-epf-core.c @@ -305,7 +305,8 @@ void *pci_epf_alloc_space(struct pci_epf *epf, size_t size, enum pci_barno bar, epf_bar[bar].size = size; epf_bar[bar].barno = bar; epf_bar[bar].flags |= upper_32_bits(size) ? - PCI_BASE_ADDRESS_MEM_TYPE_64 : + (PCI_BASE_ADDRESS_MEM_TYPE_64 | + PCI_BASE_ADDRESS_MEM_PREFETCH) : PCI_BASE_ADDRESS_MEM_TYPE_32; return space; Now when I look at it, the whole "if (epc_features->bar[i].only_64bit)" should move to pci_epf_alloc_space() IMO, so that not all EPF drivers need to duplicate this code. Kind regards, Niklas
diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c index 90d84d3bc868..96489cfdf58d 100644 --- a/drivers/pci/endpoint/functions/pci-epf-test.c +++ b/drivers/pci/endpoint/functions/pci-epf-test.c @@ -817,15 +817,19 @@ static void pci_epf_configure_bar(struct pci_epf *epf, { struct pci_epf_bar *epf_bar; bool bar_fixed_64bit; + bool bar_prefetch; int i; for (i = 0; i < PCI_STD_NUM_BARS; i++) { epf_bar = &epf->bar[i]; bar_fixed_64bit = !!(epc_features->bar_fixed_64bit & (1 << i)); + bar_prefetch = !!(epc_features->bar_prefetch & (1 << i)); if (bar_fixed_64bit) epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; if (epc_features->bar_fixed_size[i]) bar_size[i] = epc_features->bar_fixed_size[i]; + if (bar_prefetch) + epf_bar->flags |= PCI_BASE_ADDRESS_MEM_PREFETCH; } } diff --git a/include/linux/pci-epc.h b/include/linux/pci-epc.h index a48778e1a4ee..825632d581d0 100644 --- a/include/linux/pci-epc.h +++ b/include/linux/pci-epc.h @@ -161,6 +161,7 @@ struct pci_epc { * @msix_capable: indicate if the endpoint function has MSI-X capability * @reserved_bar: bitmap to indicate reserved BAR unavailable to function driver * @bar_fixed_64bit: bitmap to indicate fixed 64bit BARs + * @bar_prefetch: bitmap to indicate prefetchable BARs * @bar_fixed_size: Array specifying the size supported by each BAR * @align: alignment size required for BAR buffer allocation */ @@ -171,6 +172,7 @@ struct pci_epc_features { unsigned int msix_capable : 1; u8 reserved_bar; u8 bar_fixed_64bit; + u8 bar_prefetch; u64 bar_fixed_size[PCI_STD_NUM_BARS]; size_t align; };