set-features should be idempotent?

[iximeow]

I have barely tested this, but I built a propolis-standalone and threw it + a Windows Server 2022 onto a Cosmo and I'm no longer seeing the results of Windows repeatedly resetting the device. so this gets #1052 and apparently #1048 with it.

this probably should just be a mutex that we take somewhere in managing virtio device configuration but this was simple enough to throw together for a quick PoC/validation..

[hawkw]

would like a comment on this explaining what it means and why it's tracked here?

[hawkw]

i found the description of the suspected issue in #1048 and #1052 quite useful and felt like something like that (though not necessarily from the angle of "it was broken") would be nice to capture closer to the code.

[hawkw]

clippy is kind of obnoxiously mad about alphabetization here :|

[dancrossnyc]

I think that Windows may be in error here; or, at least, there's some ambiguity in the spec (VirtIO 1.2).

Section 2.2 of the spec reads, in part:

Each virtio device offers all the features it understands. During device initialization, the driver reads this and tells the device the subset that it accepts. The only way to renegotiate is to reset the device.

(Emphasis added)

However, sec. 2.2.2 goes on to say,

If a device has successfully negotiated a set of features at least once (by accepting the FEATURES_OK device status bit during device initialization), then it SHOULD NOT fail re-negotiation of the same set of features after a device or system reset.

(Again, emphasis added)

What this says to me is that, once the driver has written the FEATURES_OK bit in the device status register, and this has been acknowledged by the device by similarly reflecting that bit back to the driver on a read, then the driver should not write to the features register again.

However, I could also imagine that most devices should be able to tolerate a read that doesn't change the value of the register; that is, if I read the register and write it right back, perhaps that should be a tragically complicated no-op. However, if the driver tries to change anything, then that ought to fail, either by clearing (and refusing to set) FEATURES_OK or by resetting.