Re: [regression, bisected, pci/iommu] Bug 216865 - Black screen when amdgpu started during 6.2-rc1 boot with AMD IOMMU enabled

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2023/1/9 21:43, Jason Gunthorpe wrote:
On Sat, Jan 07, 2023 at 10:44:46AM +0800, Baolu Lu wrote:
On 1/6/2023 10:14 PM, Jason Gunthorpe wrote:
On Thu, Jan 05, 2023 at 03:57:28PM +0530, Vasant Hegde wrote:
Matt,

On 1/5/2023 6:39 AM, Matt Fagnani wrote:
I built 6.2-rc2 with the patch applied. The same black screen problem happened
with 6.2-rc2 with the patch. I tried to use early kdump with 6.2-rc2 with the
patch twice by panicking the kernel with sysrq+alt+c after the black screen
happened. The system rebooted after about 10-20 seconds both times, but no kdump
and dmesg files were saved in /var/crash. I'm attaching the lspci -vvv output as
requested.


Thanks for testing. As mentioned earlier I was not expecting this patch to fix
the black screen issue. It should fix kernel warnings and IOMMU page fault
related call traces. By any chance do you have the kernel boot logs?


@Baolu,
    Looking into lspci output, it doesn't list ACS feature for Graphics card. So
with your fix it didn't enable PASID and hence it failed to boot.

The ACS checks being done are feature of the path not the end point or
root port.

If we are expecting ACS on the end port then it is just a bug in how
the test was written.. The test should be a NOP because there are no
switches in this topology.

Looking at it, this seems to just be because pci_enable_pasid is
calling pci_acs_path_enabled wrong, the only other user is here:

	for (bus = pdev->bus; !pci_is_root_bus(bus); bus = bus->parent) {
		if (!bus->self)
			continue;

		if (pci_acs_path_enabled(bus->self, NULL, REQ_ACS_FLAGS))
			break;

		pdev = bus->self;

		group = iommu_group_get(&pdev->dev);
		if (group)
			return group;
	}

And notice it is calling it on pdev->bus not on pdev itself which
naturally excludes the end point from the ACS validation.

So try something like:

	if (!pci_acs_path_enabled(pdev->bus->self, NULL, PCI_ACS_RR | PCI_ACS_UF))

(and probably need to check for null ?)

Yeah! This really is a misuse of pci_acs_path_enabled().

But if @pdev is an endpoint of a multiple function device, perhaps we
still need to check acs on it?

Ah, I don't know anything about what this means from a spec
perspective.

Certainly if a function can internalize MMIO and loop it back to
another function then it surely is not OK for PASID either, nor should
those functions be in different iommu groups.

So, either this never happens for some spec reason, or the test in the
iommu code forming groups is incorrect.

The pci_device_group() path handles this like below:

/*
 * For multifunction devices which are not isolated from each other, find
 * all the other non-isolated functions and look for existing groups.  For
 * each function, we also need to look for aliases to or from other devices
 * that may already have a group.
 */
static struct iommu_group *get_pci_function_alias_group(struct pci_dev *pdev, unsigned long *devfns)
{
        struct pci_dev *tmp = NULL;
        struct iommu_group *group;

        if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
                return NULL;

It seems that all devices of an MFD shares a single iommu group if
there lacks ACS control.

--
Best regards,
baolu



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux