In D58301#1337958, @jrtc27 wrote:

In D58301#1337950, @aokblast wrote:

In D58301#1337946, @jrtc27 wrote:

In D58301#1337933, @aokblast wrote:

In D58301#1337930, @jrtc27 wrote:

In D58301#1337928, @aokblast wrote:

In D58301#1337926, @jrtc27 wrote:

bus_read_8 has been replace to bus_space_read series in sys/bus.h.

Not sure what "bus_space_read series" is though, should that not be bus_space_read_8?

Sorry, it should be bus_space_{read,write} series.

But why series? I don't understand. Why not just use the _8 suffix everywhere?

The _8 suffix remains unchanged before and after my patch. The purpose of this change is to replace the original bus_ series with the bus_space_ series.

Ok, I think I now can decipher what you're trying to get at. What you're trying to say, I think, is something like the following?

bus_{read,write}_8 are macro wrappers around the corresponding bus_space functions in sys/bus.h, so implementing bus_{read,write}_8 won't work. Implement the underlying bus_space functions instead.

Yes, sorry about my bad english expression:)

No worries; normally these things are understandable even if they don't read as perfect English, but in this case for whatever reason I couldn't quite follow it :')

Might I suggest you use my interpretation as inspiration for amending your own message? (I mean, you can just copy the whole thing if you want, but you may prefer it to sound more like you!)

In D58301#1337946, @jrtc27 wrote:

In D58301#1337933, @aokblast wrote:

In D58301#1337930, @jrtc27 wrote:

In D58301#1337928, @aokblast wrote:

In D58301#1337926, @jrtc27 wrote:

bus_read_8 has been replace to bus_space_read series in sys/bus.h.

Not sure what "bus_space_read series" is though, should that not be bus_space_read_8?

Sorry, it should be bus_space_{read,write} series.

But why series? I don't understand. Why not just use the _8 suffix everywhere?

The _8 suffix remains unchanged before and after my patch. The purpose of this change is to replace the original bus_ series with the bus_space_ series.

Ok, I think I now can decipher what you're trying to get at. What you're trying to say, I think, is something like the following?

bus_{read,write}_8 are macro wrappers around the corresponding bus_space functions in sys/bus.h, so implementing bus_{read,write}_8 won't work. Implement the underlying bus_space functions instead.

Yes, sorry about my bad english expression:)

In D58301#1337930, @jrtc27 wrote:

In D58301#1337928, @aokblast wrote:

In D58301#1337926, @jrtc27 wrote:

bus_read_8 has been replace to bus_space_read series in sys/bus.h.

Not sure what "bus_space_read series" is though, should that not be bus_space_read_8?

Sorry, it should be bus_space_{read,write} series.

But why series? I don't understand. Why not just use the _8 suffix everywhere?

In D58301#1337926, @jrtc27 wrote:

bus_read_8 has been replace to bus_space_read series in sys/bus.h.

Not sure what "bus_space_read series" is though, should that not be bus_space_read_8?

Sorry, it should be bus_space_{read,write} series.

Also should probably be using read/write not just read everywhere in the commit message.

Have some patches upstream.

HI, my patch is only the library part. It needs extra work to become a driver.

In D58302#1337163, @kib wrote:

Could f6ee1d20d36813e990691 fixed your issue?

Anyway, the useful tool is gdb' pinfo /o type.

Reflect comments

In D58302#1337157, @kib wrote:

Why? p_asig and p_tree_refcount are at the end of the struct proc.

Remove useless patch

@hs.freebsd_id.omnilan.de I have sent you a mail to ask your desired author and mail few days ago. As others might want to compile on i386, I will use Harry Schmalzb. <freebsd@omnilan.de> and commit tomorrow.

In D57885#1335409, @dch wrote:

By default this is the permissions the log file is created with:

-rw-------  1 root wheel  153B Jul 13 23:35 buildkite.log

If this is what you intend to happen then let's commit this as is. If you're expecting it to be created readable by buildkite_account then it would need changing to have a precmd script.

Tested with yubikey.

In D51865#1334521, @kevans wrote:

In D51865#1334517, @adrian wrote:

hi!

So why are a whole lot of functions getting an fd added to them?

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

IMO we really shouldn't have libraries entering capablity mode- libusb may be just one of many, and setting a precedence like that where some will insist on entering the sandbox is bound to end up with some really annoying circumstances.

