* Re: [PATCH v5 1/5] powerpc/sstep: Emulate prefixed instructions only when CPU_FTR_ARCH_31 is set
From: Ravi Bangoria @ 2020-10-12 11:07 UTC (permalink / raw)
To: Daniel Axtens
Cc: Ravi Bangoria, bala24, paulus, sandipan, jniethe5, naveen.n.rao,
linuxppc-dev
In-Reply-To: <87h7r0w6s0.fsf@dja-thinkpad.axtens.net>
Hi Daniel,
On 10/12/20 7:21 AM, Daniel Axtens wrote:
> Hi,
>
> Apologies if this has come up in a previous revision.
>
>
>> case 1:
>> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
>> + return -1;
>> +
>> prefix_r = GET_PREFIX_R(word);
>> ra = GET_PREFIX_RA(suffix);
>
> The comment above analyse_instr reads in part:
>
> * Return value is 1 if the instruction can be emulated just by
> * updating *regs with the information in *op, -1 if we need the
> * GPRs but *regs doesn't contain the full register set, or 0
> * otherwise.
>
> I was wondering why returning -1 if the instruction isn't supported the
> right thing to do - it seemed to me that it should return 0?
>
> I did look and see that there are other cases where the code returns -1
> for an unsupported operation, e.g.:
>
> #ifdef __powerpc64__
> case 4:
> if (!cpu_has_feature(CPU_FTR_ARCH_300))
> return -1;
>
> switch (word & 0x3f) {
> case 48: /* maddhd */
>
> That's from commit 930d6288a267 ("powerpc: sstep: Add support for
> maddhd, maddhdu, maddld instructions"), but it's not explained there either
>
> I see the same pattern in a number of commits: commit 6324320de609
> ("powerpc sstep: Add support for modsd, modud instructions"), commit
> 6c180071509a ("powerpc sstep: Add support for modsw, moduw
> instructions"), commit a23987ef267a ("powerpc: sstep: Add support for
> darn instruction") and a few others, all of which seem to have come
> through Sandipan in February 2019. I haven't spotted any explanation for
> why -1 was chosen, but I haven't checked the mailing list archives.
>
> If -1 is OK, would it be possible to update the comment to explain why?
Agreed. As you rightly pointed out, there are many more such cases and, yes,
we are aware of this issue and it's being worked upon as a separate patch.
(Sandipan did send a fix patch internally some time back).
Thanks for pointing it out!
Ravi
^ permalink raw reply
* Re: [PATCH] powerpc/perf: fix Threshold Event CounterMultiplier width for P10
From: Michal Suchánek @ 2020-10-12 11:29 UTC (permalink / raw)
To: Madhavan Srinivasan; +Cc: atrajeev, linuxppc-dev
In-Reply-To: <20201012103128.53243-1-maddy@linux.ibm.com>
Hello,
On Mon, Oct 12, 2020 at 04:01:28PM +0530, Madhavan Srinivasan wrote:
> Power9 and isa v3.1 has 7bit mantissa field for Threshold Event Counter
^^^ Shouldn't his be 3.0?
> Multiplier (TECM). TECM is part of Monitor Mode Control Register A (MMCRA).
> This field along with Threshold Event Counter Exponent (TECE) is used to
> get threshould counter value. In Power10, the width of TECM field is
> increase to 8bits. Patch fixes the current code to modify the MMCRA[TECM]
> extraction macro to handling this changes.
>
> Fixes: 170a315f41c64 ('powerpc/perf: Support to export MMCRA[TEC*] field to userspace')
> Signed-off-by: Madhavan Srinivasan <maddy@linux.ibm.com>
> ---
> arch/powerpc/perf/isa207-common.c | 3 +++
> arch/powerpc/perf/isa207-common.h | 4 ++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/arch/powerpc/perf/isa207-common.c b/arch/powerpc/perf/isa207-common.c
> index 964437adec18..5fe129f02290 100644
> --- a/arch/powerpc/perf/isa207-common.c
> +++ b/arch/powerpc/perf/isa207-common.c
> @@ -247,6 +247,9 @@ void isa207_get_mem_weight(u64 *weight)
> u64 sier = mfspr(SPRN_SIER);
> u64 val = (sier & ISA207_SIER_TYPE_MASK) >> ISA207_SIER_TYPE_SHIFT;
>
> + if (cpu_has_feature(CPU_FTR_ARCH_31))
> + mantissa = P10_MMCRA_THR_CTR_MANT(mmcra);
> +
> if (val == 0 || val == 7)
> *weight = 0;
> else
> diff --git a/arch/powerpc/perf/isa207-common.h b/arch/powerpc/perf/isa207-common.h
> index 044de65e96b9..71380e854f48 100644
> --- a/arch/powerpc/perf/isa207-common.h
> +++ b/arch/powerpc/perf/isa207-common.h
> @@ -219,6 +219,10 @@
> #define MMCRA_THR_CTR_EXP(v) (((v) >> MMCRA_THR_CTR_EXP_SHIFT) &\
> MMCRA_THR_CTR_EXP_MASK)
>
> +#define P10_MMCRA_THR_CTR_MANT_MASK 0xFFul
> +#define P10_MMCRA_THR_CTR_MANT(v) (((v) >> MMCRA_THR_CTR_MANT_SHIFT) &\
> + P10_MMCRA_THR_CTR_MANT_MASK)
> +
> /* MMCRA Threshold Compare bit constant for power9 */
> #define p9_MMCRA_THR_CMP_SHIFT 45
>
> --
> 2.26.2
>
^ permalink raw reply
* Re: [PATCH v5 1/5] powerpc/sstep: Emulate prefixed instructions only when CPU_FTR_ARCH_31 is set
From: Daniel Axtens @ 2020-10-12 12:55 UTC (permalink / raw)
To: Ravi Bangoria
Cc: Ravi Bangoria, bala24, paulus, sandipan, jniethe5, naveen.n.rao,
linuxppc-dev
In-Reply-To: <41dda593-0fc6-569e-2243-189d84653b4a@linux.ibm.com>
Ravi Bangoria <ravi.bangoria@linux.ibm.com> writes:
> Hi Daniel,
>
> On 10/12/20 7:21 AM, Daniel Axtens wrote:
>> Hi,
>>
>> Apologies if this has come up in a previous revision.
>>
>>
>>> case 1:
>>> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
>>> + return -1;
>>> +
>>> prefix_r = GET_PREFIX_R(word);
>>> ra = GET_PREFIX_RA(suffix);
>>
>> The comment above analyse_instr reads in part:
>>
>> * Return value is 1 if the instruction can be emulated just by
>> * updating *regs with the information in *op, -1 if we need the
>> * GPRs but *regs doesn't contain the full register set, or 0
>> * otherwise.
>>
>> I was wondering why returning -1 if the instruction isn't supported the
>> right thing to do - it seemed to me that it should return 0?
>>
>> I did look and see that there are other cases where the code returns -1
>> for an unsupported operation, e.g.:
>>
>> #ifdef __powerpc64__
>> case 4:
>> if (!cpu_has_feature(CPU_FTR_ARCH_300))
>> return -1;
>>
>> switch (word & 0x3f) {
>> case 48: /* maddhd */
>>
>> That's from commit 930d6288a267 ("powerpc: sstep: Add support for
>> maddhd, maddhdu, maddld instructions"), but it's not explained there either
>>
>> I see the same pattern in a number of commits: commit 6324320de609
>> ("powerpc sstep: Add support for modsd, modud instructions"), commit
>> 6c180071509a ("powerpc sstep: Add support for modsw, moduw
>> instructions"), commit a23987ef267a ("powerpc: sstep: Add support for
>> darn instruction") and a few others, all of which seem to have come
>> through Sandipan in February 2019. I haven't spotted any explanation for
>> why -1 was chosen, but I haven't checked the mailing list archives.
>>
>> If -1 is OK, would it be possible to update the comment to explain why?
>
> Agreed. As you rightly pointed out, there are many more such cases and, yes,
> we are aware of this issue and it's being worked upon as a separate patch.
> (Sandipan did send a fix patch internally some time back).
That sounds like a perfectly reasonable approach.
I'd love to review the patch when it's sent - if you or Sandipan could
please cc: me so I don't miss it I'd really appreciate that.
I will proceed to review the rest of the patch and series.
Kind regards,
Daniel
>
> Thanks for pointing it out!
> Ravi
^ permalink raw reply
* Re: [PATCH v5 1/5] powerpc/sstep: Emulate prefixed instructions only when CPU_FTR_ARCH_31 is set
From: Daniel Axtens @ 2020-10-12 13:44 UTC (permalink / raw)
To: Ravi Bangoria, mpe
Cc: ravi.bangoria, jniethe5, bala24, paulus, sandipan, naveen.n.rao,
linuxppc-dev
In-Reply-To: <20201011050908.72173-2-ravi.bangoria@linux.ibm.com>
Hi,
To review this, I looked through the supported instructions to see if
there were any that I thought might have been missed.
I didn't find any other v3.1 ones, although I don't have a v3.1 ISA to
hand so I was basically looking for instructions I didn't recognise.
I did, however, find a number of instructions that are new in ISA 3.0
that aren't guarded:
- addpcis
- lxvl/stxvl
- lxvll/stxvll
- lxvwsx
- stxvx
- lxsipzx
- lxvh8x
- lxsihzx
- lxvb16x/stxvb16x
- stxsibx
- stxsihx
- lxvb16x
- lxsd/stxsd
- lxssp/stxssp
- lxv/stxv
Also, I don't know how bothered we are about P7, but if I'm reading the
ISA correctly, lqarx/stqcx. are not supported before ISA 2.07. Likewise
a number of the vector instructions like lxsiwzx and lxsiwax (and the
companion stores).
I realise it's not really the point of this particular patch, so I don't
think this should block acceptance. What I would like to know - and
maybe this is something where we need mpe to weigh in - is whether we
need consistent guards for 2.07 and 3.0. We have some 3.0 guards already
but clearly not everywhere.
With all that said - the patch does what it says it does, and looks good
to me:
Reviewed-by: Daniel Axtens <dja@axtens.net>
Kind regards,
Daniel
> From: Balamuruhan S <bala24@linux.ibm.com>
>
> Unconditional emulation of prefixed instructions will allow
> emulation of them on Power10 predecessors which might cause
> issues. Restrict that.
>
> Fixes: 3920742b92f5 ("powerpc sstep: Add support for prefixed fixed-point arithmetic")
> Fixes: 50b80a12e4cc ("powerpc sstep: Add support for prefixed load/stores")
> Signed-off-by: Balamuruhan S <bala24@linux.ibm.com>
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.ibm.com>
> ---
> arch/powerpc/lib/sstep.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> index e9dcaba9a4f8..e6242744c71b 100644
> --- a/arch/powerpc/lib/sstep.c
> +++ b/arch/powerpc/lib/sstep.c
> @@ -1346,6 +1346,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> switch (opcode) {
> #ifdef __powerpc64__
> case 1:
> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
> + return -1;
> +
> prefix_r = GET_PREFIX_R(word);
> ra = GET_PREFIX_RA(suffix);
> rd = (suffix >> 21) & 0x1f;
> @@ -2733,6 +2736,9 @@ int analyse_instr(struct instruction_op *op, const struct pt_regs *regs,
> }
> break;
> case 1: /* Prefixed instructions */
> + if (!cpu_has_feature(CPU_FTR_ARCH_31))
> + return -1;
> +
> prefix_r = GET_PREFIX_R(word);
> ra = GET_PREFIX_RA(suffix);
> op->update_reg = ra;
> --
> 2.26.2
^ permalink raw reply
* Re: [RFC v1 2/2] KVM: PPC: Book3S HV: abstract secure VM related calls.
From: Bharata B Rao @ 2020-10-12 15:28 UTC (permalink / raw)
To: Ram Pai; +Cc: linuxppc-dev, kvm-ppc, farosas
In-Reply-To: <1602487663-7321-3-git-send-email-linuxram@us.ibm.com>
On Mon, Oct 12, 2020 at 12:27:43AM -0700, Ram Pai wrote:
> Abstract the secure VM related calls into generic calls.
>
> These generic calls will call the corresponding method of the
> backend that prvoides the implementation to support secure VM.
>
> Currently there is only the ultravisor based implementation.
> Modify that implementation to act as a backed to the generic calls.
>
> This plumbing will provide the flexibility to add more backends
> in the future.
>
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_book3s_uvmem.h | 100 -----------
> arch/powerpc/include/asm/kvmppc_svm_backend.h | 250 ++++++++++++++++++++++++++
> arch/powerpc/kvm/book3s_64_mmu_radix.c | 6 +-
> arch/powerpc/kvm/book3s_hv.c | 28 +--
> arch/powerpc/kvm/book3s_hv_uvmem.c | 78 ++++++--
> 5 files changed, 327 insertions(+), 135 deletions(-)
> delete mode 100644 arch/powerpc/include/asm/kvm_book3s_uvmem.h
> create mode 100644 arch/powerpc/include/asm/kvmppc_svm_backend.h
>
> diff --git a/arch/powerpc/include/asm/kvmppc_svm_backend.h b/arch/powerpc/include/asm/kvmppc_svm_backend.h
> new file mode 100644
> index 0000000..be60d80
> --- /dev/null
> +++ b/arch/powerpc/include/asm/kvmppc_svm_backend.h
> @@ -0,0 +1,250 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + *
> + * Copyright IBM Corp. 2020
> + *
> + * Authors: Ram Pai <linuxram@us.ibm.com>
> + */
> +
> +#ifndef __POWERPC_KVMPPC_SVM_BACKEND_H__
> +#define __POWERPC_KVMPPC_SVM_BACKEND_H__
> +
> +#include <linux/mutex.h>
> +#include <linux/timer.h>
> +#include <linux/types.h>
> +#include <linux/kvm_types.h>
> +#include <linux/kvm_host.h>
> +#include <linux/bug.h>
> +#ifdef CONFIG_PPC_BOOK3S
> +#include <asm/kvm_book3s.h>
> +#else
> +#include <asm/kvm_booke.h>
> +#endif
> +#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> +#include <asm/paca.h>
> +#include <asm/xive.h>
> +#include <asm/cpu_has_feature.h>
> +#endif
> +
> +struct kvmppc_hmm_backend {
Though we started with HMM initially, what we ended up with eventually
has nothing to do with HMM. Please don't introduce hmm again :-)
> + /* initialize */
> + int (*kvmppc_secmem_init)(void);
> +
> + /* cleanup */
> + void (*kvmppc_secmem_free)(void);
> +
> + /* is memory available */
> + bool (*kvmppc_secmem_available)(void);
> +
> + /* allocate a protected/secure page for the secure VM */
> + unsigned long (*kvmppc_svm_page_in)(struct kvm *kvm,
> + unsigned long gra,
> + unsigned long flags,
> + unsigned long page_shift);
> +
> + /* recover the protected/secure page from the secure VM */
> + unsigned long (*kvmppc_svm_page_out)(struct kvm *kvm,
> + unsigned long gra,
> + unsigned long flags,
> + unsigned long page_shift);
> +
> + /* initiate the transition of a VM to secure VM */
> + unsigned long (*kvmppc_svm_init_start)(struct kvm *kvm);
> +
> + /* finalize the transition of a secure VM */
> + unsigned long (*kvmppc_svm_init_done)(struct kvm *kvm);
> +
> + /* share the page on page fault */
> + int (*kvmppc_svm_page_share)(struct kvm *kvm, unsigned long gfn);
> +
> + /* abort the transition to a secure VM */
> + unsigned long (*kvmppc_svm_init_abort)(struct kvm *kvm);
> +
> + /* add a memory slot */
> + int (*kvmppc_svm_memslot_create)(struct kvm *kvm,
> + const struct kvm_memory_slot *new);
> +
> + /* free a memory slot */
> + void (*kvmppc_svm_memslot_delete)(struct kvm *kvm,
> + const struct kvm_memory_slot *old);
> +
> + /* drop pages allocated to the secure VM */
> + void (*kvmppc_svm_drop_pages)(const struct kvm_memory_slot *free,
> + struct kvm *kvm, bool skip_page_out);
> +};
Since the structure has kvmppc_ prefix, may be you can drop
the same from its members to make the fields smaller?
> +
> +extern const struct kvmppc_hmm_backend *kvmppc_svm_backend;
> +
> +static inline int kvmppc_svm_page_share(struct kvm *kvm, unsigned long gfn)
> +{
> + if (!kvmppc_svm_backend)
> + return -ENODEV;
> +
> + return kvmppc_svm_backend->kvmppc_svm_page_share(kvm,
> + gfn);
> +}
> +
> +static inline void kvmppc_svm_drop_pages(const struct kvm_memory_slot *memslot,
> + struct kvm *kvm, bool skip_page_out)
> +{
> + if (!kvmppc_svm_backend)
> + return;
> +
> + kvmppc_svm_backend->kvmppc_svm_drop_pages(memslot,
> + kvm, skip_page_out);
> +}
> +
> +static inline int kvmppc_svm_page_in(struct kvm *kvm,
> + unsigned long gpa,
> + unsigned long flags,
> + unsigned long page_shift)
> +{
> + if (!kvmppc_svm_backend)
> + return -ENODEV;
> +
> + return kvmppc_svm_backend->kvmppc_svm_page_in(kvm,
> + gpa, flags, page_shift);
> +}
> +
> +static inline int kvmppc_svm_page_out(struct kvm *kvm,
> + unsigned long gpa,
> + unsigned long flags,
> + unsigned long page_shift)
> +{
> + if (!kvmppc_svm_backend)
> + return -ENODEV;
> +
> + return kvmppc_svm_backend->kvmppc_svm_page_out(kvm,
> + gpa, flags, page_shift);
> +}
> +
> +static inline int kvmppc_svm_init_start(struct kvm *kvm)
> +{
> + if (!kvmppc_svm_backend)
> + return -ENODEV;
> +
> + return kvmppc_svm_backend->kvmppc_svm_init_start(kvm);
> +}
> +
> +static inline int kvmppc_svm_init_done(struct kvm *kvm)
> +{
> + if (!kvmppc_svm_backend)
> + return -ENODEV;
> +
> + return kvmppc_svm_backend->kvmppc_svm_init_done(kvm);
> +}
> +
> +static inline int kvmppc_svm_init_abort(struct kvm *kvm)
> +{
> + if (!kvmppc_svm_backend)
> + return -ENODEV;
> +
> + return kvmppc_svm_backend->kvmppc_svm_init_abort(kvm);
> +}
> +
> +static inline void kvmppc_svm_memslot_create(struct kvm *kvm,
> + const struct kvm_memory_slot *memslot)
> +{
> + if (!kvmppc_svm_backend)
> + return;
> +
> + kvmppc_svm_backend->kvmppc_svm_memslot_create(kvm,
> + memslot);
> +}
> +
> +static inline void kvmppc_svm_memslot_delete(struct kvm *kvm,
> + const struct kvm_memory_slot *memslot)
> +{
> + if (!kvmppc_svm_backend)
> + return;
> +
> + kvmppc_svm_backend->kvmppc_svm_memslot_delete(kvm,
> + memslot);
> +}
> +
> +static inline int kvmppc_secmem_init(void)
> +{
> +#ifdef CONFIG_PPC_UV
> + extern const struct kvmppc_hmm_backend kvmppc_uvmem_backend;
> +
> + kvmppc_svm_backend = NULL;
> + if (kvmhv_on_pseries()) {
> + /* @TODO add the protected memory backend */
> + return 0;
> + }
> +
> + kvmppc_svm_backend = &kvmppc_uvmem_backend;
> +
> + if (!kvmppc_svm_backend->kvmppc_secmem_init) {
You have a function named kvmppc_secmem_init() and the field
named the same, can be confusing.
> + pr_err("KVM-HV: kvmppc_svm_backend has no %s\n", __func__);
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_secmem_free) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_secmem_free()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_secmem_available) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_secmem_available()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_page_in) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_page_in()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_page_out) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_page_out()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_init_start) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_init_start()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_init_done) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_init_done()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_page_share) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_page_share()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_init_abort) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_init_abort()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_memslot_create) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_memslot_create()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_memslot_delete) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_memslot_delete()\n");
> + goto err;
> + }
> + if (!kvmppc_svm_backend->kvmppc_svm_drop_pages) {
> + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_drop_pages()\n");
> + goto err;
> + }
Do you really need to check each and every callback like above?
If so, may be the check can be optimized?
Regards,
Bharata.
^ permalink raw reply
* [PATCH v2] powerpc/mm: Add mask of always present MMU features
From: Christophe Leroy @ 2020-10-12 15:39 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
On the same principle as commit 773edeadf672 ("powerpc/mm: Add mask
of possible MMU features"), add mask for MMU features that are
always there in order to optimise out dead branches.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
v2: Features must be anded with MMU_FTRS_POSSIBLE instead of ~0, otherwise
MMU_FTRS_ALWAYS is ~0 when no #ifdef matches.
---
arch/powerpc/include/asm/mmu.h | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)
diff --git a/arch/powerpc/include/asm/mmu.h b/arch/powerpc/include/asm/mmu.h
index 255a1837e9f7..64e7e7f7cda9 100644
--- a/arch/powerpc/include/asm/mmu.h
+++ b/arch/powerpc/include/asm/mmu.h
@@ -201,8 +201,30 @@ enum {
0,
};
+enum {
+ MMU_FTRS_ALWAYS =
+#ifdef CONFIG_PPC_8xx
+ MMU_FTR_TYPE_8xx &
+#endif
+#ifdef CONFIG_40x
+ MMU_FTR_TYPE_40x &
+#endif
+#ifdef CONFIG_PPC_47x
+ MMU_FTR_TYPE_47x &
+#elif defined(CONFIG_44x)
+ MMU_FTR_TYPE_44x &
+#endif
+#if defined(CONFIG_E200) || defined(CONFIG_E500)
+ MMU_FTR_TYPE_FSL_E &
+#endif
+ MMU_FTRS_POSSIBLE,
+};
+
static inline bool early_mmu_has_feature(unsigned long feature)
{
+ if (MMU_FTRS_ALWAYS & feature)
+ return true;
+
return !!(MMU_FTRS_POSSIBLE & cur_cpu_spec->mmu_features & feature);
}
@@ -231,6 +253,9 @@ static __always_inline bool mmu_has_feature(unsigned long feature)
}
#endif
+ if (MMU_FTRS_ALWAYS & feature)
+ return true;
+
if (!(MMU_FTRS_POSSIBLE & feature))
return false;
--
2.25.0
^ permalink raw reply related
* Re: [RFC v1 2/2] KVM: PPC: Book3S HV: abstract secure VM related calls.
From: Ram Pai @ 2020-10-12 16:13 UTC (permalink / raw)
To: Bharata B Rao; +Cc: linuxppc-dev, kvm-ppc, farosas
In-Reply-To: <20201012152836.GK185637@in.ibm.com>
On Mon, Oct 12, 2020 at 08:58:36PM +0530, Bharata B Rao wrote:
> On Mon, Oct 12, 2020 at 12:27:43AM -0700, Ram Pai wrote:
> > Abstract the secure VM related calls into generic calls.
> >
> > These generic calls will call the corresponding method of the
> > backend that prvoides the implementation to support secure VM.
> >
> > Currently there is only the ultravisor based implementation.
> > Modify that implementation to act as a backed to the generic calls.
> >
> > This plumbing will provide the flexibility to add more backends
> > in the future.
> >
> > Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> > ---
> > arch/powerpc/include/asm/kvm_book3s_uvmem.h | 100 -----------
> > arch/powerpc/include/asm/kvmppc_svm_backend.h | 250 ++++++++++++++++++++++++++
> > arch/powerpc/kvm/book3s_64_mmu_radix.c | 6 +-
> > arch/powerpc/kvm/book3s_hv.c | 28 +--
> > arch/powerpc/kvm/book3s_hv_uvmem.c | 78 ++++++--
> > 5 files changed, 327 insertions(+), 135 deletions(-)
> > delete mode 100644 arch/powerpc/include/asm/kvm_book3s_uvmem.h
> > create mode 100644 arch/powerpc/include/asm/kvmppc_svm_backend.h
> >
> > diff --git a/arch/powerpc/include/asm/kvmppc_svm_backend.h b/arch/powerpc/include/asm/kvmppc_svm_backend.h
> > new file mode 100644
> > index 0000000..be60d80
> > --- /dev/null
> > +++ b/arch/powerpc/include/asm/kvmppc_svm_backend.h
> > @@ -0,0 +1,250 @@
> > +/* SPDX-License-Identifier: GPL-2.0-only */
> > +/*
> > + *
> > + * Copyright IBM Corp. 2020
> > + *
> > + * Authors: Ram Pai <linuxram@us.ibm.com>
> > + */
> > +
> > +#ifndef __POWERPC_KVMPPC_SVM_BACKEND_H__
> > +#define __POWERPC_KVMPPC_SVM_BACKEND_H__
> > +
> > +#include <linux/mutex.h>
> > +#include <linux/timer.h>
> > +#include <linux/types.h>
> > +#include <linux/kvm_types.h>
> > +#include <linux/kvm_host.h>
> > +#include <linux/bug.h>
> > +#ifdef CONFIG_PPC_BOOK3S
> > +#include <asm/kvm_book3s.h>
> > +#else
> > +#include <asm/kvm_booke.h>
> > +#endif
> > +#ifdef CONFIG_KVM_BOOK3S_64_HANDLER
> > +#include <asm/paca.h>
> > +#include <asm/xive.h>
> > +#include <asm/cpu_has_feature.h>
> > +#endif
> > +
> > +struct kvmppc_hmm_backend {
>
> Though we started with HMM initially, what we ended up with eventually
> has nothing to do with HMM. Please don't introduce hmm again :-)
Hmm.. its still a extension to HMM to help manage device memory.
right? What am i missing?
>
> > + /* initialize */
> > + int (*kvmppc_secmem_init)(void);
> > +
> > + /* cleanup */
> > + void (*kvmppc_secmem_free)(void);
> > +
> > + /* is memory available */
> > + bool (*kvmppc_secmem_available)(void);
> > +
> > + /* allocate a protected/secure page for the secure VM */
> > + unsigned long (*kvmppc_svm_page_in)(struct kvm *kvm,
> > + unsigned long gra,
> > + unsigned long flags,
> > + unsigned long page_shift);
> > +
> > + /* recover the protected/secure page from the secure VM */
> > + unsigned long (*kvmppc_svm_page_out)(struct kvm *kvm,
> > + unsigned long gra,
> > + unsigned long flags,
> > + unsigned long page_shift);
> > +
> > + /* initiate the transition of a VM to secure VM */
> > + unsigned long (*kvmppc_svm_init_start)(struct kvm *kvm);
> > +
> > + /* finalize the transition of a secure VM */
> > + unsigned long (*kvmppc_svm_init_done)(struct kvm *kvm);
> > +
> > + /* share the page on page fault */
> > + int (*kvmppc_svm_page_share)(struct kvm *kvm, unsigned long gfn);
> > +
> > + /* abort the transition to a secure VM */
> > + unsigned long (*kvmppc_svm_init_abort)(struct kvm *kvm);
> > +
> > + /* add a memory slot */
> > + int (*kvmppc_svm_memslot_create)(struct kvm *kvm,
> > + const struct kvm_memory_slot *new);
> > +
> > + /* free a memory slot */
> > + void (*kvmppc_svm_memslot_delete)(struct kvm *kvm,
> > + const struct kvm_memory_slot *old);
> > +
> > + /* drop pages allocated to the secure VM */
> > + void (*kvmppc_svm_drop_pages)(const struct kvm_memory_slot *free,
> > + struct kvm *kvm, bool skip_page_out);
> > +};
>
> Since the structure has kvmppc_ prefix, may be you can drop
> the same from its members to make the fields smaller?
ok.
>
> > +
> > +extern const struct kvmppc_hmm_backend *kvmppc_svm_backend;
> > +
> > +static inline int kvmppc_svm_page_share(struct kvm *kvm, unsigned long gfn)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return -ENODEV;
> > +
> > + return kvmppc_svm_backend->kvmppc_svm_page_share(kvm,
> > + gfn);
> > +}
> > +
> > +static inline void kvmppc_svm_drop_pages(const struct kvm_memory_slot *memslot,
> > + struct kvm *kvm, bool skip_page_out)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return;
> > +
> > + kvmppc_svm_backend->kvmppc_svm_drop_pages(memslot,
> > + kvm, skip_page_out);
> > +}
> > +
> > +static inline int kvmppc_svm_page_in(struct kvm *kvm,
> > + unsigned long gpa,
> > + unsigned long flags,
> > + unsigned long page_shift)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return -ENODEV;
> > +
> > + return kvmppc_svm_backend->kvmppc_svm_page_in(kvm,
> > + gpa, flags, page_shift);
> > +}
> > +
> > +static inline int kvmppc_svm_page_out(struct kvm *kvm,
> > + unsigned long gpa,
> > + unsigned long flags,
> > + unsigned long page_shift)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return -ENODEV;
> > +
> > + return kvmppc_svm_backend->kvmppc_svm_page_out(kvm,
> > + gpa, flags, page_shift);
> > +}
> > +
> > +static inline int kvmppc_svm_init_start(struct kvm *kvm)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return -ENODEV;
> > +
> > + return kvmppc_svm_backend->kvmppc_svm_init_start(kvm);
> > +}
> > +
> > +static inline int kvmppc_svm_init_done(struct kvm *kvm)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return -ENODEV;
> > +
> > + return kvmppc_svm_backend->kvmppc_svm_init_done(kvm);
> > +}
> > +
> > +static inline int kvmppc_svm_init_abort(struct kvm *kvm)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return -ENODEV;
> > +
> > + return kvmppc_svm_backend->kvmppc_svm_init_abort(kvm);
> > +}
> > +
> > +static inline void kvmppc_svm_memslot_create(struct kvm *kvm,
> > + const struct kvm_memory_slot *memslot)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return;
> > +
> > + kvmppc_svm_backend->kvmppc_svm_memslot_create(kvm,
> > + memslot);
> > +}
> > +
> > +static inline void kvmppc_svm_memslot_delete(struct kvm *kvm,
> > + const struct kvm_memory_slot *memslot)
> > +{
> > + if (!kvmppc_svm_backend)
> > + return;
> > +
> > + kvmppc_svm_backend->kvmppc_svm_memslot_delete(kvm,
> > + memslot);
> > +}
> > +
> > +static inline int kvmppc_secmem_init(void)
> > +{
> > +#ifdef CONFIG_PPC_UV
> > + extern const struct kvmppc_hmm_backend kvmppc_uvmem_backend;
> > +
> > + kvmppc_svm_backend = NULL;
> > + if (kvmhv_on_pseries()) {
> > + /* @TODO add the protected memory backend */
> > + return 0;
> > + }
> > +
> > + kvmppc_svm_backend = &kvmppc_uvmem_backend;
> > +
> > + if (!kvmppc_svm_backend->kvmppc_secmem_init) {
>
> You have a function named kvmppc_secmem_init() and the field
> named the same, can be confusing.
ok. anyway the 'kvmppc' of the field will go away as per your comment
above. So the confusion will also go away :)
>
> > + pr_err("KVM-HV: kvmppc_svm_backend has no %s\n", __func__);
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_secmem_free) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_secmem_free()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_secmem_available) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_secmem_available()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_page_in) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_page_in()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_page_out) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_page_out()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_init_start) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_init_start()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_init_done) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_init_done()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_page_share) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_page_share()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_init_abort) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_init_abort()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_memslot_create) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_memslot_create()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_memslot_delete) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_memslot_delete()\n");
> > + goto err;
> > + }
> > + if (!kvmppc_svm_backend->kvmppc_svm_drop_pages) {
> > + pr_err("KVM-HV: kvmppc_svm_backend has no kvmppc_svm_drop_pages()\n");
> > + goto err;
> > + }
>
> Do you really need to check each and every callback like above?
> If so, may be the check can be optimized?
It gets checked only the first time, when the backend is introduced.
If we dont check it during initialization, then we will have to check
everytime the method is called. So it is optimized in that sense.
Do you see a better way to optimize it?
Thanks for your comments,
RP
^ permalink raw reply
* Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
From: Eric Biggers @ 2020-10-12 16:19 UTC (permalink / raw)
To: Ira Weiny
Cc: linux-aio, linux-efi, kvm, linux-doc, Peter Zijlstra, linux-mmc,
Dave Hansen, dri-devel, linux-mm, target-devel, linux-mtd,
amd-gfx, linux-kselftest, Thomas Gleixner, drbd-dev, devel,
linux-cifs, linux-nilfs, linux-scsi, linux-nvdimm, linux-rdma,
x86, Matthew Wilcox, linux-afs, cluster-devel, Ingo Molnar,
intel-wired-lan, kexec, xen-devel, linux-ext4, bpf, Dan Williams,
Fenghua Yu, intel-gfx, ecryptfs, linux-um, reiserfs-devel,
linux-block, linux-bcache, Borislav Petkov, Andy Lutomirski,
Jaegeuk Kim, ceph-devel, io-uring, linux-cachefs, linux-nfs,
linux-ntfs-dev, netdev, linuxppc-dev, samba-technical,
linux-kernel, linux-f2fs-devel, linux-fsdevel, Andrew Morton,
linux-erofs, linux-btrfs
In-Reply-To: <20201012065635.GB2046448@iweiny-DESK2.sc.intel.com>
On Sun, Oct 11, 2020 at 11:56:35PM -0700, Ira Weiny wrote:
> >
> > And I still don't really understand. After this patchset, there is still code
> > nearly identical to the above (doing a temporary mapping just for a memcpy) that
> > would still be using kmap_atomic().
>
> I don't understand. You mean there would be other call sites calling:
>
> kmap_atomic()
> memcpy()
> kunmap_atomic()
Yes, there are tons of places that do this. Try 'git grep -A6 kmap_atomic'
and look for memcpy().
Hence why I'm asking what will be the "recommended" way to do this...
kunmap_thread() or kmap_atomic()?
> And since I don't know the call site details if there are kmap_thread() calls
> which are better off as kmap_atomic() calls I think it is worth converting
> them. But I made the assumption that kmap users would already be calling
> kmap_atomic() if they could (because it is more efficient).
Not necessarily. In cases where either one is correct, people might not have
put much thought into which of kmap() and kmap_atomic() they are using.
- Eric
^ permalink raw reply
* Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
From: Dave Hansen @ 2020-10-12 16:28 UTC (permalink / raw)
To: Eric Biggers, Ira Weiny
Cc: linux-aio, linux-efi, kvm, linux-doc, Peter Zijlstra, linux-mmc,
Dave Hansen, dri-devel, linux-mm, target-devel, linux-mtd,
amd-gfx, linux-kselftest, Thomas Gleixner, drbd-dev, devel,
linux-cifs, linux-nilfs, linux-scsi, linux-nvdimm, linux-rdma,
x86, Matthew Wilcox, linux-afs, cluster-devel, Ingo Molnar,
intel-wired-lan, kexec, xen-devel, linux-ext4, bpf, Dan Williams,
Fenghua Yu, intel-gfx, ecryptfs, linux-um, reiserfs-devel,
linux-block, linux-bcache, Borislav Petkov, Andy Lutomirski,
Jaegeuk Kim, ceph-devel, io-uring, linux-cachefs, linux-nfs,
linux-ntfs-dev, netdev, linuxppc-dev, samba-technical,
linux-kernel, linux-f2fs-devel, linux-fsdevel, Andrew Morton,
linux-erofs, linux-btrfs
In-Reply-To: <20201012161946.GA858@sol.localdomain>
On 10/12/20 9:19 AM, Eric Biggers wrote:
> On Sun, Oct 11, 2020 at 11:56:35PM -0700, Ira Weiny wrote:
>>> And I still don't really understand. After this patchset, there is still code
>>> nearly identical to the above (doing a temporary mapping just for a memcpy) that
>>> would still be using kmap_atomic().
>> I don't understand. You mean there would be other call sites calling:
>>
>> kmap_atomic()
>> memcpy()
>> kunmap_atomic()
> Yes, there are tons of places that do this. Try 'git grep -A6 kmap_atomic'
> and look for memcpy().
>
> Hence why I'm asking what will be the "recommended" way to do this...
> kunmap_thread() or kmap_atomic()?
kmap_atomic() is always preferred over kmap()/kmap_thread().
kmap_atomic() is _much_ more lightweight since its TLB invalidation is
always CPU-local and never broadcast.
So, basically, unless you *must* sleep while the mapping is in place,
kmap_atomic() is preferred.
^ permalink raw reply
* Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
From: Matthew Wilcox @ 2020-10-12 16:44 UTC (permalink / raw)
To: Dave Hansen
Cc: linux-aio, linux-efi, kvm, linux-doc, Peter Zijlstra, linux-mmc,
Dave Hansen, dri-devel, linux-mm, target-devel, linux-mtd,
linux-kselftest, linux-f2fs-devel, Ira Weiny, Thomas Gleixner,
drbd-dev, devel, linux-cifs, linux-nilfs, linux-scsi,
linux-nvdimm, linux-rdma, x86, amd-gfx, linux-afs, Eric Biggers,
Ingo Molnar, intel-wired-lan, kexec, xen-devel, linux-ext4, bpf,
Dan Williams, Fenghua Yu, intel-gfx, ecryptfs, linux-um,
reiserfs-devel, linux-block, linux-bcache, Borislav Petkov,
Andy Lutomirski, Jaegeuk Kim, ceph-devel, io-uring, linux-cachefs,
linux-nfs, linux-ntfs-dev, netdev, linuxppc-dev, samba-technical,
linux-kernel, cluster-devel, linux-fsdevel, Andrew Morton,
linux-erofs, linux-btrfs
In-Reply-To: <5d621db9-23d4-e140-45eb-d7fca2093d2b@intel.com>
On Mon, Oct 12, 2020 at 09:28:29AM -0700, Dave Hansen wrote:
> kmap_atomic() is always preferred over kmap()/kmap_thread().
> kmap_atomic() is _much_ more lightweight since its TLB invalidation is
> always CPU-local and never broadcast.
>
> So, basically, unless you *must* sleep while the mapping is in place,
> kmap_atomic() is preferred.
But kmap_atomic() disables preemption, so the _ideal_ interface would map
it only locally, then on preemption make it global. I don't even know
if that _can_ be done. But this email makes it seem like kmap_atomic()
has no downsides.
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Khalid Aziz @ 2020-10-12 17:03 UTC (permalink / raw)
To: Catalin Marinas
Cc: Jann Horn, linuxppc-dev, linux-kernel, Christoph Hellwig,
linux-mm, Paul Mackerras, sparclinux, Anthony Yznaga,
Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel
In-Reply-To: <20201010110949.GA32545@gaia>
On 10/10/20 5:09 AM, Catalin Marinas wrote:
> Hi Khalid,
>
> On Wed, Oct 07, 2020 at 02:14:09PM -0600, Khalid Aziz wrote:
>> On 10/7/20 1:39 AM, Jann Horn wrote:
>>> arch_validate_prot() is a hook that can validate whether a given set of
>>> protection flags is valid in an mprotect() operation. It is given the set
>>> of protection flags and the address being modified.
>>>
>>> However, the address being modified can currently not actually be used in
>>> a meaningful way because:
>>>
>>> 1. Only the address is given, but not the length, and the operation can
>>> span multiple VMAs. Therefore, the callee can't actually tell which
>>> virtual address range, or which VMAs, are being targeted.
>>> 2. The mmap_lock is not held, meaning that if the callee were to check
>>> the VMA at @addr, that VMA would be unrelated to the one the
>>> operation is performed on.
>>>
>>> Currently, custom arch_validate_prot() handlers are defined by
>>> arm64, powerpc and sparc.
>>> arm64 and powerpc don't care about the address range, they just check the
>>> flags against CPU support masks.
>>> sparc's arch_validate_prot() attempts to look at the VMA, but doesn't take
>>> the mmap_lock.
>>>
>>> Change the function signature to also take a length, and move the
>>> arch_validate_prot() call in mm/mprotect.c down into the locked region.
> [...]
>> As Chris pointed out, the call to arch_validate_prot() from do_mmap2()
>> is made without holding mmap_lock. Lock is not acquired until
>> vm_mmap_pgoff(). This variance is uncomfortable but I am more
>> uncomfortable forcing all implementations of validate_prot to require
>> mmap_lock be held when non-sparc implementations do not have such need
>> yet. Since do_mmap2() is in powerpc specific code, for now this patch
>> solves a current problem.
>
> I still think sparc should avoid walking the vmas in
> arch_validate_prot(). The core code already has the vmas, though not
> when calling arch_validate_prot(). That's one of the reasons I added
> arch_validate_flags() with the MTE patches. For sparc, this could be
> (untested, just copied the arch_validate_prot() code):
I am little uncomfortable with the idea of validating protection bits
inside the VMA walk loop in do_mprotect_pkey(). When ADI is being
enabled across multiple VMAs and arch_validate_flags() fails on a VMA
later, do_mprotect_pkey() will bail out with error leaving ADI enabled
on earlier VMAs. This will apply to protection bits other than ADI as
well of course. This becomes a partial failure of mprotect() call. I
think it should be all or nothing with mprotect() - when one calls
mprotect() from userspace, either the entire address range passed in
gets its protection bits updated or none of it does. That requires
validating protection bits upfront or undoing what earlier iterations of
VMA walk loop might have done.
--
Khalid
>
> static inline bool arch_validate_flags(unsigned long vm_flags)
> {
> if (!(vm_flags & VM_SPARC_ADI))
> return true;
>
> if (!adi_capable())
> return false;
>
> /* ADI can not be enabled on PFN mapped pages */
> if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
> return false;
>
> /*
> * Mergeable pages can become unmergeable if ADI is enabled on
> * them even if they have identical data on them. This can be
> * because ADI enabled pages with identical data may still not
> * have identical ADI tags on them. Disallow ADI on mergeable
> * pages.
> */
> if (vma->vm_flags & VM_MERGEABLE)
> return false;
>
> return true;
> }
>
>> That leaves open the question of should
>> generic mmap call arch_validate_prot and return EINVAL for invalid
>> combination of protection bits, but that is better addressed in a
>> separate patch.
>
> The above would cover mmap() as well.
>
> The current sparc_validate_prot() relies on finding the vma for the
> corresponding address. However, if you call this early in the mmap()
> path, there's no such vma. It is only created later in mmap_region()
> which no longer has the original PROT_* flags (all converted to VM_*
> flags).
>
> Calling arch_validate_flags() on mmap() has a small side-effect on the
> user ABI: if the CPU is not adi_capable(), PROT_ADI is currently ignored
> on mmap() but rejected by sparc_validate_prot(). Powerpc already does
> this already and I think it should be fine for arm64 (it needs checking
> though as we have another flag, PROT_BTI, hopefully dynamic loaders
> don't pass this flag unconditionally).
>
> However, as I said above, it doesn't solve the mmap() PROT_ADI checking
> for sparc since there's no vma yet. I'd strongly recommend the
> arch_validate_flags() approach and reverting the "start" parameter added
> to arch_validate_prot() if you go for the flags route.
>
> Thanks.
>
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Catalin Marinas @ 2020-10-12 17:22 UTC (permalink / raw)
To: Khalid Aziz
Cc: Jann Horn, linuxppc-dev, linux-kernel, Christoph Hellwig,
linux-mm, Paul Mackerras, sparclinux, Anthony Yznaga,
Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel
In-Reply-To: <af207cf8-3049-85eb-349d-5fed6b9be49c@oracle.com>
On Mon, Oct 12, 2020 at 11:03:33AM -0600, Khalid Aziz wrote:
> On 10/10/20 5:09 AM, Catalin Marinas wrote:
> > On Wed, Oct 07, 2020 at 02:14:09PM -0600, Khalid Aziz wrote:
> >> On 10/7/20 1:39 AM, Jann Horn wrote:
> >>> arch_validate_prot() is a hook that can validate whether a given set of
> >>> protection flags is valid in an mprotect() operation. It is given the set
> >>> of protection flags and the address being modified.
> >>>
> >>> However, the address being modified can currently not actually be used in
> >>> a meaningful way because:
> >>>
> >>> 1. Only the address is given, but not the length, and the operation can
> >>> span multiple VMAs. Therefore, the callee can't actually tell which
> >>> virtual address range, or which VMAs, are being targeted.
> >>> 2. The mmap_lock is not held, meaning that if the callee were to check
> >>> the VMA at @addr, that VMA would be unrelated to the one the
> >>> operation is performed on.
> >>>
> >>> Currently, custom arch_validate_prot() handlers are defined by
> >>> arm64, powerpc and sparc.
> >>> arm64 and powerpc don't care about the address range, they just check the
> >>> flags against CPU support masks.
> >>> sparc's arch_validate_prot() attempts to look at the VMA, but doesn't take
> >>> the mmap_lock.
> >>>
> >>> Change the function signature to also take a length, and move the
> >>> arch_validate_prot() call in mm/mprotect.c down into the locked region.
> > [...]
> >> As Chris pointed out, the call to arch_validate_prot() from do_mmap2()
> >> is made without holding mmap_lock. Lock is not acquired until
> >> vm_mmap_pgoff(). This variance is uncomfortable but I am more
> >> uncomfortable forcing all implementations of validate_prot to require
> >> mmap_lock be held when non-sparc implementations do not have such need
> >> yet. Since do_mmap2() is in powerpc specific code, for now this patch
> >> solves a current problem.
> >
> > I still think sparc should avoid walking the vmas in
> > arch_validate_prot(). The core code already has the vmas, though not
> > when calling arch_validate_prot(). That's one of the reasons I added
> > arch_validate_flags() with the MTE patches. For sparc, this could be
> > (untested, just copied the arch_validate_prot() code):
>
> I am little uncomfortable with the idea of validating protection bits
> inside the VMA walk loop in do_mprotect_pkey(). When ADI is being
> enabled across multiple VMAs and arch_validate_flags() fails on a VMA
> later, do_mprotect_pkey() will bail out with error leaving ADI enabled
> on earlier VMAs. This will apply to protection bits other than ADI as
> well of course. This becomes a partial failure of mprotect() call. I
> think it should be all or nothing with mprotect() - when one calls
> mprotect() from userspace, either the entire address range passed in
> gets its protection bits updated or none of it does. That requires
> validating protection bits upfront or undoing what earlier iterations of
> VMA walk loop might have done.
I thought the same initially but mprotect() already does this with the
VM_MAY* flag checking. If you ask it for an mprotect() that crosses
multiple vmas and one of them fails, it doesn't roll back the changes to
the prior ones. I considered that a similar approach is fine for MTE
(it's most likely a user error).
--
Catalin
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_spdif: Add support for higher sample rates
From: Nicolin Chen @ 2020-10-12 19:00 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, perex, broonie,
festevam, linux-kernel
In-Reply-To: <1602492582-3558-1-git-send-email-shengjiu.wang@nxp.com>
Hi Shengjiu,
On Mon, Oct 12, 2020 at 04:49:42PM +0800, Shengjiu Wang wrote:
> Add 88200Hz and 176400Hz sample rates support for TX.
> Add 88200Hz, 176400Hz, 192000Hz sample rates support for RX.
>
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Probably should put your own Signed-off at the bottom?
Anyway:
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Khalid Aziz @ 2020-10-12 19:14 UTC (permalink / raw)
To: Catalin Marinas
Cc: Jann Horn, linuxppc-dev, linux-kernel, Christoph Hellwig,
linux-mm, Paul Mackerras, sparclinux, Anthony Yznaga,
Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel
In-Reply-To: <20201012172218.GE6493@gaia>
On 10/12/20 11:22 AM, Catalin Marinas wrote:
> On Mon, Oct 12, 2020 at 11:03:33AM -0600, Khalid Aziz wrote:
>> On 10/10/20 5:09 AM, Catalin Marinas wrote:
>>> On Wed, Oct 07, 2020 at 02:14:09PM -0600, Khalid Aziz wrote:
>>>> On 10/7/20 1:39 AM, Jann Horn wrote:
>>>>> arch_validate_prot() is a hook that can validate whether a given set of
>>>>> protection flags is valid in an mprotect() operation. It is given the set
>>>>> of protection flags and the address being modified.
>>>>>
>>>>> However, the address being modified can currently not actually be used in
>>>>> a meaningful way because:
>>>>>
>>>>> 1. Only the address is given, but not the length, and the operation can
>>>>> span multiple VMAs. Therefore, the callee can't actually tell which
>>>>> virtual address range, or which VMAs, are being targeted.
>>>>> 2. The mmap_lock is not held, meaning that if the callee were to check
>>>>> the VMA at @addr, that VMA would be unrelated to the one the
>>>>> operation is performed on.
>>>>>
>>>>> Currently, custom arch_validate_prot() handlers are defined by
>>>>> arm64, powerpc and sparc.
>>>>> arm64 and powerpc don't care about the address range, they just check the
>>>>> flags against CPU support masks.
>>>>> sparc's arch_validate_prot() attempts to look at the VMA, but doesn't take
>>>>> the mmap_lock.
>>>>>
>>>>> Change the function signature to also take a length, and move the
>>>>> arch_validate_prot() call in mm/mprotect.c down into the locked region.
>>> [...]
>>>> As Chris pointed out, the call to arch_validate_prot() from do_mmap2()
>>>> is made without holding mmap_lock. Lock is not acquired until
>>>> vm_mmap_pgoff(). This variance is uncomfortable but I am more
>>>> uncomfortable forcing all implementations of validate_prot to require
>>>> mmap_lock be held when non-sparc implementations do not have such need
>>>> yet. Since do_mmap2() is in powerpc specific code, for now this patch
>>>> solves a current problem.
>>>
>>> I still think sparc should avoid walking the vmas in
>>> arch_validate_prot(). The core code already has the vmas, though not
>>> when calling arch_validate_prot(). That's one of the reasons I added
>>> arch_validate_flags() with the MTE patches. For sparc, this could be
>>> (untested, just copied the arch_validate_prot() code):
>>
>> I am little uncomfortable with the idea of validating protection bits
>> inside the VMA walk loop in do_mprotect_pkey(). When ADI is being
>> enabled across multiple VMAs and arch_validate_flags() fails on a VMA
>> later, do_mprotect_pkey() will bail out with error leaving ADI enabled
>> on earlier VMAs. This will apply to protection bits other than ADI as
>> well of course. This becomes a partial failure of mprotect() call. I
>> think it should be all or nothing with mprotect() - when one calls
>> mprotect() from userspace, either the entire address range passed in
>> gets its protection bits updated or none of it does. That requires
>> validating protection bits upfront or undoing what earlier iterations of
>> VMA walk loop might have done.
>
> I thought the same initially but mprotect() already does this with the
> VM_MAY* flag checking. If you ask it for an mprotect() that crosses
> multiple vmas and one of them fails, it doesn't roll back the changes to
> the prior ones. I considered that a similar approach is fine for MTE
> (it's most likely a user error).
>
You are right about the current behavior with VM_MAY* flags, but that is
not the right behavior. Adding more cases to this just perpetuates
incorrect behavior. It is not easy to roll back changes after VMAs have
potentially been split/merged which is probably why the current code
simply throws in the towel and returns with partially modified address
space. It is lot easier to do all the checks upfront and then proceed or
not proceed with modifying VMAs. One approach might be to call
arch_validate_flags() in a loop before modifying VMAs and walk all VMAs
with a read lock held. Current code also bails out with ENOMEM if it
finds a hole in the address range and leaves any modifications already
made in place. This is another case where a hole could have been
detected earlier.
--
Khalid
^ permalink raw reply
* Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
From: Ira Weiny @ 2020-10-12 19:53 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-aio, linux-efi, kvm, linux-doc, Peter Zijlstra, linux-mmc,
Dave Hansen, dri-devel, Dave Hansen, target-devel, linux-mtd,
linux-kselftest, linux-f2fs-devel, Thomas Gleixner, drbd-dev,
devel, linux-cifs, linux-nilfs, linux-scsi, linux-nvdimm,
linux-rdma, x86, amd-gfx, linux-afs, Eric Biggers, Ingo Molnar,
intel-wired-lan, kexec, xen-devel, linux-ext4, bpf, Dan Williams,
Fenghua Yu, intel-gfx, ecryptfs, linux-um, reiserfs-devel,
linux-block, linux-bcache, Borislav Petkov, Andy Lutomirski,
Jaegeuk Kim, ceph-devel, io-uring, linux-cachefs, linux-nfs,
linux-mm, linux-ntfs-dev, netdev, linuxppc-dev, samba-technical,
linux-kernel, cluster-devel, linux-fsdevel, Andrew Morton,
linux-erofs, linux-btrfs
In-Reply-To: <20201012164438.GA20115@casper.infradead.org>
On Mon, Oct 12, 2020 at 05:44:38PM +0100, Matthew Wilcox wrote:
> On Mon, Oct 12, 2020 at 09:28:29AM -0700, Dave Hansen wrote:
> > kmap_atomic() is always preferred over kmap()/kmap_thread().
> > kmap_atomic() is _much_ more lightweight since its TLB invalidation is
> > always CPU-local and never broadcast.
> >
> > So, basically, unless you *must* sleep while the mapping is in place,
> > kmap_atomic() is preferred.
>
> But kmap_atomic() disables preemption, so the _ideal_ interface would map
> it only locally, then on preemption make it global. I don't even know
> if that _can_ be done. But this email makes it seem like kmap_atomic()
> has no downsides.
And that is IIUC what Thomas was trying to solve.
Also, Linus brought up that kmap_atomic() has quirks in nesting.[1]
From what I can see all of these discussions support the need to have something
between kmap() and kmap_atomic().
However, the reason behind converting call sites to kmap_thread() are different
between Thomas' patch set and mine. Both require more kmap granularity.
However, they do so with different reasons and underlying implementations but
with the _same_ resulting semantics; a thread local mapping which is
preemptable.[2] Therefore they each focus on changing different call sites.
While this patch set is huge I think it serves a valuable purpose to identify a
large number of call sites which are candidates for this new semantic.
Ira
[1] https://lore.kernel.org/lkml/CAHk-=wgbmwsTOKs23Z=71EBTrULoeaH2U3TNqT2atHEWvkBKdw@mail.gmail.com/
[2] It is important to note these implementations are not incompatible with
each other. So I don't see yet another 'kmap_something()' being required.
^ permalink raw reply
* Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
From: Matthew Wilcox @ 2020-10-12 20:02 UTC (permalink / raw)
To: Ira Weiny
Cc: linux-aio, linux-efi, kvm, linux-doc, Peter Zijlstra, linux-mmc,
Dave Hansen, dri-devel, Dave Hansen, target-devel, linux-mtd,
linux-kselftest, linux-f2fs-devel, Thomas Gleixner, drbd-dev,
devel, linux-cifs, linux-nilfs, linux-scsi, linux-nvdimm,
linux-rdma, x86, amd-gfx, linux-afs, Eric Biggers, Ingo Molnar,
intel-wired-lan, kexec, xen-devel, linux-ext4, bpf, Dan Williams,
Fenghua Yu, intel-gfx, ecryptfs, linux-um, reiserfs-devel,
linux-block, linux-bcache, Borislav Petkov, Andy Lutomirski,
Jaegeuk Kim, ceph-devel, io-uring, linux-cachefs, linux-nfs,
linux-mm, linux-ntfs-dev, netdev, linuxppc-dev, samba-technical,
linux-kernel, cluster-devel, linux-fsdevel, Andrew Morton,
linux-erofs, linux-btrfs
In-Reply-To: <20201012195354.GC2046448@iweiny-DESK2.sc.intel.com>
On Mon, Oct 12, 2020 at 12:53:54PM -0700, Ira Weiny wrote:
> On Mon, Oct 12, 2020 at 05:44:38PM +0100, Matthew Wilcox wrote:
> > On Mon, Oct 12, 2020 at 09:28:29AM -0700, Dave Hansen wrote:
> > > kmap_atomic() is always preferred over kmap()/kmap_thread().
> > > kmap_atomic() is _much_ more lightweight since its TLB invalidation is
> > > always CPU-local and never broadcast.
> > >
> > > So, basically, unless you *must* sleep while the mapping is in place,
> > > kmap_atomic() is preferred.
> >
> > But kmap_atomic() disables preemption, so the _ideal_ interface would map
> > it only locally, then on preemption make it global. I don't even know
> > if that _can_ be done. But this email makes it seem like kmap_atomic()
> > has no downsides.
>
> And that is IIUC what Thomas was trying to solve.
>
> Also, Linus brought up that kmap_atomic() has quirks in nesting.[1]
>
> >From what I can see all of these discussions support the need to have something
> between kmap() and kmap_atomic().
>
> However, the reason behind converting call sites to kmap_thread() are different
> between Thomas' patch set and mine. Both require more kmap granularity.
> However, they do so with different reasons and underlying implementations but
> with the _same_ resulting semantics; a thread local mapping which is
> preemptable.[2] Therefore they each focus on changing different call sites.
>
> While this patch set is huge I think it serves a valuable purpose to identify a
> large number of call sites which are candidates for this new semantic.
Yes, I agree. My problem with this patch-set is that it ties it to
some Intel feature that almost nobody cares about. Maybe we should
care about it, but you didn't try very hard to make anyone care about
it in the cover letter.
For a future patch-set, I'd like to see you just introduce the new
API. Then you can optimise the Intel implementation of it afterwards.
Those patch-sets have entirely different reviewers.
^ permalink raw reply
* Re: [PATCH] powerpc/features: Remove CPU_FTR_NODSISRALIGN
From: kernel test robot @ 2020-10-12 20:10 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, kbuild-all, linux-kernel
In-Reply-To: <0346768708b69bdbfec82f6e5b0364962b9b6932.1602489812.git.christophe.leroy@csgroup.eu>
[-- Attachment #1: Type: text/plain, Size: 19524 bytes --]
Hi Christophe,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.9 next-20201012]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Christophe-Leroy/powerpc-features-Remove-CPU_FTR_NODSISRALIGN/20201012-160453
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-randconfig-r024-20201012 (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/0day-ci/linux/commit/9fd60382753ab51a10fb3d11ea7423491e32122e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Christophe-Leroy/powerpc-features-Remove-CPU_FTR_NODSISRALIGN/20201012-160453
git checkout 9fd60382753ab51a10fb3d11ea7423491e32122e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
WARNING: unmet direct dependencies detected for HOTPLUG_CPU
Depends on SMP && (PPC_PSERIES || PPC_PMAC || PPC_POWERNV || FSL_SOC_BOOKE
Selected by
- PM_SLEEP_SMP && SMP && (ARCH_SUSPEND_POSSIBLE || ARCH_HIBERNATION_POSSIBLE && PM_SLEEP
In file included from arch/powerpc/include/asm/synch.h:6,
from arch/powerpc/include/asm/bitops.h:43,
from include/linux/bitops.h:29,
from include/linux/kernel.h:12,
from include/asm-generic/bug.h:20,
from arch/powerpc/include/asm/bug.h:109,
from include/linux/bug.h:5,
from include/linux/page-flags.h:10,
from kernel/bounds.c:10:
>> arch/powerpc/include/asm/cputable.h:411:47: error: 'CPU_FTR_NODSISRALIGN' undeclared here (not in a function)
411 | #define CPU_FTRS_GENERIC_32 (CPU_FTR_COMMON | CPU_FTR_NODSISRALIGN)
| ^~~~~~~~~~~~~~~~~~~~
arch/powerpc/include/asm/cputable.h:523:6: note: in expansion of macro 'CPU_FTRS_GENERIC_32'
523 | CPU_FTRS_GENERIC_32 |
| ^~~~~~~~~~~~~~~~~~~
Makefile arch block certs crypto drivers fs include init ipc kernel lib mm net scripts security sound source usr virt [scripts/Makefile.build:117: kernel/bounds.s] Error 1
Target '__build' not remade because of errors.
Makefile arch block certs crypto drivers fs include init ipc kernel lib mm net scripts security sound source usr virt [Makefile:1202: prepare0] Error 2
Target 'prepare' not remade because of errors.
make: Makefile arch block certs crypto drivers fs include init ipc kernel lib mm net scripts security sound source usr virt [Makefile:185: __sub-make] Error 2
make: Target 'prepare' not remade because of errors.
vim +/CPU_FTR_NODSISRALIGN +411 arch/powerpc/include/asm/cputable.h
10b35d9978ac35 include/asm-powerpc/cputable.h Kumar Gala 2005-09-23 296
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 297 #define CPU_FTRS_603 (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \
385e89d5b20f5a arch/powerpc/include/asm/cputable.h Christophe Leroy 2018-11-28 298 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_PPC_LE | CPU_FTR_NOEXECUTE)
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 299 #define CPU_FTRS_604 (CPU_FTR_COMMON | CPU_FTR_PPC_LE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 300 #define CPU_FTRS_740_NOTAU (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 301 CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_L2CR | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 302 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_PPC_LE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 303 #define CPU_FTRS_740 (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 304 CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_L2CR | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 305 CPU_FTR_TAU | CPU_FTR_MAYBE_CAN_NAP | \
fab5db97e44f76 include/asm-powerpc/cputable.h Paul Mackerras 2006-06-07 306 CPU_FTR_PPC_LE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 307 #define CPU_FTRS_750 (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 308 CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_L2CR | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 309 CPU_FTR_TAU | CPU_FTR_MAYBE_CAN_NAP | \
fab5db97e44f76 include/asm-powerpc/cputable.h Paul Mackerras 2006-06-07 310 CPU_FTR_PPC_LE)
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 311 #define CPU_FTRS_750CL (CPU_FTRS_750)
b6f41cc8304ce0 include/asm-powerpc/cputable.h Josh Boyer 2007-07-03 312 #define CPU_FTRS_750FX1 (CPU_FTRS_750 | CPU_FTR_DUAL_PLL_750FX | CPU_FTR_NO_DPM)
b6f41cc8304ce0 include/asm-powerpc/cputable.h Josh Boyer 2007-07-03 313 #define CPU_FTRS_750FX2 (CPU_FTRS_750 | CPU_FTR_NO_DPM)
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 314 #define CPU_FTRS_750FX (CPU_FTRS_750 | CPU_FTR_DUAL_PLL_750FX)
b6f41cc8304ce0 include/asm-powerpc/cputable.h Josh Boyer 2007-07-03 315 #define CPU_FTRS_750GX (CPU_FTRS_750FX)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 316 #define CPU_FTRS_7400_NOTAU (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 317 CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_L2CR | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 318 CPU_FTR_ALTIVEC_COMP | \
fab5db97e44f76 include/asm-powerpc/cputable.h Paul Mackerras 2006-06-07 319 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_PPC_LE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 320 #define CPU_FTRS_7400 (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 321 CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_L2CR | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 322 CPU_FTR_TAU | CPU_FTR_ALTIVEC_COMP | \
fab5db97e44f76 include/asm-powerpc/cputable.h Paul Mackerras 2006-06-07 323 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_PPC_LE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 324 #define CPU_FTRS_7450_20 (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 325 CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 326 CPU_FTR_L3CR | CPU_FTR_SPEC7450 | \
b64f87c16f3c00 include/asm-powerpc/cputable.h Becky Bruce 2007-11-10 327 CPU_FTR_NEED_COHERENT | CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 328 #define CPU_FTRS_7450_21 (CPU_FTR_COMMON | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 329 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 330 CPU_FTR_L3CR | CPU_FTR_SPEC7450 | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 331 CPU_FTR_NAP_DISABLE_L2_PR | CPU_FTR_L3_DISABLE_NAP | \
b64f87c16f3c00 include/asm-powerpc/cputable.h Becky Bruce 2007-11-10 332 CPU_FTR_NEED_COHERENT | CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 333 #define CPU_FTRS_7450_23 (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 334 CPU_FTR_NEED_PAIRED_STWCX | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 335 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 336 CPU_FTR_L3CR | CPU_FTR_SPEC7450 | \
fab5db97e44f76 include/asm-powerpc/cputable.h Paul Mackerras 2006-06-07 337 CPU_FTR_NAP_DISABLE_L2_PR | CPU_FTR_NEED_COHERENT | CPU_FTR_PPC_LE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 338 #define CPU_FTRS_7455_1 (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 339 CPU_FTR_NEED_PAIRED_STWCX | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 340 CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | CPU_FTR_L3CR | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 341 CPU_FTR_SPEC7450 | CPU_FTR_NEED_COHERENT | CPU_FTR_PPC_LE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 342 #define CPU_FTRS_7455_20 (CPU_FTR_COMMON | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 343 CPU_FTR_NEED_PAIRED_STWCX | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 344 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 345 CPU_FTR_L3CR | CPU_FTR_SPEC7450 | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 346 CPU_FTR_NAP_DISABLE_L2_PR | CPU_FTR_L3_DISABLE_NAP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 347 CPU_FTR_NEED_COHERENT | CPU_FTR_PPC_LE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 348 #define CPU_FTRS_7455 (CPU_FTR_COMMON | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 349 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 350 CPU_FTR_L3CR | CPU_FTR_SPEC7450 | CPU_FTR_NAP_DISABLE_L2_PR | \
b64f87c16f3c00 include/asm-powerpc/cputable.h Becky Bruce 2007-11-10 351 CPU_FTR_NEED_COHERENT | CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 352 #define CPU_FTRS_7447_10 (CPU_FTR_COMMON | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 353 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 354 CPU_FTR_L3CR | CPU_FTR_SPEC7450 | CPU_FTR_NAP_DISABLE_L2_PR | \
b64f87c16f3c00 include/asm-powerpc/cputable.h Becky Bruce 2007-11-10 355 CPU_FTR_NEED_COHERENT | CPU_FTR_NO_BTIC | CPU_FTR_PPC_LE | \
b64f87c16f3c00 include/asm-powerpc/cputable.h Becky Bruce 2007-11-10 356 CPU_FTR_NEED_PAIRED_STWCX)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 357 #define CPU_FTRS_7447 (CPU_FTR_COMMON | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 358 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 359 CPU_FTR_L3CR | CPU_FTR_SPEC7450 | CPU_FTR_NAP_DISABLE_L2_PR | \
b64f87c16f3c00 include/asm-powerpc/cputable.h Becky Bruce 2007-11-10 360 CPU_FTR_NEED_COHERENT | CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 361 #define CPU_FTRS_7447A (CPU_FTR_COMMON | \
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 362 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 363 CPU_FTR_SPEC7450 | CPU_FTR_NAP_DISABLE_L2_PR | \
b64f87c16f3c00 include/asm-powerpc/cputable.h Becky Bruce 2007-11-10 364 CPU_FTR_NEED_COHERENT | CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 365 #define CPU_FTRS_7448 (CPU_FTR_COMMON | \
3d372548b4af1a include/asm-powerpc/cputable.h James.Yang 2007-05-02 366 CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_L2CR | CPU_FTR_ALTIVEC_COMP | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 367 CPU_FTR_SPEC7450 | CPU_FTR_NAP_DISABLE_L2_PR | \
b64f87c16f3c00 include/asm-powerpc/cputable.h Becky Bruce 2007-11-10 368 CPU_FTR_PPC_LE | CPU_FTR_NEED_PAIRED_STWCX)
385e89d5b20f5a arch/powerpc/include/asm/cputable.h Christophe Leroy 2018-11-28 369 #define CPU_FTRS_82XX (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | CPU_FTR_NOEXECUTE)
11af1192b75307 include/asm-powerpc/cputable.h Scott Wood 2007-09-14 370 #define CPU_FTRS_G2_LE (CPU_FTR_COMMON | CPU_FTR_MAYBE_CAN_DOZE | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 371 CPU_FTR_MAYBE_CAN_NAP)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 372 #define CPU_FTRS_E300 (CPU_FTR_MAYBE_CAN_DOZE | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 373 CPU_FTR_MAYBE_CAN_NAP | \
385e89d5b20f5a arch/powerpc/include/asm/cputable.h Christophe Leroy 2018-11-28 374 CPU_FTR_COMMON | CPU_FTR_NOEXECUTE)
4508dc21feb189 include/asm-powerpc/cputable.h David Gibson 2007-06-13 375 #define CPU_FTRS_E300C2 (CPU_FTR_MAYBE_CAN_DOZE | \
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 376 CPU_FTR_MAYBE_CAN_NAP | \
385e89d5b20f5a arch/powerpc/include/asm/cputable.h Christophe Leroy 2018-11-28 377 CPU_FTR_COMMON | CPU_FTR_FPU_UNAVAILABLE | CPU_FTR_NOEXECUTE)
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 378 #define CPU_FTRS_CLASSIC32 (CPU_FTR_COMMON)
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 379 #define CPU_FTRS_8XX (CPU_FTR_NOEXECUTE)
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 380 #define CPU_FTRS_40X (CPU_FTR_NOEXECUTE)
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 381 #define CPU_FTRS_44X (CPU_FTR_NOEXECUTE)
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 382 #define CPU_FTRS_440x6 (CPU_FTR_NOEXECUTE | \
6d2170be456129 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 383 CPU_FTR_INDEXED_DCR)
e7f75ad01d5902 arch/powerpc/include/asm/cputable.h Dave Kleikamp 2010-03-05 384 #define CPU_FTRS_47X (CPU_FTRS_440x6)
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 385 #define CPU_FTRS_E200 (CPU_FTR_SPE_COMP | \
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 386 CPU_FTR_COHERENT_ICACHE | \
e0291f1decd6e8 arch/powerpc/include/asm/cputable.h Christophe Leroy 2019-08-26 387 CPU_FTR_NOEXECUTE | \
52b066fa4e9cbf arch/powerpc/include/asm/cputable.h Scott Wood 2011-12-20 388 CPU_FTR_DEBUG_LVL_EXC)
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 389 #define CPU_FTRS_E500 (CPU_FTR_MAYBE_CAN_DOZE | \
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 390 CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | \
8309ce7280536b arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-12 391 CPU_FTR_NOEXECUTE)
c0d64cf9fefd58 arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 392 #define CPU_FTRS_E500_2 (CPU_FTR_MAYBE_CAN_DOZE | \
7c03d653cd2577 arch/powerpc/include/asm/cputable.h Benjamin Herrenschmidt 2008-12-18 393 CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | \
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 394 CPU_FTR_NOEXECUTE)
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 395 #define CPU_FTRS_E500MC ( \
dd0efb3f11cc0a arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 396 CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
73196cd364a2d9 arch/powerpc/include/asm/cputable.h Scott Wood 2011-12-20 397 CPU_FTR_DBELL | CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV)
d52459ca304743 arch/powerpc/include/asm/cputable.h Scott Wood 2013-07-23 398 /*
d52459ca304743 arch/powerpc/include/asm/cputable.h Scott Wood 2013-07-23 399 * e5500/e6500 erratum A-006958 is a timebase bug that can use the
d52459ca304743 arch/powerpc/include/asm/cputable.h Scott Wood 2013-07-23 400 * same workaround as CPU_FTR_CELL_TB_BUG.
d52459ca304743 arch/powerpc/include/asm/cputable.h Scott Wood 2013-07-23 401 */
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 402 #define CPU_FTRS_E5500 ( \
dd0efb3f11cc0a arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 403 CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
d36b4c4f3cc6ca arch/powerpc/include/asm/cputable.h Kumar Gala 2011-04-06 404 CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
d52459ca304743 arch/powerpc/include/asm/cputable.h Scott Wood 2013-07-23 405 CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_CELL_TB_BUG)
9fd60382753ab5 arch/powerpc/include/asm/cputable.h Christophe Leroy 2020-10-12 406 #define CPU_FTRS_E6500 ( \
dd0efb3f11cc0a arch/powerpc/include/asm/cputable.h Paul Mackerras 2018-03-20 407 CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
10241842fbe900 arch/powerpc/include/asm/cputable.h Kumar Gala 2011-11-06 408 CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
d52459ca304743 arch/powerpc/include/asm/cputable.h Scott Wood 2013-07-23 409 CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_ALTIVEC_COMP | \
e16c8765533a15 arch/powerpc/include/asm/cputable.h Andy Fleming 2011-12-08 410 CPU_FTR_CELL_TB_BUG | CPU_FTR_SMT)
7c92943c7b6c42 include/asm-powerpc/cputable.h Stephen Rothwell 2006-03-23 @411 #define CPU_FTRS_GENERIC_32 (CPU_FTR_COMMON | CPU_FTR_NODSISRALIGN)
0b8e2e131094d1 include/asm-powerpc/cputable.h Michael Ellerman 2006-11-23 412
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25244 bytes --]
^ permalink raw reply
* Re: [PATCH RFC PKS/PMEM 22/58] fs/f2fs: Utilize new kmap_thread()
From: Ira Weiny @ 2020-10-12 23:31 UTC (permalink / raw)
To: Matthew Wilcox
Cc: linux-aio, linux-efi, kvm, linux-doc, Peter Zijlstra, linux-mmc,
Dave Hansen, dri-devel, Dave Hansen, target-devel, linux-mtd,
linux-kselftest, linux-f2fs-devel, Thomas Gleixner, drbd-dev,
devel, linux-cifs, linux-nilfs, linux-scsi, linux-nvdimm,
linux-rdma, x86, amd-gfx, linux-afs, Eric Biggers, Ingo Molnar,
intel-wired-lan, kexec, xen-devel, linux-ext4, bpf, Dan Williams,
Fenghua Yu, intel-gfx, ecryptfs, linux-um, reiserfs-devel,
linux-block, linux-bcache, Borislav Petkov, Andy Lutomirski,
Jaegeuk Kim, ceph-devel, io-uring, linux-cachefs, linux-nfs,
linux-mm, linux-ntfs-dev, netdev, linuxppc-dev, samba-technical,
linux-kernel, cluster-devel, linux-fsdevel, Andrew Morton,
linux-erofs, linux-btrfs
In-Reply-To: <20201012200254.GB20115@casper.infradead.org>
On Mon, Oct 12, 2020 at 09:02:54PM +0100, Matthew Wilcox wrote:
> On Mon, Oct 12, 2020 at 12:53:54PM -0700, Ira Weiny wrote:
> > On Mon, Oct 12, 2020 at 05:44:38PM +0100, Matthew Wilcox wrote:
> > > On Mon, Oct 12, 2020 at 09:28:29AM -0700, Dave Hansen wrote:
> > > > kmap_atomic() is always preferred over kmap()/kmap_thread().
> > > > kmap_atomic() is _much_ more lightweight since its TLB invalidation is
> > > > always CPU-local and never broadcast.
> > > >
> > > > So, basically, unless you *must* sleep while the mapping is in place,
> > > > kmap_atomic() is preferred.
> > >
> > > But kmap_atomic() disables preemption, so the _ideal_ interface would map
> > > it only locally, then on preemption make it global. I don't even know
> > > if that _can_ be done. But this email makes it seem like kmap_atomic()
> > > has no downsides.
> >
> > And that is IIUC what Thomas was trying to solve.
> >
> > Also, Linus brought up that kmap_atomic() has quirks in nesting.[1]
> >
> > >From what I can see all of these discussions support the need to have something
> > between kmap() and kmap_atomic().
> >
> > However, the reason behind converting call sites to kmap_thread() are different
> > between Thomas' patch set and mine. Both require more kmap granularity.
> > However, they do so with different reasons and underlying implementations but
> > with the _same_ resulting semantics; a thread local mapping which is
> > preemptable.[2] Therefore they each focus on changing different call sites.
> >
> > While this patch set is huge I think it serves a valuable purpose to identify a
> > large number of call sites which are candidates for this new semantic.
>
> Yes, I agree. My problem with this patch-set is that it ties it to
> some Intel feature that almost nobody cares about.
I humbly disagree. At this level the only thing this is tied to is the idea
that there are additional memory protections available which can be enabled
quickly on a per-thread basis. PKS on Intel is but 1 implementation of that.
Even the kmap code only has knowledge that there is something which needs to be
done special on a devm page.
>
> Maybe we should
> care about it, but you didn't try very hard to make anyone care about
> it in the cover letter.
Ok my bad. We have customers who care very much about restricting access to
the PMEM pages to prevent bugs in the kernel from causing permanent damage to
their data/file systems. I'll reword the cover letter better.
>
> For a future patch-set, I'd like to see you just introduce the new
> API. Then you can optimise the Intel implementation of it afterwards.
> Those patch-sets have entirely different reviewers.
I considered doing this. But this seemed more logical because the feature is
being driven by PMEM which is behind the kmap interface not by the users of the
API.
I can introduce a patch set with a kmap_thread() call which does nothing if
that is more palatable but it seems wrong to me to do so.
Ira
^ permalink raw reply
* Re: [PATCH] ASoC: fsl_spdif: Add support for higher sample rates
From: Shengjiu Wang @ 2020-10-13 2:40 UTC (permalink / raw)
To: Nicolin Chen
Cc: alsa-devel, Timur Tabi, Xiubo Li, Fabio Estevam, Shengjiu Wang,
Takashi Iwai, linux-kernel, Mark Brown, linuxppc-dev
In-Reply-To: <20201012190037.GB17643@Asurada-Nvidia>
On Tue, Oct 13, 2020 at 3:09 AM Nicolin Chen <nicoleotsuka@gmail.com> wrote:
>
> Hi Shengjiu,
>
> On Mon, Oct 12, 2020 at 04:49:42PM +0800, Shengjiu Wang wrote:
> > Add 88200Hz and 176400Hz sample rates support for TX.
> > Add 88200Hz, 176400Hz, 192000Hz sample rates support for RX.
> >
> > Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> > Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
>
> Probably should put your own Signed-off at the bottom?
will update in v2.
>
> Anyway:
> Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
^ permalink raw reply
* [PATCH v2] ASoC: fsl_spdif: Add support for higher sample rates
From: Shengjiu Wang @ 2020-10-13 2:49 UTC (permalink / raw)
To: timur, nicoleotsuka, Xiubo.Lee, festevam, broonie, perex, tiwai,
alsa-devel
Cc: linuxppc-dev, linux-kernel
Add 88200Hz and 176400Hz sample rates support for TX.
Add 88200Hz, 176400Hz, 192000Hz sample rates support for RX.
Signed-off-by: Viorel Suman <viorel.suman@nxp.com>
Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
changes in v2
- reorder the signed-off
- add acked-by Nicolin
sound/soc/fsl/fsl_spdif.c | 16 +++++++++++++---
sound/soc/fsl/fsl_spdif.h | 9 ++++++++-
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/sound/soc/fsl/fsl_spdif.c b/sound/soc/fsl/fsl_spdif.c
index 4d14f4076ead..1c030142556a 100644
--- a/sound/soc/fsl/fsl_spdif.c
+++ b/sound/soc/fsl/fsl_spdif.c
@@ -459,10 +459,18 @@ static int spdif_set_sample_rate(struct snd_pcm_substream *substream,
rate = SPDIF_TXRATE_48000;
csfs = IEC958_AES3_CON_FS_48000;
break;
+ case 88200:
+ rate = SPDIF_TXRATE_88200;
+ csfs = IEC958_AES3_CON_FS_88200;
+ break;
case 96000:
rate = SPDIF_TXRATE_96000;
csfs = IEC958_AES3_CON_FS_96000;
break;
+ case 176400:
+ rate = SPDIF_TXRATE_176400;
+ csfs = IEC958_AES3_CON_FS_176400;
+ break;
case 192000:
rate = SPDIF_TXRATE_192000;
csfs = IEC958_AES3_CON_FS_192000;
@@ -857,7 +865,7 @@ static int fsl_spdif_rxrate_info(struct snd_kcontrol *kcontrol,
uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
uinfo->count = 1;
uinfo->value.integer.min = 16000;
- uinfo->value.integer.max = 96000;
+ uinfo->value.integer.max = 192000;
return 0;
}
@@ -1175,7 +1183,8 @@ static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
struct clk *clk, u64 savesub,
enum spdif_txrate index, bool round)
{
- static const u32 rate[] = { 32000, 44100, 48000, 96000, 192000 };
+ static const u32 rate[] = { 32000, 44100, 48000, 88200, 96000, 176400,
+ 192000, };
bool is_sysclk = clk_is_match(clk, spdif_priv->sysclk);
u64 rate_ideal, rate_actual, sub;
u32 arate;
@@ -1235,7 +1244,8 @@ static u32 fsl_spdif_txclk_caldiv(struct fsl_spdif_priv *spdif_priv,
static int fsl_spdif_probe_txclk(struct fsl_spdif_priv *spdif_priv,
enum spdif_txrate index)
{
- static const u32 rate[] = { 32000, 44100, 48000, 96000, 192000 };
+ static const u32 rate[] = { 32000, 44100, 48000, 88200, 96000, 176400,
+ 192000, };
struct platform_device *pdev = spdif_priv->pdev;
struct device *dev = &pdev->dev;
u64 savesub = 100000, ret;
diff --git a/sound/soc/fsl/fsl_spdif.h b/sound/soc/fsl/fsl_spdif.h
index e6c61e07bc1a..d5f1dfd58740 100644
--- a/sound/soc/fsl/fsl_spdif.h
+++ b/sound/soc/fsl/fsl_spdif.h
@@ -163,7 +163,9 @@ enum spdif_txrate {
SPDIF_TXRATE_32000 = 0,
SPDIF_TXRATE_44100,
SPDIF_TXRATE_48000,
+ SPDIF_TXRATE_88200,
SPDIF_TXRATE_96000,
+ SPDIF_TXRATE_176400,
SPDIF_TXRATE_192000,
};
#define SPDIF_TXRATE_MAX (SPDIF_TXRATE_192000 + 1)
@@ -177,15 +179,20 @@ enum spdif_txrate {
#define FSL_SPDIF_RATES_PLAYBACK (SNDRV_PCM_RATE_32000 | \
SNDRV_PCM_RATE_44100 | \
SNDRV_PCM_RATE_48000 | \
+ SNDRV_PCM_RATE_88200 | \
SNDRV_PCM_RATE_96000 | \
+ SNDRV_PCM_RATE_176400 | \
SNDRV_PCM_RATE_192000)
#define FSL_SPDIF_RATES_CAPTURE (SNDRV_PCM_RATE_16000 | \
SNDRV_PCM_RATE_32000 | \
SNDRV_PCM_RATE_44100 | \
SNDRV_PCM_RATE_48000 | \
+ SNDRV_PCM_RATE_88200 | \
SNDRV_PCM_RATE_64000 | \
- SNDRV_PCM_RATE_96000)
+ SNDRV_PCM_RATE_96000 | \
+ SNDRV_PCM_RATE_176400 | \
+ SNDRV_PCM_RATE_192000)
#define FSL_SPDIF_FORMATS_PLAYBACK (SNDRV_PCM_FMTBIT_S16_LE | \
SNDRV_PCM_FMTBIT_S20_3LE | \
--
2.27.0
^ permalink raw reply related
* [PATCH 2/2] selftests/powerpc: Make alignment handler test P9N DD2.1 vector CI load workaround
From: Michael Neuling @ 2020-10-13 4:37 UTC (permalink / raw)
To: mpe; +Cc: mikey, linuxppc-dev
In-Reply-To: <20201013043741.743413-1-mikey@neuling.org>
alignment_handler currently only tests the unaligned cases but it can
also be useful for testing the workaround for the P9N DD2.1 vector CI
load issue fixed by p9_hmi_special_emu(). This workaround was
introduced in 5080332c2c89 ("powerpc/64s: Add workaround for P9 vector
CI load issue").
This changes the loop to start from offset 0 rather than 1 so that we
test the kernel emulation in p9_hmi_special_emu().
Signed-off-by: Michael Neuling <mikey@neuling.org>
---
.../selftests/powerpc/alignment/alignment_handler.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/powerpc/alignment/alignment_handler.c b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
index 2a0503bc7e49..cb53a8b777e6 100644
--- a/tools/testing/selftests/powerpc/alignment/alignment_handler.c
+++ b/tools/testing/selftests/powerpc/alignment/alignment_handler.c
@@ -266,8 +266,12 @@ int do_test(char *test_name, void (*test_func)(char *, char *))
}
rc = 0;
- /* offset = 0 no alignment fault, so skip */
- for (offset = 1; offset < 16; offset++) {
+ /*
+ * offset = 0 is aligned but tests the workaround for the P9N
+ * DD2.1 vector CI load issue (see 5080332c2c89 "powerpc/64s:
+ * Add workaround for P9 vector CI load issue")
+ */
+ for (offset = 0; offset < 16; offset++) {
width = 16; /* vsx == 16 bytes */
r = 0;
--
2.26.2
^ permalink raw reply related
* [PATCH 1/2] powerpc: Fix user data corruption with P9N DD2.1 VSX CI load workaround emulation
From: Michael Neuling @ 2020-10-13 4:37 UTC (permalink / raw)
To: mpe; +Cc: mikey, linuxppc-dev
__get_user_atomic_128_aligned() stores to kaddr using stvx which is a
VMX store instruction, hence kaddr must be 16 byte aligned otherwise
the store won't occur as expected.
Unfortunately when we call __get_user_atomic_128_aligned() in
p9_hmi_special_emu(), the buffer we pass as kaddr (ie. vbuf) isn't
guaranteed to be 16B aligned. This means that the write to vbuf in
__get_user_atomic_128_aligned() has the bottom bits of the address
truncated. This results in other local variables being
overwritten. Also vbuf will not contain the correct data which results
in the userspace emulation being wrong and hence user data corruption.
In the past we've been mostly lucky as vbuf has ended up aligned but
this is fragile and isn't always true. CONFIG_STACKPROTECTOR in
particular can change the stack arrangement enough that our luck runs
out.
This issue only occurs on POWER9 Nimbus <= DD2.1 bare metal.
The fix is to align vbuf to a 16 byte boundary.
Fixes: 5080332c2c89 ("powerpc/64s: Add workaround for P9 vector CI load issue")
Signed-off-by: Michael Neuling <mikey@neuling.org>
Cc: <stable@vger.kernel.org> # v4.15+
---
arch/powerpc/kernel/traps.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index c5f39f13e96e..5006dcbe1d9f 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -885,7 +885,7 @@ static void p9_hmi_special_emu(struct pt_regs *regs)
{
unsigned int ra, rb, t, i, sel, instr, rc;
const void __user *addr;
- u8 vbuf[16], *vdst;
+ u8 vbuf[16] __aligned(16), *vdst;
unsigned long ea, msr, msr_mask;
bool swap;
--
2.26.2
^ permalink raw reply related
* Re: [PATCH] powerpc/features: Remove CPU_FTR_NODSISRALIGN
From: Aneesh Kumar K.V @ 2020-10-13 7:23 UTC (permalink / raw)
To: Christophe Leroy, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <0346768708b69bdbfec82f6e5b0364962b9b6932.1602489812.git.christophe.leroy@csgroup.eu>
Christophe Leroy <christophe.leroy@csgroup.eu> writes:
> CPU_FTR_NODSISRALIGN has not been used since
> commit 31bfdb036f12 ("powerpc: Use instruction emulation
> infrastructure to handle alignment faults")
>
> Remove it.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/cputable.h | 22 ++++++++++------------
> arch/powerpc/kernel/dt_cpu_ftrs.c | 8 --------
> arch/powerpc/kernel/prom.c | 2 +-
> 3 files changed, 11 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
> index 9780c55f9811..accdc1286f37 100644
> --- a/arch/powerpc/include/asm/cputable.h
> +++ b/arch/powerpc/include/asm/cputable.h
> @@ -137,7 +137,6 @@ static inline void cpu_feature_keys_init(void) { }
> #define CPU_FTR_DBELL ASM_CONST(0x00000004)
> #define CPU_FTR_CAN_NAP ASM_CONST(0x00000008)
> #define CPU_FTR_DEBUG_LVL_EXC ASM_CONST(0x00000010)
> -#define CPU_FTR_NODSISRALIGN ASM_CONST(0x00000020)
> #define CPU_FTR_FPU_UNAVAILABLE ASM_CONST(0x00000040)
> #define CPU_FTR_LWSYNC ASM_CONST(0x00000080)
> #define CPU_FTR_NOEXECUTE ASM_CONST(0x00000100)
> @@ -219,7 +218,7 @@ static inline void cpu_feature_keys_init(void) { }
>
> #ifndef __ASSEMBLY__
>
> -#define CPU_FTR_PPCAS_ARCH_V2 (CPU_FTR_NOEXECUTE | CPU_FTR_NODSISRALIGN)
> +#define CPU_FTR_PPCAS_ARCH_V2 (CPU_FTR_NOEXECUTE)
>
> #define MMU_FTR_PPCAS_ARCH_V2 (MMU_FTR_TLBIEL | MMU_FTR_16M_PAGE)
>
> @@ -378,33 +377,33 @@ static inline void cpu_feature_keys_init(void) { }
> CPU_FTR_COMMON | CPU_FTR_FPU_UNAVAILABLE | CPU_FTR_NOEXECUTE)
> #define CPU_FTRS_CLASSIC32 (CPU_FTR_COMMON)
> #define CPU_FTRS_8XX (CPU_FTR_NOEXECUTE)
> -#define CPU_FTRS_40X (CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
> -#define CPU_FTRS_44X (CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
> -#define CPU_FTRS_440x6 (CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE | \
> +#define CPU_FTRS_40X (CPU_FTR_NOEXECUTE)
> +#define CPU_FTRS_44X (CPU_FTR_NOEXECUTE)
> +#define CPU_FTRS_440x6 (CPU_FTR_NOEXECUTE | \
> CPU_FTR_INDEXED_DCR)
> #define CPU_FTRS_47X (CPU_FTRS_440x6)
> #define CPU_FTRS_E200 (CPU_FTR_SPE_COMP | \
> - CPU_FTR_NODSISRALIGN | CPU_FTR_COHERENT_ICACHE | \
> + CPU_FTR_COHERENT_ICACHE | \
> CPU_FTR_NOEXECUTE | \
> CPU_FTR_DEBUG_LVL_EXC)
> #define CPU_FTRS_E500 (CPU_FTR_MAYBE_CAN_DOZE | \
> - CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN | \
> + CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | \
> CPU_FTR_NOEXECUTE)
> #define CPU_FTRS_E500_2 (CPU_FTR_MAYBE_CAN_DOZE | \
> CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | \
> - CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
> -#define CPU_FTRS_E500MC (CPU_FTR_NODSISRALIGN | \
> + CPU_FTR_NOEXECUTE)
> +#define CPU_FTRS_E500MC ( \
> CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
> CPU_FTR_DBELL | CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV)
> /*
> * e5500/e6500 erratum A-006958 is a timebase bug that can use the
> * same workaround as CPU_FTR_CELL_TB_BUG.
> */
> -#define CPU_FTRS_E5500 (CPU_FTR_NODSISRALIGN | \
> +#define CPU_FTRS_E5500 ( \
> CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
> CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_CELL_TB_BUG)
> -#define CPU_FTRS_E6500 (CPU_FTR_NODSISRALIGN | \
> +#define CPU_FTRS_E6500 ( \
> CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
> CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
> CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_ALTIVEC_COMP | \
> @@ -554,7 +553,6 @@ enum {
> #define CPU_FTRS_DT_CPU_BASE \
> (CPU_FTR_LWSYNC | \
> CPU_FTR_FPU_UNAVAILABLE | \
> - CPU_FTR_NODSISRALIGN | \
> CPU_FTR_NOEXECUTE | \
> CPU_FTR_COHERENT_ICACHE | \
> CPU_FTR_STCX_CHECKS_ADDRESS | \
> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
> index 1098863e17ee..c598961d9f15 100644
> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
> @@ -273,13 +273,6 @@ static int __init feat_enable_idle_nap(struct dt_cpu_feature *f)
> return 1;
> }
>
> -static int __init feat_enable_align_dsisr(struct dt_cpu_feature *f)
> -{
> - cur_cpu_spec->cpu_features &= ~CPU_FTR_NODSISRALIGN;
> -
> - return 1;
> -}
> -
> static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
> {
> u64 lpcr;
> @@ -641,7 +634,6 @@ static struct dt_cpu_feature_match __initdata
> {"tm-suspend-hypervisor-assist", feat_enable, CPU_FTR_P9_TM_HV_ASSIST},
> {"tm-suspend-xer-so-bug", feat_enable, CPU_FTR_P9_TM_XER_SO_BUG},
> {"idle-nap", feat_enable_idle_nap, 0},
> - {"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
> {"idle-stop", feat_enable_idle_stop, 0},
> {"machine-check-power8", feat_enable_mce_power8, 0},
> {"performance-monitor-power8", feat_enable_pmu_power8, 0},
> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
> index c1545f22c077..a5a5acb627fe 100644
> --- a/arch/powerpc/kernel/prom.c
> +++ b/arch/powerpc/kernel/prom.c
> @@ -165,7 +165,7 @@ static struct ibm_pa_feature {
> #ifdef CONFIG_PPC_RADIX_MMU
> { .pabyte = 40, .pabit = 0, .mmu_features = MMU_FTR_TYPE_RADIX | MMU_FTR_GTSE },
> #endif
> - { .pabyte = 1, .pabit = 1, .invert = 1, .cpu_features = CPU_FTR_NODSISRALIGN },
> + { .pabyte = 1, .pabit = 1, .invert = 1, },
> { .pabyte = 5, .pabit = 0, .cpu_features = CPU_FTR_REAL_LE,
> .cpu_user_ftrs = PPC_FEATURE_TRUE_LE },
I didn't follow this change. Should the line be dropped?
-aneesh
^ permalink raw reply
* Re: [PATCH] powerpc/features: Remove CPU_FTR_NODSISRALIGN
From: Christophe Leroy @ 2020-10-13 7:25 UTC (permalink / raw)
To: Aneesh Kumar K.V, Benjamin Herrenschmidt, Paul Mackerras,
Michael Ellerman
Cc: linuxppc-dev, linux-kernel
In-Reply-To: <875z7ea8t7.fsf@linux.ibm.com>
Le 13/10/2020 à 09:23, Aneesh Kumar K.V a écrit :
> Christophe Leroy <christophe.leroy@csgroup.eu> writes:
>
>> CPU_FTR_NODSISRALIGN has not been used since
>> commit 31bfdb036f12 ("powerpc: Use instruction emulation
>> infrastructure to handle alignment faults")
>>
>> Remove it.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>> ---
>> arch/powerpc/include/asm/cputable.h | 22 ++++++++++------------
>> arch/powerpc/kernel/dt_cpu_ftrs.c | 8 --------
>> arch/powerpc/kernel/prom.c | 2 +-
>> 3 files changed, 11 insertions(+), 21 deletions(-)
>>
>> diff --git a/arch/powerpc/include/asm/cputable.h b/arch/powerpc/include/asm/cputable.h
>> index 9780c55f9811..accdc1286f37 100644
>> --- a/arch/powerpc/include/asm/cputable.h
>> +++ b/arch/powerpc/include/asm/cputable.h
>> @@ -137,7 +137,6 @@ static inline void cpu_feature_keys_init(void) { }
>> #define CPU_FTR_DBELL ASM_CONST(0x00000004)
>> #define CPU_FTR_CAN_NAP ASM_CONST(0x00000008)
>> #define CPU_FTR_DEBUG_LVL_EXC ASM_CONST(0x00000010)
>> -#define CPU_FTR_NODSISRALIGN ASM_CONST(0x00000020)
>> #define CPU_FTR_FPU_UNAVAILABLE ASM_CONST(0x00000040)
>> #define CPU_FTR_LWSYNC ASM_CONST(0x00000080)
>> #define CPU_FTR_NOEXECUTE ASM_CONST(0x00000100)
>> @@ -219,7 +218,7 @@ static inline void cpu_feature_keys_init(void) { }
>>
>> #ifndef __ASSEMBLY__
>>
>> -#define CPU_FTR_PPCAS_ARCH_V2 (CPU_FTR_NOEXECUTE | CPU_FTR_NODSISRALIGN)
>> +#define CPU_FTR_PPCAS_ARCH_V2 (CPU_FTR_NOEXECUTE)
>>
>> #define MMU_FTR_PPCAS_ARCH_V2 (MMU_FTR_TLBIEL | MMU_FTR_16M_PAGE)
>>
>> @@ -378,33 +377,33 @@ static inline void cpu_feature_keys_init(void) { }
>> CPU_FTR_COMMON | CPU_FTR_FPU_UNAVAILABLE | CPU_FTR_NOEXECUTE)
>> #define CPU_FTRS_CLASSIC32 (CPU_FTR_COMMON)
>> #define CPU_FTRS_8XX (CPU_FTR_NOEXECUTE)
>> -#define CPU_FTRS_40X (CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
>> -#define CPU_FTRS_44X (CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
>> -#define CPU_FTRS_440x6 (CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE | \
>> +#define CPU_FTRS_40X (CPU_FTR_NOEXECUTE)
>> +#define CPU_FTRS_44X (CPU_FTR_NOEXECUTE)
>> +#define CPU_FTRS_440x6 (CPU_FTR_NOEXECUTE | \
>> CPU_FTR_INDEXED_DCR)
>> #define CPU_FTRS_47X (CPU_FTRS_440x6)
>> #define CPU_FTRS_E200 (CPU_FTR_SPE_COMP | \
>> - CPU_FTR_NODSISRALIGN | CPU_FTR_COHERENT_ICACHE | \
>> + CPU_FTR_COHERENT_ICACHE | \
>> CPU_FTR_NOEXECUTE | \
>> CPU_FTR_DEBUG_LVL_EXC)
>> #define CPU_FTRS_E500 (CPU_FTR_MAYBE_CAN_DOZE | \
>> - CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | CPU_FTR_NODSISRALIGN | \
>> + CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | \
>> CPU_FTR_NOEXECUTE)
>> #define CPU_FTRS_E500_2 (CPU_FTR_MAYBE_CAN_DOZE | \
>> CPU_FTR_SPE_COMP | CPU_FTR_MAYBE_CAN_NAP | \
>> - CPU_FTR_NODSISRALIGN | CPU_FTR_NOEXECUTE)
>> -#define CPU_FTRS_E500MC (CPU_FTR_NODSISRALIGN | \
>> + CPU_FTR_NOEXECUTE)
>> +#define CPU_FTRS_E500MC ( \
>> CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
>> CPU_FTR_DBELL | CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV)
>> /*
>> * e5500/e6500 erratum A-006958 is a timebase bug that can use the
>> * same workaround as CPU_FTR_CELL_TB_BUG.
>> */
>> -#define CPU_FTRS_E5500 (CPU_FTR_NODSISRALIGN | \
>> +#define CPU_FTRS_E5500 ( \
>> CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
>> CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>> CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_CELL_TB_BUG)
>> -#define CPU_FTRS_E6500 (CPU_FTR_NODSISRALIGN | \
>> +#define CPU_FTRS_E6500 ( \
>> CPU_FTR_LWSYNC | CPU_FTR_NOEXECUTE | \
>> CPU_FTR_DBELL | CPU_FTR_POPCNTB | CPU_FTR_POPCNTD | \
>> CPU_FTR_DEBUG_LVL_EXC | CPU_FTR_EMB_HV | CPU_FTR_ALTIVEC_COMP | \
>> @@ -554,7 +553,6 @@ enum {
>> #define CPU_FTRS_DT_CPU_BASE \
>> (CPU_FTR_LWSYNC | \
>> CPU_FTR_FPU_UNAVAILABLE | \
>> - CPU_FTR_NODSISRALIGN | \
>> CPU_FTR_NOEXECUTE | \
>> CPU_FTR_COHERENT_ICACHE | \
>> CPU_FTR_STCX_CHECKS_ADDRESS | \
>> diff --git a/arch/powerpc/kernel/dt_cpu_ftrs.c b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> index 1098863e17ee..c598961d9f15 100644
>> --- a/arch/powerpc/kernel/dt_cpu_ftrs.c
>> +++ b/arch/powerpc/kernel/dt_cpu_ftrs.c
>> @@ -273,13 +273,6 @@ static int __init feat_enable_idle_nap(struct dt_cpu_feature *f)
>> return 1;
>> }
>>
>> -static int __init feat_enable_align_dsisr(struct dt_cpu_feature *f)
>> -{
>> - cur_cpu_spec->cpu_features &= ~CPU_FTR_NODSISRALIGN;
>> -
>> - return 1;
>> -}
>> -
>> static int __init feat_enable_idle_stop(struct dt_cpu_feature *f)
>> {
>> u64 lpcr;
>> @@ -641,7 +634,6 @@ static struct dt_cpu_feature_match __initdata
>> {"tm-suspend-hypervisor-assist", feat_enable, CPU_FTR_P9_TM_HV_ASSIST},
>> {"tm-suspend-xer-so-bug", feat_enable, CPU_FTR_P9_TM_XER_SO_BUG},
>> {"idle-nap", feat_enable_idle_nap, 0},
>> - {"alignment-interrupt-dsisr", feat_enable_align_dsisr, 0},
>> {"idle-stop", feat_enable_idle_stop, 0},
>> {"machine-check-power8", feat_enable_mce_power8, 0},
>> {"performance-monitor-power8", feat_enable_pmu_power8, 0},
>> diff --git a/arch/powerpc/kernel/prom.c b/arch/powerpc/kernel/prom.c
>> index c1545f22c077..a5a5acb627fe 100644
>> --- a/arch/powerpc/kernel/prom.c
>> +++ b/arch/powerpc/kernel/prom.c
>> @@ -165,7 +165,7 @@ static struct ibm_pa_feature {
>> #ifdef CONFIG_PPC_RADIX_MMU
>> { .pabyte = 40, .pabit = 0, .mmu_features = MMU_FTR_TYPE_RADIX | MMU_FTR_GTSE },
>> #endif
>> - { .pabyte = 1, .pabit = 1, .invert = 1, .cpu_features = CPU_FTR_NODSISRALIGN },
>> + { .pabyte = 1, .pabit = 1, .invert = 1, },
>> { .pabyte = 5, .pabit = 0, .cpu_features = CPU_FTR_REAL_LE,
>> .cpu_user_ftrs = PPC_FEATURE_TRUE_LE },
>
> I didn't follow this change. Should the line be dropped?
>
Don't know. I have to look closer, I don't know what it is used for.
Christophe
^ permalink raw reply
* Re: [PATCH 1/2] mm/mprotect: Call arch_validate_prot under mmap_lock and with length
From: Catalin Marinas @ 2020-10-13 9:16 UTC (permalink / raw)
To: Khalid Aziz
Cc: Jann Horn, linuxppc-dev, linux-kernel, Christoph Hellwig,
linux-mm, Paul Mackerras, sparclinux, Anthony Yznaga,
Andrew Morton, Will Deacon, David S. Miller, linux-arm-kernel
In-Reply-To: <20c85633-b559-c299-3e57-ae136b201526@oracle.com>
On Mon, Oct 12, 2020 at 01:14:50PM -0600, Khalid Aziz wrote:
> On 10/12/20 11:22 AM, Catalin Marinas wrote:
> > On Mon, Oct 12, 2020 at 11:03:33AM -0600, Khalid Aziz wrote:
> >> On 10/10/20 5:09 AM, Catalin Marinas wrote:
> >>> On Wed, Oct 07, 2020 at 02:14:09PM -0600, Khalid Aziz wrote:
> >>>> On 10/7/20 1:39 AM, Jann Horn wrote:
> >>>>> arch_validate_prot() is a hook that can validate whether a given set of
> >>>>> protection flags is valid in an mprotect() operation. It is given the set
> >>>>> of protection flags and the address being modified.
> >>>>>
> >>>>> However, the address being modified can currently not actually be used in
> >>>>> a meaningful way because:
> >>>>>
> >>>>> 1. Only the address is given, but not the length, and the operation can
> >>>>> span multiple VMAs. Therefore, the callee can't actually tell which
> >>>>> virtual address range, or which VMAs, are being targeted.
> >>>>> 2. The mmap_lock is not held, meaning that if the callee were to check
> >>>>> the VMA at @addr, that VMA would be unrelated to the one the
> >>>>> operation is performed on.
> >>>>>
> >>>>> Currently, custom arch_validate_prot() handlers are defined by
> >>>>> arm64, powerpc and sparc.
> >>>>> arm64 and powerpc don't care about the address range, they just check the
> >>>>> flags against CPU support masks.
> >>>>> sparc's arch_validate_prot() attempts to look at the VMA, but doesn't take
> >>>>> the mmap_lock.
> >>>>>
> >>>>> Change the function signature to also take a length, and move the
> >>>>> arch_validate_prot() call in mm/mprotect.c down into the locked region.
> >>> [...]
> >>>> As Chris pointed out, the call to arch_validate_prot() from do_mmap2()
> >>>> is made without holding mmap_lock. Lock is not acquired until
> >>>> vm_mmap_pgoff(). This variance is uncomfortable but I am more
> >>>> uncomfortable forcing all implementations of validate_prot to require
> >>>> mmap_lock be held when non-sparc implementations do not have such need
> >>>> yet. Since do_mmap2() is in powerpc specific code, for now this patch
> >>>> solves a current problem.
> >>>
> >>> I still think sparc should avoid walking the vmas in
> >>> arch_validate_prot(). The core code already has the vmas, though not
> >>> when calling arch_validate_prot(). That's one of the reasons I added
> >>> arch_validate_flags() with the MTE patches. For sparc, this could be
> >>> (untested, just copied the arch_validate_prot() code):
> >>
> >> I am little uncomfortable with the idea of validating protection bits
> >> inside the VMA walk loop in do_mprotect_pkey(). When ADI is being
> >> enabled across multiple VMAs and arch_validate_flags() fails on a VMA
> >> later, do_mprotect_pkey() will bail out with error leaving ADI enabled
> >> on earlier VMAs. This will apply to protection bits other than ADI as
> >> well of course. This becomes a partial failure of mprotect() call. I
> >> think it should be all or nothing with mprotect() - when one calls
> >> mprotect() from userspace, either the entire address range passed in
> >> gets its protection bits updated or none of it does. That requires
> >> validating protection bits upfront or undoing what earlier iterations of
> >> VMA walk loop might have done.
> >
> > I thought the same initially but mprotect() already does this with the
> > VM_MAY* flag checking. If you ask it for an mprotect() that crosses
> > multiple vmas and one of them fails, it doesn't roll back the changes to
> > the prior ones. I considered that a similar approach is fine for MTE
> > (it's most likely a user error).
>
> You are right about the current behavior with VM_MAY* flags, but that is
> not the right behavior. Adding more cases to this just perpetuates
> incorrect behavior. It is not easy to roll back changes after VMAs have
> potentially been split/merged which is probably why the current code
> simply throws in the towel and returns with partially modified address
> space. It is lot easier to do all the checks upfront and then proceed or
> not proceed with modifying VMAs. One approach might be to call
> arch_validate_flags() in a loop before modifying VMAs and walk all VMAs
> with a read lock held. Current code also bails out with ENOMEM if it
> finds a hole in the address range and leaves any modifications already
> made in place. This is another case where a hole could have been
> detected earlier.
This should be ideal indeed though with the risk of breaking the current
ABI (FWIW, FreeBSD seems to do a first pass to check for violations:
https://github.com/freebsd/freebsd/blob/master/sys/vm/vm_map.c#L2630).
However, I'm not sure it's worth the hassle. Do we expect the user to
call mprotect() across multiple mixed type mappings while relying on no
change if an error is returned? We should probably at least document the
current behaviour in the mprotect man page.
--
Catalin
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox