linux-kernel - Re: [PATCH 01/17] PCI: Add concurrency safe clear_and

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: <20230511202332.GD31598@wunner.de>
Date:   Thu, 11 May 2023 22:23:32 +0200
From:   Lukas Wunner <lukas@...ner.de>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>,
        linux-pci@...r.kernel.org, Rob Herring <robh@...nel.org>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Krzysztof Wilczy??ski <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 01/17] PCI: Add concurrency safe clear_and_set variants
 for LNKCTL{,2}

On Thu, May 11, 2023 at 10:55:06AM -0500, Bjorn Helgaas wrote:
> On Thu, May 11, 2023 at 04:14:25PM +0300, Ilpo Järvinen wrote:
> > +int pcie_capability_clear_and_set_word_locked(struct pci_dev *dev, int pos,
> > +					      u16 clear, u16 set)
> > +{
> > +	unsigned long flags;
> > +	int ret;
> > +
> > +	spin_lock_irqsave(&dev->cap_lock, flags);
> > +	ret = pcie_capability_clear_and_set_word(dev, pos, clear, set);
> > +	spin_unlock_irqrestore(&dev->cap_lock, flags);
> > +
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(pcie_capability_clear_and_set_word_locked);
> 
> I didn't see the prior discussion with Lukas, so maybe this was
> answered there, but is there any reason not to add locking to
> pcie_capability_clear_and_set_word() and friends directly?
> 
> It would be nice to avoid having to decide whether to use the locked
> or unlocked versions.

I think we definitely want to also offer lockless accessors which
can be used in hotpaths such as interrupt handlers if the accessed
registers don't need any locking (e.g. because there are no concurrent
accesses).

So the relatively lean approach chosen here which limits locking to
Link Control and Link Control 2, but allows future expansion to other
registers as well, seemed reasonable to me.

Thanks,

Lukas