linux-kernel - Re: [PATCH V4] PCI: 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: <1224f317cb45fcc5117a7d8dbd19142b0916559a.camel@dirkwinkel.cc>
Date: Mon, 07 Oct 2024 18:36:03 +0200
From: Steffen Dirkwinkel <me@...ffen.cc>
To: Jason Gunthorpe <jgg@...dia.com>, Jiri Slaby <jirislaby@...nel.org>
Cc: Vidya Sagar <vidyas@...dia.com>, corbet@....net, bhelgaas@...gle.com, 
	galshalom@...dia.com, leonro@...dia.com, treding@...dia.com,
 jonathanh@...dia.com, 	mmoshrefjava@...dia.com, shahafs@...dia.com,
 vsethi@...dia.com, 	sdonthineni@...dia.com, jan@...dia.com,
 tdave@...dia.com, 	linux-doc@...r.kernel.org, linux-pci@...r.kernel.org, 
	linux-kernel@...r.kernel.org, kthota@...dia.com, mmaddireddy@...dia.com, 
	sagar.tv@...il.com, vliaskovitis@...e.com
Subject: Re: [PATCH V4] PCI: Extend ACS configurability

On Tue, 2024-10-01 at 16:33 -0300, Jason Gunthorpe wrote:
> On Wed, Sep 25, 2024 at 07:49:59AM +0200, Jiri Slaby wrote:
> > On 25. 09. 24, 7:29, Jiri Slaby wrote:
> > > On 25. 09. 24, 7:06, Jiri Slaby wrote:
> > > > > @@ -1047,23 +1066,33 @@ static void pci_std_enable_acs(struct
> > > > > pci_dev *dev)
> > > > >    */
> > > > >   static void pci_enable_acs(struct pci_dev *dev)
> > > > >   {
> > > > > -    if (!pci_acs_enable)
> > > > > -        goto disable_acs_redir;
> > > > > +    struct pci_acs caps;
> > > > > +    int pos;
> > > > > +
> > > > > +    pos = dev->acs_cap;
> > > > > +    if (!pos)
> > > > > +        return;
> > 
> > Ignore the previous post.
> > 
> > The bridge has no ACS (see lspci below). So it used to be enabled
> > by
> > pci_quirk_enable_intel_pch_acs() by another registers. 
> 
> Er, Ok, so it overrides the whole thing with
> pci_dev_specific_acs_enabled() too..
> 
> > I am not sure how to fix this as we cannot have "caps" from these
> > quirks, so
> > that whole idea of __pci_config_acs() is nonworking for these
> > quirks.
> 
> We just need to allow the quirk to run before we try to do anything
> with the cap, which has probably always been a NOP anyhow.
> 
> Maybe like this?
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 7d85c04fbba2ae..225a6cd2e9ca3b 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -1067,8 +1067,15 @@ static void pci_std_enable_acs(struct pci_dev
> *dev, struct pci_acs *caps)
>  static void pci_enable_acs(struct pci_dev *dev)
>  {
>  	struct pci_acs caps;
> +	bool enable_acs = false;
>  	int pos;
>  
> +	/* If an iommu is present we start with kernel default caps
> */
> +	if (pci_acs_enable) {
> +		if (pci_dev_specific_enable_acs(dev))
> +			enable_acs = true;
> +	}
> +
>  	pos = dev->acs_cap;
>  	if (!pos)
>  		return;
> @@ -1077,11 +1084,8 @@ static void pci_enable_acs(struct pci_dev
> *dev)
>  	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &caps.ctrl);
>  	caps.fw_ctrl = caps.ctrl;
>  
> -	/* If an iommu is present we start with kernel default caps
> */
> -	if (pci_acs_enable) {
> -		if (pci_dev_specific_enable_acs(dev))
> -			pci_std_enable_acs(dev, &caps);
> -	}
> +	if (enable_acs)
> +		pci_std_enable_acs(dev, &caps);
>  
>  	/*
>  	 * Always apply caps from the command line, even if there is
> no iommu.

Hi,

I just ran into this issue (fewer iommu groups starting with 6.11).
Both reverting the original patch or applying your suggestion worked
for me.

Thanks
Steffen