| Message ID | 20240313105804.100168-8-cassel@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: endpoint: set prefetchable bit for 64-bit BARs | expand |
On Wed, Mar 13, 2024 at 11:57:59AM +0100, Niklas Cassel wrote: > Ever since commit f25b5fae29d4 ("PCI: endpoint: Setting a BAR size > 4 GB > is invalid if 64-bit flag is not set") it has been impossible to get the > .set_bar() callback with a BAR size > 4 GB, if the BAR was also not > requested to be configured as a 64-bit BAR. > > Thus, forcing setting the 64-bit flag for BARs larger than 4 GB in the 2 GB > lower level driver is dead code and can be removed. > > It is however possible that an EPF driver configures a BAR as 64-bit, > even if the requested size is < 4 GB. > > Respect the requested BAR configuration, just like how it is already > repected with regards to the prefetchable bit. > > Signed-off-by: Niklas Cassel <cassel@kernel.org> Okay, here you are fixing this driver. But this should be moved before patch 5/9. With that, Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > drivers/pci/controller/cadence/pcie-cadence-ep.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c > index 2d0a8d78bffb..de10e5edd1b0 100644 > --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c > +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c > @@ -99,14 +99,11 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, u8 vfn, > ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS; > } else { > bool is_prefetch = !!(flags & PCI_BASE_ADDRESS_MEM_PREFETCH); > - bool is_64bits = sz > SZ_2G; > + bool is_64bits = !!(flags & PCI_BASE_ADDRESS_MEM_TYPE_64); > > if (is_64bits && (bar & 1)) > return -EINVAL; > > - if (is_64bits && !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64)) > - epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; > - > if (is_64bits && is_prefetch) > ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS; > else if (is_prefetch) > -- > 2.44.0 >
On Fri, Mar 15, 2024 at 11:16:16AM +0530, Manivannan Sadhasivam wrote: > On Wed, Mar 13, 2024 at 11:57:59AM +0100, Niklas Cassel wrote: > > Ever since commit f25b5fae29d4 ("PCI: endpoint: Setting a BAR size > 4 GB > > is invalid if 64-bit flag is not set") it has been impossible to get the > > .set_bar() callback with a BAR size > 4 GB, if the BAR was also not > > requested to be configured as a 64-bit BAR. > > > > Thus, forcing setting the 64-bit flag for BARs larger than 4 GB in the > > 2 GB Will fix in V4. > > > lower level driver is dead code and can be removed. > > > > It is however possible that an EPF driver configures a BAR as 64-bit, > > even if the requested size is < 4 GB. > > > > Respect the requested BAR configuration, just like how it is already > > repected with regards to the prefetchable bit. > > > > Signed-off-by: Niklas Cassel <cassel@kernel.org> > > Okay, here you are fixing this driver. But this should be moved before patch > 5/9. With that, As I wrote in my reply to patch 5/9, I don't think we need to move them around. The code that sets flag PCI_BASE_ADDRESS_MEM_TYPE_64 in this driver is dead code, it couldn't happen before this commit. So unless you insist, I think it is better to keep the current ordering, such that all pci-epf-test patches are after each other. Kind regards, Niklas > > Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> > > - Mani > > > --- > > drivers/pci/controller/cadence/pcie-cadence-ep.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c > > index 2d0a8d78bffb..de10e5edd1b0 100644 > > --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c > > +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c > > @@ -99,14 +99,11 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, u8 vfn, > > ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS; > > } else { > > bool is_prefetch = !!(flags & PCI_BASE_ADDRESS_MEM_PREFETCH); > > - bool is_64bits = sz > SZ_2G; > > + bool is_64bits = !!(flags & PCI_BASE_ADDRESS_MEM_TYPE_64); > > > > if (is_64bits && (bar & 1)) > > return -EINVAL; > > > > - if (is_64bits && !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64)) > > - epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; > > - > > if (is_64bits && is_prefetch) > > ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS; > > else if (is_prefetch) > > -- > > 2.44.0 > > > > -- > மணிவண்ணன் சதாசிவம்
diff --git a/drivers/pci/controller/cadence/pcie-cadence-ep.c b/drivers/pci/controller/cadence/pcie-cadence-ep.c index 2d0a8d78bffb..de10e5edd1b0 100644 --- a/drivers/pci/controller/cadence/pcie-cadence-ep.c +++ b/drivers/pci/controller/cadence/pcie-cadence-ep.c @@ -99,14 +99,11 @@ static int cdns_pcie_ep_set_bar(struct pci_epc *epc, u8 fn, u8 vfn, ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_IO_32BITS; } else { bool is_prefetch = !!(flags & PCI_BASE_ADDRESS_MEM_PREFETCH); - bool is_64bits = sz > SZ_2G; + bool is_64bits = !!(flags & PCI_BASE_ADDRESS_MEM_TYPE_64); if (is_64bits && (bar & 1)) return -EINVAL; - if (is_64bits && !(flags & PCI_BASE_ADDRESS_MEM_TYPE_64)) - epf_bar->flags |= PCI_BASE_ADDRESS_MEM_TYPE_64; - if (is_64bits && is_prefetch) ctrl = CDNS_PCIE_LM_BAR_CFG_CTRL_PREFETCH_MEM_64BITS; else if (is_prefetch)
Ever since commit f25b5fae29d4 ("PCI: endpoint: Setting a BAR size > 4 GB is invalid if 64-bit flag is not set") it has been impossible to get the .set_bar() callback with a BAR size > 4 GB, if the BAR was also not requested to be configured as a 64-bit BAR. Thus, forcing setting the 64-bit flag for BARs larger than 4 GB in the lower level driver is dead code and can be removed. It is however possible that an EPF driver configures a BAR as 64-bit, even if the requested size is < 4 GB. Respect the requested BAR configuration, just like how it is already repected with regards to the prefetchable bit. Signed-off-by: Niklas Cassel <cassel@kernel.org> --- drivers/pci/controller/cadence/pcie-cadence-ep.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)