[PATCH 1/1] nvme-pci: set virt boundary according to capability

[PATCH 1/1] nvme-pci: set virt boundary according to capability

[Max Gurtovoy]

Some controllers advertise DWORD alignment for SGLs, so configure the
virtual boundary correctly for those devices.

Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
---
 drivers/nvme/host/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index e5ca8301bb8b..eacc89cd25eb 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -3326,7 +3326,9 @@ static unsigned long nvme_pci_get_virt_boundary(struct nvme_ctrl *ctrl,
 {
 	if (!nvme_ctrl_sgl_supported(ctrl) || is_admin)
 		return NVME_CTRL_PAGE_SIZE - 1;
-	return 0;
+	else if (ctrl->sgls & NVME_CTRL_SGLS_BYTE_ALIGNED)
+		return 0;
+	return 3;
 }
 
 static const struct nvme_ctrl_ops nvme_pci_ctrl_ops = {
-- 
2.43.5

Re: [PATCH 1/1] nvme-pci: set virt boundary according to capability

[Christoph Hellwig]

On Tue, Dec 09, 2025 at 12:26:20AM +0200, Max Gurtovoy wrote:
> Some controllers advertise DWORD alignment for SGLs, so configure the
> virtual boundary correctly for those devices.
> 
> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
> ---
>  drivers/nvme/host/pci.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index e5ca8301bb8b..eacc89cd25eb 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -3326,7 +3326,9 @@ static unsigned long nvme_pci_get_virt_boundary(struct nvme_ctrl *ctrl,
>  {
>  	if (!nvme_ctrl_sgl_supported(ctrl) || is_admin)
>  		return NVME_CTRL_PAGE_SIZE - 1;
> -	return 0;
> +	else if (ctrl->sgls & NVME_CTRL_SGLS_BYTE_ALIGNED)
> +		return 0;
> +	return 3;

I don't think this is correct.  NVME_CTRL_SGLS_BYTE_ALIGNED requires
each SGL to be aligned, but is not a virt boundary.  The dma_alignment
value in the queue_limits already takes care of that.

Re: [PATCH 1/1] nvme-pci: set virt boundary according to capability

[Max Gurtovoy]

On 09/12/2025 8:40, Christoph Hellwig wrote:
> On Tue, Dec 09, 2025 at 12:26:20AM +0200, Max Gurtovoy wrote:
>> Some controllers advertise DWORD alignment for SGLs, so configure the
>> virtual boundary correctly for those devices.
>>
>> Signed-off-by: Max Gurtovoy <mgurtovoy@nvidia.com>
>> ---
>>   drivers/nvme/host/pci.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
>> index e5ca8301bb8b..eacc89cd25eb 100644
>> --- a/drivers/nvme/host/pci.c
>> +++ b/drivers/nvme/host/pci.c
>> @@ -3326,7 +3326,9 @@ static unsigned long nvme_pci_get_virt_boundary(struct nvme_ctrl *ctrl,
>>   {
>>   	if (!nvme_ctrl_sgl_supported(ctrl) || is_admin)
>>   		return NVME_CTRL_PAGE_SIZE - 1;
>> -	return 0;
>> +	else if (ctrl->sgls & NVME_CTRL_SGLS_BYTE_ALIGNED)
>> +		return 0;
>> +	return 3;
> I don't think this is correct.  NVME_CTRL_SGLS_BYTE_ALIGNED requires
> each SGL to be aligned, but is not a virt boundary.  The dma_alignment
> value in the queue_limits already takes care of that.

I see. The virt boundary handles the gaps.

Should we modify:

dma_set_min_align_mask(dev->dev, NVME_CTRL_PAGE_SIZE - 1);

and

lim->dma_alignment = 3;

to ease the restriction for capable devices with 
NVME_CTRL_SGLS_BYTE_ALIGNED support ?

>

Re: [PATCH 1/1] nvme-pci: set virt boundary according to capability

[Keith Busch]

On Tue, Dec 09, 2025 at 01:31:21PM +0200, Max Gurtovoy wrote:
> dma_set_min_align_mask(dev->dev, NVME_CTRL_PAGE_SIZE - 1);
> 
> and
> 
> lim->dma_alignment = 3;
> 
> to ease the restriction for capable devices with NVME_CTRL_SGLS_BYTE_ALIGNED
> support ?

Yeah, the dma_alignment is the limit you'd need to change. Note, you
can't actually set the dma_alignment to be byte aligned as it's a mask,
so you'd want the value to be 0 to allow any alignment, but the block
layer currently won't let it be 0, so you'd have to set it to 1, for
word alignment.

But I'm surprised to hear of a device that can do byte aligned SGLs, as
PCIe fundamentally can't do byte aligned DMA. It's all dword based, so
if you have a device that does report byte alignment, it's still sending
dwords over the wire. It's just using the "byte enable" fields in the
TLP header to have the receiver strip off preceding and/or trailing
bytes, so it is a bit inefficient way to transfer data.

Re: [PATCH 1/1] nvme-pci: set virt boundary according to capability

[Max Gurtovoy]

On 10/12/2025 13:28, Keith Busch wrote:
> On Tue, Dec 09, 2025 at 01:31:21PM +0200, Max Gurtovoy wrote:
>> dma_set_min_align_mask(dev->dev, NVME_CTRL_PAGE_SIZE - 1);
>>
>> and
>>
>> lim->dma_alignment = 3;
>>
>> to ease the restriction for capable devices with NVME_CTRL_SGLS_BYTE_ALIGNED
>> support ?
> Yeah, the dma_alignment is the limit you'd need to change. Note, you
> can't actually set the dma_alignment to be byte aligned as it's a mask,
> so you'd want the value to be 0 to allow any alignment, but the block
> layer currently won't let it be 0, so you'd have to set it to 1, for
> word alignment.
>
> But I'm surprised to hear of a device that can do byte aligned SGLs, as
> PCIe fundamentally can't do byte aligned DMA. It's all dword based, so
> if you have a device that does report byte alignment, it's still sending
> dwords over the wire. It's just using the "byte enable" fields in the
> TLP header to have the receiver strip off preceding and/or trailing
> bytes, so it is a bit inefficient way to transfer data.

I saw some devices that report byte alignment. But if it's actually 
sending dwords over the wire than it's useless.

what about dma_set_min_align_mask(dev->dev, NVME_CTRL_PAGE_SIZE - 1); ? 
won't it mandate 4K alignment ?