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 25A74C4167B for ; Wed, 29 Nov 2023 15:11:36 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id B63436B03BB; Wed, 29 Nov 2023 10:11:35 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id B13CF6B03BF; Wed, 29 Nov 2023 10:11:35 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 9DBBD6B03C2; Wed, 29 Nov 2023 10:11:35 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 8D9976B03BB for ; Wed, 29 Nov 2023 10:11:35 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 64BD01A043D for ; Wed, 29 Nov 2023 15:11:35 +0000 (UTC) X-FDA: 81511330950.12.22AA93B Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by imf18.hostedemail.com (Postfix) with ESMTP id 9DB461C0028 for ; Wed, 29 Nov 2023 15:11:32 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=none; spf=pass (imf18.hostedemail.com: domain of joey.gouly@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=joey.gouly@arm.com; dmarc=pass (policy=none) header.from=arm.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701270693; 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: in-reply-to:in-reply-to:references:references; bh=+4xBMSnbEhsQ4XCZoi8iJgHOq3o54X61jwIHg0eOrOU=; b=5Oz/uQGGjQldcbMjdaY9YdiBtTGVt0OLvYYnDVyKAjwBY2HXFJ4gj2poOWcvoUofhdj3+m 6Ng0c2BX9GMUmfGRa+XK6x6rUJfXRih7E5ZDTvvg0sTtpGKWk1hP/w12CA8PdAASqghmsc +rEV1vNyHRTXAmZhriM+iH5iXOpfSz4= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701270693; a=rsa-sha256; cv=none; b=6/HYVd8yjswaKvzLYcgWAfBeE1b2/zpAbgAbVZpBbx/4ldxQ3/YxRZTGtX9/xOfTNrzGct As6H7TOImA5h4clVwMW3UFiD5Oe9ShgEL0leKqcn89UwHK/n7shwzJg1S+tHE4Lu3Tynk4 k9IoL8BTYb0nwTVK+GDeGrkXL9zxnus= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=none; spf=pass (imf18.hostedemail.com: domain of joey.gouly@arm.com designates 217.140.110.172 as permitted sender) smtp.mailfrom=joey.gouly@arm.com; dmarc=pass (policy=none) header.from=arm.com 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 8BCDA2F4; Wed, 29 Nov 2023 07:12:18 -0800 (PST) Received: from e124191.cambridge.arm.com (e124191.cambridge.arm.com [10.1.197.45]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 56CC33F73F; Wed, 29 Nov 2023 07:11:29 -0800 (PST) Date: Wed, 29 Nov 2023 15:11:23 +0000 From: Joey Gouly To: Marc Zyngier Cc: linux-arm-kernel@lists.infradead.org, akpm@linux-foundation.org, aneesh.kumar@linux.ibm.com, broonie@kernel.org, catalin.marinas@arm.com, dave.hansen@linux.intel.com, oliver.upton@linux.dev, shuah@kernel.org, will@kernel.org, kvmarm@lists.linux.dev, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, James Morse , Suzuki K Poulose , Zenghui Yu Subject: Re: [PATCH v3 06/25] KVM: arm64: Save/restore POE registers Message-ID: <20231129151123.GA2423241@e124191.cambridge.arm.com> References: <20231124163510.1835740-1-joey.gouly@arm.com> <20231124163510.1835740-7-joey.gouly@arm.com> <86bkbfcbyp.wl-maz@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <86bkbfcbyp.wl-maz@kernel.org> X-Stat-Signature: wqcttmeocj9hzth438jka5r1j7f9ygfz X-Rspamd-Server: rspam10 X-Rspamd-Queue-Id: 9DB461C0028 X-Rspam-User: X-HE-Tag: 1701270692-48702 X-HE-Meta: U2FsdGVkX18uM7D5cgi+6m7JvfdGjwny7dUNljCouKGYr9xT4FeMr7+f4ucKasYrB/LifaqkR19vKszdmmWZGMo0ehWddDh4WerrC2hK8crq6v/S1e6BovpT6GJc03Tj5V05utale8dNFjq43AqnGaXQKyb+lMhcubV656Vq38+xhC0zRttzdtaOvnT4xmdBIy0ofRUV7jgQKkFfMggAp1ku7E66UaOPj/FoI8JBgx35szRUnLAmKsBdJXCFlcylSVBGhUXjM1n/n3vc41M9qLixeBZ/83/ebzZ26T7YQnFIghw+caj2U6ly3/dhUl6o51WMzYcxHGkAISKXZBSCdvAVNc9qSDX+pIdwcxFtxX6ve7bxlfLrew/0yTCQTuTr15mjNM5aWhwf0pYfwF1HekZ2DIr/2xoN3/t6oog9QHlXp6DJY/6O2x6LcEFUmFIhfKrODYe+epoupTS0diDi4w8wGboTZ1p5Y6XSXotNuxRUPgGeoCp30kk/JR7aFEf2UvPItlHpkxPQowbzTRlzW2EiPzt17++Kwcl4QNYRCRHtkioR64m6S/rofxpwN2LoDRUZFRTOAtffrQad70acVBKm9PapYojkcBsUx0khYLu+/XR7GVtv0R2oY8xWQtjgKxkdwfjpMZ+aZKf4/KOUNKiuNegVahQELtK458KhRoo3qks8jIVo56RPxy5JmKruuJgB4GhHbwtTW4cfZskTpY+1HyXWvTX3teDwqV/uN3ARPAXVPf3dCQjZ3OdLsqYEDn6gWyicBxt6zcP8ygPSZ9oJrDCDJ/vc/iDxJXUFLuI2CMgPkZ+DK3wYfCB1s8P5A3tR4jBWXWnkA7gaOmuuLCWi4ZqLUCS0B22OJsU0+CuETOb+LzjXVcjFBwzgzTrr4EGo3Nbjor8LajokPKdW1ECfnY4RdFyY+Q895gLTs1G3YGhjBhBj07OiP4bOWYVsgZi7GW38D1jhdgL1dzF lDt5kOrP TRDZ/U/BtxheTC+03A0yH12jw7fdfkiXUZGIiwf9Te9nj+6ERN8kEFyqDOFtSbn3P1VvcB7NUt/qSFCIZcHeboMvq+Uhv3aBfvtcL05vBcK7ZOLMZA9cEHLxwq9Ckfk1RlXzaceBmRnRTsCvPI5twA0v1qe2WV05uuLApYgfl/JYqQCd1HDK6D1x8MJzeZ3ZRdaj4GshILgehrMp6EDTQuRPCoOx/qCQCn6H1U9ZzSxuMoWc= 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: Hi Marc, Thanks for taking a look. On Mon, Nov 27, 2023 at 06:01:18PM +0000, Marc Zyngier wrote: > On Fri, 24 Nov 2023 16:34:51 +0000, > Joey Gouly wrote: > > > > Define the new system registers that POE introduces and context switch them. > > I would really like to see a discussion on the respective lifetimes of > these two registers (see below). > > > > > Signed-off-by: Joey Gouly > > Cc: Marc Zyngier > > Cc: Oliver Upton > > Cc: Catalin Marinas > > Cc: Will Deacon > > --- > > arch/arm64/include/asm/kvm_arm.h | 4 ++-- > > arch/arm64/include/asm/kvm_host.h | 4 ++++ > > arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 10 ++++++++++ > > arch/arm64/kvm/sys_regs.c | 2 ++ > > 4 files changed, 18 insertions(+), 2 deletions(-) > > > > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h > > index b85f46a73e21..597470e0b87b 100644 > > --- a/arch/arm64/include/asm/kvm_arm.h > > +++ b/arch/arm64/include/asm/kvm_arm.h > > @@ -346,14 +346,14 @@ > > */ > > #define __HFGRTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51)) > > #define __HFGRTR_EL2_MASK GENMASK(49, 0) > > -#define __HFGRTR_EL2_nMASK (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50)) > > +#define __HFGRTR_EL2_nMASK (GENMASK(60, 57) | GENMASK(55, 54) | BIT(50)) > > > > #define __HFGWTR_EL2_RES0 (GENMASK(63, 56) | GENMASK(53, 51) | \ > > BIT(46) | BIT(42) | BIT(40) | BIT(28) | \ > > GENMASK(26, 25) | BIT(21) | BIT(18) | \ > > GENMASK(15, 14) | GENMASK(10, 9) | BIT(2)) > > #define __HFGWTR_EL2_MASK GENMASK(49, 0) > > -#define __HFGWTR_EL2_nMASK (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50)) > > +#define __HFGWTR_EL2_nMASK (GENMASK(60, 57) | GENMASK(55, 54) | BIT(50)) > > > > #define __HFGITR_EL2_RES0 GENMASK(63, 57) > > #define __HFGITR_EL2_MASK GENMASK(54, 0) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > > index 824f29f04916..fa9ebd8fce40 100644 > > --- a/arch/arm64/include/asm/kvm_host.h > > +++ b/arch/arm64/include/asm/kvm_host.h > > @@ -401,6 +401,10 @@ enum vcpu_sysreg { > > PIR_EL1, /* Permission Indirection Register 1 (EL1) */ > > PIRE0_EL1, /* Permission Indirection Register 0 (EL1) */ > > > > + /* Permission Overlay Extension registers */ > > + POR_EL1, /* Permission Overlay Register 1 (EL1) */ > > + POR_EL0, /* Permission Overlay Register 0 (EL0) */ > > + > > /* 32bit specific registers. */ > > DACR32_EL2, /* Domain Access Control Register */ > > IFSR32_EL2, /* Instruction Fault Status Register */ > > diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > > index bb6b571ec627..22f07ee43e7e 100644 > > --- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > > +++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h > > @@ -19,6 +19,9 @@ > > static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt) > > { > > ctxt_sys_reg(ctxt, MDSCR_EL1) = read_sysreg(mdscr_el1); > > + > > + if (system_supports_poe()) > > + ctxt_sys_reg(ctxt, POR_EL0) = read_sysreg_s(SYS_POR_EL0); > > So this is saved as eagerly as it gets. Why? If it only affects EL0, > it can be saved/restored in a much lazier way. Just to confirm I understand what you mean, the current code looks like: vcpu_load() // Lazy save while (ret > 0) check_vcpu_requests() kvm_arm_vcpu_enter_exit() // Eager save/restore ret = handle_exit() vcpu_put() // Lazy restore POR_EL0 does affect EL2, if it does some form of {get,put}_user. This happens in vgic_its_process_commands (as part of handle_exit), also the stolen time code (in check_vcpu_requests) and could possibly happen if perf tries to walk the user stack. So I think that it does need to happen eagerly, such that the host-userspace's POR_EL0 is used to access the VM's memory, not the guest-userspace's POR_EL0. Does that make sense? It will need a comment, I agree. > > > } > > > > static inline void __sysreg_save_user_state(struct kvm_cpu_context *ctxt) > > @@ -59,6 +62,8 @@ static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt) > > ctxt_sys_reg(ctxt, PIR_EL1) = read_sysreg_el1(SYS_PIR); > > ctxt_sys_reg(ctxt, PIRE0_EL1) = read_sysreg_el1(SYS_PIRE0); > > And the fact that you only touch PIRE0_EL1 here seems to be a good > indication that the above can be relaxed. PIREO_EL1 is not directly accessible from EL0. I'll have a think about this a bit more, and if there is a potential similar issue here. > > > } > > + if (system_supports_poe()) > > nit: missing new line before the if(). > > > + ctxt_sys_reg(ctxt, POR_EL1) = read_sysreg_el1(SYS_POR); > > ctxt_sys_reg(ctxt, PAR_EL1) = read_sysreg_par(); > > ctxt_sys_reg(ctxt, TPIDR_EL1) = read_sysreg(tpidr_el1); > > > > @@ -89,6 +94,9 @@ static inline void __sysreg_save_el2_return_state(struct kvm_cpu_context *ctxt) > > static inline void __sysreg_restore_common_state(struct kvm_cpu_context *ctxt) > > { > > write_sysreg(ctxt_sys_reg(ctxt, MDSCR_EL1), mdscr_el1); > > + > > + if (system_supports_poe()) > > + write_sysreg_s(ctxt_sys_reg(ctxt, POR_EL0), SYS_POR_EL0); > > Same thing here about the eager restore. > > > } > > > > static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt) > > @@ -135,6 +143,8 @@ static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt) > > write_sysreg_el1(ctxt_sys_reg(ctxt, PIR_EL1), SYS_PIR); > > write_sysreg_el1(ctxt_sys_reg(ctxt, PIRE0_EL1), SYS_PIRE0); > > } > > + if (system_supports_poe()) > > new line. > > > + write_sysreg_el1(ctxt_sys_reg(ctxt, POR_EL1), SYS_POR); > > write_sysreg(ctxt_sys_reg(ctxt, PAR_EL1), par_el1); > > write_sysreg(ctxt_sys_reg(ctxt, TPIDR_EL1), tpidr_el1); > > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > > index 4735e1b37fb3..a54e5eadbf29 100644 > > --- a/arch/arm64/kvm/sys_regs.c > > +++ b/arch/arm64/kvm/sys_regs.c > > @@ -2269,6 +2269,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > { SYS_DESC(SYS_MAIR_EL1), access_vm_reg, reset_unknown, MAIR_EL1 }, > > { SYS_DESC(SYS_PIRE0_EL1), NULL, reset_unknown, PIRE0_EL1 }, > > { SYS_DESC(SYS_PIR_EL1), NULL, reset_unknown, PIR_EL1 }, > > + { SYS_DESC(SYS_POR_EL1), NULL, reset_unknown, POR_EL1 }, > > { SYS_DESC(SYS_AMAIR_EL1), access_vm_reg, reset_amair_el1, AMAIR_EL1 }, > > > > { SYS_DESC(SYS_LORSA_EL1), trap_loregion }, > > @@ -2352,6 +2353,7 @@ static const struct sys_reg_desc sys_reg_descs[] = { > > .access = access_pmovs, .reg = PMOVSSET_EL0, > > .get_user = get_pmreg, .set_user = set_pmreg }, > > > > + { SYS_DESC(SYS_POR_EL0), NULL, reset_unknown, POR_EL0 }, > > { SYS_DESC(SYS_TPIDR_EL0), NULL, reset_unknown, TPIDR_EL0 }, > > { SYS_DESC(SYS_TPIDRRO_EL0), NULL, reset_unknown, TPIDRRO_EL0 }, > > { SYS_DESC(SYS_TPIDR2_EL0), undef_access }, > > Another thing that is missing is the trap routing for NV in > emulated-nested.c. Please fill in the various tables there. > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible. Thanks, Joey