Re: [PATCH] KVM: SVM: Assume a 64-bit hypercall for guests with protected state

[Tom Lendacky <thomas.lendacky@amd.com>]

On 5/24/21 12:06 PM, Paolo Bonzini wrote:
> On 24/05/21 19:03, Sean Christopherson wrote:
>>> Let's introduce a new wrapper is_64_bit_hypercall, and add a
>>> WARN_ON_ONCE(vcpu->arch.guest_state_protected) to is_64_bit_mode.
>>
>> Can we introduce the WARN(s) in a separate patch, and deploy them much more
>> widely than just is_64_bit_mode()?  I would like to have them lying in
>> wait at
>> every path that should be unreachable, e.g. get/set segments, get_cpl(),
>> etc...
> 
> Each WARN that is added must be audited separately, so this one I'd like
> to have now; it is pretty much the motivation for introducing a new
> function, as the other caller of is_64_bit_mode, kvm_get_linear_rip() is
> already "handling" SEV-ES by always returning 0.

The kvm_register_{read,write}() functions also call is_64_bit_mode(). But...

The SVM support uses those for CR and DR intercepts. CR intercepts are not
enabled under SEV-ES. DR intercepts are only set for DR7. DR7 reads don't
actually exit, the intercept is there to trigger the #VC and return a
cached value from the #VC handler. DR7 writes do exit but don't actually
do much since we don't allow guest_debug to be set so kvm_register_read()
is never called.

The x86 support also uses those functions for emulation and SMM, both of
which aren't supported under SEV-ES.

Thanks,
Tom

> 
> But yes adding more WARNs can only be good.
> 
> Paolo
> 
>> Side topic, kvm_get_cs_db_l_bits() should be moved to svm.c. 
>> Functionally, it's
>> fine to have it as a vendor-agnostic helper, but practically speaking it
>> should
>> never be called directly.
>>
>