public inbox for linux-nvme@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH] nvmet: pci-epf: Always configure BAR0 as 64-bit
@ 2025-03-14  9:58 Niklas Cassel
  2025-03-14 10:34 ` Damien Le Moal
  2025-03-14 11:06 ` Damien Le Moal
  0 siblings, 2 replies; 3+ messages in thread
From: Niklas Cassel @ 2025-03-14  9:58 UTC (permalink / raw)
  To: Christoph Hellwig, Sagi Grimberg, Chaitanya Kulkarni
  Cc: Damien Le Moal, Niklas Cassel, linux-nvme

NVMe PCIe Transport Specification 1.1, section 2.1.10, claims that the
BAR0 type is Implementation Specific.

However, in NVMe 1.1, the type is required to be 64-bit.

Thus, to make our PCI EPF work on as many host systems as possible,
always configure the BAR0 type to be 64-bit.

In the rare case that the underlying PCI EPC does not support configuring
BAR0 as 64-bit, the call to pci_epc_set_bar() will fail, and we will
return a failure back to the user.

This should not be a problem, as most PCI EPCs support configuring a BAR
as 64-bit (and those EPCs with .only_64bit set to true in epc_features
only support configuring the BAR as 64-bit).

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
Hello Damien,

please test on your platforms as well.

I think this is the way to go, as most real NVMe drives in the wild have
BAR0 as a 64-bit BAR.

 drivers/nvme/target/pci-epf.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
index 0136df45ca275..a24f6549c0d15 100644
--- a/drivers/nvme/target/pci-epf.c
+++ b/drivers/nvme/target/pci-epf.c
@@ -2096,8 +2096,15 @@ static int nvmet_pci_epf_configure_bar(struct nvmet_pci_epf *nvme_epf)
 		return -ENODEV;
 	}
 
-	if (epc_features->bar[BAR_0].only_64bit)
-		epf->bar[BAR_0].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+	/*
+	 * While NVMe PCIe Transport Specification 1.1, section 2.1.10, claims
+	 * that the BAR0 type is Implementation Specific, in NVMe 1.1, the type
+	 * is required to be 64-bit. Thus, for interoperability, always set the
+	 * type to 64-bit. In the rare case that the PCI EPC does not support
+	 * configuring BAR0 as 64-bit, the call to pci_epc_set_bar() will fail,
+	 * and we will return failure back to the user.
+	 */
+	epf->bar[BAR_0].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
 
 	/*
 	 * Calculate the size of the register bar: NVMe registers first with
-- 
2.48.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] nvmet: pci-epf: Always configure BAR0 as 64-bit
  2025-03-14  9:58 [PATCH] nvmet: pci-epf: Always configure BAR0 as 64-bit Niklas Cassel
@ 2025-03-14 10:34 ` Damien Le Moal
  2025-03-14 11:06 ` Damien Le Moal
  1 sibling, 0 replies; 3+ messages in thread
From: Damien Le Moal @ 2025-03-14 10:34 UTC (permalink / raw)
  To: Niklas Cassel, Christoph Hellwig, Sagi Grimberg,
	Chaitanya Kulkarni
  Cc: linux-nvme

On 3/14/25 18:58, Niklas Cassel wrote:
> NVMe PCIe Transport Specification 1.1, section 2.1.10, claims that the
> BAR0 type is Implementation Specific.
> 
> However, in NVMe 1.1, the type is required to be 64-bit.
> 
> Thus, to make our PCI EPF work on as many host systems as possible,
> always configure the BAR0 type to be 64-bit.
> 
> In the rare case that the underlying PCI EPC does not support configuring
> BAR0 as 64-bit, the call to pci_epc_set_bar() will fail, and we will
> return a failure back to the user.
> 
> This should not be a problem, as most PCI EPCs support configuring a BAR
> as 64-bit (and those EPCs with .only_64bit set to true in epc_features
> only support configuring the BAR as 64-bit).
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> Hello Damien,
> 
> please test on your platforms as well.

Will test. But I think this needs a Fixes tag:

Fixes: 0faa0fe6f90e ("nvmet: New NVMe PCI endpoint function target driver")

> 
> I think this is the way to go, as most real NVMe drives in the wild have
> BAR0 as a 64-bit BAR.
> 
>  drivers/nvme/target/pci-epf.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/nvme/target/pci-epf.c b/drivers/nvme/target/pci-epf.c
> index 0136df45ca275..a24f6549c0d15 100644
> --- a/drivers/nvme/target/pci-epf.c
> +++ b/drivers/nvme/target/pci-epf.c
> @@ -2096,8 +2096,15 @@ static int nvmet_pci_epf_configure_bar(struct nvmet_pci_epf *nvme_epf)
>  		return -ENODEV;
>  	}
>  
> -	if (epc_features->bar[BAR_0].only_64bit)
> -		epf->bar[BAR_0].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
> +	/*
> +	 * While NVMe PCIe Transport Specification 1.1, section 2.1.10, claims
> +	 * that the BAR0 type is Implementation Specific, in NVMe 1.1, the type
> +	 * is required to be 64-bit. Thus, for interoperability, always set the
> +	 * type to 64-bit. In the rare case that the PCI EPC does not support
> +	 * configuring BAR0 as 64-bit, the call to pci_epc_set_bar() will fail,
> +	 * and we will return failure back to the user.
> +	 */
> +	epf->bar[BAR_0].flags |= PCI_BASE_ADDRESS_MEM_TYPE_64;
>  
>  	/*
>  	 * Calculate the size of the register bar: NVMe registers first with


-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] nvmet: pci-epf: Always configure BAR0 as 64-bit
  2025-03-14  9:58 [PATCH] nvmet: pci-epf: Always configure BAR0 as 64-bit Niklas Cassel
  2025-03-14 10:34 ` Damien Le Moal
@ 2025-03-14 11:06 ` Damien Le Moal
  1 sibling, 0 replies; 3+ messages in thread
From: Damien Le Moal @ 2025-03-14 11:06 UTC (permalink / raw)
  To: Niklas Cassel, Christoph Hellwig, Sagi Grimberg,
	Chaitanya Kulkarni
  Cc: linux-nvme

On 3/14/25 18:58, Niklas Cassel wrote:
> NVMe PCIe Transport Specification 1.1, section 2.1.10, claims that the
> BAR0 type is Implementation Specific.
> 
> However, in NVMe 1.1, the type is required to be 64-bit.
> 
> Thus, to make our PCI EPF work on as many host systems as possible,
> always configure the BAR0 type to be 64-bit.
> 
> In the rare case that the underlying PCI EPC does not support configuring
> BAR0 as 64-bit, the call to pci_epc_set_bar() will fail, and we will
> return a failure back to the user.
> 
> This should not be a problem, as most PCI EPCs support configuring a BAR
> as 64-bit (and those EPCs with .only_64bit set to true in epc_features
> only support configuring the BAR as 64-bit).
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Tested-by: Damien Le Moal <dlemoal@kernel.org>

(please add the fixes tag I suggested)

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-03-14 11:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-14  9:58 [PATCH] nvmet: pci-epf: Always configure BAR0 as 64-bit Niklas Cassel
2025-03-14 10:34 ` Damien Le Moal
2025-03-14 11:06 ` Damien Le Moal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox