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 aac, ncr, ath, aacraid drivers #2

Merged
merged 10 commits into from
May 16, 2018

Conversation

lakhanshiva
Copy link
Collaborator

@lakhanshiva lakhanshiva commented May 2, 2018

The device id table for aac is

static const struct aac_ident
{
	u_int16_t		vendor;
	u_int16_t		device;
	u_int16_t		subvendor;
	u_int16_t		subdevice;
	int			hwif;
	int			quirks;
	const char		*desc;
}

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

The device id table for ncr is

typedef struct {
	unsigned long	device_id;
	unsigned short	minrevid;
	char	       *name;
	unsigned char	maxburst;
	unsigned char	maxoffs;
	unsigned char	clock_divn;
	unsigned int	features;
} ncr_chip;

unsigned long is atleast 32 bit long (hardware dependent)
unsigned short is atleast 16 bit long (hardware dependent)
Therefore i used "U32:device;U16:#;D:#" as descriptor string in MODULE_PNP_INFO

The device id table for ath is

struct pci_device_id {
	int vendor_id;
	int device_id;
	int sub_vendor_id;
	int sub_device_id;
	int driver_data;
	int match_populated:1;
	int match_vendor_id:1;
	int match_device_id:1;
	int match_sub_vendor_id:1;
	int match_sub_device_id:1;
};

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

The device id table for aacraid is

struct aac_ident
{
	u_int16_t		vendor;
	u_int16_t		device;
	u_int16_t		subvendor;
	u_int16_t		subdevice;
	int			hwif;
	int			quirks;
	char			*desc;
}

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

