On Mon, Mar 20, 2023 at 02:47:48PM -0500, Limonciello, Mario wrote: > > > > 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. > > Right. I "think" the confusion here might stem from the term "BIOS". > > The initialization of the hardware happens from a series of microcontrollers > in the APU before X86 cores are released from reset. By the time UEFI runs > all of this should have already happened. > > When I say "BIOS" I mean collectively "all" of this firmware. I don't understand the point you're making here. I don't think it matters whether this device-specific knowledge is in APU microcontroller firmware, BIOS, Linux, etc. I'm trying to suggest that if we zoom all the way in and look just at the PCIe TLPs, we would see two config writes that put the device in D3hot, then back in D0. A working device should end up either back in D0active with MSI-X fully enabled (if No_Soft_Reset is set and MSI-X was originally enabled), or in D0uninitialized (if No_Soft_Reset is clear). But with this device, apparently some additional software intervention is required, i.e., after the config write to go back to D0, we need two more writes to clear and set the MSI-X enable bit. Let's say somebody runs coreboot on this platform. Does coreboot need this device-specific knowledge? > > If we need device-specific code in BIOS or Linux, I'd say > > that's a workaround. > > Something I'd like to point out in case it wasn't apparent is Windows > doesn't actually hit this problem. It doesn't matter if the correct > hardware initialization code is included in "BIOS" or not. That's interesting because it means there's something we (I, at least) don't understand about what's going on here. Maybe Windows never puts the device in D3hot at runtime? Or maybe Windows disables MSI-X before putting it into D3 and re-enables it after going to D0? I don't know how Windows power management works, so I'm not sure this tells us anything useful about Linux. > > 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
