Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb, fxp, gem, glxsb, grackle_hb, hme drivers #5

Merged
merged 0 commits into from Jun 14, 2018

Conversation

lakhanshiva
Copy link
Collaborator

@lakhanshiva lakhanshiva commented Jun 3, 2018

The device id table for cas is

static const struct cas_pci_dev {
	uint32_t	cpd_devid;
	uint8_t		cpd_revid;
	int		cpd_variant;
	const char	*cpd_desc;
} cas_pci_devlist[];

Therefore i used "W32:vendor/device" as descriptor string in MODULE_PNP_INFO

The device id table for ena is

typedef struct _ena_vendor_info_t {
	uint16_t vendor_id;
	uint16_t device_id;
 	unsigned int index;
 } ena_vendor_info_t;

Therefore i used "U16:vendor; U16:device" as descriptor string in MODULE_PNP_INFO

The device id table for cy is

static struct cy_dev {
	uint32_t deviceid;
	const char *description;
} cy_devs[]

Therefore i used "U32:device" as descriptor string in MODULE_PNP_INFO

The device id table for esp is

static struct esp_dev {
	uint32_t deviceid;
	const char *description;
} esp_devs[]

Therefore i used "U32:device" as descriptor string in MODULE_PNP_INFO

The device id table for bktr is

static struct bktr_dev {
	uint16_t vendorid;
	uint16_t deviceid;
	uint8_t rev;
	const char *description;
} bktr_devs[]

Therefore i used "U16:vendor;U16:device" as descriptor string in MODULE_PNP_INFO

The device id table for cxgb is

struct cxgb_ident {
	uint16_t	vendor;
	uint16_t	device;
	int		index;
	char		*desc;
} cxgb_identifiers[]

Therefore i used "U16:vendor;U16:device" as descriptor string in MODULE_PNP_INFO

The device id table for fxp is

struct fxp_ident {
	uint16_t	vendor;
	uint16_t	device;
	int16_t		revid;		/* -1 matches anything */
	uint8_t		ich;
	const char	*name;
};

Therefore i used "U16:vendor;U16:device" as descriptor string in MODULE_PNP_INFO

The device id table for gem is

static const struct gem_pci_dev {
	uint32_t	gpd_devid;
	int		gpd_variant;
	const char	*gpd_desc;
} gem_pci_devlist[]

Therefore i used "U32:device" as descriptor string in MODULE_PNP_INFO

The device id table for glxsb is

static struct glxsb_dev{
	uint16_t vendorid;
	uint16_t deviceid;
	const char *description;
} glxsb_devs[]

Therefore i used "U16:vendor;U16:device" as descriptor string in MODULE_PNP_INFO

The device id table for grackle_hb is

static struct grackle_hb_dev {
	uint32_t deviceid;
	const char *description;
} grackle_hb_devs[]

Therefore i used "U32:device" as descriptor string in MODULE_PNP_INFO

The device id table for hme is

static struct hme_dev{
	uint16_t vendorid;
	uint16_t deviceid;
	const char *description;
} hme_devs[]

Therefore i used "U16:vendor;U16:device" as descriptor string in MODULE_PNP_INFO

@lakhanshiva lakhanshiva requested a review from bsdimp June 3, 2018 17:19
@lakhanshiva lakhanshiva self-assigned this Jun 3, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas drivers Add PNP info to PCI attachment of cas, ena drivers Jun 4, 2018
@@ -151,8 +151,8 @@ struct msix_entry {
};

