Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
On Thu, Mar 18, 2021, Maxim Levitsky wrote: > On Thu, 2021-03-18 at 16:35 +, Sean Christopherson wrote: > > Skipping SEV-ES guests should not be difficult; KVM could probably even > > print a message stating that the debug hook is being ignored. One thought > > would > > be to snapshot debug_intercept_exceptions at VM creation, and simply zero > > it out > > for incompatible guests. That would also allow changing > > debug_intercept_exceptions > > without reloading KVM, which IMO would be very convenient. > > > So all right I'll disable this for SEV-ES. Belated thought. KVM doesn't know a guest will be an SEV-ES guest until sev_es_guest_init(), and KVM currently doesn't prevent creating vCPUs before KVM_SEV_ES_INIT. But, I'm 99% confident that's a KVM bug. For your purposes, I think you can assume kvm->arch.debug_intercept_exceptions will _not_ change after vCPU creation. > The idea to change the debug_intercept_exceptions on the fly is also a good > idea, > I will implement it in next version of the patches. Can you also move the module param to x86? It doesn't need to be wired up for VMX right away, but it makes sense to do it at some point, and ideally folks won't have to update their scripts when that happens.
Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
On Thu, 2021-03-18 at 16:35 +, Sean Christopherson wrote: > On Thu, Mar 18, 2021, Joerg Roedel wrote: > > On Thu, Mar 18, 2021 at 11:24:25AM +0200, Maxim Levitsky wrote: > > > But again this is a debug feature, and it is intended to allow the user > > > to shoot himself in the foot. > > > > And one can't debug SEV-ES guests with it, so what is the point of > > enabling it for them too? You can create a special SEV-ES guest which does handle all exceptions via #VC, or just observe it fail which can be useful for some whatever reason. > > Agreed. I can see myself enabling debug features by default, it would be nice > to not having to go out of my way to disable them for SEV-ES/SNP guests. This does sound like a valid reason to disable this for SEV-ES. > > Skipping SEV-ES guests should not be difficult; KVM could probably even > print a message stating that the debug hook is being ignored. One thought > would > be to snapshot debug_intercept_exceptions at VM creation, and simply zero it > out > for incompatible guests. That would also allow changing > debug_intercept_exceptions > without reloading KVM, which IMO would be very convenient. > So all right I'll disable this for SEV-ES. The idea to change the debug_intercept_exceptions on the fly is also a good idea, I will implement it in next version of the patches. Thanks for the review, Best regards, Maxim Levitsky
Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
On Thu, Mar 18, 2021, Joerg Roedel wrote: > On Thu, Mar 18, 2021 at 11:24:25AM +0200, Maxim Levitsky wrote: > > But again this is a debug feature, and it is intended to allow the user > > to shoot himself in the foot. > > And one can't debug SEV-ES guests with it, so what is the point of > enabling it for them too? Agreed. I can see myself enabling debug features by default, it would be nice to not having to go out of my way to disable them for SEV-ES/SNP guests. Skipping SEV-ES guests should not be difficult; KVM could probably even print a message stating that the debug hook is being ignored. One thought would be to snapshot debug_intercept_exceptions at VM creation, and simply zero it out for incompatible guests. That would also allow changing debug_intercept_exceptions without reloading KVM, which IMO would be very convenient.
Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
On Thu, Mar 18, 2021 at 11:24:25AM +0200, Maxim Levitsky wrote: > But again this is a debug feature, and it is intended to allow the user > to shoot himself in the foot. And one can't debug SEV-ES guests with it, so what is the point of enabling it for them too? Regards, Joerg
Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
On Thu, 2021-03-18 at 10:19 +0100, Joerg Roedel wrote: > On Tue, Mar 16, 2021 at 12:51:20PM +0200, Maxim Levitsky wrote: > > I agree but what is wrong with that? > > This is a debug feature, and it only can be enabled by the root, > > and so someone might actually want this case to happen > > (e.g to see if a SEV guest can cope with extra #VC exceptions). > > That doesn't make sense, we know that and SEV-ES guest can't cope with > extra #VC exceptions, so there is no point in testing this. It is more a > way to shot oneself into the foot for the user and a potential source of > bug reports for SEV-ES guests. But again this is a debug feature, and it is intended to allow the user to shoot himself in the foot. Bug reports for a debug feature are autoclosed. It is no different from say poking kernel memory with its built-in gdbstub, for example. Best regards, Maxim Levitsky > > > > I have nothing against not allowing this for SEV-ES guests though. > > What do you think? > > I think SEV-ES guests should only have the intercept bits set which > guests acutally support > > Regards, > > Joerg >
Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
On Tue, Mar 16, 2021 at 12:51:20PM +0200, Maxim Levitsky wrote: > I agree but what is wrong with that? > This is a debug feature, and it only can be enabled by the root, > and so someone might actually want this case to happen > (e.g to see if a SEV guest can cope with extra #VC exceptions). That doesn't make sense, we know that and SEV-ES guest can't cope with extra #VC exceptions, so there is no point in testing this. It is more a way to shot oneself into the foot for the user and a potential source of bug reports for SEV-ES guests. > I have nothing against not allowing this for SEV-ES guests though. > What do you think? I think SEV-ES guests should only have the intercept bits set which guests acutally support. Regards, Joerg
Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
On Tue, 2021-03-16 at 09:32 +0100, Joerg Roedel wrote:
> Hi Maxim,
>
> On Tue, Mar 16, 2021 at 12:10:20AM +0200, Maxim Levitsky wrote:
> > -static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = {
> > +static int (*svm_exit_handlers[])(struct kvm_vcpu *vcpu) = {
>
> Can you keep this const and always set the necessary handlers? If
> exceptions are not intercepted they will not be used.
>
> > @@ -333,7 +334,9 @@ static inline void clr_exception_intercept(struct
> > vcpu_svm *svm, u32 bit)
> > struct vmcb *vmcb = svm->vmcb01.ptr;
> >
> > WARN_ON_ONCE(bit >= 32);
> > - vmcb_clr_intercept(>control, INTERCEPT_EXCEPTION_OFFSET + bit);
> > +
> > + if (!((1 << bit) & debug_intercept_exceptions))
> > + vmcb_clr_intercept(>control, INTERCEPT_EXCEPTION_OFFSET +
> > bit);
>
> This will break SEV-ES guests, as those will not cause an intercept but
> now start to get #VC exceptions on every other exception that is raised.
> SEV-ES guests are not prepared for that and will not even boot, so
> please don't enable this feature for them.
I agree but what is wrong with that?
This is a debug feature, and it only can be enabled by the root,
and so someone might actually want this case to happen
(e.g to see if a SEV guest can cope with extra #VC exceptions).
I have nothing against not allowing this for SEV-ES guests though.
What do you think?
Best regards,
Maxim Levitsky
Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
On Tue, Mar 16, 2021 at 12:10:20AM +0200, Maxim Levitsky wrote: > Add a new debug module param 'debug_intercept_exceptions' which will allow the > KVM to intercept any guest exception, and forward it to the guest. > > This can be very useful for guest debugging and/or KVM debugging with kvm > trace. > This is not intended to be used on production systems. > > This is based on an idea first shown here: > https://patchwork.kernel.org/project/kvm/patch/20160301192822.gd22...@pd.tnic/ > > CC: Borislav Petkov > Signed-off-by: Maxim Levitsky > --- > arch/x86/include/asm/kvm_host.h | 2 + > arch/x86/kvm/svm/svm.c | 77 - > arch/x86/kvm/svm/svm.h | 5 ++- > arch/x86/kvm/x86.c | 5 ++- > 4 files changed, 85 insertions(+), 4 deletions(-) Looks interesting, I'll give it a try when I get a chance in the coming days. Thx! -- Regards/Gruss, Boris. SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
Re: [PATCH 3/3] KVM: SVM: allow to intercept all exceptions for debug
Hi Maxim,
On Tue, Mar 16, 2021 at 12:10:20AM +0200, Maxim Levitsky wrote:
> -static int (*const svm_exit_handlers[])(struct kvm_vcpu *vcpu) = {
> +static int (*svm_exit_handlers[])(struct kvm_vcpu *vcpu) = {
Can you keep this const and always set the necessary handlers? If
exceptions are not intercepted they will not be used.
> @@ -333,7 +334,9 @@ static inline void clr_exception_intercept(struct
> vcpu_svm *svm, u32 bit)
> struct vmcb *vmcb = svm->vmcb01.ptr;
>
> WARN_ON_ONCE(bit >= 32);
> - vmcb_clr_intercept(>control, INTERCEPT_EXCEPTION_OFFSET + bit);
> +
> + if (!((1 << bit) & debug_intercept_exceptions))
> + vmcb_clr_intercept(>control, INTERCEPT_EXCEPTION_OFFSET +
> bit);
This will break SEV-ES guests, as those will not cause an intercept but
now start to get #VC exceptions on every other exception that is raised.
SEV-ES guests are not prepared for that and will not even boot, so
please don't enable this feature for them.
