In D60602#1389595, @gallatin wrote:

In D60602#1389590, @kbowling wrote:

Otherwise (eventually) drop that file and use __builtin_prefetch directly (my preference). The 2lines variant would be dead code after simpletx so it can remain as some short term hack.

My initial stupid test didn't catch the fact that __builtin_prefetch doesn't work on x86 because clang skips prefetch if you compile with -mno-sse. So we need the wrapper.
For now, I just want to get prefetching working on iflib for arm64. I'll try to run some tests on nt0 vs nt1, which we'd want to do before using kern_prefetch

It looks like rrs added a sys/kern_prefetch.h... I would say use/move the definitions there if you must maintain wrappers (and drop the amd64 case in it).

Could we first evaluate tuning txq_max_db_deferred() and having simpletx honor the existing policy?

It seems like e1000 has the same prior code (igc is generally copied from it)

I'm about to leave on a business trip so I can't give a proper affirmative review, but one thing jumps out without digging into anything.. the multi-line comment and another control flow make it somewhat unpleasing to me. I would investigate a couple ways of writing this, either build the ring decision up with single statement ifs (and line comments as needed), or use de morgans to move true or false around so there are early exits and only one ring decision (perhaps de-indented.. whatever ends up feeling clean after playing with it.

Looks good to me. There are probably similar elsewhere in the tree.

I thought the idea of d9c55b2e8cd6b79f6926278e10a79f1bcca27a4b was to get rid of most of the ifdef. rss_gethashconfig is a public function, why not just apply what you have in the ifdef RSS section always?

This is sufficient as a fix.

Are you waiting for internal validation? I don't like keeping head in a known broken state this long if their findings can be posted as a followup.

Rebase, use vendored TcgService.h, move new code to libefi

@olivier can you make sure I didn't break Pluton boot

In D59477#1371334, @kib wrote:

I am curious what, in your opinion, drivers should do if they get an error from PCI config space access? We tried to disable the busmaster (or any other enable bit in the config register, see below) and it failed, then what?

This now handles the close-first ordering, but the open-first ordering from my example still appears possible. Once an opener passes the SI_LASTCLOSE check, nothing records its presence until after d_open returns and the vnode lock is reacquired. A last close can still run in that interval. We need protection in both directions checking SI_LASTCLOSE only before d_open does not establish mutual exclusion.

It fixes the failed open race (from the mentioned issue) but I was able to trigger the following with an LLM mock:

Drop D_TRACKCLOSE

Should be fixed by updated D59578

In D59692#1370212, @gallatin wrote:

Why is this needed? All NICs should be using the same rss key these days and that's available via the sysctls in sys/net/rss_config.c

@erj thanks that is cleaner

I copied the existing pattern, but instead let's invert this and use bus_space_read_8 everywhere except i386 (for MFC).

Committed as 923b9a80c21f not sure why it didn't auto close

The commit above was supposed to be D58906, sorry for the mixup.

Comitted in a8c60dd5eb8f158c9488db5aa92aafd38d756348

In D59477#1368263, @kib wrote:

In D59477#1368118, @kbowling wrote:

In D59477#1367653, @kib wrote:

What is the purpose of the function? Why it is useful to drain the non-posted transactions? After clearing the bus-master enable bit, how does it make a difference?

The idea was to use this to help build fallback DMA fences in drivers for partial init or failed shutdown. There are some issues in hardware (aquantia - see Linux 7a1bb49461b1, VFs) where I want to fence off DMA, they'd still require device specific cleanup/reset and internal handling a BME fence until safe recovery.

I do not see any relation of the referenced commit to the function you propose, really.

Address review feedback

In D58742#1370364, @zlei wrote:

@kbowling I have the hardware ( QLogic FastLinQ QL41000 ) which is supported by this driver. Do you want me to test with this new feature ?

I must've not been paying attention back when these were added so I don't know the intent behind selecting between them or if we should still have two but that has no bearing on simpletx or the improvement here.

Committed as 9ea7beb173b8b5c77663ee0f86b32cc76f20357f

rebase on main

rebase on main

Looks fine. I will fix the IFF_DRV/QUIESCING feedback when I rebase D59597 / D59598

Please set MFC to 2 weeks when you commit as I intend to pull the GENERIC change back to stable/15 in about a month barring major issues

Seems to match spec as well, CRB control registers are allowed to return 00h or FFh if locality is not owned (https://trustedcomputinggroup.org/wp-content/uploads/PC-Client-Specific-Platform-TPM-Profile-for-TPM-2p0-v1p07_Pub.pdf Table 51)