Apologies, Sean.
I thought I had replied to this but found it instead in my drafts folder...
I've taken much of your feedback and incorporated that into the next
version of the patches that I submitted and updated this response based on
that, too.
On 9/15/20 7:19 PM, Sean Christopherson wrote:
> On Tue, Sep 15, 2020 at 12:22:05PM -0500, Tom Lendacky wrote:
>> On 9/14/20 5:59 PM, Sean Christopherson wrote:
>>> Given that we don't yet have publicly available KVM code for TDX, what if I
>>> generate and post a list of ioctls() that are denied by either SEV-ES or TDX,
>>> organized by the denier(s)? Then for the ioctls() that are denied by one and
>>> not the other, we add a brief explanation of why it's denied?
>>>
>>> If that sounds ok, I'll get the list and the TDX side of things posted
>>> tomorrow.
>>
>> That sounds good.
>
> TDX completely blocks the following ioctl()s:
SEV-ES doesn't need to completely block these ioctls. SEV-SNP is likely to
do more of that. SEV-ES will still allow interrupts to be injected, or
registers to be retrieved (which will only contain what was provided in
the GHCB exchange), etc.
>
> kvm_vcpu_ioctl_interrupt
> kvm_vcpu_ioctl_smi
> kvm_vcpu_ioctl_x86_setup_mce
> kvm_vcpu_ioctl_x86_set_mce
> kvm_vcpu_ioctl_x86_get_debugregs
> kvm_vcpu_ioctl_x86_set_debugregs
> kvm_vcpu_ioctl_x86_get_xsave
> kvm_vcpu_ioctl_x86_set_xsave
> kvm_vcpu_ioctl_x86_get_xcrs
> kvm_vcpu_ioctl_x86_set_xcrs
> kvm_arch_vcpu_ioctl_get_regs
> kvm_arch_vcpu_ioctl_set_regs
> kvm_arch_vcpu_ioctl_get_sregs
> kvm_arch_vcpu_ioctl_set_sregs
> kvm_arch_vcpu_ioctl_set_guest_debug
> kvm_arch_vcpu_ioctl_get_fpu
> kvm_arch_vcpu_ioctl_set_fpu
Of the listed ioctls, really the only ones I've updated are:
kvm_vcpu_ioctl_x86_get_xsave
kvm_vcpu_ioctl_x86_set_xsave
kvm_arch_vcpu_ioctl_get_sregs
This allows reading of the tracking value registers
kvm_arch_vcpu_ioctl_set_sregs
This prevents setting of register values
kvm_arch_vcpu_ioctl_set_guest_debug
kvm_arch_vcpu_ioctl_get_fpu
kvm_arch_vcpu_ioctl_set_fpu
>
> Looking through the code, I think kvm_arch_vcpu_ioctl_get_mpstate() and
> kvm_arch_vcpu_ioctl_set_mpstate() should also be disallowed, we just haven't
> actually done so.
I haven't done anything with these either.
>
> There are also two helper functions that are "blocked".
> dm_request_for_irq_injection() returns false if guest_state_protected, and
> post_kvm_run_save() shoves dummy state.
... and these.
>
> TDX also selectively blocks/skips portions of other ioctl()s so that the
> TDX code itself can yell loudly if e.g. .get_cpl() is invoked. The event
> injection restrictions are due to direct injection not being allowed (except
> for NMIs); all IRQs have to be routed through APICv (posted interrupts) and
> exception injection is completely disallowed.
For SEV-ES, we don't have those restrictions.
>
> kvm_vcpu_ioctl_x86_get_vcpu_events:
> if (!vcpu->kvm->arch.guest_state_protected)
> events->interrupt.shadow = kvm_x86_ops.get_interrupt_shadow(vcpu);
>
> kvm_arch_vcpu_put:
> if (vcpu->preempted && !vcpu->kvm->arch.guest_state_protected)
> vcpu->arch.preempted_in_kernel = !kvm_x86_ops.get_cpl(vcpu);
>
> kvm_vcpu_ioctl_x86_set_vcpu_events:
> u32 allowed_flags = KVM_VCPUEVENT_VALID_NMI_PENDING |
> KVM_VCPUEVENT_VALID_SIPI_VECTOR |
> KVM_VCPUEVENT_VALID_SHADOW |
> KVM_VCPUEVENT_VALID_SMM |
> KVM_VCPUEVENT_VALID_PAYLOAD;
>
> if (vcpu->kvm->arch.guest_state_protected)
> allowed_flags = KVM_VCPUEVENT_VALID_NMI_PENDING;
>
>
> kvm_arch_vcpu_ioctl_run:
> if (vcpu->kvm->arch.guest_state_protected)
> kvm_sync_valid_fields = KVM_SYNC_X86_EVENTS;
> else
> kvm_sync_valid_fields = KVM_SYNC_X86_VALID_FIELDS;
>
>
> In addition to the more generic guest_state_protected, we also (obviously
> tentatively) have a few other flags to deal with aspects of TDX that I'm
> fairly certain don't apply to SEV-ES:
>
> tsc_immutable - KVM doesn't have write access to the TSC offset of the
> guest.
>
> eoi_intercept_unsupported - KVM can't intercept EOIs (doesn't have access
> to EOI bitmaps) and so can't support level
> triggered interrupts, at least not without
> extra pain.
>
> readonly_mem_unsupported - Secure EPT (analagous to SNP) requires RWX
> permissions for all private/encrypted memory.
> S-EPT isn't optional, so we get the joy of
> adding this right off the bat...
Yes, most of the above stuff doesn't apply to SEV-ES.
Thanks,
Tom
>
