Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Andrew Cooper
On 23/06/2020 19:26, Andy Lutomirski wrote: > On Tue, Jun 23, 2020 at 8:23 AM Andrew Cooper > wrote: >> On 23/06/2020 14:03, Peter Zijlstra wrote: >>> On Tue, Jun 23, 2020 at 02:12:37PM +0200, Joerg Roedel wrote: On Tue, Jun 23, 2020 at 01:50:14PM +0200, Peter Zijlstra wrote: > If SNP

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Andy Lutomirski
On Tue, Jun 23, 2020 at 8:23 AM Andrew Cooper wrote: > > On 23/06/2020 14:03, Peter Zijlstra wrote: > > On Tue, Jun 23, 2020 at 02:12:37PM +0200, Joerg Roedel wrote: > >> On Tue, Jun 23, 2020 at 01:50:14PM +0200, Peter Zijlstra wrote: > >>> If SNP is the sole reason #VC needs to be IST, then I'd

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 09:03:56AM -0700, Dave Hansen wrote: > On 6/23/20 8:52 AM, Peter Zijlstra wrote: > > Isn't current #MC unconditionally fatal from kernel? But yes, I was > > sorta aware people want that changed. > > Not unconditionally. copy_to_iter_mcsafe() is a good example of one >

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Borislav Petkov
On Tue, Jun 23, 2020 at 04:39:26PM +0100, Andrew Cooper wrote: > P.S. did you also hear that with Rowhammer, userspace has a nonzero > quantity of control over generating #MC, depending on how ECC is > configured on the platform. Where does that #MC point to? Can it control for which address to

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Borislav Petkov
On Tue, Jun 23, 2020 at 04:32:22PM +0100, Andrew Cooper wrote: > MSR_MCG_STATUS.MCIP, and yes - any #MC before that point will > immediately Shutdown.  Any #MC between that point and IRET will clobber > its IST stack and end up sad. Well, at some point we should simply accept that we're living a

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Dave Hansen
On 6/23/20 8:52 AM, Peter Zijlstra wrote: > Isn't current #MC unconditionally fatal from kernel? But yes, I was > sorta aware people want that changed. Not unconditionally. copy_to_iter_mcsafe() is a good example of one thing we _can_ handle.

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 05:38:55PM +0200, Joerg Roedel wrote: > On Tue, Jun 23, 2020 at 05:23:26PM +0200, Peter Zijlstra wrote: > > Reliability of that depends on the unwinder, I wouldn't want the guess > > uwinder to OOPS me by accident. > > It doesn't use the full unwinder, it just assumes

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 04:39:26PM +0100, Andrew Cooper wrote: > On 23/06/2020 16:23, Peter Zijlstra wrote: > > On Tue, Jun 23, 2020 at 04:59:14PM +0200, Joerg Roedel wrote: > >> Yes, this is a start, it doesn't cover the case where the NMI stack is > >> in-between, so I think you need to walk

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Borislav Petkov
On Tue, Jun 23, 2020 at 12:51:03PM +0100, Andrew Cooper wrote: > Crashing out hard if the hypervisor is misbehaving is acceptable.  In a > cloud, I as a customer would (threaten to?) take my credit card > elsewhere, while for enterprise, I'd shout at my virtualisation vendor > until a fix happened

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Andrew Cooper
On 23/06/2020 16:23, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 04:59:14PM +0200, Joerg Roedel wrote: >> On Tue, Jun 23, 2020 at 04:53:44PM +0200, Peter Zijlstra wrote: >>> +noinstr void idtentry_validate_ist(struct pt_regs *regs) >>> +{ >>> + if ((regs->sp & ~(EXCEPTION_STKSZ-1)) == >>> +

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 05:23:26PM +0200, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 04:59:14PM +0200, Joerg Roedel wrote: > > On Tue, Jun 23, 2020 at 04:53:44PM +0200, Peter Zijlstra wrote: > > > +noinstr void idtentry_validate_ist(struct pt_regs *regs) > > > +{ > > > + if ((regs->sp &

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
On Tue, Jun 23, 2020 at 05:23:26PM +0200, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 04:59:14PM +0200, Joerg Roedel wrote: > > On Tue, Jun 23, 2020 at 04:53:44PM +0200, Peter Zijlstra wrote: > > > +noinstr void idtentry_validate_ist(struct pt_regs *regs) > > > +{ > > > + if ((regs->sp &

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Andrew Cooper
On 23/06/2020 16:16, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 04:49:40PM +0200, Joerg Roedel wrote: >>> We're talking about the 3rd case where the only reason things 'work' is >>> because we'll have to panic(): >>> >>> - #MC >> Okay, #MC is special and can only be handled on a best-effort

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 04:59:14PM +0200, Joerg Roedel wrote: > On Tue, Jun 23, 2020 at 04:53:44PM +0200, Peter Zijlstra wrote: > > +noinstr void idtentry_validate_ist(struct pt_regs *regs) > > +{ > > + if ((regs->sp & ~(EXCEPTION_STKSZ-1)) == > > + (_RET_IP_ & ~(EXCEPTION_STKSZ-1))) > > +

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Andrew Cooper
On 23/06/2020 14:03, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 02:12:37PM +0200, Joerg Roedel wrote: >> On Tue, Jun 23, 2020 at 01:50:14PM +0200, Peter Zijlstra wrote: >>> If SNP is the sole reason #VC needs to be IST, then I'd strongly urge >>> you to only make it IST if/when you try and

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 04:49:40PM +0200, Joerg Roedel wrote: > > We're talking about the 3rd case where the only reason things 'work' is > > because we'll have to panic(): > > > > - #MC > > Okay, #MC is special and can only be handled on a best-effort basis, as > #MC could happen anytime, also

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
On Tue, Jun 23, 2020 at 04:53:44PM +0200, Peter Zijlstra wrote: > +noinstr void idtentry_validate_ist(struct pt_regs *regs) > +{ > + if ((regs->sp & ~(EXCEPTION_STKSZ-1)) == > + (_RET_IP_ & ~(EXCEPTION_STKSZ-1))) > + die("IST stack recursion", regs, 0); > +} Yes, this is a

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 03:59:16PM +0200, Peter Zijlstra wrote: > So basically when your exception frame points to your own IST, you die. > That sounds like something we should have in generic IST code. Something like this... #DF already dies and NMI is 'magic' --- arch/x86/entry/common.c

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
On Tue, Jun 23, 2020 at 03:03:22PM +0200, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 02:12:37PM +0200, Joerg Roedel wrote: > > On Tue, Jun 23, 2020 at 01:50:14PM +0200, Peter Zijlstra wrote: > > > If SNP is the sole reason #VC needs to be IST, then I'd strongly urge > > > you to only make it

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 03:40:03PM +0200, Joerg Roedel wrote: > On Tue, Jun 23, 2020 at 02:52:01PM +0200, Peter Zijlstra wrote: > > You only have that guarantee when any SNP #VC from kernel is an > > automatic panic. But in that case, what's the point of having the > > recursion count? > > It is

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Andrew Cooper
On 23/06/2020 13:47, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 12:51:03PM +0100, Andrew Cooper wrote: > >> There are cases which are definitely non-recoverable. >> >> For both ES and SNP, a malicious hypervisor can mess with the guest >> physmap to make the the NMI, #VC and #DF stacks all

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
On Tue, Jun 23, 2020 at 02:52:01PM +0200, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 02:04:33PM +0200, Joerg Roedel wrote: > > No, the recursion check is fine, because overwriting an already used IST > > stack doesn't matter (as long as it can be detected) if we are going to > > panic anyway.

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 02:12:37PM +0200, Joerg Roedel wrote: > On Tue, Jun 23, 2020 at 01:50:14PM +0200, Peter Zijlstra wrote: > > If SNP is the sole reason #VC needs to be IST, then I'd strongly urge > > you to only make it IST if/when you try and make SNP happen, not before. > > It is not the

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 02:04:33PM +0200, Joerg Roedel wrote: > On Tue, Jun 23, 2020 at 01:48:18PM +0200, Peter Zijlstra wrote: > > On Tue, Jun 23, 2020 at 01:30:07PM +0200, Joerg Roedel wrote: > > > But you cannot do a recursion check in #VC, because the NMI can happen > > on the first

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 12:51:03PM +0100, Andrew Cooper wrote: > There are cases which are definitely non-recoverable. > > For both ES and SNP, a malicious hypervisor can mess with the guest > physmap to make the the NMI, #VC and #DF stacks all alias. > > For ES, this had better result in the

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
On Tue, Jun 23, 2020 at 01:50:14PM +0200, Peter Zijlstra wrote: > If SNP is the sole reason #VC needs to be IST, then I'd strongly urge > you to only make it IST if/when you try and make SNP happen, not before. It is not the only reason, when ES guests gain debug register support then #VC also

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
On Tue, Jun 23, 2020 at 01:48:18PM +0200, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 01:30:07PM +0200, Joerg Roedel wrote: > But you cannot do a recursion check in #VC, because the NMI can happen > on the first instruction of #VC, before we can increment our counter, > and then the #VC can

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Andrew Cooper
On 23/06/2020 12:30, Joerg Roedel wrote: > On Tue, Jun 23, 2020 at 01:07:06PM +0200, Peter Zijlstra wrote: >> On Tue, Apr 28, 2020 at 09:55:12AM +0200, Joerg Roedel wrote: >> So what happens if this #VC triggers on the first access to the #VC >> stack, because the malicious host has craftily

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 01:43:24PM +0200, Joerg Roedel wrote: > On Tue, Jun 23, 2020 at 01:14:43PM +0200, Peter Zijlstra wrote: > > On Tue, Jun 23, 2020 at 01:11:07PM +0200, Joerg Roedel wrote: > > > > The v3 patchset implements an unconditional shift of the #VC IST entry > > > in the NMI

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 01:30:07PM +0200, Joerg Roedel wrote: > Note that this is an issue only with secure nested paging (SNP), which > is not enabled yet with this patch-set. When it gets enabled a stack > recursion check in the #VC handler is needed which panics the VM. That > also fixes the

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
On Tue, Jun 23, 2020 at 01:14:43PM +0200, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 01:11:07PM +0200, Joerg Roedel wrote: > > The v3 patchset implements an unconditional shift of the #VC IST entry > > in the NMI handler, before it can trigger a #VC exception. > > Going by that other thread

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
On Tue, Jun 23, 2020 at 01:07:06PM +0200, Peter Zijlstra wrote: > On Tue, Apr 28, 2020 at 09:55:12AM +0200, Joerg Roedel wrote: > So what happens if this #VC triggers on the first access to the #VC > stack, because the malicious host has craftily mucked with only the #VC > IST stack page? > > Or

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 01:11:07PM +0200, Joerg Roedel wrote: > Hi Peter, > > On Tue, Jun 23, 2020 at 12:45:59PM +0200, Peter Zijlstra wrote: > > On Tue, Jun 23, 2020 at 11:45:19AM +0200, Joerg Roedel wrote: > > > Or maybe you have a better idea how to implement this, so I'd like to > > > hear

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
Hi Peter, On Tue, Jun 23, 2020 at 12:45:59PM +0200, Peter Zijlstra wrote: > On Tue, Jun 23, 2020 at 11:45:19AM +0200, Joerg Roedel wrote: > > Or maybe you have a better idea how to implement this, so I'd like to > > hear your opinion first before I spend too many days implementing > > something.

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Apr 28, 2020 at 09:55:12AM +0200, Joerg Roedel wrote: > On Mon, Apr 27, 2020 at 10:37:41AM -0700, Andy Lutomirski wrote: > > I have a somewhat serious question: should we use IST for #VC at all? > > As I understand it, Rome and Naples make it mandatory for hypervisors > > to intercept #DB,

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Peter Zijlstra
On Tue, Jun 23, 2020 at 11:45:19AM +0200, Joerg Roedel wrote: > Hi Andy, > > On Mon, Apr 27, 2020 at 10:37:41AM -0700, Andy Lutomirski wrote: > > 1. Use IST for #VC and deal with all the mess that entails. > > With the removal of IST shifting I wonder what you would suggest on how > to best

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-06-23 Thread Joerg Roedel
Hi Andy, On Mon, Apr 27, 2020 at 10:37:41AM -0700, Andy Lutomirski wrote: > 1. Use IST for #VC and deal with all the mess that entails. With the removal of IST shifting I wonder what you would suggest on how to best implement an NMI-safe IST handler with nesting support. My current plan is to

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-04-28 Thread Andrew Cooper
On 28/04/2020 08:55, Joerg Roedel wrote: > On Mon, Apr 27, 2020 at 10:37:41AM -0700, Andy Lutomirski wrote: >> I have a somewhat serious question: should we use IST for #VC at all? >> As I understand it, Rome and Naples make it mandatory for hypervisors >> to intercept #DB, which means that, due

Re: Should SEV-ES #VC use IST? (Re: [PATCH] Allow RDTSC and RDTSCP from userspace)

2020-04-28 Thread Joerg Roedel
On Mon, Apr 27, 2020 at 10:37:41AM -0700, Andy Lutomirski wrote: > I have a somewhat serious question: should we use IST for #VC at all? > As I understand it, Rome and Naples make it mandatory for hypervisors > to intercept #DB, which means that, due to the MOV SS mess, it's sort > of mandatory to