linux-kernel - Re: [PATCH 1/1] PCI: Fix Extend ACS configurability

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: <20250107001015.GM5556@nvidia.com>
Date: Mon, 6 Jan 2025 20:10:15 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Tushar Dave <tdave@...dia.com>
Cc: corbet@....net, bhelgaas@...gle.com, paulmck@...nel.org,
	akpm@...ux-foundation.org, thuth@...hat.com, rostedt@...dmis.org,
	xiongwei.song@...driver.com, vidyas@...dia.com,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-pci@...r.kernel.org, vsethi@...dia.com,
	sdonthineni@...dia.com
Subject: Re: [PATCH 1/1] PCI: Fix Extend ACS configurability

On Mon, Jan 06, 2025 at 12:34:00PM -0800, Tushar Dave wrote:

> > > @@ -1028,10 +1031,10 @@ static void __pci_config_acs(struct pci_dev *dev, struct pci_acs *caps,
> > >   	pci_dbg(dev, "ACS mask  = %#06x\n", mask);
> > >   	pci_dbg(dev, "ACS flags = %#06x\n", flags);
> > > +	pci_dbg(dev, "ACS control = %#06x\n", caps->ctrl);
> > > -	/* If mask is 0 then we copy the bit from the firmware setting. */
> > > -	caps->ctrl = (caps->ctrl & ~mask) | (caps->fw_ctrl & mask);
> > > -	caps->ctrl |= flags;
> > > +	caps->ctrl &= ~mask;
> > > +	caps->ctrl |= (flags & mask);
> > 
> > And why delete fw_ctrl? Doesn't that break the unchanged
> > functionality?
> 
> No, it does not break the unchanged functionality. I removed it because it
> is not needed after my fix.

I mean, the whole hunk above is not needed, right? Or at least I don't
see how it relates to your commit message..

> If it helps, using 'config_acs' the code only allows to configures the lower
> 7 bits of ACS ctrl for the specified PCI device(s).
> The bits other than the lower 7 bits of ACS ctrl remain unchanged.
> The bits specified with 'x' or 'X' that are within the 7 lower bits remain
> unchanged. Trying to configure bits other than lower 7 bits generates an
> error message "Invalid ACS flags specified"

But the fw_ctrl was how the x behavior was supposed to be implemented,
IIRC there were cases where you could not just rely on caps->ctrl as
something prior had alreaded changed it.

What about your fix to the mask changes this reasoning? If nothing
then it should be its own patch with its own explanation..

Jason