On Thu, Oct 27, 2022, David Matlack wrote: > On Thu, Oct 27, 2022 at 1:35 PM Sean Christopherson <seanjc@xxxxxxxxxx> wrote: > > > > On Thu, Oct 27, 2022, David Matlack wrote: > > > Add a new field to struct kvm that keeps track of the number of memslots > > > with dirty logging enabled. This will be used in a future commit to > > > cheaply check if any memslot is doing dirty logging. > > > > > > Signed-off-by: David Matlack <dmatlack@xxxxxxxxxx> > > > --- > > > include/linux/kvm_host.h | 2 ++ > > > virt/kvm/kvm_main.c | 10 ++++++++++ > > > > Why put this in common code? I'm having a hard time coming up with a second use > > case since the count isn't stable, i.e. it can't be used for anything except > > scenarios like x86's NX huge page mitigation where a false negative/positive is benign. > > I agree, but what is the downside of putting it in common code? The potential for misuse, e.g. outside of slots_lock. > The downside of putting it in architecture-specific code is if any other > architecture needs it (or something similar) in the future they are unlikely > to look through x86 code to see if it already exists. i.e. we're more likely > to end up with duplicate code. > > And while the count is not stable outside of slots_lock, it could > still theoretically be used under slots_lock to coordinate something > that depends on dirty logging being enabled in any slot. In our > internal kernel, for example, we use it to decide when to > create/destroy the KVM dirty log worker threads (although I doubt that > specific usecase will ever see the light of day upstream :). Yeah, I'm definitely not dead set against putting it in common code. I suspect I'm a little overly sensitive at the moment to x86 pushing a bunch of x86-centric logic into kvm_main.c and making life miserable for everyone. Been spending far too much time unwinding the mess that is kvm_init()...
