On 2022-11-09 17:42:10, Tyler Hicks (Microsoft) wrote: > On 2022-11-03 17:14:29, Bjorn Helgaas wrote: > > On Thu, Oct 27, 2022 at 05:51:49PM -0500, Bjorn Helgaas wrote: > > > On Tue, Oct 25, 2022 at 12:07:47AM -0500, Tyler Hicks wrote: > > > > On 2022-10-20 15:24:37, Bjorn Helgaas wrote: > > > > > > I've been talking to the firmware folks on why SAFE mode was selected, > > > > based on Keith's question from Wednesday. From what I've been told, > > > > U-Boot doesn't seed the RP MPS with a value so the kernel sees a value > > > > of 128 for p_mps in pci_configure_mps() when using the DEFAULT mode. > > > > Apparently UEFI does seed the RP MPS but we don't get that with U-Boot. > > > > Therefore, SAFE mode was selected to get an initial, tuned RP MPS value > > > > set to 256. > > > > > > Are there any devices below the RP at the time we set MPS=256? > > > > > > > > A subsequent hot-add will do nothing in pci_configure_mps(), and > > > > > pcie_bus_configure_settings() looks like it would set the RP and EP > > > > > MPS to the minimum of the RP and EP MPS_Supported. > > > > > > > > > > Is that what you see? What would you like to see instead? > > > > > > > > No, not quite. After hot-add, we see the EP MPS set to 128 with SAFE > > > > mode even though the RP MPS is 256. > > > > > > Can you add the relevant topology here so we can work through the > > > concrete details? > > # lspci -t > -[0000:00]---00.0-[01-ff]--+-00.0 > +-00.1 > +-00.2 > +-00.3 > \-00.4 > > > > > Is the endpoint hot-added directly below a Root Port? Or is there a > > > switch involved? > > There's not a switch involved. The multifunction endpoint is hot-added directly > below the root port. > > > > What are the MPS_Supported values for all the devices? If there's a switch > > > in the picture, it looks like we currently restrict to 128, I think because > > > it's possible an endpoint that can only do 128 may be added. > > 0000:00's MPS_Supported value is 256. > > The multifunction endpoint's MPS_Supported is 512. > > > Ping? I'd like to talk about a concrete scenario. It's too hard for > > me to imagine all the possible things that could go wrong. > > Sorry for the slow reply here. Thanks for your interest in getting more > details. Hey Bjorn - I wanted to re-ping you on this discussion since we're on the other side of the merge window now. Let me know if you need anymore details. Thanks! Tyler > > > I guess part of what's interesting here is that things work better > > when firmware has already configured MPS? It doesn't seem like we > > should *depend* on firmware having done that. > > Our firmware folks felt the same way. > > Tyler > > > > > Bjorn