* Re: [PATCH v3 -rcu] workqueue: Convert for_each_wq to use built-in list check
From: Paul E. McKenney @ 2019-08-16 16:45 UTC (permalink / raw)
To: Joel Fernandes (Google)
Cc: linux-kernel, Greg Kroah-Hartman, Jonathan Corbet, Josh Triplett,
Lai Jiangshan, linux-doc, Mathieu Desnoyers, Rafael J. Wysocki,
rcu, Steven Rostedt, Tejun Heo
In-Reply-To: <20190815141842.GB20599@google.com>
On Thu, Aug 15, 2019 at 10:18:42AM -0400, Joel Fernandes (Google) wrote:
> list_for_each_entry_rcu now has support to check for RCU reader sections
> as well as lock. Just use the support in it, instead of explicitly
> checking in the caller.
>
> Acked-by: Tejun Heo <tj@kernel.org>
> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org>
Pulled into -rcu for testing and further review, thank you!
Thanx, Paul
> ---
> v1->v3: Changed lock_is_held() to lockdep_is_held()
>
> kernel/workqueue.c | 10 ++--------
> 1 file changed, 2 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> index 601d61150b65..e882477ebf6e 100644
> --- a/kernel/workqueue.c
> +++ b/kernel/workqueue.c
> @@ -364,11 +364,6 @@ static void workqueue_sysfs_unregister(struct workqueue_struct *wq);
> !lockdep_is_held(&wq_pool_mutex), \
> "RCU or wq_pool_mutex should be held")
>
> -#define assert_rcu_or_wq_mutex(wq) \
> - RCU_LOCKDEP_WARN(!rcu_read_lock_held() && \
> - !lockdep_is_held(&wq->mutex), \
> - "RCU or wq->mutex should be held")
> -
> #define assert_rcu_or_wq_mutex_or_pool_mutex(wq) \
> RCU_LOCKDEP_WARN(!rcu_read_lock_held() && \
> !lockdep_is_held(&wq->mutex) && \
> @@ -425,9 +420,8 @@ static void workqueue_sysfs_unregister(struct workqueue_struct *wq);
> * ignored.
> */
> #define for_each_pwq(pwq, wq) \
> - list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node) \
> - if (({ assert_rcu_or_wq_mutex(wq); false; })) { } \
> - else
> + list_for_each_entry_rcu((pwq), &(wq)->pwqs, pwqs_node, \
> + lockdep_is_held(&(wq->mutex)))
>
> #ifdef CONFIG_DEBUG_OBJECTS_WORK
>
> --
> 2.23.0.rc1.153.gdeed80330f-goog
>
^ permalink raw reply
* Re: [PATCH v8 04/27] x86/fpu/xstate: Introduce XSAVES system states
From: Thomas Gleixner @ 2019-08-16 19:56 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Ingo Molnar, linux-kernel, linux-doc,
linux-mm, linux-arch, linux-api, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Borislav Petkov, Cyrill Gorcunov, Dave Hansen,
Eugene Syromiatnikov, Florian Weimer, H.J. Lu, Jann Horn,
Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <20190813205225.12032-5-yu-cheng.yu@intel.com>
On Tue, 13 Aug 2019, Yu-cheng Yu wrote:
> +/*
> + * On context switches, XSAVE states are not restored until returning
> + * to user-mode. FPU registers need to be restored before any changes,
> + * and protected by fpregs_lock()/fpregs_unlock().
I really had to read this comment twice to figure out what it means.
> + */
> +static inline void modify_fpu_regs_begin(void)
Please use a proper name space. fpu_regs_....
> +{
> + fpregs_lock();
> + if (test_thread_flag(TIF_NEED_FPU_LOAD))
> + __fpregs_load_activate();
> +}
> +
> +static inline void modify_fpu_regs_end(void)
> +{
> + fpregs_unlock();
> +}
Also why are those inlines in this particular patch? I see no relation at all.
> /*
> * MXCSR and XCR definitions:
> */
> diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
> index 9ded9532257d..970bbd303cfb 100644
> --- a/arch/x86/include/asm/fpu/xstate.h
> +++ b/arch/x86/include/asm/fpu/xstate.h
> @@ -21,9 +21,6 @@
> #define XSAVE_YMM_SIZE 256
> #define XSAVE_YMM_OFFSET (XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET)
>
> -/* Supervisor features */
> -#define XFEATURE_MASK_SUPERVISOR (XFEATURE_MASK_PT)
> -
> /* All currently supported features */
> #define SUPPORTED_XFEATURES_MASK (XFEATURE_MASK_FP | \
> XFEATURE_MASK_SSE | \
> @@ -42,6 +39,7 @@
> #endif
>
> extern u64 xfeatures_mask_user;
> +extern u64 xfeatures_mask_all;
> extern u64 xstate_fx_sw_bytes[USER_XSTATE_FX_SW_WORDS];
>
> extern void __init update_regset_xstate_info(unsigned int size,
> diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> index 12c70840980e..31d3cd70b5df 100644
> --- a/arch/x86/kernel/fpu/core.c
> +++ b/arch/x86/kernel/fpu/core.c
> @@ -294,12 +294,16 @@ void fpu__drop(struct fpu *fpu)
> * Clear FPU registers by setting them up from
> * the init fpstate:
> */
> -static inline void copy_init_fpstate_to_fpregs(void)
> +static inline void copy_init_fpstate_to_fpregs(u64 features_mask)
> {
> fpregs_lock();
>
> + /*
> + * Only XSAVES user states are copied.
> + * System states are preserved.
Fits nicely in one line and aside of that this comment is blatantly
wrong. See that caller:
> + copy_init_fpstate_to_fpregs(xfeatures_mask_all);
xfeatures_mask_all includes xfeatures_mask_system unless I'm missing
something.
> + */
> if (use_xsave())
> - copy_kernel_to_xregs(&init_fpstate.xsave, -1);
> + copy_kernel_to_xregs(&init_fpstate.xsave, features_mask);
> else if (static_cpu_has(X86_FEATURE_FXSR))
> copy_kernel_to_fxregs(&init_fpstate.fxsave);
The change of this function should also be split out into a separate
patch. This one is way too big to be reviewable.
> else
> @@ -318,7 +322,21 @@ static inline void copy_init_fpstate_to_fpregs(void)
> * Called by sys_execve(), by the signal handler code and by various
> * error paths.
> */
> -void fpu__clear(struct fpu *fpu)
> +void fpu__clear_user_states(struct fpu *fpu)
> +{
> + WARN_ON_FPU(fpu != ¤t->thread.fpu); /* Almost certainly an anomaly */
1) Please do not use tail comments. They break the reading flow.
2) Please do not comment the obvious. Put comments where they make sense. I
know you copied it, but that does not make it any better.
> + fpu__drop(fpu);
> +
> + /*
> + * Make sure fpstate is cleared and initialized.
> + */
> + fpu__initialize(fpu);
> + if (static_cpu_has(X86_FEATURE_FPU))
> + copy_init_fpstate_to_fpregs(xfeatures_mask_user);
> +}
> +
> +void fpu__clear_all(struct fpu *fpu)
> {
> WARN_ON_FPU(fpu != ¤t->thread.fpu); /* Almost certainly an anomaly */
> @@ -329,7 +347,7 @@ void fpu__clear(struct fpu *fpu)
> */
> fpu__initialize(fpu);
> if (static_cpu_has(X86_FEATURE_FPU))
> - copy_init_fpstate_to_fpregs();
> + copy_init_fpstate_to_fpregs(xfeatures_mask_all);
> }
>
> /*
> diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
> index 73fed33e5bda..0a0ba584a533 100644
> --- a/arch/x86/kernel/fpu/init.c
> +++ b/arch/x86/kernel/fpu/init.c
> @@ -217,16 +217,6 @@ static void __init fpu__init_system_xstate_size_legacy(void)
> fpu_user_xstate_size = fpu_kernel_xstate_size;
> }
>
> -/*
> - * Find supported xfeatures based on cpu features and command-line input.
> - * This must be called after fpu__init_parse_early_param() is called and
> - * xfeatures_mask is enumerated.
> - */
> -u64 __init fpu__get_supported_xfeatures_mask(void)
> -{
> - return SUPPORTED_XFEATURES_MASK;
> -}
> -
> /* Legacy code to initialize eager fpu mode. */
> static void __init fpu__init_system_ctx_switch(void)
> {
> diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> index 8a63f07cf400..4ecf1764a971 100644
> --- a/arch/x86/kernel/fpu/signal.c
> +++ b/arch/x86/kernel/fpu/signal.c
> @@ -285,7 +285,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
> IS_ENABLED(CONFIG_IA32_EMULATION));
>
> if (!buf) {
> - fpu__clear(fpu);
> + fpu__clear_user_states(fpu);
> return 0;
> }
>
> @@ -407,7 +407,7 @@ static int __fpu__restore_sig(void __user *buf, void __user *buf_fx, int size)
>
> err_out:
> if (ret)
> - fpu__clear(fpu);
> + fpu__clear_user_states(fpu);
> return ret;
> }
>
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index d560e8861a3c..9fbe73c546df 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -61,9 +61,19 @@ static short xsave_cpuid_features[] __initdata = {
> */
> u64 xfeatures_mask_user __read_mostly;
>
> +/*
> + * Supported XSAVES system states.
> + */
> +static u64 xfeatures_mask_system __read_mostly;
> +
> +/*
> + * Combined XSAVES system and user states.
> + */
> +u64 xfeatures_mask_all __read_mostly;
> +
> static unsigned int xstate_offsets[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX - 1] = -1};
> static unsigned int xstate_sizes[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX - 1] = -1};
> -static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask_user)*8];
> +static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask_all)*8];
[sizeof(...) * 8]
>
> /*
> * The XSAVE area of kernel can be in standard or compacted format;
> @@ -79,7 +89,7 @@ unsigned int fpu_user_xstate_size;
> */
> int cpu_has_xfeatures(u64 xfeatures_needed, const char **feature_name)
> {
> - u64 xfeatures_missing = xfeatures_needed & ~xfeatures_mask_user;
> + u64 xfeatures_missing = xfeatures_needed & ~xfeatures_mask_all;
>
> if (unlikely(feature_name)) {
> long xfeature_idx, max_idx;
> @@ -158,7 +168,7 @@ void fpstate_sanitize_xstate(struct fpu *fpu)
> * None of the feature bits are in init state. So nothing else
> * to do for us, as the memory layout is up to date.
> */
> - if ((xfeatures & xfeatures_mask_user) == xfeatures_mask_user)
> + if ((xfeatures & xfeatures_mask_all) == xfeatures_mask_all)
> return;
>
> /*
> @@ -213,28 +223,27 @@ void fpstate_sanitize_xstate(struct fpu *fpu)
> */
> void fpu__init_cpu_xstate(void)
> {
> - if (!boot_cpu_has(X86_FEATURE_XSAVE) || !xfeatures_mask_user)
> + if (!boot_cpu_has(X86_FEATURE_XSAVE) || !xfeatures_mask_all)
> return;
> /*
> * XCR_XFEATURE_ENABLED_MASK sets the features that are managed
> * by XSAVE{C, OPT} and XRSTOR. Only XSAVE user states can be
> * set here.
> */
> -
> - xfeatures_mask_user &= ~XFEATURE_MASK_SUPERVISOR;
> -
> cr4_set_bits(X86_CR4_OSXSAVE);
> xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask_user);
> +
> + /*
> + * MSR_IA32_XSS controls which system (not user) states are
We know that system state is not including user state. Please stop
documenting the obvious.
> + * to be managed by XSAVES.
> + */
> + if (boot_cpu_has(X86_FEATURE_XSAVES))
> + wrmsrl(MSR_IA32_XSS, xfeatures_mask_system);
> }
>
> -/*
> - * Note that in the future we will likely need a pair of
> - * functions here: one for user xstates and the other for
> - * system xstates. For now, they are the same.
> - */
> static int xfeature_enabled(enum xfeature xfeature)
> {
> - return !!(xfeatures_mask_user & BIT_ULL(xfeature));
> + return !!(xfeatures_mask_all & BIT_ULL(xfeature));
> }
>
> /*
> @@ -340,7 +349,7 @@ static int xfeature_is_aligned(int xfeature_nr)
> */
> static void __init setup_xstate_comp(void)
> {
> - unsigned int xstate_comp_sizes[sizeof(xfeatures_mask_user)*8];
> + unsigned int xstate_comp_sizes[sizeof(xfeatures_mask_all)*8];
See above
> int i;
>
> /*
> @@ -413,7 +422,7 @@ static void __init setup_init_fpu_buf(void)
> print_xstate_features();
>
> if (boot_cpu_has(X86_FEATURE_XSAVES))
> - init_fpstate.xsave.header.xcomp_bv = BIT_ULL(63) | xfeatures_mask_user;
> + init_fpstate.xsave.header.xcomp_bv = BIT_ULL(63) | xfeatures_mask_all;
>
> /*
> * Init all the features state with header.xfeatures being 0x0
> @@ -436,7 +445,7 @@ static int xfeature_uncompacted_offset(int xfeature_nr)
> * format. Checking a system state's uncompacted offset is
> * an error.
> */
> - if (XFEATURE_MASK_SUPERVISOR & BIT_ULL(xfeature_nr)) {
> + if (~xfeatures_mask_user & BIT_ULL(xfeature_nr)) {
Sigh. Why can't this use xfeatures_mask_system? That would be too obvious.
> WARN_ONCE(1, "No fixed offset for xstate %d\n", xfeature_nr);
> return -1;
> }
> @@ -608,15 +617,12 @@ static void do_extra_xstate_size_checks(void)
>
>
> /*
> - * Get total size of enabled xstates in XCR0/xfeatures_mask_user.
> + * Get total size of enabled xstates in XCR0 | IA32_XSS.
> *
> * Note the SDM's wording here. "sub-function 0" only enumerates
> * the size of the *user* states. If we use it to size a buffer
> * that we use 'XSAVES' on, we could potentially overflow the
> * buffer because 'XSAVES' saves system states too.
> - *
> - * Note that we do not currently set any bits on IA32_XSS so
> - * 'XCR0 | IA32_XSS == XCR0' for now.
> */
> static unsigned int __init get_xsaves_size(void)
> {
> @@ -698,6 +704,7 @@ static int __init init_xstate_size(void)
> */
> static void fpu__init_disable_system_xstate(void)
> {
> + xfeatures_mask_all = 0;
> xfeatures_mask_user = 0;
> cr4_clear_bits(X86_CR4_OSXSAVE);
> setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> @@ -733,10 +740,23 @@ void __init fpu__init_system_xstate(void)
> return;
> }
>
> + /*
> + * Find user states supported by the processor.
> + * Only these bits can be set in XCR0.
> + */
> cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
> xfeatures_mask_user = eax + ((u64)edx << 32);
>
> - if ((xfeatures_mask_user & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
> + /*
> + * Find system states supported by the processor.
> + * Only these bits can be set in IA32_XSS MSR.
> + */
> + cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
> + xfeatures_mask_system = ecx + ((u64)edx << 32);
> +
> + xfeatures_mask_all = xfeatures_mask_user | xfeatures_mask_system;
> +
> + if ((xfeatures_mask_all & XFEATURE_MASK_FPSSE) != XFEATURE_MASK_FPSSE) {
xfeatures_mask_all is wrong here. FPSSE is clearly user state.
> /*
> * This indicates that something really unexpected happened
> * with the enumeration. Disable XSAVE and try to continue
> @@ -751,10 +771,12 @@ void __init fpu__init_system_xstate(void)
> */
> for (i = 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) {
> if (!boot_cpu_has(xsave_cpuid_features[i]))
> - xfeatures_mask_user &= ~BIT_ULL(i);
> + xfeatures_mask_all &= ~BIT_ULL(i);
> }
>
> - xfeatures_mask_user &= fpu__get_supported_xfeatures_mask();
> + xfeatures_mask_all &= SUPPORTED_XFEATURES_MASK;
> + xfeatures_mask_user &= xfeatures_mask_all;
> + xfeatures_mask_system &= xfeatures_mask_all;
>
> /* Enable xstate instructions to be able to continue with initialization: */
> fpu__init_cpu_xstate();
> @@ -766,7 +788,7 @@ void __init fpu__init_system_xstate(void)
> * Update info used for ptrace frames; use standard-format size and no
> * system xstates:
> */
> - update_regset_xstate_info(fpu_user_xstate_size, xfeatures_mask_user & ~XFEATURE_MASK_SUPERVISOR);
> + update_regset_xstate_info(fpu_user_xstate_size, xfeatures_mask_user);
>
And exactly this hunk shows that the whole refactoring approach is wrong
from the very beginning. I stared at that in the previous patch already and
had the feeling that it's bogus.
Just doing a s/xfeatures_mask/xfeatures_mask_user/g really does not make
any sense. Simply because the current code assumes that xfeatures_mask ==
xfeatures_mask_all. So if a global rename is the right approach then
s/xfeatures_mask/xfeatures_mask_all/ and not that completely backwards
rename to _user.
That refactoring wants to be done in the following steps:
1) Introduce xfeatures_mask_user and initialize it with
xfeatures_mask_user = xfeatures_mask ^ ~XFEATURE_MASK_SUPERVISOR;
2) Fix up the usage sites in reviewable chunks. It does not matter
whether that could be folded into a larger all in one patch. What
matters is that it makes sense and is reviewable.
3) Change the signature of copy_init_fpstate_to_fpregs() so it takes a
mask and fix up the call sites accordingly. Without the bogus comment
of course.
4) Introduce xfeatures_mask_system and eventually needed helper functions.
5) Change the affected usage sites
Details may be slightly different but you get the idea.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v8 04/27] x86/fpu/xstate: Introduce XSAVES system states
From: Yu-cheng Yu @ 2019-08-16 19:59 UTC (permalink / raw)
To: Thomas Gleixner
Cc: x86, H. Peter Anvin, Ingo Molnar, linux-kernel, linux-doc,
linux-mm, linux-arch, linux-api, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Borislav Petkov, Cyrill Gorcunov, Dave Hansen,
Eugene Syromiatnikov, Florian Weimer, H.J. Lu, Jann Horn,
Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek, Peter Zijlstra, Randy Dunlap,
Ravi V. Shankar, Vedvyas Shanbhogue, Dave Martin
In-Reply-To: <alpine.DEB.2.21.1908161703010.1923@nanos.tec.linutronix.de>
On Fri, 2019-08-16 at 21:56 +0200, Thomas Gleixner wrote:
> On Tue, 13 Aug 2019, Yu-cheng Yu wrote:
> > +/*
> > + * On context switches, XSAVE states are not restored until returning
> > + * to user-mode. FPU registers need to be restored before any changes,
> > + * and protected by fpregs_lock()/fpregs_unlock().
>
> I really had to read this comment twice to figure out what it means.
>
> > + */
> > +static inline void modify_fpu_regs_begin(void)
>
> Please use a proper name space. fpu_regs_....
>
> > +{
> > + fpregs_lock();
> > + if (test_thread_flag(TIF_NEED_FPU_LOAD))
> > + __fpregs_load_activate();
> > +}
> > +
> > +static inline void modify_fpu_regs_end(void)
> > +{
> > + fpregs_unlock();
> > +}
>
> Also why are those inlines in this particular patch? I see no relation at all.
>
> > /*
> > * MXCSR and XCR definitions:
> > */
> > diff --git a/arch/x86/include/asm/fpu/xstate.h
> > b/arch/x86/include/asm/fpu/xstate.h
> > index 9ded9532257d..970bbd303cfb 100644
> > --- a/arch/x86/include/asm/fpu/xstate.h
> > +++ b/arch/x86/include/asm/fpu/xstate.h
> > @@ -21,9 +21,6 @@
> > #define XSAVE_YMM_SIZE 256
> > #define XSAVE_YMM_OFFSET (XSAVE_HDR_SIZE + XSAVE_HDR_OFFSET)
> >
> > -/* Supervisor features */
> > -#define XFEATURE_MASK_SUPERVISOR (XFEATURE_MASK_PT)
> > -
> > /* All currently supported features */
> > #define SUPPORTED_XFEATURES_MASK (XFEATURE_MASK_FP | \
> > XFEATURE_MASK_SSE | \
> > @@ -42,6 +39,7 @@
> > #endif
> >
> > extern u64 xfeatures_mask_user;
> > +extern u64 xfeatures_mask_all;
> > extern u64 xstate_fx_sw_bytes[USER_XSTATE_FX_SW_WORDS];
> >
> > extern void __init update_regset_xstate_info(unsigned int size,
> > diff --git a/arch/x86/kernel/fpu/core.c b/arch/x86/kernel/fpu/core.c
> > index 12c70840980e..31d3cd70b5df 100644
> > --- a/arch/x86/kernel/fpu/core.c
> > +++ b/arch/x86/kernel/fpu/core.c
> > @@ -294,12 +294,16 @@ void fpu__drop(struct fpu *fpu)
> > * Clear FPU registers by setting them up from
> > * the init fpstate:
> > */
> > -static inline void copy_init_fpstate_to_fpregs(void)
> > +static inline void copy_init_fpstate_to_fpregs(u64 features_mask)
> > {
> > fpregs_lock();
> >
> > + /*
> > + * Only XSAVES user states are copied.
> > + * System states are preserved.
>
> Fits nicely in one line and aside of that this comment is blatantly
> wrong. See that caller:
>
> > + copy_init_fpstate_to_fpregs(xfeatures_mask_all);
>
> xfeatures_mask_all includes xfeatures_mask_system unless I'm missing
> something.
>
> > + */
> > if (use_xsave())
> > - copy_kernel_to_xregs(&init_fpstate.xsave, -1);
> > + copy_kernel_to_xregs(&init_fpstate.xsave, features_mask);
> > else if (static_cpu_has(X86_FEATURE_FXSR))
> > copy_kernel_to_fxregs(&init_fpstate.fxsave);
>
> The change of this function should also be split out into a separate
> patch. This one is way too big to be reviewable.
>
> > else
> > @@ -318,7 +322,21 @@ static inline void copy_init_fpstate_to_fpregs(void)
> > * Called by sys_execve(), by the signal handler code and by various
> > * error paths.
> > */
> > -void fpu__clear(struct fpu *fpu)
> > +void fpu__clear_user_states(struct fpu *fpu)
> > +{
> > + WARN_ON_FPU(fpu != ¤t->thread.fpu); /* Almost certainly an
> > anomaly */
>
> 1) Please do not use tail comments. They break the reading flow.
>
> 2) Please do not comment the obvious. Put comments where they make sense. I
> know you copied it, but that does not make it any better.
>
> > + fpu__drop(fpu);
> > +
> > + /*
> > + * Make sure fpstate is cleared and initialized.
> > + */
> > + fpu__initialize(fpu);
> > + if (static_cpu_has(X86_FEATURE_FPU))
> > + copy_init_fpstate_to_fpregs(xfeatures_mask_user);
> > +}
> > +
> > +void fpu__clear_all(struct fpu *fpu)
> > {
> > WARN_ON_FPU(fpu != ¤t->thread.fpu); /* Almost certainly an
> > anomaly */
> > @@ -329,7 +347,7 @@ void fpu__clear(struct fpu *fpu)
> > */
> > fpu__initialize(fpu);
> > if (static_cpu_has(X86_FEATURE_FPU))
> > - copy_init_fpstate_to_fpregs();
> > + copy_init_fpstate_to_fpregs(xfeatures_mask_all);
> > }
> >
> > /*
> > diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c
> > index 73fed33e5bda..0a0ba584a533 100644
> > --- a/arch/x86/kernel/fpu/init.c
> > +++ b/arch/x86/kernel/fpu/init.c
> > @@ -217,16 +217,6 @@ static void __init
> > fpu__init_system_xstate_size_legacy(void)
> > fpu_user_xstate_size = fpu_kernel_xstate_size;
> > }
> >
> > -/*
> > - * Find supported xfeatures based on cpu features and command-line input.
> > - * This must be called after fpu__init_parse_early_param() is called and
> > - * xfeatures_mask is enumerated.
> > - */
> > -u64 __init fpu__get_supported_xfeatures_mask(void)
> > -{
> > - return SUPPORTED_XFEATURES_MASK;
> > -}
> > -
> > /* Legacy code to initialize eager fpu mode. */
> > static void __init fpu__init_system_ctx_switch(void)
> > {
> > diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
> > index 8a63f07cf400..4ecf1764a971 100644
> > --- a/arch/x86/kernel/fpu/signal.c
> > +++ b/arch/x86/kernel/fpu/signal.c
> > @@ -285,7 +285,7 @@ static int __fpu__restore_sig(void __user *buf, void
> > __user *buf_fx, int size)
> > IS_ENABLED(CONFIG_IA32_EMULATION));
> >
> > if (!buf) {
> > - fpu__clear(fpu);
> > + fpu__clear_user_states(fpu);
> > return 0;
> > }
> >
> > @@ -407,7 +407,7 @@ static int __fpu__restore_sig(void __user *buf, void
> > __user *buf_fx, int size)
> >
> > err_out:
> > if (ret)
> > - fpu__clear(fpu);
> > + fpu__clear_user_states(fpu);
> > return ret;
> > }
> >
> > diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> > index d560e8861a3c..9fbe73c546df 100644
> > --- a/arch/x86/kernel/fpu/xstate.c
> > +++ b/arch/x86/kernel/fpu/xstate.c
> > @@ -61,9 +61,19 @@ static short xsave_cpuid_features[] __initdata = {
> > */
> > u64 xfeatures_mask_user __read_mostly;
> >
> > +/*
> > + * Supported XSAVES system states.
> > + */
> > +static u64 xfeatures_mask_system __read_mostly;
> > +
> > +/*
> > + * Combined XSAVES system and user states.
> > + */
> > +u64 xfeatures_mask_all __read_mostly;
> > +
> > static unsigned int xstate_offsets[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX -
> > 1] = -1};
> > static unsigned int xstate_sizes[XFEATURE_MAX] = { [ 0 ... XFEATURE_MAX -
> > 1] = -1};
> > -static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask_user)*8];
> > +static unsigned int xstate_comp_offsets[sizeof(xfeatures_mask_all)*8];
>
> [sizeof(...) * 8]
>
> >
> > /*
> > * The XSAVE area of kernel can be in standard or compacted format;
> > @@ -79,7 +89,7 @@ unsigned int fpu_user_xstate_size;
> > */
> > int cpu_has_xfeatures(u64 xfeatures_needed, const char **feature_name)
> > {
> > - u64 xfeatures_missing = xfeatures_needed & ~xfeatures_mask_user;
> > + u64 xfeatures_missing = xfeatures_needed & ~xfeatures_mask_all;
> >
> > if (unlikely(feature_name)) {
> > long xfeature_idx, max_idx;
> > @@ -158,7 +168,7 @@ void fpstate_sanitize_xstate(struct fpu *fpu)
> > * None of the feature bits are in init state. So nothing else
> > * to do for us, as the memory layout is up to date.
> > */
> > - if ((xfeatures & xfeatures_mask_user) == xfeatures_mask_user)
> > + if ((xfeatures & xfeatures_mask_all) == xfeatures_mask_all)
> > return;
> >
> > /*
> > @@ -213,28 +223,27 @@ void fpstate_sanitize_xstate(struct fpu *fpu)
> > */
> > void fpu__init_cpu_xstate(void)
> > {
> > - if (!boot_cpu_has(X86_FEATURE_XSAVE) || !xfeatures_mask_user)
> > + if (!boot_cpu_has(X86_FEATURE_XSAVE) || !xfeatures_mask_all)
> > return;
> > /*
> > * XCR_XFEATURE_ENABLED_MASK sets the features that are managed
> > * by XSAVE{C, OPT} and XRSTOR. Only XSAVE user states can be
> > * set here.
> > */
> > -
> > - xfeatures_mask_user &= ~XFEATURE_MASK_SUPERVISOR;
> > -
> > cr4_set_bits(X86_CR4_OSXSAVE);
> > xsetbv(XCR_XFEATURE_ENABLED_MASK, xfeatures_mask_user);
> > +
> > + /*
> > + * MSR_IA32_XSS controls which system (not user) states are
>
> We know that system state is not including user state. Please stop
> documenting the obvious.
>
> > + * to be managed by XSAVES.
> > + */
> > + if (boot_cpu_has(X86_FEATURE_XSAVES))
> > + wrmsrl(MSR_IA32_XSS, xfeatures_mask_system);
> > }
> >
> > -/*
> > - * Note that in the future we will likely need a pair of
> > - * functions here: one for user xstates and the other for
> > - * system xstates. For now, they are the same.
> > - */
> > static int xfeature_enabled(enum xfeature xfeature)
> > {
> > - return !!(xfeatures_mask_user & BIT_ULL(xfeature));
> > + return !!(xfeatures_mask_all & BIT_ULL(xfeature));
> > }
> >
> > /*
> > @@ -340,7 +349,7 @@ static int xfeature_is_aligned(int xfeature_nr)
> > */
> > static void __init setup_xstate_comp(void)
> > {
> > - unsigned int xstate_comp_sizes[sizeof(xfeatures_mask_user)*8];
> > + unsigned int xstate_comp_sizes[sizeof(xfeatures_mask_all)*8];
>
> See above
>
> > int i;
> >
> > /*
> > @@ -413,7 +422,7 @@ static void __init setup_init_fpu_buf(void)
> > print_xstate_features();
> >
> > if (boot_cpu_has(X86_FEATURE_XSAVES))
> > - init_fpstate.xsave.header.xcomp_bv = BIT_ULL(63) |
> > xfeatures_mask_user;
> > + init_fpstate.xsave.header.xcomp_bv = BIT_ULL(63) |
> > xfeatures_mask_all;
> >
> > /*
> > * Init all the features state with header.xfeatures being 0x0
> > @@ -436,7 +445,7 @@ static int xfeature_uncompacted_offset(int xfeature_nr)
> > * format. Checking a system state's uncompacted offset is
> > * an error.
> > */
> > - if (XFEATURE_MASK_SUPERVISOR & BIT_ULL(xfeature_nr)) {
> > + if (~xfeatures_mask_user & BIT_ULL(xfeature_nr)) {
>
> Sigh. Why can't this use xfeatures_mask_system? That would be too obvious.
>
> > WARN_ONCE(1, "No fixed offset for xstate %d\n",
> > xfeature_nr);
> > return -1;
> > }
> > @@ -608,15 +617,12 @@ static void do_extra_xstate_size_checks(void)
> >
> >
> > /*
> > - * Get total size of enabled xstates in XCR0/xfeatures_mask_user.
> > + * Get total size of enabled xstates in XCR0 | IA32_XSS.
> > *
> > * Note the SDM's wording here. "sub-function 0" only enumerates
> > * the size of the *user* states. If we use it to size a buffer
> > * that we use 'XSAVES' on, we could potentially overflow the
> > * buffer because 'XSAVES' saves system states too.
> > - *
> > - * Note that we do not currently set any bits on IA32_XSS so
> > - * 'XCR0 | IA32_XSS == XCR0' for now.
> > */
> > static unsigned int __init get_xsaves_size(void)
> > {
> > @@ -698,6 +704,7 @@ static int __init init_xstate_size(void)
> > */
> > static void fpu__init_disable_system_xstate(void)
> > {
> > + xfeatures_mask_all = 0;
> > xfeatures_mask_user = 0;
> > cr4_clear_bits(X86_CR4_OSXSAVE);
> > setup_clear_cpu_cap(X86_FEATURE_XSAVE);
> > @@ -733,10 +740,23 @@ void __init fpu__init_system_xstate(void)
> > return;
> > }
> >
> > + /*
> > + * Find user states supported by the processor.
> > + * Only these bits can be set in XCR0.
> > + */
> > cpuid_count(XSTATE_CPUID, 0, &eax, &ebx, &ecx, &edx);
> > xfeatures_mask_user = eax + ((u64)edx << 32);
> >
> > - if ((xfeatures_mask_user & XFEATURE_MASK_FPSSE) !=
> > XFEATURE_MASK_FPSSE) {
> > + /*
> > + * Find system states supported by the processor.
> > + * Only these bits can be set in IA32_XSS MSR.
> > + */
> > + cpuid_count(XSTATE_CPUID, 1, &eax, &ebx, &ecx, &edx);
> > + xfeatures_mask_system = ecx + ((u64)edx << 32);
> > +
> > + xfeatures_mask_all = xfeatures_mask_user | xfeatures_mask_system;
> > +
> > + if ((xfeatures_mask_all & XFEATURE_MASK_FPSSE) !=
> > XFEATURE_MASK_FPSSE) {
>
> xfeatures_mask_all is wrong here. FPSSE is clearly user state.
>
> > /*
> > * This indicates that something really unexpected happened
> > * with the enumeration. Disable XSAVE and try to continue
> > @@ -751,10 +771,12 @@ void __init fpu__init_system_xstate(void)
> > */
> > for (i = 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) {
> > if (!boot_cpu_has(xsave_cpuid_features[i]))
> > - xfeatures_mask_user &= ~BIT_ULL(i);
> > + xfeatures_mask_all &= ~BIT_ULL(i);
> > }
> >
> > - xfeatures_mask_user &= fpu__get_supported_xfeatures_mask();
> > + xfeatures_mask_all &= SUPPORTED_XFEATURES_MASK;
> > + xfeatures_mask_user &= xfeatures_mask_all;
> > + xfeatures_mask_system &= xfeatures_mask_all;
> >
> > /* Enable xstate instructions to be able to continue with
> > initialization: */
> > fpu__init_cpu_xstate();
> > @@ -766,7 +788,7 @@ void __init fpu__init_system_xstate(void)
> > * Update info used for ptrace frames; use standard-format size and
> > no
> > * system xstates:
> > */
> > - update_regset_xstate_info(fpu_user_xstate_size, xfeatures_mask_user
> > & ~XFEATURE_MASK_SUPERVISOR);
> > + update_regset_xstate_info(fpu_user_xstate_size,
> > xfeatures_mask_user);
> >
>
> And exactly this hunk shows that the whole refactoring approach is wrong
> from the very beginning. I stared at that in the previous patch already and
> had the feeling that it's bogus.
>
> Just doing a s/xfeatures_mask/xfeatures_mask_user/g really does not make
> any sense. Simply because the current code assumes that xfeatures_mask ==
> xfeatures_mask_all. So if a global rename is the right approach then
> s/xfeatures_mask/xfeatures_mask_all/ and not that completely backwards
> rename to _user.
>
> That refactoring wants to be done in the following steps:
>
> 1) Introduce xfeatures_mask_user and initialize it with
>
> xfeatures_mask_user = xfeatures_mask ^ ~XFEATURE_MASK_SUPERVISOR;
>
> 2) Fix up the usage sites in reviewable chunks. It does not matter
> whether that could be folded into a larger all in one patch. What
> matters is that it makes sense and is reviewable.
>
> 3) Change the signature of copy_init_fpstate_to_fpregs() so it takes a
> mask and fix up the call sites accordingly. Without the bogus comment
> of course.
>
> 4) Introduce xfeatures_mask_system and eventually needed helper functions.
>
> 5) Change the affected usage sites
>
> Details may be slightly different but you get the idea.
I will work on it. Thanks!
Yu-cheng
^ permalink raw reply
* Re: [PATCH] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
From: Borislav Petkov @ 2019-08-17 8:44 UTC (permalink / raw)
To: Andrew Cooper
Cc: Lendacky, Thomas, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org, linux-pm@vger.kernel.org,
x86@kernel.org, Thomas Gleixner, Ingo Molnar, Rafael J . Wysocki,
Pavel Machek, Chen Yu, Jonathan Corbet
In-Reply-To: <312b307b-19cc-84f8-97e6-07dbdf07dd12@citrix.com>
On Thu, Aug 15, 2019 at 10:25:24PM +0100, Andrew Cooper wrote:
> I'm afraid that a number of hypervisors do write-discard, given the
> propensity of OSes (certainly traditionally) to go poking at bits like
> this without wrmsr_safe().
>
> You either need to read the MSR back and observe that the bit has really
> changed, or in this case as Thomas suggests, look at CPUID again (which
> will likely be the faster option for the non-virtualised case).
One thing I didn't think of when we talked about this: this happens only
after you resume the hypervisor. And the words "resume the hypervisor"
already means an improbable use case. Yeah, yeah, one can close the
laptop lid of her/his F15h or F16h machine while guests are running and
when the HV resumes, those guests won't get randomness but I can't seem
to find it in myself to say, uuh, that's an important use case...
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: [PATCH] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
From: Andrew Cooper @ 2019-08-17 11:43 UTC (permalink / raw)
To: Borislav Petkov
Cc: Lendacky, Thomas, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org, linux-pm@vger.kernel.org,
x86@kernel.org, Thomas Gleixner, Ingo Molnar, Rafael J . Wysocki,
Pavel Machek, Chen Yu, Jonathan Corbet
In-Reply-To: <20190817084410.GA15364@zn.tnic>
On 17/08/2019 09:44, Borislav Petkov wrote:
> On Thu, Aug 15, 2019 at 10:25:24PM +0100, Andrew Cooper wrote:
>> I'm afraid that a number of hypervisors do write-discard, given the
>> propensity of OSes (certainly traditionally) to go poking at bits like
>> this without wrmsr_safe().
>>
>> You either need to read the MSR back and observe that the bit has really
>> changed, or in this case as Thomas suggests, look at CPUID again (which
>> will likely be the faster option for the non-virtualised case).
> One thing I didn't think of when we talked about this: this happens only
> after you resume the hypervisor.
:) It hadn't escaped my notice, hence the intervention on this thread.
> And the words "resume the hypervisor" already means an improbable use case.
Qubes and OpenXT are two laptop+hypervisor oriented distros where
suspend/resume is a big deal, and these will have to follow AMD's
recommendation here.
However, for servers which don't do S3/S4, we can reason about safely
leaving RDRAND enabled, irrespective of guest configuration.
~Andrew
^ permalink raw reply
* Re: [PATCH] x86/CPU/AMD: Clear RDRAND CPUID bit on AMD family 15h/16h
From: Thomas Gleixner @ 2019-08-18 16:32 UTC (permalink / raw)
To: Andrew Cooper
Cc: Borislav Petkov, Lendacky, Thomas, linux-kernel@vger.kernel.org,
linux-doc@vger.kernel.org, linux-pm@vger.kernel.org,
x86@kernel.org, Ingo Molnar, Rafael J . Wysocki, Pavel Machek,
Chen Yu, Jonathan Corbet
In-Reply-To: <919c80f1-53a5-44d2-d785-88890e449b38@citrix.com>
On Sat, 17 Aug 2019, Andrew Cooper wrote:
> On 17/08/2019 09:44, Borislav Petkov wrote:
> > On Thu, Aug 15, 2019 at 10:25:24PM +0100, Andrew Cooper wrote:
> >> I'm afraid that a number of hypervisors do write-discard, given the
> >> propensity of OSes (certainly traditionally) to go poking at bits like
> >> this without wrmsr_safe().
> >>
> >> You either need to read the MSR back and observe that the bit has really
> >> changed, or in this case as Thomas suggests, look at CPUID again (which
> >> will likely be the faster option for the non-virtualised case).
> > One thing I didn't think of when we talked about this: this happens only
> > after you resume the hypervisor.
>
> :) It hadn't escaped my notice, hence the intervention on this thread.
>
> > And the words "resume the hypervisor" already means an improbable use case.
>
> Qubes and OpenXT are two laptop+hypervisor oriented distros where
> suspend/resume is a big deal, and these will have to follow AMD's
> recommendation here.
>
> However, for servers which don't do S3/S4, we can reason about safely
> leaving RDRAND enabled, irrespective of guest configuration.
Let the administrator reason about it. Default is off for sanity sake.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH v5 2/2] hwmon: pmbus: Add Inspur Power System power supply driver
From: Joel Stanley @ 2019-08-19 2:20 UTC (permalink / raw)
To: John Wang
Cc: Jean Delvare, Guenter Roeck, Jonathan Corbet, linux-hwmon,
linux-doc, Linux Kernel Mailing List, OpenBMC Maillist,
duanzhijia01, Lei YU
In-Reply-To: <20190816101944.3586-1-wangzqbj@inspur.com>
On Fri, 16 Aug 2019 at 10:19, John Wang <wangzqbj@inspur.com> wrote:
>
> Add the driver to monitor Inspur Power System power supplies
> with hwmon over pmbus.
>
> This driver adds sysfs attributes for additional power supply data,
> including vendor, model, part_number, serial number,
> firmware revision, hardware revision, and psu mode(active/standby).
>
> Signed-off-by: John Wang <wangzqbj@inspur.com>
> +static const struct i2c_device_id ipsps_id[] = {
> + { "inspur_ipsps1", 0 },
Convention would be to use "ipsps" here, instead of "vendor_device"?
> + {}
> +};
> +MODULE_DEVICE_TABLE(i2c, ipsps_id);
> +
> +static const struct of_device_id ipsps_of_match[] = {
> + { .compatible = "inspur,ipsps1" },
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, ipsps_of_match);
Do we need the of match table? I thought the match on the device name
from the i2c table would be enough. I will defer to Guenter here
though.
Assuming the device tables are okay:
Reviewed-by: Joel Stanley <joel@jms.id.au>
Cheers,
Joel
> +
> +static struct i2c_driver ipsps_driver = {
> + .driver = {
> + .name = "inspur-ipsps",
> + .of_match_table = ipsps_of_match,
> + },
> + .probe = ipsps_probe,
> + .remove = pmbus_do_remove,
> + .id_table = ipsps_id,
> +};
^ permalink raw reply
* Re: [PATCH v5 2/2] hwmon: pmbus: Add Inspur Power System power supply driver
From: Guenter Roeck @ 2019-08-19 2:56 UTC (permalink / raw)
To: Joel Stanley, John Wang
Cc: Jean Delvare, Jonathan Corbet, linux-hwmon, linux-doc,
Linux Kernel Mailing List, OpenBMC Maillist, duanzhijia01, Lei YU
In-Reply-To: <CACPK8XegTePdmykMzZHnW=g6hyEGr7jiW3TP8AvdzSwZGr=2gA@mail.gmail.com>
On 8/18/19 7:20 PM, Joel Stanley wrote:
> On Fri, 16 Aug 2019 at 10:19, John Wang <wangzqbj@inspur.com> wrote:
>>
>> Add the driver to monitor Inspur Power System power supplies
>> with hwmon over pmbus.
>>
>> This driver adds sysfs attributes for additional power supply data,
>> including vendor, model, part_number, serial number,
>> firmware revision, hardware revision, and psu mode(active/standby).
>>
>> Signed-off-by: John Wang <wangzqbj@inspur.com>
>
>> +static const struct i2c_device_id ipsps_id[] = {
>> + { "inspur_ipsps1", 0 },
>
> Convention would be to use "ipsps" here, instead of "vendor_device"?
ipsps1, but good catch.
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(i2c, ipsps_id);
>> +
>> +static const struct of_device_id ipsps_of_match[] = {
>> + { .compatible = "inspur,ipsps1" },
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, ipsps_of_match);
>
> Do we need the of match table? I thought the match on the device name
> from the i2c table would be enough. I will defer to Guenter here
> though.
>
Strictly speaking it is unnecessary, but it is kind of customary to have it.
The automatic detection also only works if the i2c device ID would be "ipsps1",
without vendor ID embedded.
I would recomment to have both, but name the i2c device "ipsps1" as you suggested,
for consistency.
I'll also have to check if we need of_match_ptr below in the assignment of
of_match_table. Probably not, but it would save a few bytes if CONFIG_OF
is not enabled.
Thanks,
Guenter
> Assuming the device tables are okay:
>
> Reviewed-by: Joel Stanley <joel@jms.id.au>
>
> Cheers,
>
> Joel
>
>> +
>> +static struct i2c_driver ipsps_driver = {
>> + .driver = {
>> + .name = "inspur-ipsps",
>> + .of_match_table = ipsps_of_match,
>> + },
>> + .probe = ipsps_probe,
>> + .remove = pmbus_do_remove,
>> + .id_table = ipsps_id,
>> +};
>
^ permalink raw reply
* Re: [PATCH] docs: mtd: Update spi nor reference driver
From: Vignesh Raghavendra @ 2019-08-19 4:39 UTC (permalink / raw)
To: John Garry, Schrempf Frieder, corbet@lwn.net,
mchehab+samsung@kernel.org, linux-mtd@lists.infradead.org
Cc: linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org,
marek.vasut@gmail.com, tudor.ambarus@microchip.com,
broonie@kernel.org, miquel.raynal@bootlin.com, richard@nod.at,
wanghuiqiang
In-Reply-To: <ab2d3c29-982f-cb13-e2a2-e6d8da8f1438@huawei.com>
Hi,
On 16/08/19 3:50 PM, John Garry wrote:
> On 06/08/2019 17:40, Schrempf Frieder wrote:
[...]
>
> Hi,
>
> Could someone kindly advise on the following:
>
> I am looking at ACPI support only for an mtd spi nor driver we're
> targeting for mainline support.
>
If its a new driver, please add it under drivers/spi implementing SPI
MEM framework.
There are few drivers under drivers/spi that can be used as example.
(Search for "spi_mem_ops")
> So for the host, I could use a proprietary HID in the DSDT for matching
> in the kernel driver.
>
> About the child spi flash devices, is the recommendation to just use
> PRP0001 HID and "jedec,spi-nor" compatible?
>
I am not quite familiar with ACPI systems, but child flash device should
use "jedec,spi-nor" as compatible.
Regards
Vignesh
> thanks,
> John
>
>
>>
>> @Maintainers:
>> Maybe the docs under Documentation/driver-api/mtd should be officially
>> maintained by the MTD subsystem (and added to MAINTAINERS). And if there
>> will be some driver API docs for SPI MEM it should probably live in
>> Documentation/driver-api/spi instead of Documentation/driver-api/mtd, as
>> spi-mem.c itself is in drivers/spi.
>>
>> Regards,
>> Frieder
>>
>>>
>>> Thanks,
>>> John
>>>
>>>> @@ -59,7 +59,7 @@ Part III - How can drivers use the framework?
>>>>
>>>> The main API is spi_nor_scan(). Before you call the hook, a driver
>>>> should
>>>> initialize the necessary fields for spi_nor{}. Please see
>>>> -drivers/mtd/spi-nor/spi-nor.c for detail. Please also refer to
>>>> fsl-quadspi.c
>>>> +drivers/mtd/spi-nor/spi-nor.c for detail. Please also refer to
>>>> spi-fsl-qspi.c
>>>> when you want to write a new driver for a SPI NOR controller.
>>>> Another API is spi_nor_restore(), this is used to restore the status
>>>> of SPI
>>>> flash chip such as addressing mode. Call it whenever detach the
>>>> driver from
>>>>
>>>
>>>
>>>
>>> ______________________________________________________
>>> Linux MTD discussion mailing list
>>> http://lists.infradead.org/mailman/listinfo/linux-mtd/
>
>
--
Regards
Vignesh
^ permalink raw reply
* Re: [PATCH v5 0/9] FPGA DFL updates
From: Wu Hao @ 2019-08-19 5:31 UTC (permalink / raw)
To: gregkh, mdf, linux-fpga; +Cc: linux-kernel, linux-api, linux-doc, atull
In-Reply-To: <1565578204-13969-1-git-send-email-hao.wu@intel.com>
On Mon, Aug 12, 2019 at 10:49:55AM +0800, Wu Hao wrote:
> Hi Greg,
>
> This is v5 patchset which adds more features to FPGA DFL. Marjor changes
> against v4 are sysfs related code rework to address comments on v4.
>
> Please help to take a look. Thanks!
Hi Greg,
Did you get a chance to take a look at this new version patchset? :)
Thanks
Hao
>
> Main changes from v4:
> - convert code to use dev_groups for sysfs entries (#2, #3, #4, #6, #8).
> - clean up for empty init function after remove sysfs add/remove (#1).
> - introduce is_visible for sysfs groups (#3, #4, #6, #8).
> - remove revision sysfs entries (#4, #6, #8).
> - improve naming on shared functions (#5).
> - reorganize sysfs entries for port and fme error reporting (#6, #8).
>
> Main changes from v3:
> - drop avx512 partail reconfiguration patch for now.
> - split dfl_fpga_cdev_config_port to 2 functions *_release/assign_port
> (#1).
> - split __dfl_fpga_cdev_config_port_vf into 2 functions with locking
> added (#2).
> - improve description in sysfs doc to avoid misunderstanding (#3).
> - switch to boolean in sysfs entry store function (#3).
> - remove dev_dbg in init/uinit callback function (#7, #9, #11).
> - remove uinit callback which does does nothing (#8, #9)
>
> Main changes from v2:
> - update kernel version/date in sysfs doc (patch #4, #5, #8, #10, #11).
> - add back Documentation patch (patch #12).
>
> Main changes from v1:
> - remove DRV/MODULE_VERSION modifications. (patch #1, #3, #4, #6)
> - remove argsz from new ioctls. (patch #2)
> - replace sysfs_create/remove_* with device_add/remove_* for sysfs entries.
> (patch #5, #8, #11)
>
> Wu Hao (9):
> fpga: dfl: make init callback optional
> fpga: dfl: fme: convert platform_driver to use dev_groups
> fpga: dfl: afu: convert platform_driver to use dev_groups
> fpga: dfl: afu: add userclock sysfs interfaces.
> fpga: dfl: afu: expose __afu_port_enable/disable function.
> fpga: dfl: afu: add error reporting support.
> fpga: dfl: afu: add STP (SignalTap) support
> fpga: dfl: fme: add global error reporting support
> Documentation: fpga: dfl: add descriptions for virtualization and new
> interfaces.
>
> Documentation/ABI/testing/sysfs-platform-dfl-fme | 62 ++++
> Documentation/ABI/testing/sysfs-platform-dfl-port | 53 ++++
> Documentation/fpga/dfl.rst | 105 +++++++
> drivers/fpga/Makefile | 3 +-
> drivers/fpga/dfl-afu-error.c | 230 ++++++++++++++
> drivers/fpga/dfl-afu-main.c | 230 +++++++++++---
> drivers/fpga/dfl-afu.h | 9 +
> drivers/fpga/dfl-fme-error.c | 359 ++++++++++++++++++++++
> drivers/fpga/dfl-fme-main.c | 42 +--
> drivers/fpga/dfl-fme.h | 3 +
> drivers/fpga/dfl.c | 10 +-
> drivers/fpga/dfl.h | 9 +
> 12 files changed, 1041 insertions(+), 74 deletions(-)
> create mode 100644 drivers/fpga/dfl-afu-error.c
> create mode 100644 drivers/fpga/dfl-fme-error.c
>
> --
> 1.8.3.1
^ permalink raw reply
* Re: [PATCH] docs: process: fix broken link
From: Federico Vaga @ 2019-08-19 5:50 UTC (permalink / raw)
To: Jacob Huisman
Cc: Jonathan Corbet, Harry Wei, Alex Shi, linux-doc, linux-kernel
In-Reply-To: <20190816122209.5bz4rlln5cahn7ki@jacob-MS-7A62>
On Friday, August 16, 2019 2:22:09 PM CEST Jacob Huisman wrote:
> http://linux.yyz.us/patch-format.html seems to be down since
> approximately September 2018. There is a working archive copy on
> arhive.org. Replaced the links in documenation + translations.
>
> Signed-off-by: Jacob Huisman <jacobhuisman@kernelthusiast.com>
Reviewed-by: Federico Vaga <federico.vaga@vaga.pv.it>
^ permalink raw reply
* Re: [PATCH v2 2/6] arm64: Introduce config for S32
From: Shawn Guo @ 2019-08-19 8:14 UTC (permalink / raw)
To: Stefan-gabriel Mirea
Cc: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, Leo Li, jslaby@suse.com,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Cosmin Stefan Stoica
In-Reply-To: <20190809112853.15846-3-stefan-gabriel.mirea@nxp.com>
On Fri, Aug 09, 2019 at 11:29:10AM +0000, Stefan-gabriel Mirea wrote:
> From: Mihaela Martinas <Mihaela.Martinas@freescale.com>
>
> Add configuration option for the Freescale S32 platform family in
> Kconfig.platforms. For starters, the only SoC supported will be Treerunner
> (S32V234), with a single execution target: the S32V234-EVB (rev 29288)
> board.
>
> Signed-off-by: Mihaela Martinas <Mihaela.Martinas@freescale.com>
> Signed-off-by: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
> Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
> ---
> arch/arm64/Kconfig.platforms | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/arch/arm64/Kconfig.platforms b/arch/arm64/Kconfig.platforms
> index 4778c775de1b..a9a6152d37eb 100644
> --- a/arch/arm64/Kconfig.platforms
> +++ b/arch/arm64/Kconfig.platforms
> @@ -210,6 +210,11 @@ config ARCH_ROCKCHIP
> This enables support for the ARMv8 based Rockchip chipsets,
> like the RK3368.
>
> +config ARCH_S32
> + bool "Freescale S32 SoC Family"
So you still want to use 'Freescale' than 'NXP' here?
> + help
> + This enables support for the Freescale S32 family of processors.
Shawn
> +
> config ARCH_SEATTLE
> bool "AMD Seattle SoC Family"
> help
> --
> 2.22.0
>
^ permalink raw reply
* Re: [PATCH v2 3/6] arm64: dts: fsl: Add device tree for S32V234-EVB
From: Shawn Guo @ 2019-08-19 8:58 UTC (permalink / raw)
To: Stefan-gabriel Mirea
Cc: corbet@lwn.net, robh+dt@kernel.org, mark.rutland@arm.com,
gregkh@linuxfoundation.org, catalin.marinas@arm.com,
will@kernel.org, Leo Li, jslaby@suse.com,
linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
devicetree@vger.kernel.org, linux-serial@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Cosmin Stefan Stoica,
Dan Nica, Larisa Ileana Grigore
In-Reply-To: <20190809112853.15846-4-stefan-gabriel.mirea@nxp.com>
On Fri, Aug 09, 2019 at 11:29:12AM +0000, Stefan-gabriel Mirea wrote:
> From: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
>
> Add initial version of device tree for S32V234-EVB, including nodes for the
> 4 Cortex-A53 cores, AIPS bus with UART modules, ARM architected timer and
> Generic Interrupt Controller (GIC).
>
> Keep SoC level separate from board level to let future boards with this SoC
> share common properties, while the dts files will keep board-dependent
> properties.
>
> Signed-off-by: Stoica Cosmin-Stefan <cosmin.stoica@nxp.com>
> Signed-off-by: Mihaela Martinas <Mihaela.Martinas@freescale.com>
> Signed-off-by: Dan Nica <dan.nica@nxp.com>
> Signed-off-by: Larisa Grigore <Larisa.Grigore@nxp.com>
> Signed-off-by: Phu Luu An <phu.luuan@nxp.com>
> Signed-off-by: Stefan-Gabriel Mirea <stefan-gabriel.mirea@nxp.com>
> ---
> arch/arm64/boot/dts/freescale/Makefile | 2 +
> .../boot/dts/freescale/fsl-s32v234-evb.dts | 24 ++++
The 'fsl-' prefix can be saved here, so that we can distinguish three
families by starting string: imx??? for i.MX, fsl-??? for LayerScape,
and s32??? for S32.
> .../arm64/boot/dts/freescale/fsl-s32v234.dtsi | 130 ++++++++++++++++++
> 3 files changed, 156 insertions(+)
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-s32v234-evb.dts
> create mode 100644 arch/arm64/boot/dts/freescale/fsl-s32v234.dtsi
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index c043aca66572..3af29b58a833 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -26,3 +26,5 @@ dtb-$(CONFIG_ARCH_MXC) += imx8mq-librem5-devkit.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-rmb3.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8mq-zii-ultra-zest.dtb
> dtb-$(CONFIG_ARCH_MXC) += imx8qxp-mek.dtb
> +
> +dtb-$(CONFIG_ARCH_S32) += fsl-s32v234-evb.dtb
> diff --git a/arch/arm64/boot/dts/freescale/fsl-s32v234-evb.dts b/arch/arm64/boot/dts/freescale/fsl-s32v234-evb.dts
> new file mode 100644
> index 000000000000..92bf6c5563a3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/fsl-s32v234-evb.dts
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2015-2016 Freescale Semiconductor, Inc.
> + * Copyright 2016-2017 NXP
> + */
> +
> +/dts-v1/;
> +#include "fsl-s32v234.dtsi"
> +
> +/ {
> + compatible = "fsl,s32v234-evb", "fsl,s32v234";
> +
> + chosen {
> + stdout-path = "serial0:115200n8";
> + };
> +};
> +
> +&uart0 {
> + status = "okay";
> +};
> +
> +&uart1 {
> + status = "okay";
> +};
> diff --git a/arch/arm64/boot/dts/freescale/fsl-s32v234.dtsi b/arch/arm64/boot/dts/freescale/fsl-s32v234.dtsi
> new file mode 100644
> index 000000000000..6d686d3ba997
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/fsl-s32v234.dtsi
> @@ -0,0 +1,130 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright 2015-2016 Freescale Semiconductor, Inc.
> + * Copyright 2016-2018 NXP
> + */
> +
> +/memreserve/ 0x80000000 0x00010000;
> +
> +/ {
> + model = "Freescale S32V234";
The 'model' is usually used in board level DTS to describe the board.
> + compatible = "fsl,s32v234";
> + interrupt-parent = <&gic>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + aliases {
> + serial0 = &uart0;
> + serial1 = &uart1;
> + };
> +
> + cpus {
> + #address-cells = <2>;
> + #size-cells = <0>;
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x0 0x0>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0x0 0x80000000>;
> + next-level-cache = <&cluster0_l2_cache>;
> + };
Please have a newline between nodes.
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x0 0x1>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0x0 0x80000000>;
> + next-level-cache = <&cluster0_l2_cache>;
> + };
> + cpu2: cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x0 0x100>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0x0 0x80000000>;
> + next-level-cache = <&cluster1_l2_cache>;
> + };
> + cpu3: cpu@101 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x0 0x101>;
> + enable-method = "spin-table";
> + cpu-release-addr = <0x0 0x80000000>;
> + next-level-cache = <&cluster1_l2_cache>;
> + };
> +
> + cluster0_l2_cache: l2-cache0 {
> + compatible = "cache";
> + };
> +
> + cluster1_l2_cache: l2-cache1 {
> + compatible = "cache";
> + };
> + };
> +
> + soc {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + compatible = "simple-bus";
> + interrupt-parent = <&gic>;
> + ranges;
> +
> + aips0: aips-bus@40000000 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-parent = <&gic>;
> + reg = <0x0 0x40000000 0x0 0x7D000>;
> + ranges;
> +
> + uart0: serial@40053000 {
> + compatible = "fsl,s32-linflexuart";
> + reg = <0x0 0x40053000 0x0 0x1000>;
> + interrupts = <0 59 1>;
Please use GIC_SPI and IRQ_TYPE_xxx defines to make it more readable.
> + status = "disabled";
> + };
> + };
> +
> + aips1: aips-bus@40080000 {
> + compatible = "simple-bus";
> + #address-cells = <2>;
> + #size-cells = <2>;
> + interrupt-parent = <&gic>;
> + reg = <0x0 0x40080000 0x0 0x70000>;
> + ranges;
> +
> + uart1: serial@400bc000 {
> + compatible = "fsl,s32-linflexuart";
> + reg = <0x0 0x400bc000 0x0 0x1000>;
> + interrupts = <0 60 1>;
> + status = "disabled";
> + };
> + };
> + };
> +
> + timer {
> + compatible = "arm,armv8-timer";
> + interrupts = <1 13 0xf08>,
> + <1 14 0xf08>,
> + <1 11 0xf08>,
> + <1 10 0xf08>;
> + /* clock-frequency might be modified by u-boot, depending on the
> + * chip version.
> + */
> + clock-frequency = <10000000>;
> + };
> +
> + gic: interrupt-controller@7d001000 {
> + compatible = "arm,cortex-a15-gic", "arm,cortex-a9-gic";
> + #interrupt-cells = <3>;
> + #address-cells = <0>;
> + interrupt-controller;
> + reg = <0 0x7d001000 0 0x1000>,
> + <0 0x7d002000 0 0x2000>,
> + <0 0x7d004000 0 0x2000>,
> + <0 0x7d006000 0 0x2000>;
> + interrupts = <1 9 0xf04>;
> + };
We usually put these core platform devices prior to 'soc' node.
Shawn
> +};
> --
> 2.22.0
>
^ permalink raw reply
* [PATCH v6 2/2] hwmon: pmbus: Add Inspur Power System power supply driver
From: John Wang @ 2019-08-19 9:15 UTC (permalink / raw)
To: jdelvare, linux, corbet, linux-hwmon, linux-doc, linux-kernel,
openbmc, duanzhijia01, mine260309, joel
Add the driver to monitor Inspur Power System power supplies
with hwmon over pmbus.
This driver adds sysfs attributes for additional power supply data,
including vendor, model, part_number, serial number,
firmware revision, hardware revision, and psu mode(active/standby).
Signed-off-by: John Wang <wangzqbj@inspur.com>
---
v6:
- Name i2c device as ipsps1
- Use of_match_ptr to save a few bytes if CONFIG_OF
is not enabled :)
v5:
- Align sysfs attrs description in inspur-ipsps1.rst
(Use tab instead of space to sperate names and values)
v4:
- Remove the additional tabs in the Makefile
- Rebased on 5.3-rc4, not 5.2
v3:
- Sort kconfig/makefile entries alphabetically
- Remove unnecessary initialization
- Use ATTRIBUTE_GROUPS instead of expanding directly
- Use memscan to avoid reimplementation
v2:
- Fix typos in commit message
- Invert Christmas tree
- Configure device with sysfs attrs, not debugfs entries
- Fix errno in fw_version_read, ENODATA to EPROTO
- Change the print format of fw-version
- Use sysfs_streq instead of strcmp("xxx" "\n", "xxx")
- Document sysfs attributes
---
Documentation/hwmon/inspur-ipsps1.rst | 79 +++++++++
drivers/hwmon/pmbus/Kconfig | 9 +
drivers/hwmon/pmbus/Makefile | 1 +
drivers/hwmon/pmbus/inspur-ipsps.c | 228 ++++++++++++++++++++++++++
4 files changed, 317 insertions(+)
create mode 100644 Documentation/hwmon/inspur-ipsps1.rst
create mode 100644 drivers/hwmon/pmbus/inspur-ipsps.c
diff --git a/Documentation/hwmon/inspur-ipsps1.rst b/Documentation/hwmon/inspur-ipsps1.rst
new file mode 100644
index 000000000000..2b871ae3448f
--- /dev/null
+++ b/Documentation/hwmon/inspur-ipsps1.rst
@@ -0,0 +1,79 @@
+Kernel driver inspur-ipsps1
+=======================
+
+Supported chips:
+
+ * Inspur Power System power supply unit
+
+Author: John Wang <wangzqbj@inspur.com>
+
+Description
+-----------
+
+This driver supports Inspur Power System power supplies. This driver
+is a client to the core PMBus driver.
+
+Usage Notes
+-----------
+
+This driver does not auto-detect devices. You will have to instantiate the
+devices explicitly. Please see Documentation/i2c/instantiating-devices for
+details.
+
+Sysfs entries
+-------------
+
+The following attributes are supported:
+
+======================= ======================================================
+curr1_input Measured input current
+curr1_label "iin"
+curr1_max Maximum current
+curr1_max_alarm Current high alarm
+curr2_input Measured output current in mA.
+curr2_label "iout1"
+curr2_crit Critical maximum current
+curr2_crit_alarm Current critical high alarm
+curr2_max Maximum current
+curr2_max_alarm Current high alarm
+
+fan1_alarm Fan 1 warning.
+fan1_fault Fan 1 fault.
+fan1_input Fan 1 speed in RPM.
+
+in1_alarm Input voltage under-voltage alarm.
+in1_input Measured input voltage in mV.
+in1_label "vin"
+in2_input Measured output voltage in mV.
+in2_label "vout1"
+in2_lcrit Critical minimum output voltage
+in2_lcrit_alarm Output voltage critical low alarm
+in2_max Maximum output voltage
+in2_max_alarm Output voltage high alarm
+in2_min Minimum output voltage
+in2_min_alarm Output voltage low alarm
+
+power1_alarm Input fault or alarm.
+power1_input Measured input power in uW.
+power1_label "pin"
+power1_max Input power limit
+power2_max_alarm Output power high alarm
+power2_max Output power limit
+power2_input Measured output power in uW.
+power2_label "pout"
+
+temp[1-3]_input Measured temperature
+temp[1-2]_max Maximum temperature
+temp[1-3]_max_alarm Temperature high alarm
+
+vendor Manufacturer name
+model Product model
+part_number Product part number
+serial_number Product serial number
+fw_version Firmware version
+hw_version Hardware version
+mode Work mode. Can be set to active or
+ standby, when set to standby, PSU will
+ automatically switch between standby
+ and redundancy mode.
+======================= ======================================================
diff --git a/drivers/hwmon/pmbus/Kconfig b/drivers/hwmon/pmbus/Kconfig
index b6588483fae1..d62d69bb7e49 100644
--- a/drivers/hwmon/pmbus/Kconfig
+++ b/drivers/hwmon/pmbus/Kconfig
@@ -46,6 +46,15 @@ config SENSORS_IBM_CFFPS
This driver can also be built as a module. If so, the module will
be called ibm-cffps.
+config SENSORS_INSPUR_IPSPS
+ tristate "INSPUR Power System Power Supply"
+ help
+ If you say yes here you get hardware monitoring support for the INSPUR
+ Power System power supply.
+
+ This driver can also be built as a module. If so, the module will
+ be called inspur-ipsps.
+
config SENSORS_IR35221
tristate "Infineon IR35221"
help
diff --git a/drivers/hwmon/pmbus/Makefile b/drivers/hwmon/pmbus/Makefile
index c950ea9a5d00..03bacfcfd660 100644
--- a/drivers/hwmon/pmbus/Makefile
+++ b/drivers/hwmon/pmbus/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_PMBUS) += pmbus_core.o
obj-$(CONFIG_SENSORS_PMBUS) += pmbus.o
obj-$(CONFIG_SENSORS_ADM1275) += adm1275.o
obj-$(CONFIG_SENSORS_IBM_CFFPS) += ibm-cffps.o
+obj-$(CONFIG_SENSORS_INSPUR_IPSPS) += inspur-ipsps.o
obj-$(CONFIG_SENSORS_IR35221) += ir35221.o
obj-$(CONFIG_SENSORS_IR38064) += ir38064.o
obj-$(CONFIG_SENSORS_IRPS5401) += irps5401.o
diff --git a/drivers/hwmon/pmbus/inspur-ipsps.c b/drivers/hwmon/pmbus/inspur-ipsps.c
new file mode 100644
index 000000000000..42e01549184a
--- /dev/null
+++ b/drivers/hwmon/pmbus/inspur-ipsps.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright 2019 Inspur Corp.
+ */
+
+#include <linux/debugfs.h>
+#include <linux/device.h>
+#include <linux/fs.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/pmbus.h>
+#include <linux/hwmon-sysfs.h>
+
+#include "pmbus.h"
+
+#define IPSPS_REG_VENDOR_ID 0x99
+#define IPSPS_REG_MODEL 0x9A
+#define IPSPS_REG_FW_VERSION 0x9B
+#define IPSPS_REG_PN 0x9C
+#define IPSPS_REG_SN 0x9E
+#define IPSPS_REG_HW_VERSION 0xB0
+#define IPSPS_REG_MODE 0xFC
+
+#define MODE_ACTIVE 0x55
+#define MODE_STANDBY 0x0E
+#define MODE_REDUNDANCY 0x00
+
+#define MODE_ACTIVE_STRING "active"
+#define MODE_STANDBY_STRING "standby"
+#define MODE_REDUNDANCY_STRING "redundancy"
+
+enum ipsps_index {
+ vendor,
+ model,
+ fw_version,
+ part_number,
+ serial_number,
+ hw_version,
+ mode,
+ num_regs,
+};
+
+static const u8 ipsps_regs[num_regs] = {
+ [vendor] = IPSPS_REG_VENDOR_ID,
+ [model] = IPSPS_REG_MODEL,
+ [fw_version] = IPSPS_REG_FW_VERSION,
+ [part_number] = IPSPS_REG_PN,
+ [serial_number] = IPSPS_REG_SN,
+ [hw_version] = IPSPS_REG_HW_VERSION,
+ [mode] = IPSPS_REG_MODE,
+};
+
+static ssize_t ipsps_string_show(struct device *dev,
+ struct device_attribute *devattr,
+ char *buf)
+{
+ u8 reg;
+ int rc;
+ char *p;
+ char data[I2C_SMBUS_BLOCK_MAX + 1];
+ struct i2c_client *client = to_i2c_client(dev->parent);
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+
+ reg = ipsps_regs[attr->index];
+ rc = i2c_smbus_read_block_data(client, reg, data);
+ if (rc < 0)
+ return rc;
+
+ /* filled with printable characters, ending with # */
+ p = memscan(data, '#', rc);
+ *p = '\0';
+
+ return snprintf(buf, PAGE_SIZE, "%s\n", data);
+}
+
+static ssize_t ipsps_fw_version_show(struct device *dev,
+ struct device_attribute *devattr,
+ char *buf)
+{
+ u8 reg;
+ int rc;
+ u8 data[I2C_SMBUS_BLOCK_MAX] = { 0 };
+ struct i2c_client *client = to_i2c_client(dev->parent);
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+
+ reg = ipsps_regs[attr->index];
+ rc = i2c_smbus_read_block_data(client, reg, data);
+ if (rc < 0)
+ return rc;
+
+ if (rc != 6)
+ return -EPROTO;
+
+ return snprintf(buf, PAGE_SIZE, "%u.%02u%u-%u.%02u\n",
+ data[1], data[2]/* < 100 */, data[3]/*< 10*/,
+ data[4], data[5]/* < 100 */);
+}
+
+static ssize_t ipsps_mode_show(struct device *dev,
+ struct device_attribute *devattr, char *buf)
+{
+ u8 reg;
+ int rc;
+ struct i2c_client *client = to_i2c_client(dev->parent);
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+
+ reg = ipsps_regs[attr->index];
+ rc = i2c_smbus_read_byte_data(client, reg);
+ if (rc < 0)
+ return rc;
+
+ switch (rc) {
+ case MODE_ACTIVE:
+ return snprintf(buf, PAGE_SIZE, "[%s] %s %s\n",
+ MODE_ACTIVE_STRING,
+ MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
+ case MODE_STANDBY:
+ return snprintf(buf, PAGE_SIZE, "%s [%s] %s\n",
+ MODE_ACTIVE_STRING,
+ MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
+ case MODE_REDUNDANCY:
+ return snprintf(buf, PAGE_SIZE, "%s %s [%s]\n",
+ MODE_ACTIVE_STRING,
+ MODE_STANDBY_STRING, MODE_REDUNDANCY_STRING);
+ default:
+ return snprintf(buf, PAGE_SIZE, "unspecified\n");
+ }
+}
+
+static ssize_t ipsps_mode_store(struct device *dev,
+ struct device_attribute *devattr,
+ const char *buf, size_t count)
+{
+ u8 reg;
+ int rc;
+ struct i2c_client *client = to_i2c_client(dev->parent);
+ struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr);
+
+ reg = ipsps_regs[attr->index];
+ if (sysfs_streq(MODE_STANDBY_STRING, buf)) {
+ rc = i2c_smbus_write_byte_data(client, reg,
+ MODE_STANDBY);
+ if (rc < 0)
+ return rc;
+ return count;
+ } else if (sysfs_streq(MODE_ACTIVE_STRING, buf)) {
+ rc = i2c_smbus_write_byte_data(client, reg,
+ MODE_ACTIVE);
+ if (rc < 0)
+ return rc;
+ return count;
+ }
+
+ return -EINVAL;
+}
+
+static SENSOR_DEVICE_ATTR_RO(vendor, ipsps_string, vendor);
+static SENSOR_DEVICE_ATTR_RO(model, ipsps_string, model);
+static SENSOR_DEVICE_ATTR_RO(part_number, ipsps_string, part_number);
+static SENSOR_DEVICE_ATTR_RO(serial_number, ipsps_string, serial_number);
+static SENSOR_DEVICE_ATTR_RO(hw_version, ipsps_string, hw_version);
+static SENSOR_DEVICE_ATTR_RO(fw_version, ipsps_fw_version, fw_version);
+static SENSOR_DEVICE_ATTR_RW(mode, ipsps_mode, mode);
+
+static struct attribute *ipsps_attrs[] = {
+ &sensor_dev_attr_vendor.dev_attr.attr,
+ &sensor_dev_attr_model.dev_attr.attr,
+ &sensor_dev_attr_part_number.dev_attr.attr,
+ &sensor_dev_attr_serial_number.dev_attr.attr,
+ &sensor_dev_attr_hw_version.dev_attr.attr,
+ &sensor_dev_attr_fw_version.dev_attr.attr,
+ &sensor_dev_attr_mode.dev_attr.attr,
+ NULL,
+};
+
+ATTRIBUTE_GROUPS(ipsps);
+
+static struct pmbus_driver_info ipsps_info = {
+ .pages = 1,
+ .func[0] = PMBUS_HAVE_VIN | PMBUS_HAVE_VOUT | PMBUS_HAVE_IOUT |
+ PMBUS_HAVE_IIN | PMBUS_HAVE_POUT | PMBUS_HAVE_PIN |
+ PMBUS_HAVE_FAN12 | PMBUS_HAVE_TEMP | PMBUS_HAVE_TEMP2 |
+ PMBUS_HAVE_TEMP3 | PMBUS_HAVE_STATUS_VOUT |
+ PMBUS_HAVE_STATUS_IOUT | PMBUS_HAVE_STATUS_INPUT |
+ PMBUS_HAVE_STATUS_TEMP | PMBUS_HAVE_STATUS_FAN12,
+ .groups = ipsps_groups,
+};
+
+static struct pmbus_platform_data ipsps_pdata = {
+ .flags = PMBUS_SKIP_STATUS_CHECK,
+};
+
+static int ipsps_probe(struct i2c_client *client,
+ const struct i2c_device_id *id)
+{
+ client->dev.platform_data = &ipsps_pdata;
+ return pmbus_do_probe(client, id, &ipsps_info);
+}
+
+static const struct i2c_device_id ipsps_id[] = {
+ { "ipsps1", 0 },
+ {}
+};
+MODULE_DEVICE_TABLE(i2c, ipsps_id);
+
+#ifdef CONFIG_OF
+static const struct of_device_id ipsps_of_match[] = {
+ { .compatible = "inspur,ipsps1" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, ipsps_of_match);
+#endif
+
+static struct i2c_driver ipsps_driver = {
+ .driver = {
+ .name = "inspur-ipsps",
+ .of_match_table = of_match_ptr(ipsps_of_match),
+ },
+ .probe = ipsps_probe,
+ .remove = pmbus_do_remove,
+ .id_table = ipsps_id,
+};
+
+module_i2c_driver(ipsps_driver);
+
+MODULE_AUTHOR("John Wang");
+MODULE_DESCRIPTION("PMBus driver for Inspur Power System power supplies");
+MODULE_LICENSE("GPL");
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v6 2/2] hwmon: pmbus: Add Inspur Power System power supply driver
From: Joel Stanley @ 2019-08-19 9:30 UTC (permalink / raw)
To: John Wang
Cc: Jean Delvare, Guenter Roeck, Jonathan Corbet, linux-hwmon,
linux-doc, Linux Kernel Mailing List, OpenBMC Maillist,
duanzhijia01, Lei YU
In-Reply-To: <20190819091509.29276-1-wangzqbj@inspur.com>
On Mon, 19 Aug 2019 at 09:15, John Wang <wangzqbj@inspur.com> wrote:
>
> Add the driver to monitor Inspur Power System power supplies
> with hwmon over pmbus.
>
> This driver adds sysfs attributes for additional power supply data,
> including vendor, model, part_number, serial number,
> firmware revision, hardware revision, and psu mode(active/standby).
>
> Signed-off-by: John Wang <wangzqbj@inspur.com>
Reviewed-by: Joel Stanley <joel@jms.id.au>
^ permalink raw reply
* [PATCH V3 0/3] KVM/Hyper-V: Add Hyper-V direct tlb flush support
From: lantianyu1986 @ 2019-08-19 13:17 UTC (permalink / raw)
To: pbonzini, rkrcmar, corbet, kys, haiyangz, sthemmin, sashal, tglx,
mingo, bp, hpa, x86, michael.h.kelley
Cc: Tianyu Lan, kvm, linux-doc, linux-hyperv, linux-kernel, vkuznets
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
This patchset is to add Hyper-V direct tlb support in KVM. Hyper-V
in L0 can delegate L1 hypervisor to handle tlb flush request from
L2 guest when direct tlb flush is enabled in L1.
Patch 2 introduces new cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH to enable
feature from user space. User space should enable this feature only
when Hyper-V hypervisor capability is exposed to guest and KVM profile
is hided. There is a parameter conflict between KVM and Hyper-V hypercall.
We hope L2 guest doesn't use KVM hypercall when the feature is
enabled. Detail please see comment of new API "KVM_CAP_HYPERV_DIRECT_TLBFLUSH"
Change since v2:
- Move hv assist page(hv_pa_pg) from struct kvm to struct kvm_hv.
Change since v1:
- Fix offset issue in the patch 1.
- Update description of KVM KVM_CAP_HYPERV_DIRECT_TLBFLUSH.
Tianyu Lan (2):
x86/Hyper-V: Fix definition of struct hv_vp_assist_page
KVM/Hyper-V: Add new KVM cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH
Vitaly Kuznetsov (1):
KVM/Hyper-V/VMX: Add direct tlb flush support
Documentation/virtual/kvm/api.txt | 13 +++++++++++++
arch/x86/include/asm/hyperv-tlfs.h | 24 ++++++++++++++++++-----
arch/x86/include/asm/kvm_host.h | 4 ++++
arch/x86/kvm/vmx/evmcs.h | 2 ++
arch/x86/kvm/vmx/vmx.c | 39 ++++++++++++++++++++++++++++++++++++++
arch/x86/kvm/x86.c | 8 ++++++++
include/uapi/linux/kvm.h | 1 +
7 files changed, 86 insertions(+), 5 deletions(-)
--
2.14.5
^ permalink raw reply
* [PATCH V3 1/3] x86/Hyper-V: Fix definition of struct hv_vp_assist_page
From: lantianyu1986 @ 2019-08-19 13:17 UTC (permalink / raw)
To: pbonzini, rkrcmar, corbet, kys, haiyangz, sthemmin, sashal, tglx,
mingo, bp, hpa, x86, michael.h.kelley
Cc: Tianyu Lan, kvm, linux-doc, linux-kernel, linux-hyperv, vkuznets
In-Reply-To: <20190819131737.26942-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
The struct hv_vp_assist_page was defined incorrectly.
The "vtl_control" should be u64[3], "nested_enlightenments
_control" should be a u64 and there is 7 reserved bytes
following "enlighten_vmentry". This patch is to fix it.
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
--
Change since v1:
Move definition of struct hv_nested_enlightenments_control
into this patch to fix offset issue.
---
arch/x86/include/asm/hyperv-tlfs.h | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/x86/include/asm/hyperv-tlfs.h b/arch/x86/include/asm/hyperv-tlfs.h
index af78cd72b8f3..cf0b2a04271d 100644
--- a/arch/x86/include/asm/hyperv-tlfs.h
+++ b/arch/x86/include/asm/hyperv-tlfs.h
@@ -514,14 +514,24 @@ struct hv_timer_message_payload {
__u64 delivery_time; /* When the message was delivered */
} __packed;
+struct hv_nested_enlightenments_control {
+ struct {
+ __u32 directhypercall:1;
+ __u32 reserved:31;
+ } features;
+ struct {
+ __u32 reserved;
+ } hypercallControls;
+} __packed;
+
/* Define virtual processor assist page structure. */
struct hv_vp_assist_page {
__u32 apic_assist;
- __u32 reserved;
- __u64 vtl_control[2];
- __u64 nested_enlightenments_control[2];
- __u32 enlighten_vmentry;
- __u32 padding;
+ __u32 reserved1;
+ __u64 vtl_control[3];
+ struct hv_nested_enlightenments_control nested_control;
+ __u8 enlighten_vmentry;
+ __u8 reserved2[7];
__u64 current_nested_vmcs;
} __packed;
--
2.14.5
^ permalink raw reply related
* [PATCH V3 2/3] KVM/Hyper-V: Add new KVM cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH
From: lantianyu1986 @ 2019-08-19 13:17 UTC (permalink / raw)
To: pbonzini, rkrcmar, corbet, kys, haiyangz, sthemmin, sashal, tglx,
mingo, bp, hpa, x86, michael.h.kelley
Cc: Tianyu Lan, kvm, linux-doc, linux-kernel, linux-hyperv, vkuznets
In-Reply-To: <20190819131737.26942-1-Tianyu.Lan@microsoft.com>
From: Tianyu Lan <Tianyu.Lan@microsoft.com>
This patch adds new KVM cap KVM_CAP_HYPERV_DIRECT_TLBFLUSH and let
user space to enable direct tlb flush function when only Hyper-V
hypervsior capability is exposed to VM. This patch also adds
enable_direct_tlbflush callback in the struct kvm_x86_ops and
platforms may use it to implement direct tlb flush support.
Signed-off-by: Tianyu Lan <Tianyu.Lan@microsoft.com>
---
Change since v1:
Update description of KVM_CAP_HYPERV_DIRECT_TLBFLUSH
in the KVM API doc.
---
Documentation/virtual/kvm/api.txt | 13 +++++++++++++
arch/x86/include/asm/kvm_host.h | 2 ++
arch/x86/kvm/x86.c | 8 ++++++++
include/uapi/linux/kvm.h | 1 +
4 files changed, 24 insertions(+)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 2cd6250b2896..0c6e1b25d0c8 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -5289,3 +5289,16 @@ Architectures: x86
This capability indicates that KVM supports paravirtualized Hyper-V IPI send
hypercalls:
HvCallSendSyntheticClusterIpi, HvCallSendSyntheticClusterIpiEx.
+8.21 KVM_CAP_HYPERV_DIRECT_TLBFLUSH
+
+Architecture: x86
+
+This capability indicates that KVM running on top of Hyper-V hypervisor
+enables Direct TLB flush for its guests meaning that TLB flush
+hypercalls are handled by Level 0 hypervisor (Hyper-V) bypassing KVM.
+Due to the different ABI for hypercall parameters between Hyper-V and
+KVM, enabling this capability effectively disables all hypercall
+handling by KVM (as some KVM hypercall may be mistakenly treated as TLB
+flush hypercalls by Hyper-V) so userspace should disable KVM identification
+in CPUID and only exposes Hyper-V identification. In this case, guest
+thinks it's running on Hyper-V and only use Hyper-V hypercalls.
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 0cc5b611a113..667d154e89d4 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -1205,6 +1205,8 @@ struct kvm_x86_ops {
uint16_t (*nested_get_evmcs_version)(struct kvm_vcpu *vcpu);
bool (*need_emulation_on_page_fault)(struct kvm_vcpu *vcpu);
+
+ int (*enable_direct_tlbflush)(struct kvm_vcpu *vcpu);
};
struct kvm_arch_async_pf {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 9d7b9e6a0939..a9d8ee7f7bf0 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -3183,6 +3183,9 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
r = kvm_x86_ops->get_nested_state ?
kvm_x86_ops->get_nested_state(NULL, NULL, 0) : 0;
break;
+ case KVM_CAP_HYPERV_DIRECT_TLBFLUSH:
+ r = kvm_x86_ops->enable_direct_tlbflush ? 1 : 0;
+ break;
default:
break;
}
@@ -3953,6 +3956,11 @@ static int kvm_vcpu_ioctl_enable_cap(struct kvm_vcpu *vcpu,
r = -EFAULT;
}
return r;
+ case KVM_CAP_HYPERV_DIRECT_TLBFLUSH:
+ if (!kvm_x86_ops->enable_direct_tlbflush)
+ return -ENOTTY;
+
+ return kvm_x86_ops->enable_direct_tlbflush(vcpu);
default:
return -EINVAL;
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index a7c19540ce21..cb959bc925b1 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -996,6 +996,7 @@ struct kvm_ppc_resize_hpt {
#define KVM_CAP_ARM_PTRAUTH_ADDRESS 171
#define KVM_CAP_ARM_PTRAUTH_GENERIC 172
#define KVM_CAP_PMU_EVENT_FILTER 173
+#define KVM_CAP_HYPERV_DIRECT_TLBFLUSH 174
#ifdef KVM_CAP_IRQ_ROUTING
--
2.14.5
^ permalink raw reply related
* [PATCH 00/11 v3] Cleanup IOMMU passthrough setting (and disable IOMMU Passthrough when SME is active)
From: Joerg Roedel @ 2019-08-19 13:22 UTC (permalink / raw)
To: Joerg Roedel
Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
Suravee.Suthikulpanit, Joerg Roedel
From: Joerg Roedel <jroedel@suse.de>
Hi,
This patch-set started out small to overwrite the default passthrough
setting (through CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y) when SME is active.
But on the way to that Tom reminded me that the current ways to
configure passthrough/no-passthrough modes for IOMMU on x86 is a mess.
So I added a few more patches to clean that up a bit, getting rid of the
iommu_pass_through variable on the way.This information is now kept only
in iommu code, with helpers to change that setting from architecture
code.
And of course this patch-set still disables IOMMU Passthrough mode when
SME is active even when CONFIG_IOMMU_DEFAULT_PASSTHROUGH=y is set.
The reason for that change is that SME with passthrough mode turned out
to be fragile with devices requiring SWIOTLB, mainly because SWIOTLB has
a maximum allocation size of 256kb and a limit overall size of the
bounce buffer.
Therefore having IOMMU in translation mode by default is better when SME
is active on a system.
Please review.
Thanks,
Joerg
Changes since v2:
- Added 'bool cmd_line' parameter to iommu_set_default_*()
functions, so that we correctly recognize iommu=pt/nopt
command line parameters
Changes since v1:
- Cleaned up the kernel command line parameters to
configure passthrough/translated mode, getting rid
of the global iommu_pass_through variable
Joerg Roedel (11):
iommu: Remember when default domain type was set on kernel command line
iommu: Add helpers to set/get default domain type
iommu: Use Functions to set default domain type in iommu_set_def_domain_type()
iommu/amd: Request passthrough mode from IOMMU core
iommu/vt-d: Request passthrough mode from IOMMU core
x86/dma: Get rid of iommu_pass_through
ia64: Get rid of iommu_pass_through
iommu: Print default domain type on boot
iommu: Set default domain type at runtime
iommu: Disable passthrough mode when SME is active
Documentation: Update Documentation for iommu.passthrough
Documentation/admin-guide/kernel-parameters.txt | 2 +-
arch/ia64/include/asm/iommu.h | 2 -
arch/ia64/kernel/pci-dma.c | 2 -
arch/x86/include/asm/iommu.h | 1 -
arch/x86/kernel/pci-dma.c | 20 +-----
drivers/iommu/amd_iommu.c | 6 +-
drivers/iommu/intel-iommu.c | 2 +-
drivers/iommu/iommu.c | 93 +++++++++++++++++++++++--
include/linux/iommu.h | 16 +++++
9 files changed, 110 insertions(+), 34 deletions(-)
--
2.16.4
^ permalink raw reply
* [PATCH 04/11] iommu/amd: Request passthrough mode from IOMMU core
From: Joerg Roedel @ 2019-08-19 13:22 UTC (permalink / raw)
To: Joerg Roedel
Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190819132256.14436-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
Get rid of the iommu_pass_through variable and request
passthrough mode via the new iommu core function.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/iommu/amd_iommu.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index b607a92791d3..7434b34d7a94 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -436,7 +436,7 @@ static int iommu_init_device(struct device *dev)
* invalid address), we ignore the capability for the device so
* it'll be forced to go into translation mode.
*/
- if ((iommu_pass_through || !amd_iommu_force_isolation) &&
+ if ((iommu_default_passthrough() || !amd_iommu_force_isolation) &&
dev_is_pci(dev) && pci_iommuv2_capable(to_pci_dev(dev))) {
struct amd_iommu *iommu;
@@ -2226,7 +2226,7 @@ static int amd_iommu_add_device(struct device *dev)
BUG_ON(!dev_data);
- if (iommu_pass_through || dev_data->iommu_v2)
+ if (dev_data->iommu_v2)
iommu_request_dm_for_dev(dev);
/* Domains are initialized for this device - have a look what we ended up with */
@@ -2805,7 +2805,7 @@ int __init amd_iommu_init_api(void)
int __init amd_iommu_init_dma_ops(void)
{
- swiotlb = (iommu_pass_through || sme_me_mask) ? 1 : 0;
+ swiotlb = (iommu_default_passthrough() || sme_me_mask) ? 1 : 0;
iommu_detected = 1;
if (amd_iommu_unmap_flush)
--
2.16.4
^ permalink raw reply related
* [PATCH 07/11] ia64: Get rid of iommu_pass_through
From: Joerg Roedel @ 2019-08-19 13:22 UTC (permalink / raw)
To: Joerg Roedel
Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190819132256.14436-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
This variable has no users anymore so it can be removed.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
arch/ia64/include/asm/iommu.h | 2 --
arch/ia64/kernel/pci-dma.c | 2 --
2 files changed, 4 deletions(-)
diff --git a/arch/ia64/include/asm/iommu.h b/arch/ia64/include/asm/iommu.h
index 7429a72f3f92..92aceef63710 100644
--- a/arch/ia64/include/asm/iommu.h
+++ b/arch/ia64/include/asm/iommu.h
@@ -8,10 +8,8 @@
extern void no_iommu_init(void);
#ifdef CONFIG_INTEL_IOMMU
extern int force_iommu, no_iommu;
-extern int iommu_pass_through;
extern int iommu_detected;
#else
-#define iommu_pass_through (0)
#define no_iommu (1)
#define iommu_detected (0)
#endif
diff --git a/arch/ia64/kernel/pci-dma.c b/arch/ia64/kernel/pci-dma.c
index fe988c49f01c..f5d49cd3fbb0 100644
--- a/arch/ia64/kernel/pci-dma.c
+++ b/arch/ia64/kernel/pci-dma.c
@@ -22,8 +22,6 @@ int force_iommu __read_mostly = 1;
int force_iommu __read_mostly;
#endif
-int iommu_pass_through;
-
static int __init pci_iommu_init(void)
{
if (iommu_detected)
--
2.16.4
^ permalink raw reply related
* [PATCH 11/11] Documentation: Update Documentation for iommu.passthrough
From: Joerg Roedel @ 2019-08-19 13:22 UTC (permalink / raw)
To: Joerg Roedel
Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190819132256.14436-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
This kernel parameter now takes also effect on X86.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
Documentation/admin-guide/kernel-parameters.txt | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 47d981a86e2f..2d5dfa46e88a 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1811,7 +1811,7 @@
synchronously.
iommu.passthrough=
- [ARM64] Configure DMA to bypass the IOMMU by default.
+ [ARM64, X86] Configure DMA to bypass the IOMMU by default.
Format: { "0" | "1" }
0 - Use IOMMU translation for DMA.
1 - Bypass the IOMMU for DMA.
--
2.16.4
^ permalink raw reply related
* [PATCH 10/11] iommu: Disable passthrough mode when SME is active
From: Joerg Roedel @ 2019-08-19 13:22 UTC (permalink / raw)
To: Joerg Roedel
Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190819132256.14436-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
Using Passthrough mode when SME is active causes certain
devices to use the SWIOTLB bounce buffer. The bounce buffer
code has an upper limit of 256kb for the size of DMA
allocations, which is too small for certain devices and
causes them to fail.
With this patch we enable IOMMU by default when SME is
active in the system, making the default configuration work
for more systems than it does now.
Users that don't want IOMMUs to be enabled still can disable
them with kernel parameters.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/iommu/iommu.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 01759d4ac70b..ec18c9630e93 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -119,6 +119,11 @@ static int __init iommu_subsys_init(void)
iommu_set_default_passthrough(false);
else
iommu_set_default_translated(false);
+
+ if (iommu_default_passthrough() && sme_active()) {
+ pr_info("SME detected - Disabling default IOMMU Passthrough\n");
+ iommu_set_default_translated(false);
+ }
}
pr_info("Default domain type: %s %s\n",
--
2.16.4
^ permalink raw reply related
* [PATCH 05/11] iommu/vt-d: Request passthrough mode from IOMMU core
From: Joerg Roedel @ 2019-08-19 13:22 UTC (permalink / raw)
To: Joerg Roedel
Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190819132256.14436-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
Get rid of the iommu_pass_through variable and request
passthrough mode via the new iommu core function.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
drivers/iommu/intel-iommu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c
index 12d094d08c0a..a66c2a1bbbe3 100644
--- a/drivers/iommu/intel-iommu.c
+++ b/drivers/iommu/intel-iommu.c
@@ -3267,7 +3267,7 @@ static int __init init_dmars(void)
iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
}
- if (iommu_pass_through)
+ if (iommu_default_passthrough())
iommu_identity_mapping |= IDENTMAP_ALL;
#ifdef CONFIG_INTEL_IOMMU_BROKEN_GFX_WA
--
2.16.4
^ permalink raw reply related
* [PATCH 06/11] x86/dma: Get rid of iommu_pass_through
From: Joerg Roedel @ 2019-08-19 13:22 UTC (permalink / raw)
To: Joerg Roedel
Cc: corbet, tony.luck, fenghua.yu, tglx, mingo, bp, hpa, x86,
linux-doc, linux-ia64, iommu, linux-kernel, Thomas.Lendacky,
Suravee.Suthikulpanit, Joerg Roedel
In-Reply-To: <20190819132256.14436-1-joro@8bytes.org>
From: Joerg Roedel <jroedel@suse.de>
This variable has no users anymore. Remove it and tell the
IOMMU code via its new functions about requested DMA modes.
Signed-off-by: Joerg Roedel <jroedel@suse.de>
---
arch/x86/include/asm/iommu.h | 1 -
arch/x86/kernel/pci-dma.c | 20 +++-----------------
2 files changed, 3 insertions(+), 18 deletions(-)
diff --git a/arch/x86/include/asm/iommu.h b/arch/x86/include/asm/iommu.h
index baedab8ac538..b91623d521d9 100644
--- a/arch/x86/include/asm/iommu.h
+++ b/arch/x86/include/asm/iommu.h
@@ -4,7 +4,6 @@
extern int force_iommu, no_iommu;
extern int iommu_detected;
-extern int iommu_pass_through;
/* 10 seconds */
#define DMAR_OPERATION_TIMEOUT ((cycles_t) tsc_khz*10*1000)
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index f62b498b18fb..fa4352dce491 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0
#include <linux/dma-direct.h>
#include <linux/dma-debug.h>
+#include <linux/iommu.h>
#include <linux/dmar.h>
#include <linux/export.h>
#include <linux/memblock.h>
@@ -34,21 +35,6 @@ int no_iommu __read_mostly;
/* Set this to 1 if there is a HW IOMMU in the system */
int iommu_detected __read_mostly = 0;
-/*
- * This variable becomes 1 if iommu=pt is passed on the kernel command line.
- * If this variable is 1, IOMMU implementations do no DMA translation for
- * devices and allow every device to access to whole physical memory. This is
- * useful if a user wants to use an IOMMU only for KVM device assignment to
- * guests and not for driver dma translation.
- * It is also possible to disable by default in kernel config, and enable with
- * iommu=nopt at boot time.
- */
-#ifdef CONFIG_IOMMU_DEFAULT_PASSTHROUGH
-int iommu_pass_through __read_mostly = 1;
-#else
-int iommu_pass_through __read_mostly;
-#endif
-
extern struct iommu_table_entry __iommu_table[], __iommu_table_end[];
void __init pci_iommu_alloc(void)
@@ -120,9 +106,9 @@ static __init int iommu_setup(char *p)
swiotlb = 1;
#endif
if (!strncmp(p, "pt", 2))
- iommu_pass_through = 1;
+ iommu_set_default_passthrough(true);
if (!strncmp(p, "nopt", 4))
- iommu_pass_through = 0;
+ iommu_set_default_translated(true);
gart_parse_options(p);
--
2.16.4
^ permalink raw reply related
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