| Message ID | 20190930015740.84362-2-justin.he@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fix double page fault on arm64 | expand |
On Mon, Sep 30, 2019 at 09:57:38AM +0800, Jia He wrote: > We unconditionally set the HW_AFDBM capability and only enable it on > CPUs which really have the feature. But sometimes we need to know > whether this cpu has the capability of HW AF. So decouple AF from > DBM by new helper cpu_has_hw_af(). > > Signed-off-by: Jia He <justin.he@arm.com> > Suggested-by: Suzuki Poulose <Suzuki.Poulose@arm.com> > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > --- > arch/arm64/include/asm/cpufeature.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > index 9cde5d2e768f..949bc7c85030 100644 > --- a/arch/arm64/include/asm/cpufeature.h > +++ b/arch/arm64/include/asm/cpufeature.h > @@ -659,6 +659,16 @@ static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange) > default: return CONFIG_ARM64_PA_BITS; > } > } > + > +/* Check whether hardware update of the Access flag is supported */ > +static inline bool cpu_has_hw_af(void) > +{ > + if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM)) > + return read_cpuid(ID_AA64MMFR1_EL1) & 0xf; 0xf? I think we should have a mask in sysreg.h for this constant. Will
On Tue, 1 Oct 2019 13:54:47 +0100 Will Deacon <will@kernel.org> wrote: > On Mon, Sep 30, 2019 at 09:57:38AM +0800, Jia He wrote: > > We unconditionally set the HW_AFDBM capability and only enable it on > > CPUs which really have the feature. But sometimes we need to know > > whether this cpu has the capability of HW AF. So decouple AF from > > DBM by new helper cpu_has_hw_af(). > > > > Signed-off-by: Jia He <justin.he@arm.com> > > Suggested-by: Suzuki Poulose <Suzuki.Poulose@arm.com> > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > > --- > > arch/arm64/include/asm/cpufeature.h | 10 ++++++++++ > > 1 file changed, 10 insertions(+) > > > > diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h > > index 9cde5d2e768f..949bc7c85030 100644 > > --- a/arch/arm64/include/asm/cpufeature.h > > +++ b/arch/arm64/include/asm/cpufeature.h > > @@ -659,6 +659,16 @@ static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange) > > default: return CONFIG_ARM64_PA_BITS; > > } > > } > > + > > +/* Check whether hardware update of the Access flag is supported */ > > +static inline bool cpu_has_hw_af(void) > > +{ > > + if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM)) > > + return read_cpuid(ID_AA64MMFR1_EL1) & 0xf; > > 0xf? I think we should have a mask in sysreg.h for this constant. We don't have the mask, but we certainly have the shift. GENMASK(ID_AA64MMFR1_HADBS_SHIFT + 3, ID_AA64MMFR1_HADBS_SHIFT) is a bit of a mouthful though. Ideally, we'd have a helper for that. M.
Hi Will and Marc Sorry for the late response, just came back from a vacation. > -----Original Message----- > From: Marc Zyngier <maz@kernel.org> > Sent: 2019年10月1日 21:19 > To: Will Deacon <will@kernel.org> > Cc: Justin He (Arm Technology China) <Justin.He@arm.com>; Catalin > Marinas <Catalin.Marinas@arm.com>; Mark Rutland > <Mark.Rutland@arm.com>; James Morse <James.Morse@arm.com>; > Matthew Wilcox <willy@infradead.org>; Kirill A. Shutemov > <kirill.shutemov@linux.intel.com>; linux-arm-kernel@lists.infradead.org; > linux-kernel@vger.kernel.org; linux-mm@kvack.org; Punit Agrawal > <punitagrawal@gmail.com>; Thomas Gleixner <tglx@linutronix.de>; > Andrew Morton <akpm@linux-foundation.org>; hejianet@gmail.com; Kaly > Xin (Arm Technology China) <Kaly.Xin@arm.com> > Subject: Re: [PATCH v10 1/3] arm64: cpufeature: introduce helper > cpu_has_hw_af() > > On Tue, 1 Oct 2019 13:54:47 +0100 > Will Deacon <will@kernel.org> wrote: > > > On Mon, Sep 30, 2019 at 09:57:38AM +0800, Jia He wrote: > > > We unconditionally set the HW_AFDBM capability and only enable it on > > > CPUs which really have the feature. But sometimes we need to know > > > whether this cpu has the capability of HW AF. So decouple AF from > > > DBM by new helper cpu_has_hw_af(). > > > > > > Signed-off-by: Jia He <justin.he@arm.com> > > > Suggested-by: Suzuki Poulose <Suzuki.Poulose@arm.com> > > > Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> > > > --- > > > arch/arm64/include/asm/cpufeature.h | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/arch/arm64/include/asm/cpufeature.h > b/arch/arm64/include/asm/cpufeature.h > > > index 9cde5d2e768f..949bc7c85030 100644 > > > --- a/arch/arm64/include/asm/cpufeature.h > > > +++ b/arch/arm64/include/asm/cpufeature.h > > > @@ -659,6 +659,16 @@ static inline u32 > id_aa64mmfr0_parange_to_phys_shift(int parange) > > > default: return CONFIG_ARM64_PA_BITS; > > > } > > > } > > > + > > > +/* Check whether hardware update of the Access flag is supported */ > > > +static inline bool cpu_has_hw_af(void) > > > +{ > > > + if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM)) > > > + return read_cpuid(ID_AA64MMFR1_EL1) & 0xf; > > > > 0xf? I think we should have a mask in sysreg.h for this constant. > > We don't have the mask, but we certainly have the shift. > > GENMASK(ID_AA64MMFR1_HADBS_SHIFT + 3, > ID_AA64MMFR1_HADBS_SHIFT) is a bit > of a mouthful though. Ideally, we'd have a helper for that. > Ok, I will implement the helper if there isn't so far. And then replace the 0xf with it. -- Cheers, Justin (Jia He) > M. > -- > Without deviation from the norm, progress is not possible. IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On 08/10/2019 02:12, Justin He (Arm Technology China) wrote: > Hi Will and Marc > Sorry for the late response, just came back from a vacation. > >> -----Original Message----- >> From: Marc Zyngier <maz@kernel.org> >> Sent: 2019年10月1日 21:19 >> To: Will Deacon <will@kernel.org> >> Cc: Justin He (Arm Technology China) <Justin.He@arm.com>; Catalin >> Marinas <Catalin.Marinas@arm.com>; Mark Rutland >> <Mark.Rutland@arm.com>; James Morse <James.Morse@arm.com>; >> Matthew Wilcox <willy@infradead.org>; Kirill A. Shutemov >> <kirill.shutemov@linux.intel.com>; linux-arm-kernel@lists.infradead.org; >> linux-kernel@vger.kernel.org; linux-mm@kvack.org; Punit Agrawal >> <punitagrawal@gmail.com>; Thomas Gleixner <tglx@linutronix.de>; >> Andrew Morton <akpm@linux-foundation.org>; hejianet@gmail.com; Kaly >> Xin (Arm Technology China) <Kaly.Xin@arm.com> >> Subject: Re: [PATCH v10 1/3] arm64: cpufeature: introduce helper >> cpu_has_hw_af() >> >> On Tue, 1 Oct 2019 13:54:47 +0100 >> Will Deacon <will@kernel.org> wrote: >> >>> On Mon, Sep 30, 2019 at 09:57:38AM +0800, Jia He wrote: >>>> We unconditionally set the HW_AFDBM capability and only enable it on >>>> CPUs which really have the feature. But sometimes we need to know >>>> whether this cpu has the capability of HW AF. So decouple AF from >>>> DBM by new helper cpu_has_hw_af(). >>>> >>>> Signed-off-by: Jia He <justin.he@arm.com> >>>> Suggested-by: Suzuki Poulose <Suzuki.Poulose@arm.com> >>>> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> >>>> --- >>>> arch/arm64/include/asm/cpufeature.h | 10 ++++++++++ >>>> 1 file changed, 10 insertions(+) >>>> >>>> diff --git a/arch/arm64/include/asm/cpufeature.h >> b/arch/arm64/include/asm/cpufeature.h >>>> index 9cde5d2e768f..949bc7c85030 100644 >>>> --- a/arch/arm64/include/asm/cpufeature.h >>>> +++ b/arch/arm64/include/asm/cpufeature.h >>>> @@ -659,6 +659,16 @@ static inline u32 >> id_aa64mmfr0_parange_to_phys_shift(int parange) >>>> default: return CONFIG_ARM64_PA_BITS; >>>> } >>>> } >>>> + >>>> +/* Check whether hardware update of the Access flag is supported */ >>>> +static inline bool cpu_has_hw_af(void) >>>> +{ >>>> + if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM)) >>>> + return read_cpuid(ID_AA64MMFR1_EL1) & 0xf; >>> >>> 0xf? I think we should have a mask in sysreg.h for this constant. >> >> We don't have the mask, but we certainly have the shift. >> >> GENMASK(ID_AA64MMFR1_HADBS_SHIFT + 3, >> ID_AA64MMFR1_HADBS_SHIFT) is a bit >> of a mouthful though. Ideally, we'd have a helper for that. >> > Ok, I will implement the helper if there isn't so far. > And then replace the 0xf with it. Or could we simpl reuse existing cpuid_feature_extract_unsigned_field() ? u64 mmfr1 = read_cpuid(ID_AA64MMFR1_EL1); return cpuid_feature_extract_unsigned_field(mmfr1, ID_AA64MMFR1_HADBS_SHIFT) ? Cheers Suzuki
Hi Suzuki On 2019/10/8 23:32, Suzuki K Poulose wrote: > > > On 08/10/2019 02:12, Justin He (Arm Technology China) wrote: >> Hi Will and Marc >> Sorry for the late response, just came back from a vacation. >> >>> -----Original Message----- >>> From: Marc Zyngier <maz@kernel.org> >>> Sent: 2019年10月1日 21:19 >>> To: Will Deacon <will@kernel.org> >>> Cc: Justin He (Arm Technology China) <Justin.He@arm.com>; Catalin >>> Marinas <Catalin.Marinas@arm.com>; Mark Rutland >>> <Mark.Rutland@arm.com>; James Morse <James.Morse@arm.com>; >>> Matthew Wilcox <willy@infradead.org>; Kirill A. Shutemov >>> <kirill.shutemov@linux.intel.com>; linux-arm-kernel@lists.infradead.org; >>> linux-kernel@vger.kernel.org; linux-mm@kvack.org; Punit Agrawal >>> <punitagrawal@gmail.com>; Thomas Gleixner <tglx@linutronix.de>; >>> Andrew Morton <akpm@linux-foundation.org>; hejianet@gmail.com; Kaly >>> Xin (Arm Technology China) <Kaly.Xin@arm.com> >>> Subject: Re: [PATCH v10 1/3] arm64: cpufeature: introduce helper >>> cpu_has_hw_af() >>> >>> On Tue, 1 Oct 2019 13:54:47 +0100 >>> Will Deacon <will@kernel.org> wrote: >>> >>>> On Mon, Sep 30, 2019 at 09:57:38AM +0800, Jia He wrote: >>>>> We unconditionally set the HW_AFDBM capability and only enable it on >>>>> CPUs which really have the feature. But sometimes we need to know >>>>> whether this cpu has the capability of HW AF. So decouple AF from >>>>> DBM by new helper cpu_has_hw_af(). >>>>> >>>>> Signed-off-by: Jia He <justin.he@arm.com> >>>>> Suggested-by: Suzuki Poulose <Suzuki.Poulose@arm.com> >>>>> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com> >>>>> --- >>>>> arch/arm64/include/asm/cpufeature.h | 10 ++++++++++ >>>>> 1 file changed, 10 insertions(+) >>>>> >>>>> diff --git a/arch/arm64/include/asm/cpufeature.h >>> b/arch/arm64/include/asm/cpufeature.h >>>>> index 9cde5d2e768f..949bc7c85030 100644 >>>>> --- a/arch/arm64/include/asm/cpufeature.h >>>>> +++ b/arch/arm64/include/asm/cpufeature.h >>>>> @@ -659,6 +659,16 @@ static inline u32 >>> id_aa64mmfr0_parange_to_phys_shift(int parange) >>>>> default: return CONFIG_ARM64_PA_BITS; >>>>> } >>>>> } >>>>> + >>>>> +/* Check whether hardware update of the Access flag is supported */ >>>>> +static inline bool cpu_has_hw_af(void) >>>>> +{ >>>>> + if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM)) >>>>> + return read_cpuid(ID_AA64MMFR1_EL1) & 0xf; >>>> >>>> 0xf? I think we should have a mask in sysreg.h for this constant. >>> >>> We don't have the mask, but we certainly have the shift. >>> >>> GENMASK(ID_AA64MMFR1_HADBS_SHIFT + 3, >>> ID_AA64MMFR1_HADBS_SHIFT) is a bit >>> of a mouthful though. Ideally, we'd have a helper for that. >>> >> Ok, I will implement the helper if there isn't so far. >> And then replace the 0xf with it. > > Or could we simpl reuse existing cpuid_feature_extract_unsigned_field() ? > > u64 mmfr1 = read_cpuid(ID_AA64MMFR1_EL1); > > return cpuid_feature_extract_unsigned_field(mmfr1, ID_AA64MMFR1_HADBS_SHIFT) ? > Yes, we can, I will send the new version --- Cheers, Justin (Jia He)
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h index 9cde5d2e768f..949bc7c85030 100644 --- a/arch/arm64/include/asm/cpufeature.h +++ b/arch/arm64/include/asm/cpufeature.h @@ -659,6 +659,16 @@ static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange) default: return CONFIG_ARM64_PA_BITS; } } + +/* Check whether hardware update of the Access flag is supported */ +static inline bool cpu_has_hw_af(void) +{ + if (IS_ENABLED(CONFIG_ARM64_HW_AFDBM)) + return read_cpuid(ID_AA64MMFR1_EL1) & 0xf; + + return false; +} + #endif /* __ASSEMBLY__ */ #endif