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=-12.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham 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 A24DCC4743E for ; Tue, 8 Jun 2021 12:06:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 832666124C for ; Tue, 8 Jun 2021 12:06:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232432AbhFHMIp (ORCPT ); Tue, 8 Jun 2021 08:08:45 -0400 Received: from mail.kernel.org ([198.145.29.99]:55928 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232294AbhFHMIo (ORCPT ); Tue, 8 Jun 2021 08:08:44 -0400 Received: by mail.kernel.org (Postfix) with ESMTPSA id 0081160FDB; Tue, 8 Jun 2021 12:06:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1623154012; bh=szHxzIi0roevoqKfazzQiVRpREg75Q1zH65M51z19xM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=X6PGuCn19pZRTaq4LmYkkSPIFYciDjpNRne40QQLuwDNqVNv/CtZdN7eQTzFor/Ns x+a16zyRcQFPEXEy5l3EpniIuKQABpTutruJAjSymJfAj3NF6BQ65m0FbCZlViWhao 5csPWnJCGskVDEPFhUX5sauXY3a2g6B4NvOlunsQVnVKMZD6y8Oggbq6+9A025x4Qe dsAfQVXy8ZT8x9Xygcw9M7gmSIW8DAfftMiKd353BawwOd1yrpISbBrMt/JBYMrl0i jmmvSY66fFkWo07eUJs/oEvrBte0Kjd9tNKd5FMs99kFAs3HsrriR0YK/oWhUUMDmw 5uousyqtR6Psw== Date: Tue, 8 Jun 2021 13:06:46 +0100 From: Will Deacon To: Mark Rutland Cc: kvmarm@lists.cs.columbia.edu, Marc Zyngier , James Morse , Alexandru Elisei , Suzuki K Poulose , Christoffer Dall , Paolo Bonzini , Fuad Tabba , Quentin Perret , Sean Christopherson , David Brazdil , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC PATCH 4/4] KVM: arm64: Introduce KVM_CAP_ARM_PROTECTED_VM Message-ID: <20210608120645.GD10174@willie-the-truck> References: <20210603183347.1695-1-will@kernel.org> <20210603183347.1695-5-will@kernel.org> <20210604144110.GD69333@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20210604144110.GD69333@C02TD0UTHF1T.local> User-Agent: Mutt/1.10.1 (2018-07-13) Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On Fri, Jun 04, 2021 at 03:41:10PM +0100, Mark Rutland wrote: > On Thu, Jun 03, 2021 at 07:33:47PM +0100, Will Deacon wrote: > > +7.26.1 KVM_CAP_ARM_PROTECTED_VM_FLAGS_ENABLE > > +-------------------------------------------- > > + > > +:Capability: 'flag' parameter to KVM_CAP_ARM_PROTECTED_VM > > +:Architectures: arm64 > > +:Target: VM > > +:Parameters: args[0] contains memory slot ID to hold guest firmware > > +:Returns: 0 on success; negative error code on failure > > + > > +Enabling this capability causes all memory slots of the specified VM to be > > +unmapped from the host system and put into a state where they are no longer > > +configurable. The memory slot corresponding to the ID passed in args[0] is > > +populated with the guest firmware image provided by the host firmware. > > As on the prior patch, I don't quite follow the rationale for the guest > fw coming from the host fw, and it seems to go against the usual design > for VM contents, so I fear it could be a problem in future (even if not > in android's specific model for usage). Hopefully my other reply helps here. As I say, it would be trivial to extend the interface to make this optional. > > +The first vCPU to enter the guest is defined to be the primary vCPU. All other > > +vCPUs belonging to the VM are secondary vCPUs. > > + > > +All vCPUs belonging to a VM with this capability enabled are initialised to a > > +pre-determined reset state > > What is that "pre-determined reset state"? e.g. is that just what KVM > does today, or is there something more specific (e.g. that might change > with the "Boot protocol version" below)? Yes, it's the KVM reset state as described by KVM_ARM_VCPU_INIT, as I state later in the sentence: > > irrespective of any prior configuration according to > > +the KVM_ARM_VCPU_INIT ioctl here ^^^ So I should probably reword it. How about: | All vCPUs belonging to a VM with this capability enabled are initialised to | a pre-determined reset state according to the KVM_ARM_VCPU_INIT ioctl, | irrespective of any prior configuration, with the following exceptions | for the primary vCPU: ? > > with the following exceptions for the primary > > +vCPU: > > + > > + =========== =========== > > + Register(s) Reset value > > + =========== =========== > > + X0-X14: Preserved (see KVM_SET_ONE_REG) > > + X15: Boot protocol version (0) > > What's the "Boot protocol" in this context? Is that just referring to > this handover state, or is that something more involved? It's just versioning this state, in case we have to change/extend it in future. If you prefer, I can just say: X15-X30 Reserved (0) instead? > > + X16-X30: Reserved (0) > > + PC: IPA base of firmware memory slot > > + SP: IPA end of firmware memory slot > > + =========== =========== > > + > > +Secondary vCPUs belonging to a VM with this capability enabled will return > > +-EPERM in response to a KVM_RUN ioctl() if the vCPU was not initialised with > > +the KVM_ARM_VCPU_POWER_OFF feature. > > I assume this means that protected VMs always get a trusted PSCI > implementation? It might be worth mentioning so (and worth consdiering > if that should always have the SMCCC bits too). "trusted PSCI implementation" might be stretching it, but the idea is that you can at least ensure that vCPUs won't start executing until the guest is ready for them. I don't think we need any particular SMCCC revision for that. > > + > > +There is no support for AArch32 at any exception level. > > Is this only going to run on CPUs without AArch32 EL0? ... or does this > mean behaviour will be erratic if someone tries to run AArch32 EL0? It means that we don't permit creation of 32-bit vCPUs, or exception return to an AArch32 guest from EL2. Of course, if the guest itself decides to try this and the hardware happens to support it then there's not a lot we can about it. But that's the guest being silly, so we don't need to care. > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > > index 24223adae150..cdb3298ba8ae 100644 > > --- a/arch/arm64/include/uapi/asm/kvm.h > > +++ b/arch/arm64/include/uapi/asm/kvm.h > > @@ -402,6 +402,15 @@ struct kvm_vcpu_events { > > #define KVM_PSCI_RET_INVAL PSCI_RET_INVALID_PARAMS > > #define KVM_PSCI_RET_DENIED PSCI_RET_DENIED > > > > +/* Protected KVM */ > > +#define KVM_CAP_ARM_PROTECTED_VM_FLAGS_ENABLE 0 > > +#define KVM_CAP_ARM_PROTECTED_VM_FLAGS_INFO 1 > > + > > +struct kvm_protected_vm_info { > > + __u64 firmware_size; > > + __u64 __reserved[7]; > > +}; > > Do you have anything in mind for those 7 fields, or was this just a > number that sounded big enough? I just padded it to a cacheline, as that's plenty enough space to version the thing if we need to. > I wonder if it's worth adding an size field, and having a size argument > to the ioctl, so that you can discover the full size if we need to grow > this, but you can always get a truncated copy if you just need the early > fields. Possibly, but the rest of the KVM UAPI doesn't seem to be designed like that, so I followed what was there already. I defer to the KVM maintainers on this one... Will 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=-10.3 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 E99A0C47082 for ; Tue, 8 Jun 2021 12:06:59 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id 5E4416100A for ; Tue, 8 Jun 2021 12:06:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5E4416100A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id D952D40573; Tue, 8 Jun 2021 08:06:58 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@kernel.org Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 6tQHcNrdJ4ng; Tue, 8 Jun 2021 08:06:56 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 34A2D40629; Tue, 8 Jun 2021 08:06:56 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 522A240573 for ; Tue, 8 Jun 2021 08:06:55 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 3dYwKSGhgvIG for ; Tue, 8 Jun 2021 08:06:54 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 1B2674048B for ; Tue, 8 Jun 2021 08:06:54 -0400 (EDT) Received: by mail.kernel.org (Postfix) with ESMTPSA id 0081160FDB; Tue, 8 Jun 2021 12:06:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1623154012; bh=szHxzIi0roevoqKfazzQiVRpREg75Q1zH65M51z19xM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=X6PGuCn19pZRTaq4LmYkkSPIFYciDjpNRne40QQLuwDNqVNv/CtZdN7eQTzFor/Ns x+a16zyRcQFPEXEy5l3EpniIuKQABpTutruJAjSymJfAj3NF6BQ65m0FbCZlViWhao 5csPWnJCGskVDEPFhUX5sauXY3a2g6B4NvOlunsQVnVKMZD6y8Oggbq6+9A025x4Qe dsAfQVXy8ZT8x9Xygcw9M7gmSIW8DAfftMiKd353BawwOd1yrpISbBrMt/JBYMrl0i jmmvSY66fFkWo07eUJs/oEvrBte0Kjd9tNKd5FMs99kFAs3HsrriR0YK/oWhUUMDmw 5uousyqtR6Psw== Date: Tue, 8 Jun 2021 13:06:46 +0100 From: Will Deacon To: Mark Rutland Subject: Re: [RFC PATCH 4/4] KVM: arm64: Introduce KVM_CAP_ARM_PROTECTED_VM Message-ID: <20210608120645.GD10174@willie-the-truck> References: <20210603183347.1695-1-will@kernel.org> <20210603183347.1695-5-will@kernel.org> <20210604144110.GD69333@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210604144110.GD69333@C02TD0UTHF1T.local> User-Agent: Mutt/1.10.1 (2018-07-13) Cc: kvm@vger.kernel.org, Marc Zyngier , linux-arm-kernel@lists.infradead.org, Sean Christopherson , Paolo Bonzini , kvmarm@lists.cs.columbia.edu X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On Fri, Jun 04, 2021 at 03:41:10PM +0100, Mark Rutland wrote: > On Thu, Jun 03, 2021 at 07:33:47PM +0100, Will Deacon wrote: > > +7.26.1 KVM_CAP_ARM_PROTECTED_VM_FLAGS_ENABLE > > +-------------------------------------------- > > + > > +:Capability: 'flag' parameter to KVM_CAP_ARM_PROTECTED_VM > > +:Architectures: arm64 > > +:Target: VM > > +:Parameters: args[0] contains memory slot ID to hold guest firmware > > +:Returns: 0 on success; negative error code on failure > > + > > +Enabling this capability causes all memory slots of the specified VM to be > > +unmapped from the host system and put into a state where they are no longer > > +configurable. The memory slot corresponding to the ID passed in args[0] is > > +populated with the guest firmware image provided by the host firmware. > > As on the prior patch, I don't quite follow the rationale for the guest > fw coming from the host fw, and it seems to go against the usual design > for VM contents, so I fear it could be a problem in future (even if not > in android's specific model for usage). Hopefully my other reply helps here. As I say, it would be trivial to extend the interface to make this optional. > > +The first vCPU to enter the guest is defined to be the primary vCPU. All other > > +vCPUs belonging to the VM are secondary vCPUs. > > + > > +All vCPUs belonging to a VM with this capability enabled are initialised to a > > +pre-determined reset state > > What is that "pre-determined reset state"? e.g. is that just what KVM > does today, or is there something more specific (e.g. that might change > with the "Boot protocol version" below)? Yes, it's the KVM reset state as described by KVM_ARM_VCPU_INIT, as I state later in the sentence: > > irrespective of any prior configuration according to > > +the KVM_ARM_VCPU_INIT ioctl here ^^^ So I should probably reword it. How about: | All vCPUs belonging to a VM with this capability enabled are initialised to | a pre-determined reset state according to the KVM_ARM_VCPU_INIT ioctl, | irrespective of any prior configuration, with the following exceptions | for the primary vCPU: ? > > with the following exceptions for the primary > > +vCPU: > > + > > + =========== =========== > > + Register(s) Reset value > > + =========== =========== > > + X0-X14: Preserved (see KVM_SET_ONE_REG) > > + X15: Boot protocol version (0) > > What's the "Boot protocol" in this context? Is that just referring to > this handover state, or is that something more involved? It's just versioning this state, in case we have to change/extend it in future. If you prefer, I can just say: X15-X30 Reserved (0) instead? > > + X16-X30: Reserved (0) > > + PC: IPA base of firmware memory slot > > + SP: IPA end of firmware memory slot > > + =========== =========== > > + > > +Secondary vCPUs belonging to a VM with this capability enabled will return > > +-EPERM in response to a KVM_RUN ioctl() if the vCPU was not initialised with > > +the KVM_ARM_VCPU_POWER_OFF feature. > > I assume this means that protected VMs always get a trusted PSCI > implementation? It might be worth mentioning so (and worth consdiering > if that should always have the SMCCC bits too). "trusted PSCI implementation" might be stretching it, but the idea is that you can at least ensure that vCPUs won't start executing until the guest is ready for them. I don't think we need any particular SMCCC revision for that. > > + > > +There is no support for AArch32 at any exception level. > > Is this only going to run on CPUs without AArch32 EL0? ... or does this > mean behaviour will be erratic if someone tries to run AArch32 EL0? It means that we don't permit creation of 32-bit vCPUs, or exception return to an AArch32 guest from EL2. Of course, if the guest itself decides to try this and the hardware happens to support it then there's not a lot we can about it. But that's the guest being silly, so we don't need to care. > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > > index 24223adae150..cdb3298ba8ae 100644 > > --- a/arch/arm64/include/uapi/asm/kvm.h > > +++ b/arch/arm64/include/uapi/asm/kvm.h > > @@ -402,6 +402,15 @@ struct kvm_vcpu_events { > > #define KVM_PSCI_RET_INVAL PSCI_RET_INVALID_PARAMS > > #define KVM_PSCI_RET_DENIED PSCI_RET_DENIED > > > > +/* Protected KVM */ > > +#define KVM_CAP_ARM_PROTECTED_VM_FLAGS_ENABLE 0 > > +#define KVM_CAP_ARM_PROTECTED_VM_FLAGS_INFO 1 > > + > > +struct kvm_protected_vm_info { > > + __u64 firmware_size; > > + __u64 __reserved[7]; > > +}; > > Do you have anything in mind for those 7 fields, or was this just a > number that sounded big enough? I just padded it to a cacheline, as that's plenty enough space to version the thing if we need to. > I wonder if it's worth adding an size field, and having a size argument > to the ioctl, so that you can discover the full size if we need to grow > this, but you can always get a truncated copy if you just need the early > fields. Possibly, but the rest of the KVM UAPI doesn't seem to be designed like that, so I followed what was there already. I defer to the KVM maintainers on this one... Will _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm 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=-10.7 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,URIBL_BLOCKED,USER_AGENT_SANE_1 autolearn=unavailable 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 17C67C47082 for ; Tue, 8 Jun 2021 12:33:00 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id DDAD46128D for ; Tue, 8 Jun 2021 12:32:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org DDAD46128D Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=Z3BYrsDNlBV2zPOTVQAc3ftpA8IqFYjhoYnUriPBz2U=; b=KwuDkXd2OnyQ6F oDTSiEBKV9G9kj21GMt1yd6u4KmTkfCuJsE2o+AD7RipSXASPwk00xHvnvG3NrUfilY+NV4rpXedI +ipocSCQ9JHQOKi+malCJN6l3HYudjWCNyHX33Sw6ok1Bn/udSsVoOY30suFkOwmijkjwwuHMzBw7 axbqFyaKPcjnrxnmMsXaK97x/NDIsAaNxSZPO7Q/I7fJ6RSGywTqFvPSnCUqALn6M1asriOtS6JnE r+8pjujlX4cuHXlAELo8JzLq2g25AjppXEQ2EI+nTTGz/C1DoXZSwl7oYeB5o3pq+KZfzMRKkSJKP A/tf2VOtyJN4U8oGTAow==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqasi-008MJW-5V; Tue, 08 Jun 2021 12:30:56 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1lqaVQ-008D5Z-Tg for linux-arm-kernel@lists.infradead.org; Tue, 08 Jun 2021 12:06:54 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id 0081160FDB; Tue, 8 Jun 2021 12:06:49 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1623154012; bh=szHxzIi0roevoqKfazzQiVRpREg75Q1zH65M51z19xM=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=X6PGuCn19pZRTaq4LmYkkSPIFYciDjpNRne40QQLuwDNqVNv/CtZdN7eQTzFor/Ns x+a16zyRcQFPEXEy5l3EpniIuKQABpTutruJAjSymJfAj3NF6BQ65m0FbCZlViWhao 5csPWnJCGskVDEPFhUX5sauXY3a2g6B4NvOlunsQVnVKMZD6y8Oggbq6+9A025x4Qe dsAfQVXy8ZT8x9Xygcw9M7gmSIW8DAfftMiKd353BawwOd1yrpISbBrMt/JBYMrl0i jmmvSY66fFkWo07eUJs/oEvrBte0Kjd9tNKd5FMs99kFAs3HsrriR0YK/oWhUUMDmw 5uousyqtR6Psw== Date: Tue, 8 Jun 2021 13:06:46 +0100 From: Will Deacon To: Mark Rutland Cc: kvmarm@lists.cs.columbia.edu, Marc Zyngier , James Morse , Alexandru Elisei , Suzuki K Poulose , Christoffer Dall , Paolo Bonzini , Fuad Tabba , Quentin Perret , Sean Christopherson , David Brazdil , kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [RFC PATCH 4/4] KVM: arm64: Introduce KVM_CAP_ARM_PROTECTED_VM Message-ID: <20210608120645.GD10174@willie-the-truck> References: <20210603183347.1695-1-will@kernel.org> <20210603183347.1695-5-will@kernel.org> <20210604144110.GD69333@C02TD0UTHF1T.local> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20210604144110.GD69333@C02TD0UTHF1T.local> User-Agent: Mutt/1.10.1 (2018-07-13) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210608_050653_040083_D2816259 X-CRM114-Status: GOOD ( 44.15 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Jun 04, 2021 at 03:41:10PM +0100, Mark Rutland wrote: > On Thu, Jun 03, 2021 at 07:33:47PM +0100, Will Deacon wrote: > > +7.26.1 KVM_CAP_ARM_PROTECTED_VM_FLAGS_ENABLE > > +-------------------------------------------- > > + > > +:Capability: 'flag' parameter to KVM_CAP_ARM_PROTECTED_VM > > +:Architectures: arm64 > > +:Target: VM > > +:Parameters: args[0] contains memory slot ID to hold guest firmware > > +:Returns: 0 on success; negative error code on failure > > + > > +Enabling this capability causes all memory slots of the specified VM to be > > +unmapped from the host system and put into a state where they are no longer > > +configurable. The memory slot corresponding to the ID passed in args[0] is > > +populated with the guest firmware image provided by the host firmware. > > As on the prior patch, I don't quite follow the rationale for the guest > fw coming from the host fw, and it seems to go against the usual design > for VM contents, so I fear it could be a problem in future (even if not > in android's specific model for usage). Hopefully my other reply helps here. As I say, it would be trivial to extend the interface to make this optional. > > +The first vCPU to enter the guest is defined to be the primary vCPU. All other > > +vCPUs belonging to the VM are secondary vCPUs. > > + > > +All vCPUs belonging to a VM with this capability enabled are initialised to a > > +pre-determined reset state > > What is that "pre-determined reset state"? e.g. is that just what KVM > does today, or is there something more specific (e.g. that might change > with the "Boot protocol version" below)? Yes, it's the KVM reset state as described by KVM_ARM_VCPU_INIT, as I state later in the sentence: > > irrespective of any prior configuration according to > > +the KVM_ARM_VCPU_INIT ioctl here ^^^ So I should probably reword it. How about: | All vCPUs belonging to a VM with this capability enabled are initialised to | a pre-determined reset state according to the KVM_ARM_VCPU_INIT ioctl, | irrespective of any prior configuration, with the following exceptions | for the primary vCPU: ? > > with the following exceptions for the primary > > +vCPU: > > + > > + =========== =========== > > + Register(s) Reset value > > + =========== =========== > > + X0-X14: Preserved (see KVM_SET_ONE_REG) > > + X15: Boot protocol version (0) > > What's the "Boot protocol" in this context? Is that just referring to > this handover state, or is that something more involved? It's just versioning this state, in case we have to change/extend it in future. If you prefer, I can just say: X15-X30 Reserved (0) instead? > > + X16-X30: Reserved (0) > > + PC: IPA base of firmware memory slot > > + SP: IPA end of firmware memory slot > > + =========== =========== > > + > > +Secondary vCPUs belonging to a VM with this capability enabled will return > > +-EPERM in response to a KVM_RUN ioctl() if the vCPU was not initialised with > > +the KVM_ARM_VCPU_POWER_OFF feature. > > I assume this means that protected VMs always get a trusted PSCI > implementation? It might be worth mentioning so (and worth consdiering > if that should always have the SMCCC bits too). "trusted PSCI implementation" might be stretching it, but the idea is that you can at least ensure that vCPUs won't start executing until the guest is ready for them. I don't think we need any particular SMCCC revision for that. > > + > > +There is no support for AArch32 at any exception level. > > Is this only going to run on CPUs without AArch32 EL0? ... or does this > mean behaviour will be erratic if someone tries to run AArch32 EL0? It means that we don't permit creation of 32-bit vCPUs, or exception return to an AArch32 guest from EL2. Of course, if the guest itself decides to try this and the hardware happens to support it then there's not a lot we can about it. But that's the guest being silly, so we don't need to care. > > diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h > > index 24223adae150..cdb3298ba8ae 100644 > > --- a/arch/arm64/include/uapi/asm/kvm.h > > +++ b/arch/arm64/include/uapi/asm/kvm.h > > @@ -402,6 +402,15 @@ struct kvm_vcpu_events { > > #define KVM_PSCI_RET_INVAL PSCI_RET_INVALID_PARAMS > > #define KVM_PSCI_RET_DENIED PSCI_RET_DENIED > > > > +/* Protected KVM */ > > +#define KVM_CAP_ARM_PROTECTED_VM_FLAGS_ENABLE 0 > > +#define KVM_CAP_ARM_PROTECTED_VM_FLAGS_INFO 1 > > + > > +struct kvm_protected_vm_info { > > + __u64 firmware_size; > > + __u64 __reserved[7]; > > +}; > > Do you have anything in mind for those 7 fields, or was this just a > number that sounded big enough? I just padded it to a cacheline, as that's plenty enough space to version the thing if we need to. > I wonder if it's worth adding an size field, and having a size argument > to the ioctl, so that you can discover the full size if we need to grow > this, but you can always get a truncated copy if you just need the early > fields. Possibly, but the rest of the KVM UAPI doesn't seem to be designed like that, so I followed what was there already. I defer to the KVM maintainers on this one... Will _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel