From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-1.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id D6905C4332B for ; Thu, 19 Mar 2020 18:40:54 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id ABCED20789 for ; Thu, 19 Mar 2020 18:40:54 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584643254; bh=Nnz8DKyINwp4/CvCy4G7iAxgaPQesPB+hm/vCt15eTU=; h=References:In-Reply-To:From:Date:Subject:To:Cc:List-ID:From; b=Zw3AaW0wH59o1FuX3BjDF6WnHwqZOolucJslY0DQxh1+06/L2hCYCVTHBYP31+XgX XDcIQ5PGPIc2Nkzw/drDGdp57O8wnRSXQ0jPwGJdzbbmkgtK/aD3gG+CJpCXrKuxRj pLmjtTnVP1CrUrY0ugmH1w3lkXbIra+MRLeMeFOc= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727231AbgCSSkx (ORCPT ); Thu, 19 Mar 2020 14:40:53 -0400 Received: from mail.kernel.org ([198.145.29.99]:60764 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725787AbgCSSkx (ORCPT ); Thu, 19 Mar 2020 14:40:53 -0400 Received: from mail-wm1-f51.google.com (mail-wm1-f51.google.com [209.85.128.51]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 5AB8F20836 for ; Thu, 19 Mar 2020 18:40:52 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1584643252; bh=Nnz8DKyINwp4/CvCy4G7iAxgaPQesPB+hm/vCt15eTU=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=R55Y0XT+kDu4C+QH9pSvmerrrpvk8vLjCLyhz6RGjBEy+fuKyV/rBtZviY5CH2bji i224dYCMGcoHWaxtOQJty0JTZzlS1PN6iuSbG/Wuu4j5JlPKdprJK/yKOP6Xvpa3E7 eUIY9FLatITEQ3vemNqnCSh3CuYYaov+WXaxPVpE= Received: by mail-wm1-f51.google.com with SMTP id a9so503585wmj.4 for ; Thu, 19 Mar 2020 11:40:52 -0700 (PDT) X-Gm-Message-State: ANhLgQ0kcbyyjad9jSmkzqdDMqYQP9LXZ9NTPmzttNY62y/ZbiHEpVg8 XAl2vprBGHF6OZllmqy35Xfn4/ptz7gtwIkV+CRkSg== X-Google-Smtp-Source: ADFU+vvWBH0AH3H+Jf2dJbQy+fo/OsWleVD+wRMeOKLdEDzg7ZJ7+mROEy3ck/x1sUe+/gBAEu7toV/J5yrTI9VoQ/4= X-Received: by 2002:a1c:b0c3:: with SMTP id z186mr5129118wme.36.1584643250721; Thu, 19 Mar 2020 11:40:50 -0700 (PDT) MIME-Version: 1.0 References: <20200319091407.1481-1-joro@8bytes.org> <20200319091407.1481-71-joro@8bytes.org> <20200319160749.GC5122@8bytes.org> In-Reply-To: <20200319160749.GC5122@8bytes.org> From: Andy Lutomirski Date: Thu, 19 Mar 2020 11:40:39 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH 70/70] x86/sev-es: Add NMI state tracking To: Joerg Roedel Cc: Andy Lutomirski , X86 ML , "H. Peter Anvin" , Dave Hansen , Peter Zijlstra , Thomas Hellstrom , Jiri Slaby , Dan Williams , Tom Lendacky , Juergen Gross , Kees Cook , LKML , kvm list , Linux Virtualization , Joerg Roedel Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Mar 19, 2020 at 9:07 AM Joerg Roedel wrote: > > Hi Andy, > > On Thu, Mar 19, 2020 at 08:35:59AM -0700, Andy Lutomirski wrote: > > On Thu, Mar 19, 2020 at 2:14 AM Joerg Roedel wrote: > > > > > > From: Joerg Roedel > > > > > > Keep NMI state in SEV-ES code so the kernel can re-enable NMIs for the > > > vCPU when it reaches IRET. > > > > IIRC I suggested just re-enabling NMI in C from do_nmi(). What was > > wrong with that approach? > > If I understand the code correctly a nested NMI will just reset the > interrupted NMI handler to start executing again at 'restart_nmi'. > The interrupted NMI handler could be in the #VC handler, and it is not > safe to just jump back to the start of the NMI handler from somewhere > within the #VC handler. Nope. A nested NMI will reset the interrupted NMI's return frame to cause it to run again when it's done. I don't think this will have any real interaction with #VC. There's no longjmp() here. > > So I decided to not allow NMI nesting for SEV-ES and only re-enable the > NMI window when the first NMI returns. This is not implemented in this > patch, but I will do that once Thomas' entry-code rewrite is upstream. > I certainly *like* preventing nesting, but I don't think we really want a whole alternate NMI path just for a couple of messed-up AMD generations. And the TF trick is not so pretty either. > > This causes us to pop the NMI frame off the stack. Assuming the NMI > > restart logic is invoked (which is maybe impossible?), we get #DB, > > which presumably is actually delivered. And we end up on the #DB > > stack, which might already have been in use, so we have a potential > > increase in nesting. Also, #DB may be called from an unexpected > > context. > > An SEV-ES hypervisor is required to intercept #DB, which means that the > #DB exception actually ends up being a #VC exception. So it will not end > up on the #DB stack. With your patch set, #DB doesn't seem to end up on the #DB stack either. > > > I think there are two credible ways to approach this: > > > > 1. Just put the NMI unmask in do_nmi(). The kernel *already* knows > > how to handle running do_nmi() with NMIs unmasked. This is much, much > > simpler than your code. > > Right, and I thought about that, but the implication is that the > complexity is moved somewhere else, namely into the #VC handler, which > then has to be restartable. As above, I don't think there's an actual problem here. > > > 2. Have an entirely separate NMI path for the > > SEV-ES-on-misdesigned-CPU case. And have very clear documentation for > > what prevents this code from being executed on future CPUs (Zen3?) > > that have this issue fixed for real? > > That sounds like a good alternative, I will investigate this approach. > The NMI handler should be much simpler as it doesn't need to allow NMI > nesting. The question is, does the C code down the NMI path depend on > the NMI handlers stack frame layout (e.g. the in-nmi flag)? Nope. In particular, the 32-bit path doesn't have all this.