From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 4C0221116; Wed, 1 Mar 2023 11:54:41 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 81E9F2F4; Wed, 1 Mar 2023 03:55:22 -0800 (PST) Received: from [10.57.16.41] (unknown [10.57.16.41]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1DA213F587; Wed, 1 Mar 2023 03:54:35 -0800 (PST) Message-ID: <8e803abd-8856-3c44-6262-40c026216c9a@arm.com> Date: Wed, 1 Mar 2023 11:54:34 +0000 Precedence: bulk X-Mailing-List: linux-coco@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [RFC PATCH 05/28] arm64: RME: Define the user ABI To: Zhi Wang Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, Catalin Marinas , Marc Zyngier , Will Deacon , James Morse , Oliver Upton , Suzuki K Poulose , Zenghui Yu , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Joey Gouly , Alexandru Elisei , Christoffer Dall , Fuad Tabba , linux-coco@lists.linux.dev References: <20230127112248.136810-1-suzuki.poulose@arm.com> <20230127112932.38045-1-steven.price@arm.com> <20230127112932.38045-6-steven.price@arm.com> <20230213180413.00000392@gmail.com> Content-Language: en-GB From: Steven Price In-Reply-To: <20230213180413.00000392@gmail.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 13/02/2023 16:04, Zhi Wang wrote: > On Fri, 27 Jan 2023 11:29:09 +0000 > Steven Price wrote: > >> There is one (multiplexed) CAP which can be used to create, populate and >> then activate the realm. >> >> Signed-off-by: Steven Price >> --- >> Documentation/virt/kvm/api.rst | 1 + >> arch/arm64/include/uapi/asm/kvm.h | 63 +++++++++++++++++++++++++++++++ >> include/uapi/linux/kvm.h | 2 + >> 3 files changed, 66 insertions(+) >> >> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst >> index 0dd5d8733dd5..f1a59d6fb7fc 100644 >> --- a/Documentation/virt/kvm/api.rst >> +++ b/Documentation/virt/kvm/api.rst >> @@ -4965,6 +4965,7 @@ Recognised values for feature: >> >> ===== =========================================== >> arm64 KVM_ARM_VCPU_SVE (requires KVM_CAP_ARM_SVE) >> + arm64 KVM_ARM_VCPU_REC (requires KVM_CAP_ARM_RME) >> ===== =========================================== >> >> Finalizes the configuration of the specified vcpu feature. >> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h >> index a7a857f1784d..fcc0b8dce29b 100644 >> --- a/arch/arm64/include/uapi/asm/kvm.h >> +++ b/arch/arm64/include/uapi/asm/kvm.h >> @@ -109,6 +109,7 @@ struct kvm_regs { >> #define KVM_ARM_VCPU_SVE 4 /* enable SVE for this CPU */ >> #define KVM_ARM_VCPU_PTRAUTH_ADDRESS 5 /* VCPU uses address authentication */ >> #define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */ >> +#define KVM_ARM_VCPU_REC 7 /* VCPU REC state as part of Realm */ >> >> struct kvm_vcpu_init { >> __u32 target; >> @@ -401,6 +402,68 @@ enum { >> #define KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES 3 >> #define KVM_DEV_ARM_ITS_CTRL_RESET 4 >> >> +/* KVM_CAP_ARM_RME on VM fd */ >> +#define KVM_CAP_ARM_RME_CONFIG_REALM 0 >> +#define KVM_CAP_ARM_RME_CREATE_RD 1 >> +#define KVM_CAP_ARM_RME_INIT_IPA_REALM 2 >> +#define KVM_CAP_ARM_RME_POPULATE_REALM 3 >> +#define KVM_CAP_ARM_RME_ACTIVATE_REALM 4 >> + > > It is a little bit confusing here. These seems more like 'commands' not caps. > Will leave more comments after reviewing the later patches. Sorry for the slow response. Thank you for your review - I hope to post a new version of this series (rebased on 6.3-rc1) in the coming weeks with your comments addressed. They are indeed commands - and using caps is a bit of a hack. The benefit here is that all the Realm commands are behind the one KVM_CAP_ARM_RME. The options I can see are: a) What I've got here - (ab)using KVM_ENABLE_CAP to perform commands. b) Add new ioctls for each of the above stages (so 5 new ioctls on top of the CAP for discovery). With any future extensions requiring new ioctls. c) Add a single new multiplexing ioctl (along with the CAP for discovery). I'm not massively keen on defining a new multiplexing scheme (c), but equally (b) seems like it's burning through ioctl numbers. Which led me to stick with (a) which at least keeps the rebasing simple (there's only the one CAP number which could conflict) and there's already a multiplexing scheme. But I'm happy to change if there's consensus a different approach would be preferable. Thanks, Steve >> +#define KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA256 0 >> +#define KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA512 1 >> + >> +#define KVM_CAP_ARM_RME_RPV_SIZE 64 >> + >> +/* List of configuration items accepted for KVM_CAP_ARM_RME_CONFIG_REALM */ >> +#define KVM_CAP_ARM_RME_CFG_RPV 0 >> +#define KVM_CAP_ARM_RME_CFG_HASH_ALGO 1 >> +#define KVM_CAP_ARM_RME_CFG_SVE 2 >> +#define KVM_CAP_ARM_RME_CFG_DBG 3 >> +#define KVM_CAP_ARM_RME_CFG_PMU 4 >> + >> +struct kvm_cap_arm_rme_config_item { >> + __u32 cfg; >> + union { >> + /* cfg == KVM_CAP_ARM_RME_CFG_RPV */ >> + struct { >> + __u8 rpv[KVM_CAP_ARM_RME_RPV_SIZE]; >> + }; >> + >> + /* cfg == KVM_CAP_ARM_RME_CFG_HASH_ALGO */ >> + struct { >> + __u32 hash_algo; >> + }; >> + >> + /* cfg == KVM_CAP_ARM_RME_CFG_SVE */ >> + struct { >> + __u32 sve_vq; >> + }; >> + >> + /* cfg == KVM_CAP_ARM_RME_CFG_DBG */ >> + struct { >> + __u32 num_brps; >> + __u32 num_wrps; >> + }; >> + >> + /* cfg == KVM_CAP_ARM_RME_CFG_PMU */ >> + struct { >> + __u32 num_pmu_cntrs; >> + }; >> + /* Fix the size of the union */ >> + __u8 reserved[256]; >> + }; >> +}; >> + >> +struct kvm_cap_arm_rme_populate_realm_args { >> + __u64 populate_ipa_base; >> + __u64 populate_ipa_size; >> +}; >> + >> +struct kvm_cap_arm_rme_init_ipa_args { >> + __u64 init_ipa_base; >> + __u64 init_ipa_size; >> +}; >> + >> /* Device Control API on vcpu fd */ >> #define KVM_ARM_VCPU_PMU_V3_CTRL 0 >> #define KVM_ARM_VCPU_PMU_V3_IRQ 0 >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >> index 20522d4ba1e0..fec1909e8b73 100644 >> --- a/include/uapi/linux/kvm.h >> +++ b/include/uapi/linux/kvm.h >> @@ -1176,6 +1176,8 @@ struct kvm_ppc_resize_hpt { >> #define KVM_CAP_S390_PROTECTED_ASYNC_DISABLE 224 >> #define KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP 225 >> >> +#define KVM_CAP_ARM_RME 300 // FIXME: Large number to prevent conflicts >> + >> #ifdef KVM_CAP_IRQ_ROUTING >> >> struct kvm_irq_routing_irqchip { > 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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 9FE8EC7EE2D for ; Wed, 1 Mar 2023 11:56:03 +0000 (UTC) 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:From:References:Cc:To: Subject:MIME-Version:Date:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=cuC8OTGafQ95bpNSwHvtToijbSnAC3PrP2IgkbGdwmo=; b=k7CRQF4oEHaccS b5hGBGvJ6DvzBLp3igej37kqAobcuvCvRd9QGTVseVgdFV3IiCGMjPq7GMNC2J2dZlYa51ZED4CcT 2sAM55JK3puXyUjGimVcLEaHRErlvqaR/SLxaJmLv/EDKF3aUna6ZDMaKN4uw2TNyBOX9okUVgj7V 2b/Sr0RqL+89OgHfUhHFKRXxOatjqe0IBPZP46G4yQGPRcaYCmZNTPNknE+fv8hIwFkMGW+bdqAmq RvTjri/DhdMG6dY5cmWZ5Q1xlQ5cZu4WdxZvzQ9cWMu1qWI9DOOHEV3VcWgErSR3jCidWi9TCZzTk PItSeSsGnDmYyB/1XiAQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pXL2l-00G0vR-NY; Wed, 01 Mar 2023 11:54:47 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1pXL2h-00G0uL-9a for linux-arm-kernel@lists.infradead.org; Wed, 01 Mar 2023 11:54:45 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 81E9F2F4; Wed, 1 Mar 2023 03:55:22 -0800 (PST) Received: from [10.57.16.41] (unknown [10.57.16.41]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 1DA213F587; Wed, 1 Mar 2023 03:54:35 -0800 (PST) Message-ID: <8e803abd-8856-3c44-6262-40c026216c9a@arm.com> Date: Wed, 1 Mar 2023 11:54:34 +0000 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.7.1 Subject: Re: [RFC PATCH 05/28] arm64: RME: Define the user ABI To: Zhi Wang Cc: kvm@vger.kernel.org, kvmarm@lists.linux.dev, Catalin Marinas , Marc Zyngier , Will Deacon , James Morse , Oliver Upton , Suzuki K Poulose , Zenghui Yu , linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, Joey Gouly , Alexandru Elisei , Christoffer Dall , Fuad Tabba , linux-coco@lists.linux.dev References: <20230127112248.136810-1-suzuki.poulose@arm.com> <20230127112932.38045-1-steven.price@arm.com> <20230127112932.38045-6-steven.price@arm.com> <20230213180413.00000392@gmail.com> Content-Language: en-GB From: Steven Price In-Reply-To: <20230213180413.00000392@gmail.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20230301_035443_480225_3B5D71BB X-CRM114-Status: GOOD ( 25.69 ) 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 13/02/2023 16:04, Zhi Wang wrote: > On Fri, 27 Jan 2023 11:29:09 +0000 > Steven Price wrote: > >> There is one (multiplexed) CAP which can be used to create, populate and >> then activate the realm. >> >> Signed-off-by: Steven Price >> --- >> Documentation/virt/kvm/api.rst | 1 + >> arch/arm64/include/uapi/asm/kvm.h | 63 +++++++++++++++++++++++++++++++ >> include/uapi/linux/kvm.h | 2 + >> 3 files changed, 66 insertions(+) >> >> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst >> index 0dd5d8733dd5..f1a59d6fb7fc 100644 >> --- a/Documentation/virt/kvm/api.rst >> +++ b/Documentation/virt/kvm/api.rst >> @@ -4965,6 +4965,7 @@ Recognised values for feature: >> >> ===== =========================================== >> arm64 KVM_ARM_VCPU_SVE (requires KVM_CAP_ARM_SVE) >> + arm64 KVM_ARM_VCPU_REC (requires KVM_CAP_ARM_RME) >> ===== =========================================== >> >> Finalizes the configuration of the specified vcpu feature. >> diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h >> index a7a857f1784d..fcc0b8dce29b 100644 >> --- a/arch/arm64/include/uapi/asm/kvm.h >> +++ b/arch/arm64/include/uapi/asm/kvm.h >> @@ -109,6 +109,7 @@ struct kvm_regs { >> #define KVM_ARM_VCPU_SVE 4 /* enable SVE for this CPU */ >> #define KVM_ARM_VCPU_PTRAUTH_ADDRESS 5 /* VCPU uses address authentication */ >> #define KVM_ARM_VCPU_PTRAUTH_GENERIC 6 /* VCPU uses generic authentication */ >> +#define KVM_ARM_VCPU_REC 7 /* VCPU REC state as part of Realm */ >> >> struct kvm_vcpu_init { >> __u32 target; >> @@ -401,6 +402,68 @@ enum { >> #define KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES 3 >> #define KVM_DEV_ARM_ITS_CTRL_RESET 4 >> >> +/* KVM_CAP_ARM_RME on VM fd */ >> +#define KVM_CAP_ARM_RME_CONFIG_REALM 0 >> +#define KVM_CAP_ARM_RME_CREATE_RD 1 >> +#define KVM_CAP_ARM_RME_INIT_IPA_REALM 2 >> +#define KVM_CAP_ARM_RME_POPULATE_REALM 3 >> +#define KVM_CAP_ARM_RME_ACTIVATE_REALM 4 >> + > > It is a little bit confusing here. These seems more like 'commands' not caps. > Will leave more comments after reviewing the later patches. Sorry for the slow response. Thank you for your review - I hope to post a new version of this series (rebased on 6.3-rc1) in the coming weeks with your comments addressed. They are indeed commands - and using caps is a bit of a hack. The benefit here is that all the Realm commands are behind the one KVM_CAP_ARM_RME. The options I can see are: a) What I've got here - (ab)using KVM_ENABLE_CAP to perform commands. b) Add new ioctls for each of the above stages (so 5 new ioctls on top of the CAP for discovery). With any future extensions requiring new ioctls. c) Add a single new multiplexing ioctl (along with the CAP for discovery). I'm not massively keen on defining a new multiplexing scheme (c), but equally (b) seems like it's burning through ioctl numbers. Which led me to stick with (a) which at least keeps the rebasing simple (there's only the one CAP number which could conflict) and there's already a multiplexing scheme. But I'm happy to change if there's consensus a different approach would be preferable. Thanks, Steve >> +#define KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA256 0 >> +#define KVM_CAP_ARM_RME_MEASUREMENT_ALGO_SHA512 1 >> + >> +#define KVM_CAP_ARM_RME_RPV_SIZE 64 >> + >> +/* List of configuration items accepted for KVM_CAP_ARM_RME_CONFIG_REALM */ >> +#define KVM_CAP_ARM_RME_CFG_RPV 0 >> +#define KVM_CAP_ARM_RME_CFG_HASH_ALGO 1 >> +#define KVM_CAP_ARM_RME_CFG_SVE 2 >> +#define KVM_CAP_ARM_RME_CFG_DBG 3 >> +#define KVM_CAP_ARM_RME_CFG_PMU 4 >> + >> +struct kvm_cap_arm_rme_config_item { >> + __u32 cfg; >> + union { >> + /* cfg == KVM_CAP_ARM_RME_CFG_RPV */ >> + struct { >> + __u8 rpv[KVM_CAP_ARM_RME_RPV_SIZE]; >> + }; >> + >> + /* cfg == KVM_CAP_ARM_RME_CFG_HASH_ALGO */ >> + struct { >> + __u32 hash_algo; >> + }; >> + >> + /* cfg == KVM_CAP_ARM_RME_CFG_SVE */ >> + struct { >> + __u32 sve_vq; >> + }; >> + >> + /* cfg == KVM_CAP_ARM_RME_CFG_DBG */ >> + struct { >> + __u32 num_brps; >> + __u32 num_wrps; >> + }; >> + >> + /* cfg == KVM_CAP_ARM_RME_CFG_PMU */ >> + struct { >> + __u32 num_pmu_cntrs; >> + }; >> + /* Fix the size of the union */ >> + __u8 reserved[256]; >> + }; >> +}; >> + >> +struct kvm_cap_arm_rme_populate_realm_args { >> + __u64 populate_ipa_base; >> + __u64 populate_ipa_size; >> +}; >> + >> +struct kvm_cap_arm_rme_init_ipa_args { >> + __u64 init_ipa_base; >> + __u64 init_ipa_size; >> +}; >> + >> /* Device Control API on vcpu fd */ >> #define KVM_ARM_VCPU_PMU_V3_CTRL 0 >> #define KVM_ARM_VCPU_PMU_V3_IRQ 0 >> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h >> index 20522d4ba1e0..fec1909e8b73 100644 >> --- a/include/uapi/linux/kvm.h >> +++ b/include/uapi/linux/kvm.h >> @@ -1176,6 +1176,8 @@ struct kvm_ppc_resize_hpt { >> #define KVM_CAP_S390_PROTECTED_ASYNC_DISABLE 224 >> #define KVM_CAP_DIRTY_LOG_RING_WITH_BITMAP 225 >> >> +#define KVM_CAP_ARM_RME 300 // FIXME: Large number to prevent conflicts >> + >> #ifdef KVM_CAP_IRQ_ROUTING >> >> struct kvm_irq_routing_irqchip { > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel