* Re: [PATCH v4 1/8] KVM: PPC: Ultravisor: Introduce the MSR_S bit
From: Claudio Carvalho @ 2019-07-12 21:07 UTC (permalink / raw)
To: Nicholas Piggin, linuxppc-dev
Cc: Madhavan Srinivasan, Michael Anderson, Ram Pai, kvm-ppc,
Bharata B Rao, Ryan Grimm, Sukadev Bhattiprolu, Thiago Bauermann,
Anshuman Khandual
In-Reply-To: <1562892336.boqkwvamhq.astroid@bobo.none>
On 7/11/19 9:57 PM, Nicholas Piggin wrote:
> Claudio Carvalho's on June 29, 2019 6:08 am:
>> From: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
>>
>> The ultravisor processor mode is introduced in POWER platforms that
>> supports the Protected Execution Facility (PEF). Ultravisor is higher
>> privileged than hypervisor mode.
>>
>> In PEF enabled platforms, the MSR_S bit is used to indicate if the
>> thread is in secure state. With the MSR_S bit, the privilege state of
>> the thread is now determined by MSR_S, MSR_HV and MSR_PR, as follows:
>>
>> S HV PR
>> -----------------------
>> 0 x 1 problem
>> 1 0 1 problem
>> x x 0 privileged
>> x 1 0 hypervisor
>> 1 1 0 ultravisor
>> 1 1 1 reserved
> What does this table mean? I thought 'x' meant either
Yes, it means either. The table was arranged that way to say that:
- hypervisor state is also a privileged state,
- ultravisor state is also a hypervisor state.
> , but in that
> case there are several states that can apply to the same
> combination of bits.
>
> Would it be clearer to rearrange the table so the columns are the HV
> and PR bits we know and love, plus the effect of S=1 on each of them?
>
> HV PR S=0 S=1
> ---------------------------------------------
> 0 0 privileged privileged (secure guest kernel)
> 0 1 problem problem (secure guest userspace)
> 1 0 hypervisor ultravisor
> 1 1 problem reserved
>
> Is that accurate?
Yes, it is. I also like this format. I will consider it.
>
>
>> The hypervisor doesn't (and can't) run with the MSR_S bit set, but a
>> secure guest and the ultravisor firmware do.
>>
>> Signed-off-by: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
>> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
>> [ Update the commit message ]
>> Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
>> ---
>> arch/powerpc/include/asm/reg.h | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
>> index 10caa145f98b..39b4c0a519f5 100644
>> --- a/arch/powerpc/include/asm/reg.h
>> +++ b/arch/powerpc/include/asm/reg.h
>> @@ -38,6 +38,7 @@
>> #define MSR_TM_LG 32 /* Trans Mem Available */
>> #define MSR_VEC_LG 25 /* Enable AltiVec */
>> #define MSR_VSX_LG 23 /* Enable VSX */
>> +#define MSR_S_LG 22 /* Secure VM bit */
>> #define MSR_POW_LG 18 /* Enable Power Management */
>> #define MSR_WE_LG 18 /* Wait State Enable */
>> #define MSR_TGPR_LG 17 /* TLB Update registers in use */
>> @@ -71,11 +72,13 @@
>> #define MSR_SF __MASK(MSR_SF_LG) /* Enable 64 bit mode */
>> #define MSR_ISF __MASK(MSR_ISF_LG) /* Interrupt 64b mode valid on 630 */
>> #define MSR_HV __MASK(MSR_HV_LG) /* Hypervisor state */
>> +#define MSR_S __MASK(MSR_S_LG) /* Secure state */
> This is a real nitpick, but why two different comments for the bit
> number and the mask?
Fixed for the next version. Both comments will be /* Secure state */
Thanks
Claudio
^ permalink raw reply
* Re: [PATCH 01/12] Documentation: move architectures together
From: Jonathan Corbet @ 2019-07-12 17:34 UTC (permalink / raw)
To: Alex Shi
Cc: linux-s390, linux-fbdev, linux-samsung-soc, linux-ia64,
linux-scsi, linux-parisc, linux-doc, linux-sh, linux-kernel,
linux-mips, kvm, linux-riscv, linux-omap, linuxppc-dev,
linux-stm32, linux-arm-kernel
In-Reply-To: <20190712022018.27989-1-alex.shi@linux.alibaba.com>
On Fri, 12 Jul 2019 10:20:07 +0800
Alex Shi <alex.shi@linux.alibaba.com> wrote:
> There are many different archs in Documentation/ dir, it's better to
> move them together in 'Documentation/arch' which follows from kernel source.
So this seems certain to collide badly with Mauro's RST-conversion monster
patch set.
More to the point, though...if we are going to thrash up things this
badly, we want to be sure that we're doing it right so we don't end up
renaming everything again. Grouping stuff into a new arch/ subdirectory
adds a bit of order, but it doesn't do much toward trying to organize our
documentation for its readers, and it doesn't help us to modernize the
docs and get rid of the old, useless stuff. A quick check shows that many
of these files have seen no changes other than typo fixes since the
beginning of the Git era.
So, in my mind, this needs some thought. Maybe we want a
Documentation/arch in the end, but I'm not convinced that we should just
create it and fill it with a snow shovel. This might be a good thing to
discuss at the kernel summit in September.
Thanks,
jon
^ permalink raw reply
* Re: [PATCH 2/3] DMA mapping: Move SME handling to x86-specific files
From: Thomas Gleixner @ 2019-07-12 16:09 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-fsdevel,
linuxppc-dev, Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20190712053631.9814-3-bauerman@linux.ibm.com>
On Fri, 12 Jul 2019, Thiago Jung Bauermann wrote:
> diff --git a/include/linux/mem_encrypt.h b/include/linux/mem_encrypt.h
> index b310a9c18113..f2e399fb626b 100644
> --- a/include/linux/mem_encrypt.h
> +++ b/include/linux/mem_encrypt.h
> @@ -21,23 +21,11 @@
>
> #else /* !CONFIG_ARCH_HAS_MEM_ENCRYPT */
>
> -#define sme_me_mask 0ULL
> -
> -static inline bool sme_active(void) { return false; }
> static inline bool sev_active(void) { return false; }
You want to move out sev_active as well, the only relevant thing is
mem_encrypt_active(). Everything SME/SEV is an architecture detail.
> +static inline bool mem_encrypt_active(void) { return false; }
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 1/3] x86/Kconfig: Move ARCH_HAS_MEM_ENCRYPT to arch/Kconfig
From: Thomas Gleixner @ 2019-07-12 16:04 UTC (permalink / raw)
To: Thiago Jung Bauermann
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, Halil Pasic, iommu,
Ingo Molnar, Borislav Petkov, H. Peter Anvin, linux-fsdevel,
linuxppc-dev, Christoph Hellwig, Marek Szyprowski
In-Reply-To: <20190712053631.9814-2-bauerman@linux.ibm.com>
On Fri, 12 Jul 2019, Thiago Jung Bauermann wrote:
> powerpc and s390 are going to use this feature as well, so put it in a
> generic location.
>
> Signed-off-by: Thiago Jung Bauermann <bauerman@linux.ibm.com>
Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
^ permalink raw reply
* [Bug 204125] FTBFS on ppc64 big endian and gcc9 because of -mcall-aixdesc and missing __linux__
From: bugzilla-daemon @ 2019-07-12 15:36 UTC (permalink / raw)
To: linuxppc-dev
In-Reply-To: <bug-204125-206035@https.bugzilla.kernel.org/>
https://bugzilla.kernel.org/show_bug.cgi?id=204125
--- Comment #8 from Daniel Kolesa (linux@octaforge.org) ---
Using this patch on my machines now:
https://gist.github.com/q66/625cbec5d7317829a302773f89533b51 seems to work well
--
You are receiving this mail because:
You are watching the assignee of the bug.
^ permalink raw reply
* Re: [PATCH] treewide: Rename rcu_dereference_raw_notrace to _check
From: Joel Fernandes @ 2019-07-12 15:34 UTC (permalink / raw)
To: Paul E. McKenney
Cc: rcu, kernel-team, Jonathan Corbet, linux-doc, Lai Jiangshan,
linux-kernel, kvm-ppc, Josh Triplett, Ingo Molnar,
Mathieu Desnoyers, Steven Rostedt, byungchul.park, linuxppc-dev
In-Reply-To: <20190712150107.GT26519@linux.ibm.com>
On Fri, Jul 12, 2019 at 08:01:07AM -0700, Paul E. McKenney wrote:
> On Thu, Jul 11, 2019 at 04:45:41PM -0400, Joel Fernandes (Google) wrote:
> > The rcu_dereference_raw_notrace() API name is confusing.
> > It is equivalent to rcu_dereference_raw() except that it also does
> > sparse pointer checking.
> >
> > There are only a few users of rcu_dereference_raw_notrace(). This
> > patches renames all of them to be rcu_dereference_raw_check with the
> > "check" indicating sparse checking.
> >
> > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
>
> I queued this, but reworked the commit log and fixed a couple of
> irritating checkpatch issues that were in the original code.
> Does this work for you?
Thanks, yes it looks good to me.
thanks,
- Joel
>
> Thanx, Paul
>
> ------------------------------------------------------------------------
>
> commit bd5c0fea6016c90cf7a9eb0435cd0c373dfdac2f
> Author: Joel Fernandes (Google) <joel@joelfernandes.org>
> Date: Thu Jul 11 16:45:41 2019 -0400
>
> treewide: Rename rcu_dereference_raw_notrace() to _check()
>
> The rcu_dereference_raw_notrace() API name is confusing. It is equivalent
> to rcu_dereference_raw() except that it also does sparse pointer checking.
>
> There are only a few users of rcu_dereference_raw_notrace(). This patches
> renames all of them to be rcu_dereference_raw_check() with the "_check()"
> indicating sparse checking.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
> [ paulmck: Fix checkpatch warnings about parentheses. ]
> Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
>
> diff --git a/Documentation/RCU/Design/Requirements/Requirements.html b/Documentation/RCU/Design/Requirements/Requirements.html
> index f04c467e55c5..467251f7fef6 100644
> --- a/Documentation/RCU/Design/Requirements/Requirements.html
> +++ b/Documentation/RCU/Design/Requirements/Requirements.html
> @@ -2514,7 +2514,7 @@ disabled across the entire RCU read-side critical section.
> <p>
> It is possible to use tracing on RCU code, but tracing itself
> uses RCU.
> -For this reason, <tt>rcu_dereference_raw_notrace()</tt>
> +For this reason, <tt>rcu_dereference_raw_check()</tt>
> is provided for use by tracing, which avoids the destructive
> recursion that could otherwise ensue.
> This API is also used by virtualization in some architectures,
> diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
> index 21b1ed5df888..53388a311967 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_64.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_64.h
> @@ -546,7 +546,7 @@ static inline void note_hpte_modification(struct kvm *kvm,
> */
> static inline struct kvm_memslots *kvm_memslots_raw(struct kvm *kvm)
> {
> - return rcu_dereference_raw_notrace(kvm->memslots[0]);
> + return rcu_dereference_raw_check(kvm->memslots[0]);
> }
>
> extern void kvmppc_mmu_debugfs_init(struct kvm *kvm);
> diff --git a/include/linux/rculist.h b/include/linux/rculist.h
> index e91ec9ddcd30..932296144131 100644
> --- a/include/linux/rculist.h
> +++ b/include/linux/rculist.h
> @@ -622,7 +622,7 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> * as long as the traversal is guarded by rcu_read_lock().
> */
> #define hlist_for_each_entry_rcu(pos, head, member) \
> - for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
> + for (pos = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),\
> typeof(*(pos)), member); \
> pos; \
> pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(\
> @@ -642,10 +642,10 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
> * not do any RCU debugging or tracing.
> */
> #define hlist_for_each_entry_rcu_notrace(pos, head, member) \
> - for (pos = hlist_entry_safe (rcu_dereference_raw_notrace(hlist_first_rcu(head)),\
> + for (pos = hlist_entry_safe(rcu_dereference_raw_check(hlist_first_rcu(head)),\
> typeof(*(pos)), member); \
> pos; \
> - pos = hlist_entry_safe(rcu_dereference_raw_notrace(hlist_next_rcu(\
> + pos = hlist_entry_safe(rcu_dereference_raw_check(hlist_next_rcu(\
> &(pos)->member)), typeof(*(pos)), member))
>
> /**
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 0c9b92799abc..e5161e377ad4 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -478,7 +478,7 @@ do { \
> * The no-tracing version of rcu_dereference_raw() must not call
> * rcu_read_lock_held().
> */
> -#define rcu_dereference_raw_notrace(p) __rcu_dereference_check((p), 1, __rcu)
> +#define rcu_dereference_raw_check(p) __rcu_dereference_check((p), 1, __rcu)
>
> /**
> * rcu_dereference_protected() - fetch RCU pointer when updates prevented
> diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
> index 0515a2096f90..0456e0a3dab1 100644
> --- a/kernel/trace/ftrace_internal.h
> +++ b/kernel/trace/ftrace_internal.h
> @@ -6,22 +6,22 @@
>
> /*
> * Traverse the ftrace_global_list, invoking all entries. The reason that we
> - * can use rcu_dereference_raw_notrace() is that elements removed from this list
> + * can use rcu_dereference_raw_check() is that elements removed from this list
> * are simply leaked, so there is no need to interact with a grace-period
> - * mechanism. The rcu_dereference_raw_notrace() calls are needed to handle
> + * mechanism. The rcu_dereference_raw_check() calls are needed to handle
> * concurrent insertions into the ftrace_global_list.
> *
> * Silly Alpha and silly pointer-speculation compiler optimizations!
> */
> #define do_for_each_ftrace_op(op, list) \
> - op = rcu_dereference_raw_notrace(list); \
> + op = rcu_dereference_raw_check(list); \
> do
>
> /*
> * Optimized for just a single item in the list (as that is the normal case).
> */
> #define while_for_each_ftrace_op(op) \
> - while (likely(op = rcu_dereference_raw_notrace((op)->next)) && \
> + while (likely(op = rcu_dereference_raw_check((op)->next)) && \
> unlikely((op) != &ftrace_list_end))
>
> extern struct ftrace_ops __rcu *ftrace_ops_list;
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 2c92b3d9ea30..1d69110d9e5b 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -2642,10 +2642,10 @@ static void ftrace_exports(struct ring_buffer_event *event)
>
> preempt_disable_notrace();
>
> - export = rcu_dereference_raw_notrace(ftrace_exports_list);
> + export = rcu_dereference_raw_check(ftrace_exports_list);
> while (export) {
> trace_process_export(export, event);
> - export = rcu_dereference_raw_notrace(export->next);
> + export = rcu_dereference_raw_check(export->next);
> }
>
> preempt_enable_notrace();
^ permalink raw reply
* Re: [PATCH v9 00/10] namei: openat2(2) path resolution restrictions
From: Aleksa Sarai @ 2019-07-12 15:32 UTC (permalink / raw)
To: Al Viro
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, containers, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, Chanho Min
In-Reply-To: <20190712151118.GP17978@ZenIV.linux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 734 bytes --]
On 2019-07-12, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Sun, Jul 07, 2019 at 12:57:27AM +1000, Aleksa Sarai wrote:
> > Patch changelog:
> > v9:
> > * Replace resolveat(2) with openat2(2). [Linus]
> > * Output a warning to dmesg if may_open_magiclink() is violated.
> > * Add an openat2(O_CREAT) testcase.
>
> One general note for the future, BTW: for such series it's generally
> a good idea to put it into a public git tree somewhere and mention that
> in the announcement...
Sure, I'll mention it next time. For the record the tree is
<https://github.com/cyphar/linux/tree/resolveat/master>
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3] powerpc/setup_64: fix -Wempty-body warnings
From: Qian Cai @ 2019-07-12 15:30 UTC (permalink / raw)
To: mpe; +Cc: linux-kernel, paulus, tyreld, joe, linuxppc-dev
In-Reply-To: <1561730629-5025-1-git-send-email-cai@lca.pw>
Ping.
On Fri, 2019-06-28 at 10:03 -0400, Qian Cai wrote:
> At the beginning of setup_64.c, it has,
>
> #ifdef DEBUG
> #define DBG(fmt...) udbg_printf(fmt)
> #else
> #define DBG(fmt...)
> #endif
>
> where DBG() could be compiled away, and generate warnings,
>
> arch/powerpc/kernel/setup_64.c: In function 'initialize_cache_info':
> arch/powerpc/kernel/setup_64.c:579:49: warning: suggest braces around
> empty body in an 'if' statement [-Wempty-body]
> DBG("Argh, can't find dcache properties !\n");
> ^
> arch/powerpc/kernel/setup_64.c:582:49: warning: suggest braces around
> empty body in an 'if' statement [-Wempty-body]
> DBG("Argh, can't find icache properties !\n");
>
> Fix it by using the no_printk() macro, and make sure that format and
> argument are always verified by the compiler.
>
> Suggested-by: Tyrel Datwyler <tyreld@linux.vnet.ibm.com>
> Suggested-by: Joe Perches <joe@perches.com>
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
>
> v3: Use no_printk() macro, and make sure that format and argument are always
> verified by the compiler using a more generic form ##__VA_ARGS__ per Joe.
>
> v2: Fix it by using a NOP while loop per Tyrel.
>
> arch/powerpc/kernel/setup_64.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/setup_64.c b/arch/powerpc/kernel/setup_64.c
> index 44b4c432a273..cea933a43f0a 100644
> --- a/arch/powerpc/kernel/setup_64.c
> +++ b/arch/powerpc/kernel/setup_64.c
> @@ -69,9 +69,9 @@
> #include "setup.h"
>
> #ifdef DEBUG
> -#define DBG(fmt...) udbg_printf(fmt)
> +#define DBG(fmt, ...) udbg_printf(fmt, ##__VA_ARGS__)
> #else
> -#define DBG(fmt...)
> +#define DBG(fmt, ...) no_printk(fmt, ##__VA_ARGS__)
> #endif
>
> int spinning_secondaries;
^ permalink raw reply
* Re: [PATCH] powerpc/powernv: fix a W=1 compilation warning
From: Qian Cai @ 2019-07-12 15:29 UTC (permalink / raw)
To: benh, paulus, mpe; +Cc: aik, linuxppc-dev, linux-kernel
In-Reply-To: <1558541369-8263-1-git-send-email-cai@lca.pw>
Ping.
On Wed, 2019-05-22 at 12:09 -0400, Qian Cai wrote:
> The commit b575c731fe58 ("powerpc/powernv/npu: Add set/unset window
> helpers") called pnv_npu_set_window() in a void function
> pnv_npu_dma_set_32(), but the return code from pnv_npu_set_window() has
> no use there as all the error logging happen in pnv_npu_set_window(),
> so just remove the unused variable to avoid a compilation warning,
>
> arch/powerpc/platforms/powernv/npu-dma.c: In function
> 'pnv_npu_dma_set_32':
> arch/powerpc/platforms/powernv/npu-dma.c:198:10: warning: variable ‘rc’
> set but not used [-Wunused-but-set-variable]
>
> Signed-off-by: Qian Cai <cai@lca.pw>
> ---
> arch/powerpc/platforms/powernv/npu-dma.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/powernv/npu-dma.c
> b/arch/powerpc/platforms/powernv/npu-dma.c
> index 495550432f3d..035208ed591f 100644
> --- a/arch/powerpc/platforms/powernv/npu-dma.c
> +++ b/arch/powerpc/platforms/powernv/npu-dma.c
> @@ -195,7 +195,6 @@ static void pnv_npu_dma_set_32(struct pnv_ioda_pe *npe)
> {
> struct pci_dev *gpdev;
> struct pnv_ioda_pe *gpe;
> - int64_t rc;
>
> /*
> * Find the assoicated PCI devices and get the dma window
> @@ -208,8 +207,8 @@ static void pnv_npu_dma_set_32(struct pnv_ioda_pe *npe)
> if (!gpe)
> return;
>
> - rc = pnv_npu_set_window(&npe->table_group, 0,
> - gpe->table_group.tables[0]);
> + pnv_npu_set_window(&npe->table_group, 0,
> + gpe->table_group.tables[0]);
>
> /*
> * NVLink devices use the same TCE table configuration as
^ permalink raw reply
* Re: [PATCH kernel v4 0/4 repost] powerpc/ioda2: Yet another attempt to allow DMA masks between 32 and 59
From: Christoph Hellwig @ 2019-07-12 15:28 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Sam Bobroff, Alistair Popple, Oliver O'Halloran, linuxppc-dev,
David Gibson
In-Reply-To: <20190712094509.56695-1-aik@ozlabs.ru>
On Fri, Jul 12, 2019 at 07:45:05PM +1000, Alexey Kardashevskiy wrote:
> This is an attempt to allow DMA masks between 32..59 which are not large
> enough to use either a PHB3 bypass mode or a sketchy bypass. Depending
> on the max order, up to 40 is usually available.
Can you elaborate what you man with supported in detail? In the end
a DMA devices DMA capability is only really interesting as a lower
bound.
e.g. if you have a DMA that supports 40-bit DMA addressing we could
always treat it as if supports 32-bit addressing, and I thought the
powerpc code does that, as the DMA API now relies on that. Did I miss
something and it explicitly rejected that (in which case I didn't spot
the fix in this series), or is this just an optimization to handle these
devices more optimally, in which case maybe the changelog could be
improved a bit.
^ permalink raw reply
* Re: [PATCH kernel v4 4/4] powerpc/powernv/ioda2: Create bigger default window with 64k IOMMU pages
From: Christoph Hellwig @ 2019-07-12 15:25 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Sam Bobroff, Alistair Popple, Oliver O'Halloran, linuxppc-dev,
David Gibson
In-Reply-To: <20190712094509.56695-5-aik@ozlabs.ru>
> -extern struct iommu_table *iommu_init_table(struct iommu_table * tbl,
> - int nid);
> +extern struct iommu_table *iommu_init_table_res(struct iommu_table *tbl,
> + int nid, unsigned long res_start, unsigned long res_end);
> +#define iommu_init_table(tbl, nid) iommu_init_table_res((tbl), (nid), 0, 0)
I'd just pass the two additional paramters to iommu_init_table, there
are only 10 callers of it.
> + * the max order of allocation possible. The TCE tableis likely to
Missing whitespace between tabke and is.
> + * end up being multilevel and with on-demand allocation in place,
> + * the initial use is not going to be huge as the default window aims
> + * to support cripplied devices (i.e. not fully 64bit DMAble) only.
s/cripplied/crippled/
^ permalink raw reply
* Re: [PATCH kernel v4 2/4] powerpc/iommu: Allow bypass-only for DMA
From: Christoph Hellwig @ 2019-07-12 15:20 UTC (permalink / raw)
To: Alexey Kardashevskiy
Cc: Sam Bobroff, Alistair Popple, Oliver O'Halloran, linuxppc-dev,
David Gibson
In-Reply-To: <20190712094509.56695-3-aik@ozlabs.ru>
> This skips the 32bit DMA setup check if the bypass is can be selected.
That sentence does not parse. I think you need to dop the "can be"
based on the actual patch.
^ permalink raw reply
* Re: [PATCH v9 00/10] namei: openat2(2) path resolution restrictions
From: Al Viro @ 2019-07-12 15:11 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, containers, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, Chanho Min
In-Reply-To: <20190706145737.5299-1-cyphar@cyphar.com>
On Sun, Jul 07, 2019 at 12:57:27AM +1000, Aleksa Sarai wrote:
> Patch changelog:
> v9:
> * Replace resolveat(2) with openat2(2). [Linus]
> * Output a warning to dmesg if may_open_magiclink() is violated.
> * Add an openat2(O_CREAT) testcase.
One general note for the future, BTW: for such series it's generally
a good idea to put it into a public git tree somewhere and mention that
in the announcement...
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Christoph Hellwig @ 2019-07-12 15:11 UTC (permalink / raw)
To: Halil Pasic
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
linuxppc-dev, Christoph Hellwig, Thiago Jung Bauermann,
Marek Szyprowski
In-Reply-To: <20190712165153.78d49095.pasic@linux.ibm.com>
On Fri, Jul 12, 2019 at 04:51:53PM +0200, Halil Pasic wrote:
> Thank you very much! I will have another look, but it seems to me,
> without further measures taken, this would break protected virtualization
> support on s390. The effect of the che for s390 is that
> force_dma_unencrypted() will always return false instead calling into
> the platform code like it did before the patch, right?
>
> Should I send a Fixes: e67a5ed1f86f "dma-direct: Force unencrypted DMA
> under SME for certain DMA masks" (Tom Lendacky, 2019-07-10) patch that
> rectifies things for s390 or how do we want handle this?
Yes, please do. I hadn't noticed the s390 support had landed in
mainline already.
^ permalink raw reply
* Re: [PATCH] treewide: Rename rcu_dereference_raw_notrace to _check
From: Paul E. McKenney @ 2019-07-12 15:01 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: rcu, kernel-team, Jonathan Corbet, linux-doc, Lai Jiangshan,
linux-kernel, kvm-ppc, Josh Triplett, Ingo Molnar,
Mathieu Desnoyers, Steven Rostedt, byungchul.park, linuxppc-dev
In-Reply-To: <20190711204541.28940-1-joel@joelfernandes.org>
On Thu, Jul 11, 2019 at 04:45:41PM -0400, Joel Fernandes (Google) wrote:
> The rcu_dereference_raw_notrace() API name is confusing.
> It is equivalent to rcu_dereference_raw() except that it also does
> sparse pointer checking.
>
> There are only a few users of rcu_dereference_raw_notrace(). This
> patches renames all of them to be rcu_dereference_raw_check with the
> "check" indicating sparse checking.
>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
I queued this, but reworked the commit log and fixed a couple of
irritating checkpatch issues that were in the original code.
Does this work for you?
Thanx, Paul
------------------------------------------------------------------------
commit bd5c0fea6016c90cf7a9eb0435cd0c373dfdac2f
Author: Joel Fernandes (Google) <joel@joelfernandes.org>
Date: Thu Jul 11 16:45:41 2019 -0400
treewide: Rename rcu_dereference_raw_notrace() to _check()
The rcu_dereference_raw_notrace() API name is confusing. It is equivalent
to rcu_dereference_raw() except that it also does sparse pointer checking.
There are only a few users of rcu_dereference_raw_notrace(). This patches
renames all of them to be rcu_dereference_raw_check() with the "_check()"
indicating sparse checking.
Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
[ paulmck: Fix checkpatch warnings about parentheses. ]
Signed-off-by: Paul E. McKenney <paulmck@linux.ibm.com>
diff --git a/Documentation/RCU/Design/Requirements/Requirements.html b/Documentation/RCU/Design/Requirements/Requirements.html
index f04c467e55c5..467251f7fef6 100644
--- a/Documentation/RCU/Design/Requirements/Requirements.html
+++ b/Documentation/RCU/Design/Requirements/Requirements.html
@@ -2514,7 +2514,7 @@ disabled across the entire RCU read-side critical section.
<p>
It is possible to use tracing on RCU code, but tracing itself
uses RCU.
-For this reason, <tt>rcu_dereference_raw_notrace()</tt>
+For this reason, <tt>rcu_dereference_raw_check()</tt>
is provided for use by tracing, which avoids the destructive
recursion that could otherwise ensue.
This API is also used by virtualization in some architectures,
diff --git a/arch/powerpc/include/asm/kvm_book3s_64.h b/arch/powerpc/include/asm/kvm_book3s_64.h
index 21b1ed5df888..53388a311967 100644
--- a/arch/powerpc/include/asm/kvm_book3s_64.h
+++ b/arch/powerpc/include/asm/kvm_book3s_64.h
@@ -546,7 +546,7 @@ static inline void note_hpte_modification(struct kvm *kvm,
*/
static inline struct kvm_memslots *kvm_memslots_raw(struct kvm *kvm)
{
- return rcu_dereference_raw_notrace(kvm->memslots[0]);
+ return rcu_dereference_raw_check(kvm->memslots[0]);
}
extern void kvmppc_mmu_debugfs_init(struct kvm *kvm);
diff --git a/include/linux/rculist.h b/include/linux/rculist.h
index e91ec9ddcd30..932296144131 100644
--- a/include/linux/rculist.h
+++ b/include/linux/rculist.h
@@ -622,7 +622,7 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
* as long as the traversal is guarded by rcu_read_lock().
*/
#define hlist_for_each_entry_rcu(pos, head, member) \
- for (pos = hlist_entry_safe (rcu_dereference_raw(hlist_first_rcu(head)),\
+ for (pos = hlist_entry_safe(rcu_dereference_raw(hlist_first_rcu(head)),\
typeof(*(pos)), member); \
pos; \
pos = hlist_entry_safe(rcu_dereference_raw(hlist_next_rcu(\
@@ -642,10 +642,10 @@ static inline void hlist_add_behind_rcu(struct hlist_node *n,
* not do any RCU debugging or tracing.
*/
#define hlist_for_each_entry_rcu_notrace(pos, head, member) \
- for (pos = hlist_entry_safe (rcu_dereference_raw_notrace(hlist_first_rcu(head)),\
+ for (pos = hlist_entry_safe(rcu_dereference_raw_check(hlist_first_rcu(head)),\
typeof(*(pos)), member); \
pos; \
- pos = hlist_entry_safe(rcu_dereference_raw_notrace(hlist_next_rcu(\
+ pos = hlist_entry_safe(rcu_dereference_raw_check(hlist_next_rcu(\
&(pos)->member)), typeof(*(pos)), member))
/**
diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 0c9b92799abc..e5161e377ad4 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -478,7 +478,7 @@ do { \
* The no-tracing version of rcu_dereference_raw() must not call
* rcu_read_lock_held().
*/
-#define rcu_dereference_raw_notrace(p) __rcu_dereference_check((p), 1, __rcu)
+#define rcu_dereference_raw_check(p) __rcu_dereference_check((p), 1, __rcu)
/**
* rcu_dereference_protected() - fetch RCU pointer when updates prevented
diff --git a/kernel/trace/ftrace_internal.h b/kernel/trace/ftrace_internal.h
index 0515a2096f90..0456e0a3dab1 100644
--- a/kernel/trace/ftrace_internal.h
+++ b/kernel/trace/ftrace_internal.h
@@ -6,22 +6,22 @@
/*
* Traverse the ftrace_global_list, invoking all entries. The reason that we
- * can use rcu_dereference_raw_notrace() is that elements removed from this list
+ * can use rcu_dereference_raw_check() is that elements removed from this list
* are simply leaked, so there is no need to interact with a grace-period
- * mechanism. The rcu_dereference_raw_notrace() calls are needed to handle
+ * mechanism. The rcu_dereference_raw_check() calls are needed to handle
* concurrent insertions into the ftrace_global_list.
*
* Silly Alpha and silly pointer-speculation compiler optimizations!
*/
#define do_for_each_ftrace_op(op, list) \
- op = rcu_dereference_raw_notrace(list); \
+ op = rcu_dereference_raw_check(list); \
do
/*
* Optimized for just a single item in the list (as that is the normal case).
*/
#define while_for_each_ftrace_op(op) \
- while (likely(op = rcu_dereference_raw_notrace((op)->next)) && \
+ while (likely(op = rcu_dereference_raw_check((op)->next)) && \
unlikely((op) != &ftrace_list_end))
extern struct ftrace_ops __rcu *ftrace_ops_list;
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 2c92b3d9ea30..1d69110d9e5b 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -2642,10 +2642,10 @@ static void ftrace_exports(struct ring_buffer_event *event)
preempt_disable_notrace();
- export = rcu_dereference_raw_notrace(ftrace_exports_list);
+ export = rcu_dereference_raw_check(ftrace_exports_list);
while (export) {
trace_process_export(export, event);
- export = rcu_dereference_raw_notrace(export->next);
+ export = rcu_dereference_raw_check(export->next);
}
preempt_enable_notrace();
^ permalink raw reply related
* Re: [PATCH v9 05/10] namei: O_BENEATH-style path resolution flags
From: Al Viro @ 2019-07-12 15:00 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, Chanho Min, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190712132553.GN17978@ZenIV.linux.org.uk>
On Fri, Jul 12, 2019 at 02:25:53PM +0100, Al Viro wrote:
> if (flags & LOOKUP_BENEATH) {
> nd->root = nd->path;
> if (!(flags & LOOKUP_RCU))
> path_get(&nd->root);
> else
> nd->root_seq = nd->seq;
BTW, this assignment is needed for LOOKUP_RCU case. Without it
you are pretty much guaranteed that lazy pathwalk will fail,
when it comes to complete_walk().
Speaking of which, what would happen if LOOKUP_ROOT/LOOKUP_BENEATH
combination would someday get passed?
^ permalink raw reply
* Re: [PATCH v2 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Michal Hocko @ 2019-07-12 15:00 UTC (permalink / raw)
To: Will Deacon
Cc: Heiko Carstens, open list:MEMORY MANAGEMENT, Paul Mackerras,
H . Peter Anvin, sparclinux@vger.kernel.org, Alexander Duyck,
linux-s390@vger.kernel.org, x86@kernel.org, willy, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Hoan Tran OS, Catalin Marinas,
Open Source Submission, Pavel Tatashin, Vasily Gorbik,
Will Deacon, Borislav Petkov, Thomas Gleixner, Vlastimil Babka,
Oscar Salvador, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org, David S . Miller
In-Reply-To: <20190712143730.au3662g4ua2tjudu@willie-the-truck>
On Fri 12-07-19 15:37:30, Will Deacon wrote:
> Hi all,
>
> On Fri, Jul 12, 2019 at 02:12:23PM +0200, Michal Hocko wrote:
> > On Fri 12-07-19 10:56:47, Hoan Tran OS wrote:
> > [...]
> > > It would be good if we can enable it by-default. Otherwise, let arch
> > > enables it by them-self. Do you have any suggestions?
> >
> > I can hardly make any suggestions when it is not really clear _why_ you
> > want to remove this config option in the first place. Please explain
> > what motivated you to make this change.
>
> Sorry, I think this confusion might actually be my fault and Hoan has just
> been implementing my vague suggestion here:
>
> https://lore.kernel.org/linux-arm-kernel/20190625101245.s4vxfosoop52gl4e@willie-the-truck/
>
> If the preference of the mm folks is to leave CONFIG_NODES_SPAN_OTHER_NODES
> as it is, then we can define it for arm64. I just find it a bit weird that
> the majority of NUMA-capable architectures have to add a symbol in the arch
> Kconfig file, for what appears to be a performance optimisation applicable
> only to ia64, mips and sh.
>
> At the very least we could make the thing selectable.
Hmm, I thought this was selectable. But I am obviously wrong here.
Looking more closely, it seems that this is indeed only about
__early_pfn_to_nid and as such not something that should add a config
symbol. This should have been called out in the changelog though.
Also while at it, does HAVE_MEMBLOCK_NODE_MAP fall into a similar
bucket? Do we have any NUMA architecture that doesn't enable it?
Thanks!
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v2 0/5] mm: Enable CONFIG_NODES_SPAN_OTHER_NODES by default for NUMA
From: Will Deacon @ 2019-07-12 14:37 UTC (permalink / raw)
To: Michal Hocko
Cc: Heiko Carstens, open list:MEMORY MANAGEMENT, Paul Mackerras,
H . Peter Anvin, sparclinux@vger.kernel.org, Alexander Duyck,
linux-s390@vger.kernel.org, x86@kernel.org, willy, Mike Rapoport,
Christian Borntraeger, Ingo Molnar, Hoan Tran OS, Catalin Marinas,
Open Source Submission, Pavel Tatashin, Vasily Gorbik,
Will Deacon, Borislav Petkov, Thomas Gleixner, Vlastimil Babka,
Oscar Salvador, linux-arm-kernel@lists.infradead.org,
linux-kernel@vger.kernel.org, Andrew Morton,
linuxppc-dev@lists.ozlabs.org, David S . Miller
In-Reply-To: <20190712121223.GR29483@dhcp22.suse.cz>
Hi all,
On Fri, Jul 12, 2019 at 02:12:23PM +0200, Michal Hocko wrote:
> On Fri 12-07-19 10:56:47, Hoan Tran OS wrote:
> [...]
> > It would be good if we can enable it by-default. Otherwise, let arch
> > enables it by them-self. Do you have any suggestions?
>
> I can hardly make any suggestions when it is not really clear _why_ you
> want to remove this config option in the first place. Please explain
> what motivated you to make this change.
Sorry, I think this confusion might actually be my fault and Hoan has just
been implementing my vague suggestion here:
https://lore.kernel.org/linux-arm-kernel/20190625101245.s4vxfosoop52gl4e@willie-the-truck/
If the preference of the mm folks is to leave CONFIG_NODES_SPAN_OTHER_NODES
as it is, then we can define it for arm64. I just find it a bit weird that
the majority of NUMA-capable architectures have to add a symbol in the arch
Kconfig file, for what appears to be a performance optimisation applicable
only to ia64, mips and sh.
At the very least we could make the thing selectable.
Will
^ permalink raw reply
* Re: [PATCH 3/3] fs/core/vmcore: Move sev_active() reference to x86 arch code
From: Christoph Hellwig @ 2019-07-12 14:08 UTC (permalink / raw)
To: Halil Pasic
Cc: linux-s390, Mike Anderson, Konrad Rzeszutek Wilk, Robin Murphy,
x86, Ram Pai, linux-kernel, Alexey Dobriyan, iommu, Ingo Molnar,
Borislav Petkov, H. Peter Anvin, linux-fsdevel, Thomas Gleixner,
linuxppc-dev, Christoph Hellwig, Thiago Jung Bauermann,
Marek Szyprowski
In-Reply-To: <20190712150912.3097215e.pasic@linux.ibm.com>
On Fri, Jul 12, 2019 at 03:09:12PM +0200, Halil Pasic wrote:
> This is the implementation for the guys that don't
> have ARCH_HAS_MEM_ENCRYPT.
>
> Means sev_active() may not be used in such code after this
> patch. What about
>
> static inline bool force_dma_unencrypted(void)
> {
> return sev_active();
> }
>
> in kernel/dma/direct.c?
FYI, I have this pending in the dma-mapping tree:
http://git.infradead.org/users/hch/dma-mapping.git/commitdiff/e67a5ed1f86f4370991c601f2fcad9ebf9e1eebb
^ permalink raw reply
* Re: [PATCH v9 05/10] namei: O_BENEATH-style path resolution flags
From: Al Viro @ 2019-07-12 13:25 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, Chanho Min, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190712125552.GL17978@ZenIV.linux.org.uk>
On Fri, Jul 12, 2019 at 01:55:52PM +0100, Al Viro wrote:
> On Fri, Jul 12, 2019 at 01:39:24PM +0100, Al Viro wrote:
> > On Fri, Jul 12, 2019 at 08:57:45PM +1000, Aleksa Sarai wrote:
> >
> > > > > @@ -2350,9 +2400,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> > > > > s = ERR_PTR(error);
> > > > > return s;
> > > > > }
> > > > > - error = dirfd_path_init(nd);
> > > > > - if (unlikely(error))
> > > > > - return ERR_PTR(error);
> > > > > + if (likely(!nd->path.mnt)) {
> > > >
> > > > Is that a weird way of saying "if we hadn't already called dirfd_path_init()"?
> > >
> > > Yes. I did it to be more consistent with the other "have we got the
> > > root" checks elsewhere. Is there another way you'd prefer I do it?
> >
> > "Have we got the root" checks are inevitable evil; here you are making the
> > control flow in a single function hard to follow.
> >
> > I *think* what you are doing is
> > absolute pathname, no LOOKUP_BENEATH:
> > set_root
> > error = nd_jump_root(nd)
> > else
> > error = dirfd_path_init(nd)
> > return unlikely(error) ? ERR_PTR(error) : s;
> > which should be a lot easier to follow (not to mention shorter), but I might
> > be missing something in all of that.
>
> PS: if that's what's going on, I would be tempted to turn the entire
> path_init() part into this:
> if (flags & LOOKUP_BENEATH)
> while (*s == '/')
> s++;
> in the very beginning (plus the handling of nd_jump_root() prototype
> change, but that belongs with nd_jump_root() change itself, obviously).
> Again, I might be missing something here...
Argh... I am, at that - you have setting path->root (and grabbing it)
in LOOKUP_BENEATH cases and you do it after dirfd_path_init(). So
how about
if (flags & LOOKUP_BENEATH)
while (*s == '/')
s++;
before the whole thing and
if (*s == '/') { /* can happen only without LOOKUP_BENEATH */
set_root(nd);
error = nd_jump_root(nd);
if (unlikely(error))
return ERR_PTR(error);
} else if (nd->dfd == AT_FDCWD) {
if (flags & LOOKUP_RCU) {
struct fs_struct *fs = current->fs;
unsigned seq;
do {
seq = read_seqcount_begin(&fs->seq);
nd->path = fs->pwd;
nd->inode = nd->path.dentry->d_inode;
nd->seq = __read_seqcount_begin(&nd->path.dentry->d_seq);
} while (read_seqcount_retry(&fs->seq, seq));
} else {
get_fs_pwd(current->fs, &nd->path);
nd->inode = nd->path.dentry->d_inode;
}
} else {
/* Caller must check execute permissions on the starting path component */
struct fd f = fdget_raw(nd->dfd);
struct dentry *dentry;
if (!f.file)
return ERR_PTR(-EBADF);
dentry = f.file->f_path.dentry;
if (*s && unlikely(!d_can_lookup(dentry))) {
fdput(f);
return ERR_PTR(-ENOTDIR);
}
nd->path = f.file->f_path;
if (flags & LOOKUP_RCU) {
nd->inode = nd->path.dentry->d_inode;
nd->seq = read_seqcount_begin(&nd->path.dentry->d_seq);
} else {
path_get(&nd->path);
nd->inode = nd->path.dentry->d_inode;
}
fdput(f);
}
if (flags & LOOKUP_BENEATH) {
nd->root = nd->path;
if (!(flags & LOOKUP_RCU))
path_get(&nd->root);
else
nd->root_seq = nd->seq;
}
return s;
replacing the part in the end? Makes for much smaller change; it might
very well still make sense to add dirfd_path_init() as a separate
cleanup (perhaps with the *s == '/' case included), though.
^ permalink raw reply
* Re: [PATCH v9 01/10] namei: obey trailing magic-link DAC permissions
From: Al Viro @ 2019-07-12 13:10 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, Chanho Min, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190712122017.xkowq2cjreylpotm@yavin>
On Fri, Jul 12, 2019 at 10:20:17PM +1000, Aleksa Sarai wrote:
> On 2019-07-12, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > On Sun, Jul 07, 2019 at 12:57:28AM +1000, Aleksa Sarai wrote:
> > > @@ -514,7 +516,14 @@ static void set_nameidata(struct nameidata *p, int dfd, struct filename *name)
> > > p->stack = p->internal;
> > > p->dfd = dfd;
> > > p->name = name;
> > > - p->total_link_count = old ? old->total_link_count : 0;
> > > + p->total_link_count = 0;
> > > + p->acc_mode = 0;
> > > + p->opath_mask = FMODE_PATH_READ | FMODE_PATH_WRITE;
> > > + if (old) {
> > > + p->total_link_count = old->total_link_count;
> > > + p->acc_mode = old->acc_mode;
> > > + p->opath_mask = old->opath_mask;
> > > + }
> >
> > Huh? Could somebody explain why traversals of NFS4 referrals should inherit
> > ->acc_mode and ->opath_mask?
>
> I'll be honest -- I don't understand what set_nameidata() did so I just
> did what I thought would be an obvious change (to just copy the
> contents). I thought it was related to some aspect of the symlink stack
> handling.
No. It's handling of (very rare) nested pathwalk. The only case I can think
of is handling of NFS4 referrals - they are triggered by ->d_automount()
and include NFS4 mount. Which does internal pathwalk of its own, to get
to the root of subtree being automounted.
NFS has its own recursion protection on that path (no deeper nesting than
one level of referral traversals), but there some nesting is inevitable;
we do get another nameidata instance on stack. And for nd_jump_link() we
need to keep track of the innermost one.
For symlinks nothing of that sort happens - they are dealt with on the same
struct nameidata. ->total_link_count copying is there for one reason only -
we want the total amount of symlinks traversed during the pathwalk (including
the referral processing, etc.) to count towards MAXSYMLINKS check. It could've
been moved from nameidata to task_struct, but it's cheaper to handle it that
way.
Again, nesting is *rare*.
> In that case, should they both be set to 0 on set_nameidata()? This will
> mean that fd re-opening (or magic-link opening) through a
> set_nameidata() would always fail.
Huh? set_nameidata() is done for *all* instances - it's pretty much the
constructor of that object (and restore_nameidata() - a destructor).
Everything goes through it.
And again, I'm not sure we want these fields in nameidata - IMO they belong
in open_flags. Things like e.g. stat() don't need them at all.
Incidentally, O_PATH opening of symlinks combined with subsequent procfs
symlink traversals is worth testing - that's where the things get subtle
and that's where it's easy to get in trouble on modifications.
^ permalink raw reply
* Re: [PATCH] powerpc: mm: Limit rma_size to 1TB when running without HV mode
From: Michael Ellerman @ 2019-07-12 13:09 UTC (permalink / raw)
To: Suraj Jitindar Singh, linuxppc-dev; +Cc: sjitindarsingh, kvm-ppc, david
In-Reply-To: <20190710052018.14628-1-sjitindarsingh@gmail.com>
Suraj Jitindar Singh <sjitindarsingh@gmail.com> writes:
> The virtual real mode addressing (VRMA) mechanism is used when a
> partition is using HPT (Hash Page Table) translation and performs
> real mode accesses (MSR[IR|DR] = 0) in non-hypervisor mode. In this
> mode effective address bits 0:23 are treated as zero (i.e. the access
> is aliased to 0) and the access is performed using an implicit 1TB SLB
> entry.
>
> The size of the RMA (Real Memory Area) is communicated to the guest as
> the size of the first memory region in the device tree. And because of
> the mechanism described above can be expected to not exceed 1TB. In the
> event that the host erroneously represents the RMA as being larger than
> 1TB, guest accesses in real mode to memory addresses above 1TB will be
> aliased down to below 1TB. This means that a memory access performed in
> real mode may differ to one performed in virtual mode for the same memory
> address, which would likely have unintended consequences.
>
> To avoid this outcome have the guest explicitly limit the size of the
> RMA to the current maximum, which is 1TB. This means that even if the
> first memory block is larger than 1TB, only the first 1TB should be
> accessed in real mode.
>
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
I added:
Fixes: c3ab300ea555 ("powerpc: Add POWER9 cputable entry")
Cc: stable@vger.kernel.org # v4.6+
Which is not exactly correct, but probably good enough?
cheers
> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c b/arch/powerpc/mm/book3s64/hash_utils.c
> index 28ced26f2a00..4d0e2cce9cd5 100644
> --- a/arch/powerpc/mm/book3s64/hash_utils.c
> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
> @@ -1901,11 +1901,19 @@ void hash__setup_initial_memory_limit(phys_addr_t first_memblock_base,
> *
> * For guests on platforms before POWER9, we clamp the it limit to 1G
> * to avoid some funky things such as RTAS bugs etc...
> + * On POWER9 we limit to 1TB in case the host erroneously told us that
> + * the RMA was >1TB. Effective address bits 0:23 are treated as zero
> + * (meaning the access is aliased to zero i.e. addr = addr % 1TB)
> + * for virtual real mode addressing and so it doesn't make sense to
> + * have an area larger than 1TB as it can't be addressed.
> */
> if (!early_cpu_has_feature(CPU_FTR_HVMODE)) {
> ppc64_rma_size = first_memblock_size;
> if (!early_cpu_has_feature(CPU_FTR_ARCH_300))
> ppc64_rma_size = min_t(u64, ppc64_rma_size, 0x40000000);
> + else
> + ppc64_rma_size = min_t(u64, ppc64_rma_size,
> + 1UL << SID_SHIFT_1T);
>
> /* Finally limit subsequent allocations */
> memblock_set_current_limit(ppc64_rma_size);
> --
> 2.13.6
^ permalink raw reply
* Re: [PATCH v9 05/10] namei: O_BENEATH-style path resolution flags
From: Al Viro @ 2019-07-12 12:55 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, Chanho Min, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190712123924.GK17978@ZenIV.linux.org.uk>
On Fri, Jul 12, 2019 at 01:39:24PM +0100, Al Viro wrote:
> On Fri, Jul 12, 2019 at 08:57:45PM +1000, Aleksa Sarai wrote:
>
> > > > @@ -2350,9 +2400,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> > > > s = ERR_PTR(error);
> > > > return s;
> > > > }
> > > > - error = dirfd_path_init(nd);
> > > > - if (unlikely(error))
> > > > - return ERR_PTR(error);
> > > > + if (likely(!nd->path.mnt)) {
> > >
> > > Is that a weird way of saying "if we hadn't already called dirfd_path_init()"?
> >
> > Yes. I did it to be more consistent with the other "have we got the
> > root" checks elsewhere. Is there another way you'd prefer I do it?
>
> "Have we got the root" checks are inevitable evil; here you are making the
> control flow in a single function hard to follow.
>
> I *think* what you are doing is
> absolute pathname, no LOOKUP_BENEATH:
> set_root
> error = nd_jump_root(nd)
> else
> error = dirfd_path_init(nd)
> return unlikely(error) ? ERR_PTR(error) : s;
> which should be a lot easier to follow (not to mention shorter), but I might
> be missing something in all of that.
PS: if that's what's going on, I would be tempted to turn the entire
path_init() part into this:
if (flags & LOOKUP_BENEATH)
while (*s == '/')
s++;
in the very beginning (plus the handling of nd_jump_root() prototype
change, but that belongs with nd_jump_root() change itself, obviously).
Again, I might be missing something here...
^ permalink raw reply
* Re: [PATCH v9 05/10] namei: O_BENEATH-style path resolution flags
From: Al Viro @ 2019-07-12 12:39 UTC (permalink / raw)
To: Aleksa Sarai
Cc: linux-ia64, linux-sh, Alexei Starovoitov, linux-kernel,
David Howells, linux-kselftest, sparclinux, Shuah Khan,
linux-arch, linux-s390, Tycho Andersen, Aleksa Sarai,
linux-arm-kernel, linux-mips, linux-xtensa, Kees Cook,
Arnd Bergmann, Jann Horn, linuxppc-dev, linux-m68k,
Andy Lutomirski, Shuah Khan, David Drysdale, Christian Brauner,
J. Bruce Fields, linux-parisc, linux-api, Chanho Min, Jeff Layton,
Oleg Nesterov, Eric Biederman, linux-alpha, linux-fsdevel,
Andrew Morton, Linus Torvalds, containers
In-Reply-To: <20190712105745.nruaftgeat6irhzr@yavin>
On Fri, Jul 12, 2019 at 08:57:45PM +1000, Aleksa Sarai wrote:
> > > @@ -2350,9 +2400,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
> > > s = ERR_PTR(error);
> > > return s;
> > > }
> > > - error = dirfd_path_init(nd);
> > > - if (unlikely(error))
> > > - return ERR_PTR(error);
> > > + if (likely(!nd->path.mnt)) {
> >
> > Is that a weird way of saying "if we hadn't already called dirfd_path_init()"?
>
> Yes. I did it to be more consistent with the other "have we got the
> root" checks elsewhere. Is there another way you'd prefer I do it?
"Have we got the root" checks are inevitable evil; here you are making the
control flow in a single function hard to follow.
I *think* what you are doing is
absolute pathname, no LOOKUP_BENEATH:
set_root
error = nd_jump_root(nd)
else
error = dirfd_path_init(nd)
return unlikely(error) ? ERR_PTR(error) : s;
which should be a lot easier to follow (not to mention shorter), but I might
be missing something in all of that.
^ permalink raw reply
* Re: [PATCH v2] powerpc/book3s/mm: Update Oops message to print the correct translation in use
From: Christophe Leroy @ 2019-07-12 12:25 UTC (permalink / raw)
To: Michael Ellerman, Aneesh Kumar K.V, npiggin, paulus; +Cc: linuxppc-dev
In-Reply-To: <87r26ve93k.fsf@concordia.ellerman.id.au>
Le 12/07/2019 à 14:22, Michael Ellerman a écrit :
> Christophe Leroy <christophe.leroy@c-s.fr> writes:
>> Le 12/07/2019 à 08:25, Michael Ellerman a écrit :
>>> "Aneesh Kumar K.V" <aneesh.kumar@linux.ibm.com> writes:
> ...
>>>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>>>> index 11caa0291254..b181d6860f28 100644
>>>> --- a/arch/powerpc/kernel/traps.c
>>>> +++ b/arch/powerpc/kernel/traps.c
>>>> @@ -250,15 +250,22 @@ static void oops_end(unsigned long flags, struct pt_regs *regs,
>>>> }
>>>> NOKPROBE_SYMBOL(oops_end);
>>>>
>>>> +static char *get_mmu_str(void)
>>>> +{
>>>> + if (early_radix_enabled())
>>>> + return " MMU=Radix";
>>>> + if (early_mmu_has_feature(MMU_FTR_HPTE_TABLE))
>>>> + return " MMU=Hash";
>>>> + return "";
>>>> +}
>>>
>>> We don't change MMU once we're up, so just do this logic once and stash
>>> it into a static string, rather than rechecking on every oops.
>>
>> Do we really have oops so often that we have to worry about that ?
>
> Sometimes :)
>
> But no I don't mean it's a performance issue, it just seems simpler to
> compute the value once and store it. In fact for most platforms it can
> just be a static string at compile time, it's only 64-bit Book3S that
> needs to do anything at runtime.
Right, but I'm sure GCC will take care of that since the function is
static and called only once.
Christophe
>
> cheers
>
^ 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