On 13/9/2023 11:24 pm, David Woodhouse wrote:
On Wed, 2023-09-13 at 15:15 +0000, Sean Christopherson wrote:
e.g. if userspace writes '0' immediately after creating, and then later writes a
small delta, the v6 code wouldn't trigger synchronization because "user_set_tsc"
would be left unseft by the write of '0'.
True, but that's the existing behaviour,
No? The existing code will fall into the "conditionally sync" logic for any
non-zero value.
Yeah, OK. This isn't one of the cases we set out to deliberately
change, but it would be changed by v6 of the patch, and I suppose
you're right that we should accept a small amount of extra code
complexity just to avoid making any changes we don't *need* to, even
for stupid cases like this.
I don't care (in the Tommy Lee Jones[*] sense). All I care about is minimizing
the probability of breaking userspace, which means making the smallest possible
change to KVM's ABI. For me, whether or not userspace is doing something sensible
doesn't factor into that equation.
Ack.
If we combine the v5 code diff (the u64 *user_value proposal) with the refined
changelog in v6,
it seems like we've reached a point of equilibrium on this issue, doesn't it ?
Please let me know you have more concerns.
Although there's a strong argument that adding further warts to an
already fundamentally broken API probably isn't a great idea in the
first place. Just deprecate it and use the saner replacement API...
which I just realised we don't have (qv). Ooops :)
