Re: [PATCH V4] PCI: Extend ACS configurability

[Jiri Slaby <jirislaby@kernel.org>]

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. But the "if 
(!pos)" does not let it run now.

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.


00:1c.0 PCI bridge: Intel Corporation C610/X99 series chipset PCI 
Express Root Port #1 (rev d5) (prog-if 00 [Normal decode])
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 36
	NUMA node: 0
	Bus: primary=00, secondary=08, subordinate=08, sec-latency=0
	I/O behind bridge: 00001000-00001fff [size=4K]
	Memory behind bridge: f3600000-f37fffff [size=2M]
	Prefetchable memory behind bridge: 00000000f3800000-00000000f39fffff 
[size=2M]
	Secondary status: 66MHz- FastB2B- ParErr- DEVSEL=fast >TAbort- <TAbort- 
<MAbort- <SERR- <PERR-
	BridgeCtl: Parity- SERR+ NoISA- VGA- MAbort- >Reset- FastB2B-
		PriDiscTmr- SecDiscTmr- DiscTmrStat- DiscTmrSERREn-
	Capabilities: [40] Express (v2) Root Port (Slot+), MSI 00
		DevCap:	MaxPayload 128 bytes, PhantFunc 0
			ExtTag- RBE+
		DevCtl:	Report errors: Correctable- Non-Fatal- Fatal- Unsupported-
			RlxdOrd- ExtTag- PhantFunc- AuxPwr- NoSnoop-
			MaxPayload 128 bytes, MaxReadReq 128 bytes
		DevSta:	CorrErr- UncorrErr- FatalErr- UnsuppReq- AuxPwr+ TransPend-
		LnkCap:	Port #1, Speed 5GT/s, Width x1, ASPM L0s L1, Exit Latency L0s 
<1us, L1 <4us
			ClockPM- Surprise- LLActRep+ BwNot+ ASPMOptComp-
		LnkCtl:	ASPM Disabled; RCB 64 bytes Disabled- CommClk-
			ExtSynch- ClockPM- AutWidDis- BWInt- AutBWInt-
		LnkSta:	Speed 2.5GT/s, Width x0, TrErr- Train- SlotClk+ DLActive- 
BWMgmt- ABWMgmt-
		SltCap:	AttnBtn- PwrCtrl- MRL- AttnInd- PwrInd- HotPlug+ Surprise+
			Slot #0, PowerLimit 0.000W; Interlock- NoCompl+
		SltCtl:	Enable: AttnBtn- PwrFlt- MRL- PresDet- CmdCplt- HPIrq- LinkChg-
			Control: AttnInd Unknown, PwrInd Unknown, Power- Interlock-
		SltSta:	Status: AttnBtn- PowerFlt- MRL- CmdCplt- PresDet- Interlock-
			Changed: MRL- PresDet- LinkState-
		RootCtl: ErrCorrectable- ErrNon-Fatal- ErrFatal- PMEIntEna- CRSVisible-
		RootCap: CRSVisible-
		RootSta: PME ReqID 0000, PMEStatus- PMEPending-
		DevCap2: Completion Timeout: Range ABC, TimeoutDis+, LTR+, OBFF Via 
WAKE# ARIFwd-
			 AtomicOpsCap: Routing- 32bit- 64bit- 128bitCAS-
		DevCtl2: Completion Timeout: 50us to 50ms, TimeoutDis-, LTR-, OBFF 
Disabled ARIFwd-
			 AtomicOpsCtl: ReqEn- EgressBlck-
		LnkCtl2: Target Link Speed: 5GT/s, EnterCompliance- SpeedDis-
			 Transmit Margin: Normal Operating Range, EnterModifiedCompliance- 
ComplianceSOS-
			 Compliance De-emphasis: -6dB
		LnkSta2: Current De-emphasis Level: -3.5dB, EqualizationComplete-, 
EqualizationPhase1-
			 EqualizationPhase2-, EqualizationPhase3-, LinkEqualizationRequest-
	Capabilities: [80] MSI: Enable+ Count=1/1 Maskable- 64bit-
		Address: fee00358  Data: 0000
	Capabilities: [90] Subsystem: Dell Device 0618
	Capabilities: [a0] Power Management version 3
		Flags: PMEClk- DSI- D1- D2- AuxCurrent=0mA PME(D0+,D1-,D2-,D3hot+,D3cold+)
		Status: D0 NoSoftRst- PME-Enable- DSel=0 DScale=0 PME-
	Kernel driver in use: pcieport

>>> -    if (!pci_dev_specific_enable_acs(dev))
>>> -        goto disable_acs_redir;
>>> +    pci_read_config_word(dev, pos + PCI_ACS_CAP, &caps.cap);
>>> +    pci_read_config_word(dev, pos + PCI_ACS_CTRL, &caps.ctrl);
>>> +    caps.fw_ctrl = caps.ctrl;
>>> -    pci_std_enable_acs(dev);
>>> +    /* If an iommu is present we start with kernel default caps */
>>> +    if (pci_acs_enable) {
> 
> AFAIU pci_acs_enable is set from iommus' code via pci_request_acs(). 
> Which is much later than when bridges are initialized here, right?



-- 
js
suse labs