In D51865#1334517, @adrian wrote:

hi!

So why are a whole lot of functions getting an fd added to them?

As we need to provide the require file descriptor that need to be opened before cap_enter.

Do the FDs change during the runtime of the tool? Or are they something that could be set
once during some init phase (maybe in a new function which would call cap_enter, rather than
the owner having to call it) and then pulled out of some the libusb backend state?

In D57885#1334360, @dch wrote:

Sigh, it's already set in L28... but if the file isn't present, it needs to be created with correct permissions, otherwise it won't be set. Can you add that please?

In D57885#1334350, @dch wrote:

• needs a PORTREVISION bump

In D52166#1330369, @adrian wrote:

Hey, any further traction on this? What's left before it can be landed?

Thanks for your review @bnovkov

Reflect bnovkov's review

LGTM!

Fixes

reflect olce@'s comment

Reflect olce@'s comment

In D57825#1333484, @olce wrote:

Looks good. One suggestion in an inline comment. Only thing now is the dependency on the CPPC lib, I'll review the other revisions and come back to here.

In D55474#1333882, @olce wrote:

Two more things to consider. First, you finally reduce items expected to be read-only to an integer (field value in struct acpi_cppc_rdonly_item), but some of these are dynamic and may change during runtime, in which case we have to keep the full register specification. Second, in the _CPC example I sent you offline for an AMD machine where processors do not report the CPPC extended feature flag, most present items have the PCC access method, so won't be covered by the code here. In the same example, there are only two FFH, for the ReferencePerformanceCounterRegister and DeliveredPerformanceCounterRegister, and the addresses seem to correspond to the MPERF and APERF MSR numbers respectively.

In D55474#1333742, @olce wrote:

As noted on previous revisions, SYS_RES_FFH should not exist, nor should acpi_ffh_write() and acpi_ffh_read(). So, basically, this needs a full rework.

LGTM! Thanks.

In D58152#1333581, @kevans wrote:

In D58152#1333577, @aokblast wrote:

In D58152#1333568, @kevans wrote:

In D58152#1333559, @kib wrote:

You do understand, that the change is only changes something for the statically linked binaries?

We had already implemented a workaround in rtld, _dl_iterate_phdr_locked iirc. I'm not sure offhand why that wasn't sufficient here, I might need to re-check which lsan drop I sent to @aokblast

Yes, I see the comment from your brach and see both glibc and NetBSD (although very strange in their implemetation) has recursive dl_phdr_info_lock lock.

Yes, I think @kib was at least mildly concerned when I raised the thought of doing the same, and we decided it was reasonably fine to just bypass the locks because we do dl_iterate_phdr, rfork (vfork) inside the handler, then ptrace the parent. The thread that holds the relevanr locks is suspended in rfork and safely not racing against what we do in the child.

In D58152#1333568, @kevans wrote:

In D58152#1333559, @kib wrote:

You do understand, that the change is only changes something for the statically linked binaries?

We had already implemented a workaround in rtld, _dl_iterate_phdr_locked iirc. I'm not sure offhand why that wasn't sufficient here, I might need to re-check which lsan drop I sent to @aokblast

In D58152#1333559, @kib wrote:

You do understand, that the change is only changes something for the statically linked binaries?

This fixes the kvm_clock detection on a guest on KVM on HyperV.

@imp Any suggestion? This is for your request:).

Add manual

Is there anything else barring us from merging this?

@dch ping!

• AI fixed
• AI fix: 2
• fixup! xhci: Refactor xhci_generic_setup code

@kib I don't have the machine now so it needs test from the user.

@adrian If you don't have any concern, I will merge this patch tomorrow.

In D51045#1182411, @jhb wrote:

For the cross-build error, it seems like clang isn't honoring --sysroot when looking for the libraries. This can be correct in some cases (it would be correct for the LLVM port cross-building a binary as the port should use the sanitizer libs shipped as part of the port). We may want a local patch for the version of LLVM in the base system to honor --sysroot for the sanitizer runtimes though. Perhaps @dim has thoughts?

LGTM! irq_res = bus_alloc_resource_any is not guarded with IICHID_SAMPLING. It should be paired.

Minor fixes

Enable FFH

Minor fixes

Minor fixes