typedef struct _ena_vendor_info_t {
unsigned int vendor_id;
unsigned int device_id;
uint16_t vendor_id;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unsigned int is not necessarily the same size as uint16_t. Is this ok?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Supported PCI vendor and device id's fit into a uint16_t

/*
 * Supported PCI vendor and devices IDs
 */
#define	PCI_VENDOR_ID_AMAZON	0x1d0f

#define	PCI_DEV_ID_ENA_PF	0x0ec2
#define	PCI_DEV_ID_ENA_LLQ_PF	0x1ec2
#define	PCI_DEV_ID_ENA_VF	0xec20
#define	PCI_DEV_ID_ENA_LLQ_VF	0xec21

Even in ena_probe, it is being compared to a uint16_t type..

static int
ena_probe(device_t dev)
{
	ena_vendor_info_t *ent;
	char		adapter_name[60];
	uint16_t	pci_vendor_id = 0;
	uint16_t	pci_device_id = 0;

	pci_vendor_id = pci_get_vendor(dev);
	pci_device_id = pci_get_device(dev);

	ent = ena_vendor_info_array;
	while (ent->vendor_id != 0) {
		if ((pci_vendor_id == ent->vendor_id) &&
		    (pci_device_id == ent->device_id))

So i think this is the appropriate type for this..

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After a review with @bsdimp , I'm ok with this change

@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena drivers Add PNP info to PCI attachment of cas, ena, cy drivers Jun 6, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena, cy drivers Add PNP info to PCI attachment of cas, ena, cy, esp, bktr drivers Jun 6, 2018
@lakhanshiva lakhanshiva force-pushed the lakhan-pnpinfo branch 6 times, most recently from 9371117 to 8052c0e Compare June 6, 2018 12:57
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena, cy, esp, bktr drivers Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, em, igb drivers Jun 6, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, em, igb drivers Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, em, igb, cxgb drivers Jun 6, 2018
@lakhanshiva lakhanshiva force-pushed the lakhan-pnpinfo branch 3 times, most recently from 2a2c6cf to 2469de3 Compare June 7, 2018 04:41
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, em, igb, cxgb drivers Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb drivers Jun 7, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb drivers Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb, fxp drivers Jun 8, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb, fxp drivers Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb, fxp, gem drivers Jun 8, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb, fxp, gem drivers Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb, fxp, gem, glxsb, grackle_hb drivers Jun 10, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb, fxp, gem, glxsb, grackle_hb drivers Add PNP info to PCI attachment of cas, ena, cy, esp, bktr, cxgb, fxp, gem, glxsb, grackle_hb, hme drivers Jun 11, 2018
@lakhanshiva lakhanshiva force-pushed the lakhan-pnpinfo branch 4 times, most recently from 8e50e92 to e1b82c0 Compare June 14, 2018 13:03
lakhanshiva added a commit to lakhanshiva/freebsd that referenced this pull request Jun 14, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5
lakhanshiva added a commit to lakhanshiva/freebsd that referenced this pull request Jun 14, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5
uqs pushed a commit to freebsd/freebsd-src that referenced this pull request Jul 8, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5
uqs pushed a commit to freebsd/freebsd-src that referenced this pull request Jul 8, 2018
Move device table earlier in the file so we can reference it in the
PNP_INFO macro.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5
bsdimp pushed a commit that referenced this pull request Jul 8, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: #5
bsdimp pushed a commit that referenced this pull request Jul 8, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: #5
bsdimp pushed a commit that referenced this pull request Jul 8, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: #5
bsdimp pushed a commit that referenced this pull request Jul 8, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: #5
bsdimp pushed a commit that referenced this pull request Jul 8, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: #5
bsdimp pushed a commit that referenced this pull request Jul 8, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: #5
bsdimp pushed a commit that referenced this pull request Jul 8, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: #5
mat813 pushed a commit to mat813/freebsd that referenced this pull request Jul 12, 2018
Move module delcaration to be after device table. The PNP_INFO must
follow the module declaration.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5


git-svn-id: https://svn.freebsd.org/base/head@336098 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
mat813 pushed a commit to mat813/freebsd that referenced this pull request Jul 12, 2018
Make unsigned values uint16_t for pnp table. They are properly
uint16_t befause they are 16-bit PCI IDs. The PNP_INFO language has no
type for bare unsigned.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5


git-svn-id: https://svn.freebsd.org/base/head@336099 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
mat813 pushed a commit to mat813/freebsd that referenced this pull request Jul 12, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5


git-svn-id: https://svn.freebsd.org/base/head@336100 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
mat813 pushed a commit to mat813/freebsd that referenced this pull request Jul 12, 2018
Move device table earlier in the file so we can reference it in the
PNP_INFO macro.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5


git-svn-id: https://svn.freebsd.org/base/head@336101 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
bdrewery pushed a commit to bdrewery/freebsd that referenced this pull request Jul 31, 2018
Move module delcaration to be after device table. The PNP_INFO must
follow the module declaration.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5


git-svn-id: svn+ssh://svn.freebsd.org/base/head@336098 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
bdrewery pushed a commit to bdrewery/freebsd that referenced this pull request Jul 31, 2018
Make unsigned values uint16_t for pnp table. They are properly
uint16_t befause they are 16-bit PCI IDs. The PNP_INFO language has no
type for bare unsigned.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5


git-svn-id: svn+ssh://svn.freebsd.org/base/head@336099 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
bdrewery pushed a commit to bdrewery/freebsd that referenced this pull request Jul 31, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5


git-svn-id: svn+ssh://svn.freebsd.org/base/head@336100 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
bdrewery pushed a commit to bdrewery/freebsd that referenced this pull request Jul 31, 2018
Move device table earlier in the file so we can reference it in the
PNP_INFO macro.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp#5


git-svn-id: svn+ssh://svn.freebsd.org/base/head@336101 ccf9f872-aa2e-dd11-9fc8-001c23d0bc1f
brooksdavis pushed a commit to CTSRD-CHERI/cheribsd that referenced this pull request Sep 7, 2018
Move module delcaration to be after device table. The PNP_INFO must
follow the module declaration.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp/freebsd#5
brooksdavis pushed a commit to CTSRD-CHERI/cheribsd that referenced this pull request Sep 7, 2018
Make unsigned values uint16_t for pnp table. They are properly
uint16_t befause they are 16-bit PCI IDs. The PNP_INFO language has no
type for bare unsigned.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp/freebsd#5
brooksdavis pushed a commit to CTSRD-CHERI/cheribsd that referenced this pull request Sep 7, 2018
Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp/freebsd#5
brooksdavis pushed a commit to CTSRD-CHERI/cheribsd that referenced this pull request Sep 7, 2018
Move device table earlier in the file so we can reference it in the
PNP_INFO macro.

Reviewed by: imp, chuck
Submitted by: Lakhan Shiva Kamireddy <lakhanshiva@gmail.com>
Sponsored by: Google, Inc. (GSoC 2018)
Pull Request: bsdimp/freebsd#5
bsdimp pushed a commit that referenced this pull request May 10, 2023
Under certain loads, the following panic is hit:

    panic: page fault
    KDB: stack backtrace:
    #0 0xffffffff805db025 at kdb_backtrace+0x65
    #1 0xffffffff8058e86f at vpanic+0x17f
    #2 0xffffffff8058e6e3 at panic+0x43
    #3 0xffffffff808adc15 at trap_fatal+0x385
    #4 0xffffffff808adc6f at trap_pfault+0x4f
    #5 0xffffffff80886da8 at calltrap+0x8
    #6 0xffffffff80669186 at vgonel+0x186
    #7 0xffffffff80669841 at vgone+0x31
    #8 0xffffffff8065806d at vfs_hash_insert+0x26d
    #9 0xffffffff81a39069 at sfs_vgetx+0x149
    #10 0xffffffff81a39c54 at zfsctl_snapdir_lookup+0x1e4
    #11 0xffffffff8065a28c at lookup+0x45c
    #12 0xffffffff806594b9 at namei+0x259
    #13 0xffffffff80676a33 at kern_statat+0xf3
    #14 0xffffffff8067712f at sys_fstatat+0x2f
    #15 0xffffffff808ae50c at amd64_syscall+0x10c
    freebsd#16 0xffffffff808876bb at fast_syscall_common+0xf8

The page fault occurs because vgonel() will call VOP_CLOSE() for active
vnodes. For this reason, define vop_close for zfsctl_ops_snapshot. While
here, define vop_open for consistency.

After adding the necessary vop, the bug progresses to the following
panic:

    panic: VERIFY3(vrecycle(vp) == 1) failed (0 == 1)
    cpuid = 17
    KDB: stack backtrace:
    #0 0xffffffff805e29c5 at kdb_backtrace+0x65
    #1 0xffffffff8059620f at vpanic+0x17f
    #2 0xffffffff81a27f4a at spl_panic+0x3a
    #3 0xffffffff81a3a4d0 at zfsctl_snapshot_inactive+0x40
    #4 0xffffffff8066fdee at vinactivef+0xde
    #5 0xffffffff80670b8a at vgonel+0x1ea
    #6 0xffffffff806711e1 at vgone+0x31
    #7 0xffffffff8065fa0d at vfs_hash_insert+0x26d
    #8 0xffffffff81a39069 at sfs_vgetx+0x149
    #9 0xffffffff81a39c54 at zfsctl_snapdir_lookup+0x1e4
    #10 0xffffffff80661c2c at lookup+0x45c
    #11 0xffffffff80660e59 at namei+0x259
    #12 0xffffffff8067e3d3 at kern_statat+0xf3
    #13 0xffffffff8067eacf at sys_fstatat+0x2f
    #14 0xffffffff808b5ecc at amd64_syscall+0x10c
    #15 0xffffffff8088f07b at fast_syscall_common+0xf8

This is caused by a race condition that can occur when allocating a new
vnode and adding that vnode to the vfs hash. If the newly created vnode
loses the race when being inserted into the vfs hash, it will not be
recycled as its usecount is greater than zero, hitting the above
assertion.

Fix this by dropping the assertion.

FreeBSD-issue: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=252700
Reviewed-by: Andriy Gapon <avg@FreeBSD.org>
Reviewed-by: Mateusz Guzik <mjguzik@gmail.com>
Reviewed-by: Alek Pinchuk <apinchuk@axcient.com>
Reviewed-by: Ryan Moeller <ryan@iXsystems.com>
Signed-off-by: Rob Wing <rob.wing@klarasystems.com>
Co-authored-by: Rob Wing <rob.wing@klarasystems.com>
Submitted-by: Klara, Inc.
Sponsored-by: rsync.net
Closes #14501
bsdimp pushed a commit that referenced this pull request Aug 16, 2023
Avoid locking issues when if_allmulti() calls the driver's if_ioctl,
because that may acquire sleepable locks (while we hold a non-sleepable
rwlock).

Fortunately there's no pressing need to hold the mroute lock while we
do this, so we can postpone the call slightly, until after we've
released the lock.

This avoids the following WITNESS warning (with iflib drivers):

	lock order reversal: (sleepable after non-sleepable)
	 1st 0xffffffff82f64960 IPv4 multicast forwarding (IPv4 multicast forwarding, rw) @ /usr/src/sys/netinet/ip_mroute.c:1050
	 2nd 0xfffff8000480f180 iflib ctx lock (iflib ctx lock, sx) @ /usr/src/sys/net/iflib.c:4525
	lock order IPv4 multicast forwarding -> iflib ctx lock attempted at:
	#0 0xffffffff80bbd6ce at witness_checkorder+0xbbe
	#1 0xffffffff80b56d10 at _sx_xlock+0x60
	#2 0xffffffff80c9ce5c at iflib_if_ioctl+0x2dc
	#3 0xffffffff80c7c395 at if_setflag+0xe5
	#4 0xffffffff82f60a0e at del_vif_locked+0x9e
	#5 0xffffffff82f5f0d5 at X_ip_mrouter_set+0x265
	#6 0xffffffff80bfd402 at sosetopt+0xc2
	#7 0xffffffff80c02105 at kern_setsockopt+0xa5
	#8 0xffffffff80c02054 at sys_setsockopt+0x24
	#9 0xffffffff81046be8 at amd64_syscall+0x138
	#10 0xffffffff8101930b at fast_syscall_common+0xf8

See also:	https://redmine.pfsense.org/issues/12079
Reviewed by:	mjg
Sponsored by:	Rubicon Communications, LLC ("Netgate")
Differential Revision:	https://reviews.freebsd.org/D41209
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants