Re: Writable module parameters in KVM

Re: Writable module parameters in KVM

[Paolo Bonzini]

On 26/05/21 01:45, Ben Gardon wrote:
> 
> At Google we have an informal practice of adding sysctls to control some 
> KVM features. Usually these just act as simple "chicken bits" which 
> allow us to turn off a feature without having to stall a kernel rollout 
> if some feature causes problems. (Sysctls were used for reasons specific 
> to Google infrastructure, not because they're necessarily better.)
> 
> We'd like to get rid of this divergence with upstream by converting the 
> sysctls to writable module parameters, but I'm not sure what the general 
> guidance is on writable module parameters. Looking through KVM, it seems 
> like we have several writable parameters, but they're mostly read-only.

Sure, making them writable is okay.  Most KVM parameters are read-only 
because it's much simpler (the usecase for introducing them was simply 
"test what would happen on old processors").  What are these features 
that you'd like to control?

> I also don't see central documentation of the module parameters. They're 
> mentioned in the documentation for other features, but don't have their 
> own section / file. Should they?

They probably should, yes.

Paolo

Re: Writable module parameters in KVM

[Maxim Levitsky]

On Wed, 2021-05-26 at 12:49 +0200, Paolo Bonzini wrote:
> On 26/05/21 01:45, Ben Gardon wrote:
> > At Google we have an informal practice of adding sysctls to control some 
> > KVM features. Usually these just act as simple "chicken bits" which 
> > allow us to turn off a feature without having to stall a kernel rollout 
> > if some feature causes problems. (Sysctls were used for reasons specific 
> > to Google infrastructure, not because they're necessarily better.)
> > 
> > We'd like to get rid of this divergence with upstream by converting the 
> > sysctls to writable module parameters, but I'm not sure what the general 
> > guidance is on writable module parameters. Looking through KVM, it seems 
> > like we have several writable parameters, but they're mostly read-only.
> 
> Sure, making them writable is okay.  Most KVM parameters are read-only 
> because it's much simpler (the usecase for introducing them was simply 
> "test what would happen on old processors").  What are these features 
> that you'd like to control?
> 
> > I also don't see central documentation of the module parameters. They're 
> > mentioned in the documentation for other features, but don't have their 
> > own section / file. Should they?
> 
> They probably should, yes.
> 
> Paolo
> 
I vote (because I have fun with my win98 once in a while),
to make 'npt' writable, since that is the only way
to make it run on KVM on AMD.
My personal itch only though!

Best regards,
	Maxim Levitsky

Re: Writable module parameters in KVM

[Sean Christopherson]

On Wed, May 26, 2021, Maxim Levitsky wrote:
> On Wed, 2021-05-26 at 12:49 +0200, Paolo Bonzini wrote:
> > On 26/05/21 01:45, Ben Gardon wrote:
> > > At Google we have an informal practice of adding sysctls to control some 
> > > KVM features. Usually these just act as simple "chicken bits" which 
> > > allow us to turn off a feature without having to stall a kernel rollout 
> > > if some feature causes problems. (Sysctls were used for reasons specific 
> > > to Google infrastructure, not because they're necessarily better.)
> > > 
> > > We'd like to get rid of this divergence with upstream by converting the 
> > > sysctls to writable module parameters, but I'm not sure what the general 
> > > guidance is on writable module parameters. Looking through KVM, it seems 
> > > like we have several writable parameters, but they're mostly read-only.
> > 
> > Sure, making them writable is okay.  Most KVM parameters are read-only 
> > because it's much simpler (the usecase for introducing them was simply 
> > "test what would happen on old processors").  What are these features 
> > that you'd like to control?

My $0.02 is that most parameters should remain read-only, and making a param
writable (new or existing) must come with strong justification for taking on the
extra complexity.

I absolutely agree that making select params writable adds a ton of value, e.g.
being able to switch to/from the TDP MMU without reloading KVM saves a lot of
time when testing, toggling forced flush/sync on PGD reuse is extremely valuable
for triage and/or mitigation, etc...  But writable params should either bring a
lot of value and/or add near-zero complexity.

> > > I also don't see central documentation of the module parameters. They're 
> > > mentioned in the documentation for other features, but don't have their 
> > > own section / file. Should they?
> > 
> > They probably should, yes.
> > 
> > Paolo
> > 
> I vote (because I have fun with my win98 once in a while), to make 'npt'
> writable, since that is the only way to make it run on KVM on AMD.

For posterity, "that" refers to disabling NPT, not making 'npt' writable :-)

