From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (NAM12-MW2-obe.outbound.protection.outlook.com [40.107.244.70]) by mx.groups.io with SMTP id smtpd.web11.18247.1602783565713098314 for ; Thu, 15 Oct 2020 10:39:26 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amdcloud.onmicrosoft.com header.s=selector2-amdcloud-onmicrosoft-com header.b=VH4n1hAE; spf=none, err=SPF record not found (domain: amd.com, ip: 40.107.244.70, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ta3SatYFNzLjZOHTjvPGIVyULv/iGCeIdjyM8uW1g9WEu63U0TqnaoqzsZaxJWxWpgw3Wj9bHescQo/HUfYp/BYGqQY3WHLkvRd2qwxfk6sqo3VpP1HKM953SmwMuhYbmT4A+6cfq2uXC/FiCdGS+H/N/uCCjtND9LLDcukxbqUEhUisFCr9oHfERR3mi5HLhiVVl35LFm+B1fW9vEYME4r1ao36EYDVFtgikM+2Lbyl7jrmIMLgeaZgHR1yjn7WHHk1fToQj0D9xFyTYRYk5c3qwcqcsKn9uHphvtkJviqpXwv1H8drvgQnb1RN84sD4JC6PlC4avjjpgstrhq9/g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=WU6jKrVpeDRLlTaSw0Js3t7PsMXYZt0ylxIujomWPHk=; b=aiH7nI0/2gCthygA2REeaJGDqxCop66HNDfBNl/qxajhjV9ric62z1X7JadHWnJpmeBwJfKqJfv0/PRI+roNuhDtL9Tw33/dXGs8yf+GcQJ5kBeklNstR0YjK7lQ5i2o1cdNlf2/zK6A8RUpZm4qPvV7VlVi+Nnn2XMTdwp8k4BdIyPcQ9WA7XPi7ku+5xi9ht/qOsqsoe0ljQYvSQMDkf+ED6E2sgrRv5PvGsR5Y+pdIsadKrWFIyRS9Uq2qzPfCpqhjDIc71cBWblzF++F9nOBpkr/nah3IEac/09dfNNgRhiR+Sz628XPnvSommb4o+Mk638iZ6IjaNBFGaKmXw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amdcloud.onmicrosoft.com; s=selector2-amdcloud-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=WU6jKrVpeDRLlTaSw0Js3t7PsMXYZt0ylxIujomWPHk=; b=VH4n1hAEhralcYkFvX97eYOzjhq5jQt0dhcXj23aYsIURWtVcAlziPQN0moZINrbo8JvaHINS5HAbe+XBnhJC5goYiRPROrVAAcZHtmtsnbf4+Vh3V7PXplco12XVHnDCg3sZB4OSUCBo8Te/HLdJ7AjCEBKw9dryDsXeqs2OOQ= Authentication-Results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM6PR12MB4372.namprd12.prod.outlook.com (2603:10b6:5:2af::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3455.24; Thu, 15 Oct 2020 17:39:23 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4d88:9239:2419:7348]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::4d88:9239:2419:7348%2]) with mapi id 15.20.3455.030; Thu, 15 Oct 2020 17:39:22 +0000 Subject: Re: [PATCH 8/9] OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Disable interrupts when using GHCB To: Laszlo Ersek , devel@edk2.groups.io Cc: Brijesh Singh , Jordan Justen , Ard Biesheuvel References: <4f66dc48ae127d8b42313e7a0a8c7cf10667dcb3.1602346027.git.thomas.lendacky@amd.com> From: "Lendacky, Thomas" Message-ID: <636194c8-ea53-15ab-faa1-72e2f32e6e6b@amd.com> Date: Thu, 15 Oct 2020 12:39:20 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 In-Reply-To: X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN4PR0401CA0007.namprd04.prod.outlook.com (2603:10b6:803:21::17) To DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from office-linux.texastahm.com (67.79.209.213) by SN4PR0401CA0007.namprd04.prod.outlook.com (2603:10b6:803:21::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3477.21 via Frontend Transport; Thu, 15 Oct 2020 17:39:22 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 183a4e79-793a-4811-afa7-08d87131409c X-MS-TrafficTypeDiagnostic: DM6PR12MB4372: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: 6QNRtS1BWaZojormW3IzGSuQ7izg/kMLJ8qut20sb8dSjFRSvX4Q+agya/ZImyeUkza3+JyE0GZHnyvjQ/za9hN2oMWHXOZvIKkZprUdNy1+R4R/E6EieN/avvjuofATTVPhD9Brte8L+vAH0LtJ1HwOV68khLK90YytSYyvQkEzYtraOvY2++9pVDtf03kJiEa/YI+mKk6zynSEbKMOgkJBHyBjUhowu0IR/7pIrz9no6Qg7AeyK5ohY6uXHWRUQCl8864frqq1GZNDPAcg1tw1Wpj1QPZuODo8DhMiJTOfGdWMGuMSQ8dP2ofZTxhWEcEIm78uik/JPYaKEfcHnUIRvtMon6vIYtiBsnzwUcmP2yn8ZCbzVt6VOqnqt9k7FN1QQIS+UoL3waxgd/90iLGFt2F8yZmwiAwffPWfnBxdgQtui819t50NYakkRm0R X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR12MB1355.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(136003)(346002)(376002)(396003)(366004)(39860400002)(8676002)(31696002)(66556008)(26005)(6512007)(54906003)(6486002)(4326008)(5660300002)(31686004)(83380400001)(34490700002)(66946007)(52116002)(86362001)(316002)(8936002)(53546011)(478600001)(36756003)(6506007)(2616005)(956004)(66476007)(16526019)(186003)(2906002)(213903007)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: C5ZiQNZP+cJAGyQ/Zmvta1i7cy74QZ2dKNIxOXNGljlBW0E6hp14gpuyR9yliQ8akkv4f7y4gnKiBtoVtrbqoEvDMmMx8G8YQpGca4O+tfNSOeWSyG4MR++TEuIFNivFvZ2fwjS/blr7F8YTgbhr1YTlIcoElxIMbbjggzTbJWkTp1H97y1vxOHPOeA+TfY1CPe0n3rUf7NBooQLLvRlinEDQtS9Awbf24sJHk+sgGniOQ8AAoT2pkYJikEgAL0OKiSbaql97Hjak/94FK4OQvn5obGISXtnrP8CeIGGX/gHS3OYh70dQr1PYwx5Ijruaw14+imcCffUWkcUfK9eaOY4ddokuoMoRrPjjE12A59jSKoU5Er/HihY6Bhs+YxVAa1xIaNGiRhyEl58el7022NbglE6JWJEc1VgtUkNaoye5vSSLNw4PvwlZDncWx9iD/ARVvrJ0+4CU3VNdyzRVOVTOkOfe93fyuB7l1BOpbLNfjScrzYZsbeXxR/XfYTXsi52HgpOWPxxE5c23DhaAyMKvxY8vY16gUq62a+Q8NX0WYX2wtj8IpXfQpiZBjcZdhsC7oX57dCfmFpjzYWbG2OvJL5c/2s5dl6VRiF0/Hv6RDjmyfVPNzNeBO1XJlRN2XpBLbZ8+ZR8n0+rINCKeQ== X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 183a4e79-793a-4811-afa7-08d87131409c X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 Oct 2020 17:39:22.6500 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: UiagcOzLohT7noOgwQsyEeEHuMOFgr8XhE0lFpvGaz3YvQgiHmBFFJcjnuYroc5lj+u+lt20Etg42Zjwyp4IkA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4372 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 10/15/20 4:45 AM, Laszlo Ersek wrote: > On 10/10/20 18:07, Tom Lendacky wrote: >> From: Tom Lendacky >> >> The QemuFlashPtrWrite() flash services runtime uses the GHCB and VmgExit() >> directly to perform the flash write when running as an SEV-ES guest. If an >> interrupt arrives > > (1) please clarify what kind of interrupt you've seen in practice (my > guess: timer interrupt) > >> between VmgInit() and VmgDone(), > > (2) VmgDone() is currently an empty function (both library instances) -- > did you mean VmgExit()? > > >> the Dr7 read in the >> interrupt handler > > (3) please clarify what interrupt handler you have in mind (function > name and file with full path would be helpful) > >> will generate a #VC, which can overwrite information in >> the GHCB that QemuFlashPtrWrite() has set. Prevent this from occurring by >> disabling interrupts around the usage of the GHCB. > > (4) I like the last sentence, because it seems to support my suspicion > that the problem is generic. Should we push the interrupt disablement / > re-enablement logic into VmgInit() and VmgDone()? > > For that, the pre-VmgInit() interrupt state would have to be saved (for > restoration) somewhere. > > (5) I wonder if raising the TPL to TPL_HIGH_LEVEL, rather than messing > with interrupts explicitly, works too. (Search the UEFI spec for > "TPL_HIGH_LEVEL".) Managing the TPL feels cleaner. > > ... Either way, VmgInit() would have to output either the "old TPL", or > the "old interrupt state", for VmgDone() to restore. > > ... Hm wait, VmgInit() is called from ApWakeupFunction() > [UefiCpuPkg/Library/MpInitLib/MpLib.c], which is built for PEI too -- > there is no "TPL" concept in PEI. > > So I guess we should indeed manipulate the interrupts briefly, but I > believe we should still have that logic occur every time we are setting > up a VmgExit(). And so VmgInit() / VmgDone() look like the perfect > bracket for that. What's your take? Yes, I believe that will work nicely. I can add a second parameter to VmgInit() that takes a pointer to a BOOLEAN to return the interrupt state and then pass that to VmgDone(). To keep builds from breaking, I'll need to do the UefiCpuPkg and OvmfPkg changes in the same patch. Thanks, Tom > > Thanks! > Laszlo > >> >> Fixes: 437eb3f7a8db ("OvmfPkg/QemuFlashFvbServicesRuntimeDxe: Bypass flash detection with SEV-ES") >> Cc: Jordan Justen >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Signed-off-by: Tom Lendacky >> --- >> OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c | 15 +++++++++++++++ >> 1 file changed, 15 insertions(+) >> >> diff --git a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c >> index 5d5a117c48e0..872e58db7cc0 100644 >> --- a/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c >> +++ b/OvmfPkg/QemuFlashFvbServicesRuntimeDxe/QemuFlashDxe.c >> @@ -9,6 +9,7 @@ >> >> **/ >> >> +#include >> #include >> #include >> #include >> @@ -54,6 +55,7 @@ QemuFlashPtrWrite ( >> GHCB *Ghcb; >> UINT32 ScratchIndex; >> UINT32 ScratchBit; >> + BOOLEAN InterruptsEnabled; >> >> Msr.GhcbPhysicalAddress = AsmReadMsr64 (MSR_SEV_ES_GHCB); >> Ghcb = Msr.Ghcb; >> @@ -61,6 +63,15 @@ QemuFlashPtrWrite ( >> ScratchIndex = GhcbSwScratch / 8; >> ScratchBit = GhcbSwScratch & 0x07; >> >> + // >> + // Be sure that an interrupt can't cause a #VC while the GHCB is >> + // being used. >> + // >> + InterruptsEnabled = GetInterruptState (); >> + if (InterruptsEnabled) { >> + DisableInterrupts (); >> + } >> + >> // >> // Writing to flash is emulated by the hypervisor through the use of write >> // protection. This won't work for an SEV-ES guest because the write won't >> @@ -74,6 +85,10 @@ QemuFlashPtrWrite ( >> Ghcb->SaveArea.ValidBitmap[ScratchIndex] |= (1 << ScratchBit); >> VmgExit (Ghcb, SVM_EXIT_MMIO_WRITE, (UINT64) (UINTN) Ptr, 1); >> VmgDone (Ghcb); >> + >> + if (InterruptsEnabled) { >> + EnableInterrupts (); >> + } >> } else { >> *Ptr = Value; >> } >> >