In D52232#1193211, @markj wrote:

Well, 8 bits per sample, but yes this might turn out to be excessive. Perhaps I should include a tunable to override that value.

8 bits per byte might be high for some sources... I'm not sure. It's probably fine for the relatively simplistic tests in the health test?

Should we add a comment on the hc_source_mask definition about its locking/access protocol? LGTM.

I'll take your word for it :)

LGTM.

In D48683#1176553, @obiwac wrote:

It might make sense for amdsmu to use amdsmn's amdsmn_read, though its not much extra work for amdsmu. @cem thoughts on this?

Otherwise seems unobjectionable to me.

Oops. :-)

Thanks -- lgtm otherwise.

Commit message nit:

Seems reasonable.

In D51112#1166640, @markj wrote:

...

Thanks!

No real objection to this overall approach, but we could also just fix the MPSC queue, right? (release stores for ring.in on the producer side and acquire loads on the consumer side.) (More generally, maybe it would be good to have some sort of generic, correct MPSC implementation that this subsystem can use, instead of the bespoke thing.)

lgtm otherwise

FYI, it's best practice to generate diffs with full context (-U999999 or whatever).

Is the associated hardware documentation public, and if so, could you link to it?

I have no objection.

Yes, yes. It is easier to analyze with e.g. rand2() % 3. But you and Ed have both mentioned a way to measure this for rand32 more tractably -- great!

Yeah, you can do a simple bias test with an upper limit of 3 for example. Bucket 0 is more likely than buckets 1 and 2 under a simplistic rand32() % 3 implementation. Or something like that. It is probably easier to measure with a smaller power of 2 generator.

Some context on the Lemire idea if others haven't seen it:

Thanks.

This is now sort of a layering violation, right? We're assuming Fortuna implements devrandom, but the interface is pluggable. We have 2-3 implementations in tree, and integrators might write their own for compliance reasons.

LGTM.

In random_early_prime we divide the input into blocks of size sizeof(event.he_entropy)) and then process those one by one, with each block only being fed into one pool. so simply padding the entropy with zeros would result in most of the pools having no entropy.

I don't think there's any reason to spread this using pbkdf2 just to defeat the Fortuna input seeding thresholds. You might as well seed the 64 bytes and then 2048-64 bytes of zero, or whatever. (Internally Fortuna is compressing the provided seed material using SHA2.)

Lgtm

Nice observation and fix.

No objection. I'll let someone else stamp it.

Lgtm.

Nevermind

LGTM

As mentioned in email, we might also consider publishing a seed generation to the VDSO page from Fortuna.

Something to keep in mind re: exactly 64 is that it's possible for entropy sources to return less entropy than was requested, if for example some internal state means they don't have anything available at the moment. That's one reason I prefer polling repeatedly until seeded (or some spin threshold is crossed).

Discussed with: cem

Thanks.

Seems reasonable to me.

Seems very reasonable to me. Thanks!

Seems reasonable to me!

LGTM. My only bikeshed contribution is maybe gctl_get_devnameparam or get_devparam. But I don’t object to the current name.

Thanks.

Setting cstd seems strongly preferable vs making upstream updates more difficult.

Either way seems fine to me. I’d ask vangyzen or bdrewery for input.

What workflow?

In D34220#774261, @sg2342_googlemail.com wrote:

subtract one from vt_logo_sprite_height as well

In D34186#773413, @dim wrote:

In D34186#773402, @cem wrote:

Looks like TESTAB expects boolean (0 and 1) values, too.

Well, the interesting thing is that this macro produces values 0 through 4, and it's a sort of weird shortcut to make the switch(TESTAB()) construct possible.

Looks like TESTAB expects boolean (0 and 1) values, too.

In D34152#772589, @hselasky wrote:

@cem: You mean "iff" ? if and only if?

Looks fine to me. We should probably also actively feed jitter entropy if we hit the blocked state on boot, but that doesn't mean we shouldn't just do this first.

Typo in summary: s/path/patch/

Thanks.

I’m hoping emaste will be able to test and commit it.

LGTM, thanks.

LGTM modulo overflow concern and what looks like a typo.

Lgtm. I didn’t attempt to verify the asm implementation.