On 2021/10/29 1:24, Bjorn Helgaas wrote:

On Thu, Oct 28, 2021 at 03:44:49PM +0800, Dongdong Liu wrote:

quoted

On 2021/10/28 6:28, Bjorn Helgaas wrote:

quoted

On Sat, Oct 09, 2021 at 06:49:34PM +0800, Dongdong Liu wrote:

quoted

PCIe spec 5.0 r1.0 section 2.2.6.2 says:

  If an Endpoint supports sending Requests to other Endpoints (as
  opposed to host memory), the Endpoint must not send 10-Bit Tag
  Requests to another given Endpoint unless an implementation-specific
  mechanism determines that the Endpoint supports 10-Bit Tag Completer
  capability.

Add a 10bit_tag sysfs file, write 0 to disable 10-Bit Tag Requester
when the driver does not bind the device. The typical use case is for
p2pdma when the peer device does not support 10-Bit Tag Completer.
Write 1 to enable 10-Bit Tag Requester when RC supports 10-Bit Tag
Completer capability. The typical use case is for host memory targeted
by DMA Requests. The 10bit_tag file content indicate current status of
10-Bit Tag Requester Enable.

Don't we have a hole here?  We're adding knobs to control 10-Bit Tag
usage, but don't we have basically the same issues with Extended
(8-bit) Tags?

All PCIe completers are required to support 8-bit tags
from the "[PATCH] PCI: enable extended tags support for PCIe endpoints"
(https://patchwork.kernel.org/project/linux-arm-msm/patch/1474769434-5756-1-git-send-email-okaya@codeaurora.org/).

I ask hardware colleagues, also says all PCIe devices should support
8-bit tags completer default, so seems no need to do this for 8-bit tags.

Oh, right, I forgot that, thanks for the reminder!  Let's add a
comment in pci_configure_extended_tags() to that effect so I'll
remember next time.

Ok, Will do.

I think the appropriate reference is PCIe r5.0, sec 2.2.6.2, which
says "Receivers/Completers must handle 8-bit Tag values correctly
regardless of the setting of their Extended Tag Field Enable bit (see
Section 7.5.3.4)."

The Tag field was 8 bits all the way from PCIe r1.0, but until r2.1 it
said that by default, only the lower 5 bits are used.

The text about all Completers explicitly being required to support
8-bit Tags wasn't added until PCIe r3.0, which might explain some
confusion and the presence of the Extended Tag Field Enable bit.

Thanks for the clarification.

At the same time, can you fold pci_configure_10bit_tags() directly
into pci_configure_extended_tags()?  It's pretty small and I think it
will be easier if it's all in one place.

OK, will do.

quoted

quoted

I wonder if we should be adding a more general "tags" file that can
manage both 8-bit and 10-bit tag usage.

I'm still thinking that maybe a generic name (without "10") would be
better, even though we don't need it to manage 8-bit tags.  It's
conceivable that there could be even more tag bits in the future, and
it would be nice if we didn't have to add yet another file.

Looks good, will do.

Thanks,
Dongdong.

Bjorn
.