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=-15.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, 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 20910C433E1 for ; Mon, 22 Mar 2021 10:31:35 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id CCCE5619A7 for ; Mon, 22 Mar 2021 10:31:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230294AbhCVKbG (ORCPT ); Mon, 22 Mar 2021 06:31:06 -0400 Received: from foss.arm.com ([217.140.110.172]:57088 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230046AbhCVKay (ORCPT ); Mon, 22 Mar 2021 06:30:54 -0400 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 190FA1063; Mon, 22 Mar 2021 03:30:54 -0700 (PDT) Received: from [192.168.0.110] (unknown [172.31.20.19]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 934DB3F718; Mon, 22 Mar 2021 03:30:53 -0700 (PDT) Subject: Re: [kvm-unit-tests PATCH 2/4] arm/arm64: Read system registers to get the state of the MMU To: Nikos Nikoleris , kvm@vger.kernel.org Cc: drjones@redhat.com References: <20210319122414.129364-1-nikos.nikoleris@arm.com> <20210319122414.129364-3-nikos.nikoleris@arm.com> From: Alexandru Elisei Message-ID: Date: Mon, 22 Mar 2021 10:30:43 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1 MIME-Version: 1.0 In-Reply-To: <20210319122414.129364-3-nikos.nikoleris@arm.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Content-Language: en-US Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Hi Nikos, On 3/19/21 12:24 PM, Nikos Nikoleris wrote: > When we are in EL1 we can directly tell if the local cpu's MMU is on > by reading a system register (SCTRL/SCTRL_EL1). In EL0, we use the > relevant cpumask. This way we don't have to rely on the cpu id in > thread_info when we are in setup executing in EL1. In subsequent > patches we resolve the problem of identifying safely whether we are > executing in EL1 or EL0. The commit message doesn't explain why mmu_disabled_cpu_count has been removed. It also doesn't explain why the disabled cpumask has been replaced with an enabled cpumask. Other than that, it is a good idea to stop mmu_enabled() from checking the cpumask because marking the MMU as enabled/disabled requires modifying the cpumask, which calls mmu_enabled(). That's not a problem at EL0 because EL0 cannot turn on or off the MMU. Thanks, Alex > > Signed-off-by: Nikos Nikoleris > --- > lib/arm/asm/mmu-api.h | 7 +------ > lib/arm/asm/processor.h | 7 +++++++ > lib/arm64/asm/processor.h | 1 + > lib/arm/mmu.c | 16 ++++++++-------- > lib/arm/processor.c | 5 +++++ > lib/arm64/processor.c | 5 +++++ > 6 files changed, 27 insertions(+), 14 deletions(-) > > diff --git a/lib/arm/asm/mmu-api.h b/lib/arm/asm/mmu-api.h > index 3d04d03..05fc12b 100644 > --- a/lib/arm/asm/mmu-api.h > +++ b/lib/arm/asm/mmu-api.h > @@ -5,12 +5,7 @@ > #include > > extern pgd_t *mmu_idmap; > -extern unsigned int mmu_disabled_cpu_count; > -extern bool __mmu_enabled(void); > -static inline bool mmu_enabled(void) > -{ > - return mmu_disabled_cpu_count == 0 || __mmu_enabled(); > -} > +extern bool mmu_enabled(void); > extern void mmu_mark_enabled(int cpu); > extern void mmu_mark_disabled(int cpu); > extern void mmu_enable(pgd_t *pgtable); > diff --git a/lib/arm/asm/processor.h b/lib/arm/asm/processor.h > index 273366d..af54c09 100644 > --- a/lib/arm/asm/processor.h > +++ b/lib/arm/asm/processor.h > @@ -67,11 +67,13 @@ extern int mpidr_to_cpu(uint64_t mpidr); > ((mpidr >> MPIDR_LEVEL_SHIFT(level)) & 0xff) > > extern void start_usr(void (*func)(void *arg), void *arg, unsigned long sp_usr); > +extern bool __mmu_enabled(void); > extern bool is_user(void); > > #define CNTVCT __ACCESS_CP15_64(1, c14) > #define CNTFRQ __ACCESS_CP15(c14, 0, c0, 0) > #define CTR __ACCESS_CP15(c0, 0, c0, 1) > +#define SCTRL __ACCESS_CP15(c1, 0, c0, 0) > > static inline u64 get_cntvct(void) > { > @@ -89,6 +91,11 @@ static inline u32 get_ctr(void) > return read_sysreg(CTR); > } > > +static inline u32 get_sctrl(void) > +{ > + return read_sysreg(SCTRL); > +} > + > extern unsigned long dcache_line_size; > > #endif /* _ASMARM_PROCESSOR_H_ */ > diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h > index 771b2d1..8e2037e 100644 > --- a/lib/arm64/asm/processor.h > +++ b/lib/arm64/asm/processor.h > @@ -98,6 +98,7 @@ extern int mpidr_to_cpu(uint64_t mpidr); > > extern void start_usr(void (*func)(void *arg), void *arg, unsigned long sp_usr); > extern bool is_user(void); > +extern bool __mmu_enabled(void); > > static inline u64 get_cntvct(void) > { > diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c > index a1862a5..d806c63 100644 > --- a/lib/arm/mmu.c > +++ b/lib/arm/mmu.c > @@ -24,10 +24,9 @@ extern unsigned long etext; > pgd_t *mmu_idmap; > > /* CPU 0 starts with disabled MMU */ > -static cpumask_t mmu_disabled_cpumask = { {1} }; > -unsigned int mmu_disabled_cpu_count = 1; > +static cpumask_t mmu_enabled_cpumask = { {0} }; > > -bool __mmu_enabled(void) > +bool mmu_enabled(void) > { > int cpu = current_thread_info()->cpu; > > @@ -38,19 +37,20 @@ bool __mmu_enabled(void) > * spinlock, atomic bitop, etc., otherwise we'll recurse. > * [cpumask_]test_bit is safe though. > */ > - return !cpumask_test_cpu(cpu, &mmu_disabled_cpumask); > + if (is_user()) > + return cpumask_test_cpu(cpu, &mmu_enabled_cpumask); > + else > + return __mmu_enabled(); > } > > void mmu_mark_enabled(int cpu) > { > - if (cpumask_test_and_clear_cpu(cpu, &mmu_disabled_cpumask)) > - --mmu_disabled_cpu_count; > + cpumask_set_cpu(cpu, &mmu_enabled_cpumask); > } > > void mmu_mark_disabled(int cpu) > { > - if (!cpumask_test_and_set_cpu(cpu, &mmu_disabled_cpumask)) > - ++mmu_disabled_cpu_count; > + cpumask_clear_cpu(cpu, &mmu_enabled_cpumask); > } > > extern void asm_mmu_enable(phys_addr_t pgtable); > diff --git a/lib/arm/processor.c b/lib/arm/processor.c > index 773337e..a2d39a4 100644 > --- a/lib/arm/processor.c > +++ b/lib/arm/processor.c > @@ -145,3 +145,8 @@ bool is_user(void) > { > return current_thread_info()->flags & TIF_USER_MODE; > } > + > +bool __mmu_enabled(void) > +{ > + return get_sctrl() & CR_M; > +} > diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c > index 2a024e3..ef55862 100644 > --- a/lib/arm64/processor.c > +++ b/lib/arm64/processor.c > @@ -257,3 +257,8 @@ bool is_user(void) > { > return current_thread_info()->flags & TIF_USER_MODE; > } > + > +bool __mmu_enabled(void) > +{ > + return read_sysreg(sctlr_el1) & SCTLR_EL1_M; > +}