Re: [PATCH V4] PCI: Extend ACS configurability

[Jason Gunthorpe]

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.