On 4/24/20 2:27 PM, Tom Lendacky wrote:> On 4/24/20 4:24 PM, Dave Hansen wrote: >> On 4/24/20 2:03 PM, Mike Stunes wrote: >>> I needed to allow RDTSC(P) from userspace and in early boot in order to >>> get userspace started properly. Patch below. >>> >>> --- >>> SEV-ES guests will need to execute rdtsc and rdtscp from userspace and >>> during early boot. Move the rdtsc(p) #VC handler into common code and >>> extend the #VC handlers. >> >> Do SEV-ES guests _always_ #VC on rdtsc(p)? > > Only if the hypervisor is intercepting those instructions.Ahh, so any instruction that can have an instruction intercept set potentially needs to be able to tolerate a #VC? Those instruction intercepts are under the control of the (untrusted relative to the guest) hypervisor, right?>From the main sev-es series:+#ifdef CONFIG_AMD_MEM_ENCRYPT +idtentry vmm_communication do_vmm_communication has_error_code=1 +#endif Since this is set as non-paranoid, that both limits the instructions that can be used in entry paths *and* limits the future architecture from being able add instructions that a current SEV-ES guest doesn't know about. Does SEV-ES have versioning so guests can tell if they might be subject to new interrupt intercepts for which they are not prepared? I didn't see anything obvious in section 15.35 of the manual. There's also a nugget in the manual that says:> Similarly, the hypervisor should avoid setting intercept bits for > events that would occur in the #VC handler (such as IRET).That's a fun point because it means that the (untrusted) hypervisor can cause endless faults. I *guess* we have mitigation for this with our stack guard pages, but it's still a bit nasty that the hypervisor can arbitrarily land a guest in the double-fault handler. It just all seems a bit weak for the hypervisor to be considered untrusted. But, it's _certainly_ a steep in the right direction from SEV.
Hi Dave, On Fri, Apr 24, 2020 at 03:53:09PM -0700, Dave Hansen wrote:> Ahh, so any instruction that can have an instruction intercept set > potentially needs to be able to tolerate a #VC? Those instruction > intercepts are under the control of the (untrusted relative to the > guest) hypervisor, right? > > >From the main sev-es series: > > +#ifdef CONFIG_AMD_MEM_ENCRYPT > +idtentry vmm_communication do_vmm_communication has_error_code=1 > +#endifThe next version of the patch-set (which I will hopefully have ready next week) will have this changed. The #VC exception handler uses an IST stack and is set to paranoid=1 and shift_ist. The IST stacks for the #VC handler are only allocated when SEV-ES is active.> That's a fun point because it means that the (untrusted) hypervisor can > cause endless faults. I *guess* we have mitigation for this with our > stack guard pages, but it's still a bit nasty that the hypervisor can > arbitrarily land a guest in the double-fault handler. > > It just all seems a bit weak for the hypervisor to be considered > untrusted. But, it's _certainly_ a steep in the right direction from SEV.Yeah, a malicious hypervisor can do bad things to an SEV-ES VM, but it can't easily steal its secrets from memory or registers. The #VC handler does its best to just crash the VM if unexpected hypervisor behavior is detected. Regards, Joerg
On Sat, Apr 25, 2020 at 5:49 AM Joerg Roedel <jroedel at suse.de> wrote:> > Hi Dave, > > On Fri, Apr 24, 2020 at 03:53:09PM -0700, Dave Hansen wrote: > > Ahh, so any instruction that can have an instruction intercept set > > potentially needs to be able to tolerate a #VC? Those instruction > > intercepts are under the control of the (untrusted relative to the > > guest) hypervisor, right? > > > > >From the main sev-es series: > > > > +#ifdef CONFIG_AMD_MEM_ENCRYPT > > +idtentry vmm_communication do_vmm_communication has_error_code=1 > > +#endif > > The next version of the patch-set (which I will hopefully have ready > next week) will have this changed. The #VC exception handler uses an IST > stack and is set to paranoid=1 and shift_ist. The IST stacks for the #VC > handler are only allocated when SEV-ES is active.shift_ist is gross. What's it for? If it's not needed, I'd rather not use it, and I eventually want to get rid of it for #DB as well. --Andy
On 4/25/20 5:49 AM, Joerg Roedel wrote:>> That's a fun point because it means that the (untrusted) hypervisor can >> cause endless faults. I *guess* we have mitigation for this with our >> stack guard pages, but it's still a bit nasty that the hypervisor can >> arbitrarily land a guest in the double-fault handler. >> >> It just all seems a bit weak for the hypervisor to be considered >> untrusted. But, it's _certainly_ a steep in the right direction from SEV. > Yeah, a malicious hypervisor can do bad things to an SEV-ES VM, but it > can't easily steal its secrets from memory or registers. The #VC handler > does its best to just crash the VM if unexpected hypervisor behavior is > detected.This is the kind of design information that would be very useful to reviewers. Will some of this information make it into the cover letter eventually? Or, Documentation/? Also, for the security purists, an SEV-ES host is still trusted (in the same TCB as the guest). Truly guest-untrusted VMMs won't be available until SEV-SNP, right?