linux-kernel - Re: [RFC PATCH v5 092/104] KVM: TDX: Handle TDX PV HLT hypercall

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Yk79A4EdiZoVQMsV@google.com>
Date:   Thu, 7 Apr 2022 15:02:27 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     isaku.yamahata@...el.com, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org, isaku.yamahata@...il.com,
        Jim Mattson <jmattson@...gle.com>, erdemaktas@...gle.com,
        Connor Kuehl <ckuehl@...hat.com>
Subject: Re: [RFC PATCH v5 092/104] KVM: TDX: Handle TDX PV HLT hypercall

On Thu, Apr 07, 2022, Paolo Bonzini wrote:
> On 3/4/22 20:49, isaku.yamahata@...el.com wrote:
> > +	bool interrupt_disabled = tdvmcall_p1_read(vcpu);
> 
> Where is R12 documented for TDG.VP.VMCALL<Instruction.HLT>?
> 
> > +		 * Virtual interrupt can arrive after TDG.VM.VMCALL<HLT> during
> > +		 * the TDX module executing.  On the other hand, KVM doesn't
> > +		 * know if vcpu was executing in the guest TD or the TDX module.
> 
> I don't understand this; why isn't it enough to check PI.ON or something
> like that as part of HLT emulation?

Ooh, I think I remember what this is.  This is for the case where the virtual
interrupt is recognized, i.e. set in vmcs.RVI, between the STI and "HLT".  KVM
doesn't have access to RVI and the interrupt is no longer in the PID (because it
was "recognized".  It doesn't get delivered in the guest because the TDCALL
completes before interrupts are enabled.

I lobbied to get this fixed in the TDX module by immediately resuming the guest
in this case, but obviously that was unsuccessful.

> > +		details.full = td_state_non_arch_read64(
> > +			to_tdx(vcpu), TD_VCPU_STATE_DETAILS_NON_ARCH);
> 
> TDX documentation says "the meaning of the field may change with Intel TDX
> module version", where is this field documented?  I cannot find any "other
> guest state" fields in the TDX documentation.

IMO we should put a stake in the ground and refuse to accept code that consumes
"non-architectural" state.  It's all software, having non-architectural APIs is
completely ridiculous.