On Mon, Mar 20, 2023 at 05:20:55PM +0000, Limonciello, Mario wrote: > > -----Original Message----- > > From: Bjorn Helgaas <helgaas@xxxxxxxxxx> > > Sent: Monday, March 20, 2023 12:15 > > To: Limonciello, Mario <Mario.Limonciello@xxxxxxx> > > Cc: Natikar, Basavaraj <Basavaraj.Natikar@xxxxxxx>; > > bhelgaas@xxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; thomas@xxxxxxxxxxxx; > > Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> > > Subject: Re: [PATCH] PCI: Add quirk to clear MSI-X Whatever you're using for email adds all these redundant headers to every response... > > On Mon, Mar 20, 2023 at 01:32:16AM +0000, Limonciello, Mario wrote: > > > > -----Original Message----- > > > > From: Bjorn Helgaas <helgaas@xxxxxxxxxx> > > > > Sent: Friday, March 10, 2023 16:14 > > > > To: Limonciello, Mario <Mario.Limonciello@xxxxxxx> > > > > Cc: Natikar, Basavaraj <Basavaraj.Natikar@xxxxxxx>; Natikar, Basavaraj > > > > <Basavaraj.Natikar@xxxxxxx>; bhelgaas@xxxxxxxxxx; linux- > > > > pci@xxxxxxxxxxxxxxx; thomas@xxxxxxxxxxxx > > > > Subject: Re: [PATCH] PCI: Add quirk to clear MSI-X > > > > > > > > On Thu, Mar 09, 2023 at 06:57:38PM -0600, Mario Limonciello wrote: > > > > > On 3/9/23 16:30, Bjorn Helgaas wrote: > > > > > > On Thu, Mar 09, 2023 at 12:32:41PM -0600, Limonciello, Mario wrote: > > > > > > > On 3/9/2023 12:25, Bjorn Helgaas wrote: > > > > > > > ... > > it's important that the commit log is accurate and makes sense even to > > people who don't know the internals of the device. > > > > It *sounds* like what's happening is: > > > > - OS writes PMCSR to put device in D3hot > > - BIOS traps D0->D3hot transition via something like SMI and > > captures MSI-X state > > - Device enters D3hot > > - Device internal MSI-X state is lost > > - BIOS traps D3hot->D0 transition via SMI > > - Device enters D0 > > - BIOS restores MSI-X state > > - OS resumes use of device > > > > If that's what's happening, the fact that the device loses the > > internal state in D3hot sounds like a *hardware* defect -- if you put > > the device in a system without a BIOS, the D0->D3hot->D0 transitions > > would not work as required by the PCIe spec. > > Actually it's a controller integrated into the APU. > > So any system you put this APU into has a BIOS. Because it's a socketed > APU people can very easily move it from one motherboard to another and one > vendor may have the BIOS properly configuring but another might not. > > > We can call the fact that BIOS lacks the MSI-X save/restore a BIOS > > defect, but the only reason BIOS would *need* that save/restore is > > because of the underlying *hardware* defect. > > > > If that's the case, I would expect a commit log something like this: > > > > The AMD [1022:15b8] USB controller loses some internal functional > > MSI-X context when transitioning from D0 to D3hot. BIOS normally > > traps D0->D3hot and D3hot->D0 transitions so it can save and restore > > that internal context, but some firmware in the field lacks this > > workaround. > > I wouldn't call it a workaround. The hardware is doing exactly as it's > intended for how the firmware programmed. The whole point of the PCI spec is to build devices where standard features like power management can be operated without device-specific knowledge. If we need device-specific code in BIOS or Linux, I'd say that's a workaround. Does this device set No_Soft_Reset? If it does, when it receives a config write to PMCSR that puts it in D3hot, followed by a config write that puts it back in D0, it's supposed to return to D0 with no additional software intervention required. I think that also means no BIOS intervention is required. If it does not set No_Soft_Reset, pci_pm_reset() assumes that a D0->D3hot->D0 transition resets the device. We save and restore the MSI-X Capability in that case, but we do NOT run the DECLARE_PCI_FIXUP_RESUME_EARLY quirks. I think that means MSI-X would not work after a PM reset of this device. Bjorn
