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 Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B945AC4828F for ; Fri, 9 Feb 2024 01:52:39 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 0EB9C6B0074; Thu, 8 Feb 2024 20:52:39 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 09B716B0075; Thu, 8 Feb 2024 20:52:39 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id E7DBF6B0078; Thu, 8 Feb 2024 20:52:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id D9DD06B0074 for ; Thu, 8 Feb 2024 20:52:38 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 7D24CA2570 for ; Fri, 9 Feb 2024 01:52:38 +0000 (UTC) X-FDA: 81770591196.15.7292A57 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (mail-bn8nam11on2041.outbound.protection.outlook.com [40.107.236.41]) by imf27.hostedemail.com (Postfix) with ESMTP id 78E6040004 for ; Fri, 9 Feb 2024 01:52:35 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=pass header.d=amd.com header.s=selector1 header.b=vc2ZM0uO; arc=pass ("microsoft.com:s=arcselector9901:i=1"); dmarc=pass (policy=quarantine) header.from=amd.com; spf=pass (imf27.hostedemail.com: domain of Michael.Roth@amd.com designates 40.107.236.41 as permitted sender) smtp.mailfrom=Michael.Roth@amd.com ARC-Seal: i=2; s=arc-20220608; d=hostedemail.com; t=1707443555; a=rsa-sha256; cv=pass; b=g170a/bhbLNDWbpFjRqq4/8dffepCTKVkQ78WP+YUrXnNtmowyPQ8PkfYO8izGIiyJv6GN 5uPxPg0quadm/rJi6BEIysUEKHBKK3ck1IVMpus1/VrWoU5vwthZ3xt4nvTvSHyHtDdtnX 5o7U1A+puG3V8H3TWXLzQAGuccLRxRU= ARC-Authentication-Results: i=2; imf27.hostedemail.com; dkim=pass header.d=amd.com header.s=selector1 header.b=vc2ZM0uO; arc=pass ("microsoft.com:s=arcselector9901:i=1"); dmarc=pass (policy=quarantine) header.from=amd.com; spf=pass (imf27.hostedemail.com: domain of Michael.Roth@amd.com designates 40.107.236.41 as permitted sender) smtp.mailfrom=Michael.Roth@amd.com ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707443555; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=UxuYHw6h45Qj/UCXjWNQNek+lV/wpatv3BONnJE2QpA=; b=CsPW0MzzQzOdsK1kyzlMWpxUNFN3AcaO813eaLSJLyECIx74MHpk8viLZB8pxUEitGz7Dc ahGlgc6TajhqKhuuzQQ7YKK6N8zF9mXKwkgelryRNFPXNseouXjVx4yQ7k2PckortDdj6c lPWg/v7Ow+hc4YWOvYw5OgCai+ebK/0= ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=eSfRpxSib2XslGPPtpqUwtzvDRlIM17eRLW1pr/60HQwdcFaNADaMy+Om4KxMyN/GVsj1oLnr1g5itudAKFS5z87VaKDWJ/Gg0qWwolq5gQSuZHh7IZTjjmEy0AHbObqWytqM7rpTnH+IwMvbhuofS8eU65KNDBXGvjrpoT8MjvK6gDgKkoN0nu88Nsv+7Ta72NjU1oK9HIHfzElxca4wXQw7r8RFbJhF42ljpH4XUiENGUYi2vFkOSPVfvZUQUyk0pf/C7vhaQPNnSc6lML1/BcZPF2GDvrXg4RAz16ZiBDlPFKoS4YscmDcoyMl1sBogAPqspSIKcLTO3KwO0afw== 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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=UxuYHw6h45Qj/UCXjWNQNek+lV/wpatv3BONnJE2QpA=; b=lxABmzCcwSN58TldKCeeWGXMfSLIfi3Hg9RhCACxQ6Z6/+GoH7GBLhelPjpM/ARfrjweGahJGYh2DSb9gya/DkvmJNNaRQlOiV4/AZKqkqBS7HVnQ1eUP3FAcBSwyUEA8JWKTh9RsUiYcy6fb7Eky2OUWzUMOBoPPlU3HzaUD/R20u0lmRDV3p10jxfl8sPW6s2TmeOs21qLT2okkITbEYDRsWpYtyhaDv/A+bMhx8cdi18LXG2noQhDe8gb4EHc0O7M04D/vqE248TCKCVvSFd3sVbnbZrr8MB80lgzuS0PpbS1+1mqKginvRynM0rvuvzhaF4Vc+Jk0o9wzAhbsg== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass (sender ip is 165.204.84.17) smtp.rcpttodomain=redhat.com smtp.mailfrom=amd.com; dmarc=pass (p=quarantine sp=quarantine pct=100) action=none header.from=amd.com; dkim=none (message not signed); arc=none (0) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=UxuYHw6h45Qj/UCXjWNQNek+lV/wpatv3BONnJE2QpA=; b=vc2ZM0uOXKRf0IUgJkuA4lxG7+JxTPkIY3lSL6/POxyRSw+dl0O+4Y/sYiknXjRHU5QY6Qgc9ZFCoXFnunjVMphCRy2YxIfPUq773L/5zVYUnroUAYZ0C7EnDaHZfVZnSm/HTKZwgg+wA26i6KAYxj7JVK4QGs4toiyquRqWuJc= Received: from MN2PR04CA0011.namprd04.prod.outlook.com (2603:10b6:208:d4::24) by CY8PR12MB7266.namprd12.prod.outlook.com (2603:10b6:930:56::10) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7270.16; Fri, 9 Feb 2024 01:52:31 +0000 Received: from BL6PEPF0001AB52.namprd02.prod.outlook.com (2603:10b6:208:d4:cafe::65) by MN2PR04CA0011.outlook.office365.com (2603:10b6:208:d4::24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7249.36 via Frontend Transport; Fri, 9 Feb 2024 01:52:31 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 165.204.84.17) smtp.mailfrom=amd.com; dkim=none (message not signed) header.d=none;dmarc=pass action=none header.from=amd.com; Received-SPF: Pass (protection.outlook.com: domain of amd.com designates 165.204.84.17 as permitted sender) receiver=protection.outlook.com; client-ip=165.204.84.17; helo=SATLEXMB04.amd.com; pr=C Received: from SATLEXMB04.amd.com (165.204.84.17) by BL6PEPF0001AB52.mail.protection.outlook.com (10.167.241.4) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.20.7181.14 via Frontend Transport; Fri, 9 Feb 2024 01:52:31 +0000 Received: from localhost (10.180.168.240) by SATLEXMB04.amd.com (10.181.40.145) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.34; Thu, 8 Feb 2024 19:52:30 -0600 Date: Thu, 8 Feb 2024 19:52:05 -0600 From: Michael Roth To: Paolo Bonzini CC: Sean Christopherson , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , , Brijesh Singh Subject: Re: [PATCH v11 18/35] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command Message-ID: <20240209015205.xv66udh6hqz7a6t7@amd.com> References: <20231230172351.574091-1-michael.roth@amd.com> <20231230172351.574091-19-michael.roth@amd.com> <20240116041457.wver7acnwthjaflr@amd.com> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Originating-IP: [10.180.168.240] X-ClientProxiedBy: SATLEXMB03.amd.com (10.181.40.144) To SATLEXMB04.amd.com (10.181.40.145) X-EOPAttributedMessage: 0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: BL6PEPF0001AB52:EE_|CY8PR12MB7266:EE_ X-MS-Office365-Filtering-Correlation-Id: 0bc132a9-4cf1-4242-be94-08dc2911c720 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Rp4KlCA6h9GSEFM4xw3CHc36Ockr2TWiLUpM6af5/38SK97bGJwtGNqAoZALe0x341TrKAOcBb1u6jCN6ioWufngeVoQBMlfUbhSIyWMT8vqllrTcu2sf21j6J8wZLJtbXfVAwEDZP+2XkXz/nz7N5f/cM1CcHdJUqKXL8f6tvM/wY2eojtp3UcFunjJYlFR1csxA1wla1uBetex583xlfRsS2194VoL5R6m32EFXPavzWRiv2hpj0UEngTlucpa2VavepWhWL3JFMNgpAtV3RpIn49VckeoPo980OsvUtHbc41fiDxZrDRauTh3hexDTT1H31DC59sDMWGd5+pW1luLJQyNQ1ZHCj7ny0IWGpT6rdivMO3hh4XeqdaXAwIboUdFExgAKrxyOoDiQRI10xqursc8jhsiZJyRnzXlBYGXm8tklfHYrmG7P4ZodKkB/JflzygJVBFgR2iNakxXZ/tgVAuqgCn+VtYbTmMJibNShzHEp8IhAKS+/WFAk882clBD+SqoWQsVvcWc41W9sQNbXGPxPpzNP+0b4vo5BGwfhzP7xvFfzUVFm4qfX5TRZRlngl8+Pw0kt56E5W+8DXWwW+oA+ez+1qEDCVZZ9HY= X-Forefront-Antispam-Report: CIP:165.204.84.17;CTRY:US;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:SATLEXMB04.amd.com;PTR:InfoDomainNonexistent;CAT:NONE;SFS:(13230031)(4636009)(346002)(396003)(376002)(39860400002)(136003)(230922051799003)(82310400011)(64100799003)(186009)(1800799012)(451199024)(40470700004)(36840700001)(46966006)(2616005)(26005)(426003)(336012)(16526019)(83380400001)(1076003)(478600001)(53546011)(86362001)(70586007)(316002)(6916009)(8676002)(54906003)(4326008)(70206006)(8936002)(81166007)(6666004)(356005)(82740400003)(44832011)(36756003)(41300700001)(2906002)(7406005)(7416002)(5660300002);DIR:OUT;SFP:1101; X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Feb 2024 01:52:31.0981 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 0bc132a9-4cf1-4242-be94-08dc2911c720 X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=3dd8961f-e488-4e60-8e11-a82d994e183d;Ip=[165.204.84.17];Helo=[SATLEXMB04.amd.com] X-MS-Exchange-CrossTenant-AuthSource: BL6PEPF0001AB52.namprd02.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR12MB7266 X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 78E6040004 X-Stat-Signature: 1wn3qzmtrxuztiszke6b3wgwxzgfs86o X-HE-Tag: 1707443555-428374 X-HE-Meta: U2FsdGVkX1/5G9/iS/+7KsuwtIZnSIAKlHKRhVLqCuZfnUo8sz7NtEff8zSEnDGxiAO3ybTq8cQub1/RHTP7oxXpjQKbuqoJ0DhhUDuftTpsOZb2NG+uZIBJ3pIw8dXsOICrZf9x4AssK5WZa0vDqQPpJvNRJqdpxlhp5kexG8YhuZORQ3dKVR0CFN5rPA5YNbkBaixMAhtcMc/V2ZApC3QkIm40Pj6ZSNcEwxBK5onZ1u/k9GKwpudbjVG8opRf4Izr9HYqhgLxijOzHoNSjtyJbAqWp74rKTNOaLJq2NxgTAJBN9Lamw9/iepIKplmHm1MlGHgAociYFZj3WUIWHDwVkuqkrTi+nHSVz1kvJwonNuFP5mQ6yBue54Ayl+GHun6SMSzlCvAGtnAMpJFTmKknq5l1WGGus67wdZJnVl+sj/+Ffr5cbOc6reUKtGaFv4Sl3jnGrmeoZr/ZAIgV0w7fnayrpj3YE+b5qcLwn+ZeokMltfjx5NBcO8sov99ygIQ+f1mQoCbLj27nnCCN6OALSzO4mCiDVB+xZOKifvBmvu6YMz6mLLlzJjgRrJcFRcuo+Fb8yOkRw9LaS5wKxthUm4nhZSRR0+SB7JAjEp9kv9SbMm0t9q/ppfwa42nOxwdjKzox8Dtjqcg/D3UVFfM9gtv5CVGPgz4BeZHqWSyjTDbISm4io9CTCXfzf6L8OVmqXSoigeaPbY382oLvuq7Nqpg4kinjQR5fTqz6fIYPokouOniEBAgHQwY0hGi12WaalnNYusYKxsepJ0RUM/PzwlMu6aW4th9BDH7pPbBHXCPx5IEyJmA2gXllU4NgF/xYGN6z4Qtth1stYWqwblIiCsDN/7V9YhQDQxpC8DfxU4MPlKSREomAwDOm3iIQru62KgVY4wzPmI6Bwm7+CzieWSjYb8XCTE6VAx4o+8fasW4ossXbQgIhbirICgzZG22gMqeDq6P47uEy+1 UjMQlHNJ POSANQ3viJpXQ88Tsj8rZ851CaXAuORlQ3tA6WX7bBFCfoqroHbGgsJE5TJgmWzqeyn16phq9KaaV8xBGztbe05TSKmJh43ZSOYes4GQmjP8hj2AjppVaNdFp5qHTYE7JkmUcPGUhfRCuDdGacPjJIdWg3NnuTr5BxfOWITTrp1Y1itDs6t2o/jEmJtlynnsPWx5a9f8JAhF+jSVAyx8ItOCd37+e2iFWSpJ5JvKbIfhZeUI1Y/g0S4BAkdYTZEoqf63esYZeg1cYRgJu4KCt4PYq+uFuFj3V2TUsyXwXpW8shygQVmlDLYEmQro7XBDfL4Jd0N6K2JxvdAEUnTSgQC2Fv1B5ot4nUeuhRxHT5SP9cmealVtI0nNbfDv3t3aoJr3n42vcSVxKrYcq181mEaU5HM1mZT/T0/FFEhMc/XDU+DyEXpgWrffcTHd/R3rcG1oTYeuNwwdO9kk= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Wed, Feb 07, 2024 at 12:43:02AM +0100, Paolo Bonzini wrote: > On Fri, Feb 2, 2024 at 11:55 PM Sean Christopherson wrote: > > > > > + struct kvm_sev_snp_launch_update { > > > > > + __u64 start_gfn; /* Guest page number to start from. */ > > > > > + __u64 uaddr; /* userspace address need to be encrypted */ > > > > > > > > Huh? Why is KVM taking a userspace address? IIUC, the address unconditionally > > > > gets translated into a gfn, so why not pass a gfn? > > > > > > > > And speaking of gfns, AFAICT start_gfn is never used. > > > > > > I think having both the uaddr and start_gfn parameters makes sense. I > > > think it's only awkward because how I'm using the memslot to translate > > > the uaddr to a GFN in the current implementation, > > > > Yes. > > > > > > Oof, reading more of the code, this *requires* an effective in-place copy-and-convert > > > > of guest memory. > > > > > > So that's how it's done here, KVM_SNP_LAUNCH_UPDATE copies the pages into > > > gmem, then passes those pages on to firmware for encryption. Then the > > > VMM is expected to mark the GFN range as private via > > > KVM_SET_MEMORY_ATTRIBUTES, since the VMM understands what constitutes > > > initial private/encrypted payload. I should document that better in > > > KVM_SNP_LAUNCH_UPDATE docs however. > > > > That's fine. As above, my complaints are that the unencrypted source *must* be > > covered by a memslot. That's beyond ugly. > > Yes, if there's one field that has to go it's uaddr, and then you'll > have a non-in-place encrypt (any copy performed by KVM it is hidden). > > > > > > + kvaddr = pfn_to_kaddr(pfns[i]); > > > > > + if (!virt_addr_valid(kvaddr)) { > > > > > > > > I really, really don't like that this assume guest_memfd is backed by struct page. > > > > > > There are similar enforcements in the SEV/SEV-ES code. There was some > > > initial discussion about relaxing this for SNP so we could support > > > things like /dev/mem-mapped guest memory, but then guest_memfd came > > > along and made that to be an unlikely use-case in the near-term given > > > that it relies on alloc_pages() currently and explicitly guards against > > > mmap()'ing pages in userspace. > > > > > > I think it makes to keep the current tightened restrictions in-place > > > until such a use-case comes along, since otherwise we are relaxing a > > > bunch of currently-useful sanity checks that span all throughout the code > > What sanity is being checked for, in other words why are they useful? > If all you get for breaking the promise is a KVM_BUG_ON, for example, > that's par for the course. If instead you get an oops, then we have a > problem. > > I may be a bit less draconian than Sean, but the assumptions need to > be documented and explained because they _are_ going to go away. Maybe in this case sanity-check isn't the right word, but for instance the occurance Sean objected to: kvaddr = pfn_to_kaddr(pfns[i]); if (!virt_addr_valid(kvaddr)) { ... ret = -EINVAL; where there are pfn_valid() checks underneath the covers that provide some assurance this is normal struct-page-backed/kernel-tracked memory that has a mapping in the directmap we can use here. Dropping that assumption means we need to create temporary mappings to access the PFN, which complicates the code for a potential use-case that doesn't yet exist. But if the maintainers are telling me this will change then I have no objection to making those changes :) That was just my thinking at the time. And yes, if we move more of this sort of functionality closer to gmem then those assumptions become reality and we can keep the code more closely in sync will how memory is actually allocated. I'll rework this to something closer to what Sean mentioned during the PUCK call: a gmem interface that can be called to handle populating initial gmem pages, and drop remaining any assumptions about struct-backed/directmapped in PFNs in the code that remains afterward. I'm hoping if we do move to a unified KVM API that a similar approach will work in that case too. It may be a bit tricky with how TDX does a lot of this through KVM MMU / SecureEPT hooks, this may complicate locking expectations and not necessarily fit nicely into the same flow as SNP, but we'll see how it goes. -Mike