Paths

Table of Contentst

Differential D56997

sys: Use is_pci_device instead of direct comparisons to devclasses
ClosedPublic
Actions

Authored by jhb on Wed, May 13, 8:27 PM.

Tags

None

Referenced Files

	F157853127: D56997.id178028.diff
	Mon, May 25, 8:44 PM

	F157800794: D56997.id178015.diff
	Mon, May 25, 8:25 AM

	F157798394: D56997.id178028.diff
	Mon, May 25, 7:46 AM

	F157788500: D56997.diff
	Mon, May 25, 5:10 AM

	F157788318: D56997.diff
	Mon, May 25, 5:07 AM

	Unknown Object (File)
	Sun, May 24, 9:12 AM

	Unknown Object (File)
	Sat, May 23, 5:58 AM

	Unknown Object (File)
	Sat, May 23, 3:53 AM

Subscribers

Details

Reviewers

imp
andrew
manu
bz
kib

Commits

rG6dc813301a17: sys: Use is_pci_device instead of direct comparisons to devclasses

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

jhb created this revision.Wed, May 13, 8:27 PM

Herald added a reviewer: andrew. · View Herald TranscriptWed, May 13, 8:27 PM

Herald added a reviewer: andrew. · View Herald Transcript

Herald added a reviewer: manu. · View Herald Transcript

Herald added subscribers: linuxkpi, emaste. · View Herald Transcript

Harbormaster completed remote builds in B73088: Diff 177815.Wed, May 13, 8:27 PM

jhb requested review of this revision.Wed, May 13, 8:27 PM

jhb added a parent revision: D56996: pci: Add is_pci_device helper function.

jhb added a child revision: D56998: sys: Use is_pci_device instead of checking device or devclass names.

No necessarily changes but the LinuxKPI dev_is_pci() variant should get more careful checking (especially in drm land; I believe wireless is fine).

sys/compat/linuxkpi/common/include/linux/pci.h
401–402	This seems wrong, doesn't it one or the other way (before or now)? We were not getting the parent here but now. In theory that should be fine unless it's the acpi0 -> pcib0 lookup (unlikely to be hit with this function) or we hit any special "drmn" magic which is not a PCI device.
sys/dev/iommu/busdma_iommu.c
120	Could move some of them into a smaller scope; at least for pcip and pcibp that would make sense now.

This revision now requires changes to proceed.Wed, May 13, 9:01 PM

imp added inline comments.Thu, May 14, 1:26 AM

sys/compat/linuxkpi/common/include/linux/pci.h
401–402	Yea. All the others check the parent. This checks the device

jhb added inline comments.Thu, May 14, 1:16 PM

sys/compat/linuxkpi/common/include/linux/pci.h
401–402	Oh humm, so it is. I wonder if the old version of this was broken though. The code on Linux appears to be testing a leaf device not a PCI bus device, in particular see how it is used in `dev_is_pf` in the next line which clearly assumes that it is operating on a PF, not a logical bus: https://github.com/torvalds/linux/blob/e1914add2799225a87502051415fc5c32aeb02ae/include/linux/pci.h#L1278 That said, that means this change should be its own followup commit as a fix for a pre-existing bug, or perhaps I should fix it to check the parent first?

jhb added a reviewer: kib.Thu, May 14, 1:16 PM

jhb added inline comments.

sys/dev/iommu/busdma_iommu.c
120	Possibly, though that adds more noise to the diff? It's also true that `pcip` isn't really needed either if one is content with a nested call to device_get_parent.

x86/iommu part is certainly fine.

bz added inline comments.Thu, May 14, 9:29 PM

sys/compat/linuxkpi/common/include/linux/pci.h
401–402	Can you do a quick fix in main and then rebase this on top? I'd try to approve as son as I see it.

Rebase on top of LinuxKPI fixup

Harbormaster completed remote builds in B73192: Diff 178015.Mon, May 18, 3:23 PM

jhb added a parent revision: D57066: LinuxKPI: Check the devclass of the parent device in dev_is_pci.Mon, May 18, 3:25 PM

bz accepted this revision.Mon, May 18, 3:40 PM

This revision is now accepted and ready to land.Mon, May 18, 3:40 PM

Closed by commit rG6dc813301a17: sys: Use is_pci_device instead of direct comparisons to devclasses (authored by jhb). · Explain WhyMon, May 18, 6:53 PM

This revision was automatically updated to reflect the committed changes.

jhb added a commit: rG6dc813301a17: sys: Use is_pci_device instead of direct comparisons to devclasses.

Revision Contents
Changeset List

Path

Size

sys/

arm64/

cavium/

thunder_pcie_pem.c

6 lines

iommu/

6 lines

compat/

linuxkpi/

common/

include/

linux/

6 lines

dev/

ata/

3 lines

bge/

13 lines

ichwd/

5 lines

iommu/

32 lines

pci/

10 lines

6 lines

vnic/

thunder_bgx_fdt.c

9 lines

x86/

iommu/

3 lines

13 lines

Diff 178028

sys/arm64/cavium/thunder_pcie_pem.c

Loading...

sys/arm64/iommu/smmu.c

Loading...

sys/compat/linuxkpi/common/include/linux/pci.h

Loading...

sys/dev/ata/ata-all.c

Loading...

sys/dev/bge/if_bge.c

Loading...

sys/dev/ichwd/ichwd.c

Loading...

sys/dev/iommu/busdma_iommu.c

Loading...

sys/dev/pci/pci.c

Loading...

sys/dev/pci/vga_pci.c

Loading...

sys/dev/vnic/thunder_bgx_fdt.c

Loading...

sys/x86/iommu/amd_drv.c

Loading...

sys/x86/iommu/intel_drv.c

Loading...