Making 'npt' writable is probably feasible ('ept' would be beyond messy), but I
strongly prefer to keep it read-only.  The direct impacts on the MMU and SVM
aren't too bad, but NPT is required for SEV and VLS, affects kvm_cpu_caps, etc...
And, no offense to win98, there's isn't a strong use case because outside of
personal usage, the host admin/VMM doesn't know that the guest will be running a
broken kernel.

> My personal itch only though!
> 
> Best regards,
> 	Maxim Levitsky
> 

Re: Writable module parameters in KVM

[Paolo Bonzini]

On 26/05/21 17:44, Sean Christopherson wrote:
>> Sure, making them writable is okay.
>
> making a param writable (new or existing) must come with strong
> justification for taking on the extra complexity.

I agree.  It's the same for every change, and it's the reason why most 
parameters are read-only: no justification for the extra complexity. 
But if somebody has a usecase, it can be considered.

> Making 'npt' writable is probably feasible ('ept' would be beyond messy), but I
> strongly prefer to keep it read-only.  The direct impacts on the MMU and SVM
> aren't too bad, but NPT is required for SEV and VLS, affects kvm_cpu_caps, etc...
> And, no offense to win98, there's isn't a strong use case because outside of
> personal usage, the host admin/VMM doesn't know that the guest will be running a
> broken kernel.

Making 'npt' writable would be beyond messy too; allowing select VMs to 
disable EPT/NPT might be simpler, but not that much.  I can't say 
offhand if the code would be ugly or not.

Paolo

Re: Writable module parameters in KVM

[Ben Gardon]

On Wed, May 26, 2021 at 9:30 AM Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 26/05/21 17:44, Sean Christopherson wrote:
> >> Sure, making them writable is okay.
> >
> > making a param writable (new or existing) must come with strong
> > justification for taking on the extra complexity.
>
> I agree.  It's the same for every change, and it's the reason why most
> parameters are read-only: no justification for the extra complexity.
> But if somebody has a usecase, it can be considered.
>
> > Making 'npt' writable is probably feasible ('ept' would be beyond messy), but I
> > strongly prefer to keep it read-only.  The direct impacts on the MMU and SVM
> > aren't too bad, but NPT is required for SEV and VLS, affects kvm_cpu_caps, etc...
> > And, no offense to win98, there's isn't a strong use case because outside of
> > personal usage, the host admin/VMM doesn't know that the guest will be running a
> > broken kernel.
>
> Making 'npt' writable would be beyond messy too; allowing select VMs to
> disable EPT/NPT might be simpler, but not that much.  I can't say
> offhand if the code would be ugly or not.

Thanks for the guidance all. We'll probably send out more writable
module params in the future in that case, and will add a Documentation
file whenever we send out the first one.

I don't know if there's a great way to formally encode this
distinction, but I see two major classes of writable params in terms
of complexity:
1. parameters that are captured on VM creation and follow the life of
the VM e.g. the TDP MMU
2. parameters which have an effect on all VMs on the system when
changed e.g. internally we have sysctls to change NX reclaim
parameters

I think class 1 is substantially easier to reason about from a code
perspective, but might be more confusing to userspace, as the current
value of the parameter has no bearing on the value captured by the VM.
Class 2 will probably be more complex to implement, require
synchronization, and need a better justification.

>
> Paolo
>

Re: Writable module parameters in KVM

[Sean Christopherson]

On Wed, May 26, 2021, Ben Gardon wrote:
> I don't know if there's a great way to formally encode this distinction, but
> I see two major classes of writable params in terms of complexity:
>
> 1. parameters that are captured on VM creation and follow the life of
> the VM e.g. the TDP MMU
>
> 2. parameters which have an effect on all VMs on the system when
> changed e.g. internally we have sysctls to change NX reclaim
> parameters
> 
> I think class 1 is substantially easier to reason about from a code
> perspective, but might be more confusing to userspace, as the current
> value of the parameter has no bearing on the value captured by the VM.
> Class 2 will probably be more complex to implement, require
> synchronization, and need a better justification.

That assessement isn't universally true, e.g. 'npt' and 'ept' could be snapshotted
and put into (1), but as discussed, the fallout would be spectactular.  And on
the other side, the flush/sync on reuse flag is fully dynamic and falls into (2),
yet is trivial to implement.

That said, I don't think it matters because I don't think classifying params
will change anyone's behavior.  Each param would still need to be justified and
reviewed on a case-by-case basis.