@lakhanshiva lakhanshiva requested a review from bsdimp May 3, 2018 05:47
@lakhanshiva lakhanshiva changed the title Adding PNP_INFO to aac Add PNP info to PCI attachment of aac, ncr drivers May 3, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of aac, ncr drivers Add PNP info to PCI attachment of aac, ncr, ath drivers May 3, 2018
@lakhanshiva lakhanshiva changed the title Add PNP info to PCI attachment of aac, ncr, ath drivers Add PNP info to PCI attachment of aac, ncr, ath, aacraid drivers May 11, 2018
@@ -493,6 +493,8 @@ static driver_t aacch_driver = {

static devclass_t aacch_devclass;
DRIVER_MODULE(aacch, pci, aacch_driver, aacch_devclass, NULL, NULL);
MODULE_PNP_INFO("U16:vendor;U16:device;U16:#;U16:#", pci, aacch,
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think we need the trailing two U16:#; on these. You use those to skip bits that you need to.

@@ -86,6 +86,9 @@ static driver_t aacraid_pci_driver = {
static devclass_t aacraid_devclass;

DRIVER_MODULE(aacraid, pci, aacraid_pci_driver, aacraid_devclass, 0, 0);
MODULE_PNP_INFO("U16:vendor;U16:device;U16:#;U16:#", pci, aacraid,
Copy link
Owner

Choose a reason for hiding this comment

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

Same here.

@lakhanshiva
Copy link
Collaborator Author

lakhanshiva commented May 14, 2018 via email

@lakhanshiva
Copy link
Collaborator Author

The pull request is building fine. Can i merge this, if you can give LGTM ?

@lakhanshiva lakhanshiva merged this pull request into bsdimp:lakhan-pnpinfo May 16, 2018
bsdimp pushed a commit that referenced this pull request Jul 8, 2018
(or peel off the band-aid, whatever floats your boat)

This addresses two separate issues:

1.) Nothing within bsdgrep actually knew whether it cared about line numbers
  or not.

2.) The file layer knew nothing about the context in which it was being
  called.

#1 is only important when we're *not* processing line-by-line. #2 is
debatably a good idea; the parsing context is only handy because that's
where we store current offset information and, as of this commit, whether or
not it needs to be line-aware.
bsdimp pushed a commit that referenced this pull request Dec 9, 2019
Currently NMIs are sent over event channels, but that defeats the
purpose of NMIs since event channels can be masked. Fix this by
issuing NMIs using a hypercall, which injects a NMI (vector #2) to the
desired vCPU.

Note that NMIs could also be triggered using the emulated local APIC,
but using a hypercall is better from a performance point of view
since it doesn't involve instruction decoding when not using x2APIC
mode.

Reported and Tested by:	avg
Sponsored by:		Citrix Systems R&D
bsdimp pushed a commit that referenced this pull request Aug 25, 2020
…2832,

r332850-r332852, r332856, r332858, r332876, r333351, r334803,
r334806-r334809, r334821, r334837, r334889, r335188, r351769, r352691

r320414:
Expect :mmap_eof_not_eol to fail

It relies on a jemalloc feature (opt.redzone) no longer available after
r319971.

r328559:
Remove t_grep:mmap_eof_not_eol test

The test was marked as an expected failure in r320414 after r319971's import
of a newer jemalloc removed an essential feature (opt.redzone) for
reproducing the behavior it was testing. Since then, no way has been found
or demonstrated to reliably test the behavior, so remove the test.

r332805:
bsdgrep: Split match processing out of procfile

procfile is getting kind of hairy, and it's not going to get better as we
correct some more bits that assume we process one line at a time.

r332806:
bsdgrep: Clean up procmatches a little bit

r332809:
bsdgrep: Add some TODOs for future work on operating on chunks

r332832:
bsdgrep: Break procmatches down a little bit more

Split the matching and non-matching cases out into their own functions to
reduce future complexity. As the name implies, procmatches will eventually
process more than one match itself in the future.

r332850:
bsdgrep: Some light cleanup

There's no point checking for a bunch of file modes if we're not a
practicing believer of DIR_SKIP or DEV_SKIP.

This also reduces some style violations that were particularly ugly looking
when browsing through.

r332851:
bsdgrep: More trivial cleanup/style cleanup

We can avoid branching for these easily reduced patterns

r332852:
bsdgrep: if chain => switch

This makes some of this a little easier to follow (in my opinion).

r332856:
bsdgrep: Fix --include/--exclude ordering issues

Prior to r332851:
* --exclude always win out over --include
* --exclude-dir always wins out over --include-dir

r332851 broke that behavior, resulting in:
* First of --exclude, --include wins
* First of --exclude-dir, --include-dir wins

As it turns out, both behaviors are wrong by modern grep standards- the
latest rule wins. e.g.:

`grep --exclude foo --include foo 'thing' foo`
foo is included

`grep --include foo --exclude foo 'thing' foo`
foo is excluded

As tested with GNU grep 3.1.

This commit makes bsdgrep follow this behavior.

r332858:
bsdgrep: Use grep_strdup instead of grep_malloc+strcpy

r332876:
bsdgrep: Fix build failure WITHOUT_LZMA (incorrect bracket placement)

r333351:
bsdgrep: Allow "-" to be passed to -f to mean "standard input"

A version of this patch was originally sent to me by se@, matching behavior
from newer versions of GNU grep.

While there have been some differences of opinion on whether stdin should be
closed or not after depleting it in process of -f, I've opted to leave stdin
open and just let the later matching stuff fail and result in a no-match.
I'm not married to the current behavior- it was generally chosen since we
are adopting this in particular from GNU grep, and I would like to stay
consistent without a strong argument to the contrary. The current behavior
isn't technically wrong, it's just fairly unfriendly to the developer-user
of grep that may not realize their usage is trivially invalid.

r334803:
netbsd-tests: grep(1): Add test for -c flag

Someone might be inclined to accidentally break this. someone might have
written said test because they broke it locally.

r334806:
bsdgrep(1): Do some less dirty things with return types

Neither procfile nor grep_tree return anything meaningful to their callers.
None of the callers actually care about how many lines were matched in all
of the files they processed; it's all about "did anything match?"

This is generally just a light refactoring to remind me of what actually
matters as I'm rewriting these bits to care less about 'stuff'.

r334807:
bsdgrep(1): whoops, garbage collect the now write-only variable

r334808:
bsdgrep(1): Don't initialize fts_flags twice

Admittedly, this is a clang-scan complaint... but it wasn't wrong. fts_flags
is initialized by all cases in the switch(), which should be fairly obvious.
Annotate this anyways.

r334809:
netbsd-tests: bsdgrep(1): Add a test for -m, too

r334821:
bsdgrep(1): Slooowly peel away the chunky onion

(or peel off the band-aid, whatever floats your boat)

This addresses two separate issues:

1.) Nothing within bsdgrep actually knew whether it cared about line numbers
  or not.

2.) The file layer knew nothing about the context in which it was being
  called.

#1 is only important when we're *not* processing line-by-line. #2 is
debatably a good idea; the parsing context is only handy because that's
where we store current offset information and, as of this commit, whether or
not it needs to be line-aware.

r334837:
bsdgrep(1): Evict character sequence that moved in

r334889:
bsdgrep(1): Some more int -> bool conversions and name changes

Again motivated by upcoming work to rewrite a bunch of this- single-letter
variable names and slightly misleading variable names ("lastmatches" to
indicate that the last matched) are not helpful.

r335188:
bsdgrep(1): Remove redundant initialization; unconditionally assigned later

r351769:
bsdgrep(1): add some basic tests for some GNU Extension support

These will be expanded later as I come up with good test cases; for now,
these seem to be enough to trigger bugs in base gnugrep and expose missing
features in bsdgrep.

r352691:
bsdgrep(1): various fixes of empty pattern/exit code/-c behavior

When an empty pattern is encountered in the pattern list, I had previously
broken bsdgrep to count that as a "match all" and ignore any other patterns
in the list. This commit rectifies that mistake, among others:

- The -v flag semantics were not quite right; lines matched should have been
  counted differently based on whether the -v flag was set or not. procline
  now definitively returns whether it's matched or not, and interpreting
  that result has been kicked up a level.
- Empty patterns with the -x flag was broken similarly to empty patterns
  with the -w flag. The former is a whole-line match and should be more
  strict, only matching blank lines. No -x and no -w will will match the
  empty string at the beginning of each line.
- The exit code with -L was broken, w.r.t. modern grep. Modern grap will
  exit(0) if any file that didn't match was output, so our interpretation
  was simply backwards. The new interpretation makes sense to me.

Tests updated and added to try and catch some of this.

This misbehavior was found by autoconf while fixing ports found in PR 229925
expecting either a more sane or a more GNU-like sed.
bsdimp pushed a commit that referenced this pull request Sep 29, 2022
* Use same filter func (rib_filter_f_t) for nexhtop groups to
 simplify callbacks.
* simplify conditional route deletion & remove the need to pass
 rt_addrinfo to the low-level deletion functions
* speedup rib_walk_del() by removing an additional per-prefix lookup

Differential Revision: https://reviews.freebsd.org/D36071
MFC after:	1 month
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 Jun 10, 2023
Fix all -Wparameter-unused and cast alignment

Differential Revision: https://reviews.freebsd.org/D40303
MFC after:	2 weeks
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
bsdimp pushed a commit that referenced this pull request Jan 6, 2024
Specifically, altering the console list with conscontrol has some weird
behavior:

1. If you remove the first configured console, /dev/console will become
  unconfigured
2. Any console added becomes the /dev/console

In a multicons situation, #1 is clearly a bug and #2 is perhaps slightly
less clear.  If we have ttyu0, ttyv0, then it seems obvious that one
would want ttyv0 to take over the console if ttyu0 is removed.  If we
add ttyu0 back in, then it's debatable whether it should take over the
console or not.

Fix it now to make the /dev/console selection more FIFO-ish, with
respect to how conscontrol affects it.  A `primary` verb for
conscontrol(8) might be a good addition.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants