netdev - Re: [PATCH V10 4/8] PCI/sysfs: Add a 10-Bit Tag sysfs file PCIe Endpoint devices

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 28 Oct 2021 15:44:49 +0800
From:   Dongdong Liu <liudongdong3@...wei.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     <hch@...radead.org>, <kw@...ux.com>, <logang@...tatee.com>,
        <leon@...nel.org>, <linux-pci@...r.kernel.org>,
        <rajur@...lsio.com>, <hverkuil-cisco@...all.nl>,
        <linux-media@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH V10 4/8] PCI/sysfs: Add a 10-Bit Tag sysfs file PCIe
 Endpoint devices

Hi Bjorn

Many thanks for your review.
On 2021/10/28 6:28, Bjorn Helgaas wrote:
> On Sat, Oct 09, 2021 at 06:49:34PM +0800, Dongdong Liu wrote:
>> 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.
>
> I wonder if we should be adding a more general "tags" file that can
> manage both 8-bit and 10-bit tag usage.
>
>> +static struct device_attribute dev_attr_10bit_tag = __ATTR(10bit_tag, 0644,
>> +							   pci_10bit_tag_show,
>> +							   pci_10bit_tag_store);
>
> I think this should use DEVICE_ATTR().
Yes, will do.

Thanks,
Dongdong
> Or even better, if the name doesn't start with a digit, DEVICE_ATTR_RW().
> .
>