linux-kernel - Re: [PATCH v3 2/2] PCI: pciehp: Clear the optional capabilities in DEVCTL2 on a hot-plug

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]

Message-ID: <4599d885-219f-3ee0-f425-62746f31cc67@amd.com>
Date:   Tue, 27 Jun 2023 10:38:54 -0700
From:   Smita Koralahalli <Smita.KoralahalliChannabasappa@....com>
To:     Lukas Wunner <lukas@...ner.de>
Cc:     linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        Bjorn Helgaas <bhelgaas@...gle.com>, oohall@...il.com,
        Mahesh J Salgaonkar <mahesh@...ux.ibm.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        Yazen Ghannam <yazen.ghannam@....com>,
        Fontenot Nathan <Nathan.Fontenot@....com>,
        Jay Cornwall <Jay.Cornwall@....com>,
        Felix Kuehling <Felix.Kuehling@....com>
Subject: Re: [PATCH v3 2/2] PCI: pciehp: Clear the optional capabilities in
 DEVCTL2 on a hot-plug

On 6/22/2023 2:42 PM, Lukas Wunner wrote:
> [cc += Jay, Felix]
> 
> On Thu, Jun 22, 2023 at 02:02:12PM -0700, Smita Koralahalli wrote:
>> Would it be fair to just reuse pci_enable_atomic_ops_to_root() for
>> Atomic_Ops configuration?
> 
> Hm, that's a good question.  I'm not an expert on that corner of
> the PCI core.
> 
> But indeed what you could try is amend that function to not only
> *set* PCI_EXP_DEVCTL2_ATOMIC_REQ if it's supported, but to also
> *clear* it if it's not supported.
> 
> And you'd have to call pci_enable_atomic_ops_to_root() on enumeration,
> e.g. from pci_init_capabilities().
> 
> That should obviate the need to call pci_enable_atomic_ops_to_root()
> from drivers, so you could probably remove the call from all the
> drivers which currently call it (amdgpu, infiniband, mellanox),
> in one separate patch per driver.
> 
> An then you could drop the EXPORT clause for pci_enable_atomic_ops_to_root()
> and make it private to the PCI core.
> 
> So that would be 5 patches (enablement/disablement on enumeration,
> amendmend of the 3 drivers, making the call private).
> 
> I'm not sure if anyone will cry foul if you do that but if you want
> to give it a try, go for it. :)

Okay, I see there are no objections except for Bjorn/Jay's comments on

"But there could be devices where AtomicOps are nominally supported but 
untested or broken.."

Would this be an issue?

If not, I will start working on those 5 patches.

Thanks,
Smita
> 
> I don't now why commit 430a23689dea, which introduced
> pci_enable_atomic_ops_to_root(), chose to add it as a library function
> which is only called from specific drivers, instead of universally
> enabling the feature for all devices.  Adding the commit authors to cc
> so they can chime in.
> 
> Thanks,
> 
> Lukas