Re: [PATCH] x86/fault: Print "SUPERVISOR" and "READ" when decoding #PF oops
On Wed, Dec 5, 2018 at 11:27 AM Randy Dunlap wrote: > > BTW, what does PK mean? "Protection Key" Linus
Re: [PATCH] x86/fault: Print "SUPERVISOR" and "READ" when decoding #PF oops
On Wed, Dec 5, 2018 at 11:27 AM Randy Dunlap wrote: > > BTW, what does PK mean? "Protection Key" Linus
Re: [PATCH v14 06/11] livepatch: Use lists to manage patches, objects and functions
On Thu, Nov 29, 2018 at 10:44:26AM +0100, Petr Mladek wrote: > From: Jason Baron > > Currently klp_patch contains a pointer to a statically allocated array of > struct klp_object and struct klp_objects contains a pointer to a statically > allocated array of klp_func. In order to allow for the dynamic allocation > of objects and functions, link klp_patch, klp_object, and klp_func together > via linked lists. This allows us to more easily allocate new objects and > functions, while having the iterator be a simple linked list walk. > > The static structures are added to the lists early. It allows to add > the dynamically allocated objects before klp_init_object() and > klp_init_func() calls. Therefore it reduces the further changes > to the code. > > This patch does not change the existing behavior. > > Signed-off-by: Jason Baron > [pmla...@suse.com: Initialize lists before init calls] > Signed-off-by: Petr Mladek > Cc: Josh Poimboeuf > Cc: Jessica Yu > Cc: Jiri Kosina > Cc: Miroslav Benes > --- Acked-by: Joe Lawrence -- Joe
Re: [PATCH v14 06/11] livepatch: Use lists to manage patches, objects and functions
On Thu, Nov 29, 2018 at 10:44:26AM +0100, Petr Mladek wrote: > From: Jason Baron > > Currently klp_patch contains a pointer to a statically allocated array of > struct klp_object and struct klp_objects contains a pointer to a statically > allocated array of klp_func. In order to allow for the dynamic allocation > of objects and functions, link klp_patch, klp_object, and klp_func together > via linked lists. This allows us to more easily allocate new objects and > functions, while having the iterator be a simple linked list walk. > > The static structures are added to the lists early. It allows to add > the dynamically allocated objects before klp_init_object() and > klp_init_func() calls. Therefore it reduces the further changes > to the code. > > This patch does not change the existing behavior. > > Signed-off-by: Jason Baron > [pmla...@suse.com: Initialize lists before init calls] > Signed-off-by: Petr Mladek > Cc: Josh Poimboeuf > Cc: Jessica Yu > Cc: Jiri Kosina > Cc: Miroslav Benes > --- Acked-by: Joe Lawrence -- Joe
Re: [PATCH v14 05/11] livepatch: Simplify API by removing registration step
On Thu, Nov 29, 2018 at 10:44:25AM +0100, Petr Mladek wrote:
> The possibility to re-enable a registered patch was useful for immediate
> patches where the livepatch module had to stay until the system reboot.
> The improved consistency model allows to achieve the same result by
> unloading and loading the livepatch module again.
>
> Also we are going to add a feature called atomic replace. It will allow
> to create a patch that would replace all already registered patches.
> The aim is to handle dependent patches more securely. It will obsolete
> the stack of patches that helped to handle the dependencies so far.
> Then it might be unclear when a cumulative patch re-enabling is safe.
>
> It would be complicated to support the many modes. Instead we could
> actually make the API and code easier to understand.
>
> This patch removes the two step public API. All the checks and init calls
> are moved from klp_register_patch() to klp_enabled_patch(). Also the patch
> is automatically freed, including the sysfs interface when the transition
> to the disabled state is completed.
>
> As a result, there is never a disabled patch on the top of the stack.
> Therefore we do not need to check the stack in __klp_enable_patch().
> And we could simplify the check in __klp_disable_patch().
>
> Also the API and logic is much easier. It is enough to call
> klp_enable_patch() in module_init() call. The patch patch can be disabled
^^^
s/patch patch/patch
> by writing '0' into /sys/kernel/livepatch//enabled. Then the module
> can be removed once the transition finishes and sysfs interface is freed.
>
> The only problem is how to free the structures and kobjects a safe way.
> The operation is triggered from the sysfs interface. We could not put
> the related kobject from there because it would cause lock inversion
> between klp_mutex and kernfs locks, see kn->count lockdep map.
>
> This patch solved the problem by offloading the free task to
> a workqueue. It is perfectly fine:
>
> + The patch cannot not longer be used in the livepatch operations.
>
> + The module could not be removed until the free operation finishes
> and module_put() is called.
>
> + The operation is asynchronous already when the first
> klp_try_complete_transition() fails and another call
> is queued with a delay.
>
> Suggested-by: Josh Poimboeuf
> Signed-off-by: Petr Mladek
> ---
Acked-by: Joe Lawrence
> Documentation/livepatch/livepatch.txt| 137 ++---
> include/linux/livepatch.h| 5 +-
> kernel/livepatch/core.c | 275
> +--
> kernel/livepatch/core.h | 2 +
> kernel/livepatch/transition.c| 19 +-
> samples/livepatch/livepatch-callbacks-demo.c | 13 +-
> samples/livepatch/livepatch-sample.c | 13 +-
> samples/livepatch/livepatch-shadow-fix1.c| 14 +-
> samples/livepatch/livepatch-shadow-fix2.c| 14 +-
> 9 files changed, 166 insertions(+), 326 deletions(-)
>
> diff --git a/Documentation/livepatch/livepatch.txt
> b/Documentation/livepatch/livepatch.txt
> index 2d7ed09dbd59..d849af312576 100644
> --- a/Documentation/livepatch/livepatch.txt
> +++ b/Documentation/livepatch/livepatch.txt
>
> [ ... snip ... ]
>
> +5.1. Loading
> +
>
> -5.1. Registration
> --
> +The only reasonable way is to enable the patch when the livepatch kernel
> +module is being loaded. For this, klp_enable_patch() has to be called
> +in module_init() callback. There are two main reasons:
s/in module_init()/in the module_init()
>
> [ ... snip ... ]
>
> +5.4. Removing
> +-
>
> -At this stage, all the relevant sys-fs entries are removed and the patch
> -is removed from the list of known patches.
> +Module removal is only safe when there are no users of the underlying
^^
Could a reader confuse "underlying functions" for functions in the
livepatching-core? Would "modified functions" or adding "(struct
klp_func) " make this more specific?
> +functions. This is the reason why the force feature permanently disables
> +the removal. The forced tasks entered the functions but we cannot say
^
Same ambiguity here.
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index b71892693da5..1366dbb159ab 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -144,6 +144,7 @@ struct klp_object {
> * @kobj_alive: @kobj has been added and needs freeing
> * @enabled: the patch is enabled (but operation may be incomplete)
> * @forced: was involved in a forced transition
> + * @free_work: work freeing the patch that has to be done in another
> context
Re: [PATCH v14 05/11] livepatch: Simplify API by removing registration step
On Thu, Nov 29, 2018 at 10:44:25AM +0100, Petr Mladek wrote:
> The possibility to re-enable a registered patch was useful for immediate
> patches where the livepatch module had to stay until the system reboot.
> The improved consistency model allows to achieve the same result by
> unloading and loading the livepatch module again.
>
> Also we are going to add a feature called atomic replace. It will allow
> to create a patch that would replace all already registered patches.
> The aim is to handle dependent patches more securely. It will obsolete
> the stack of patches that helped to handle the dependencies so far.
> Then it might be unclear when a cumulative patch re-enabling is safe.
>
> It would be complicated to support the many modes. Instead we could
> actually make the API and code easier to understand.
>
> This patch removes the two step public API. All the checks and init calls
> are moved from klp_register_patch() to klp_enabled_patch(). Also the patch
> is automatically freed, including the sysfs interface when the transition
> to the disabled state is completed.
>
> As a result, there is never a disabled patch on the top of the stack.
> Therefore we do not need to check the stack in __klp_enable_patch().
> And we could simplify the check in __klp_disable_patch().
>
> Also the API and logic is much easier. It is enough to call
> klp_enable_patch() in module_init() call. The patch patch can be disabled
^^^
s/patch patch/patch
> by writing '0' into /sys/kernel/livepatch//enabled. Then the module
> can be removed once the transition finishes and sysfs interface is freed.
>
> The only problem is how to free the structures and kobjects a safe way.
> The operation is triggered from the sysfs interface. We could not put
> the related kobject from there because it would cause lock inversion
> between klp_mutex and kernfs locks, see kn->count lockdep map.
>
> This patch solved the problem by offloading the free task to
> a workqueue. It is perfectly fine:
>
> + The patch cannot not longer be used in the livepatch operations.
>
> + The module could not be removed until the free operation finishes
> and module_put() is called.
>
> + The operation is asynchronous already when the first
> klp_try_complete_transition() fails and another call
> is queued with a delay.
>
> Suggested-by: Josh Poimboeuf
> Signed-off-by: Petr Mladek
> ---
Acked-by: Joe Lawrence
> Documentation/livepatch/livepatch.txt| 137 ++---
> include/linux/livepatch.h| 5 +-
> kernel/livepatch/core.c | 275
> +--
> kernel/livepatch/core.h | 2 +
> kernel/livepatch/transition.c| 19 +-
> samples/livepatch/livepatch-callbacks-demo.c | 13 +-
> samples/livepatch/livepatch-sample.c | 13 +-
> samples/livepatch/livepatch-shadow-fix1.c| 14 +-
> samples/livepatch/livepatch-shadow-fix2.c| 14 +-
> 9 files changed, 166 insertions(+), 326 deletions(-)
>
> diff --git a/Documentation/livepatch/livepatch.txt
> b/Documentation/livepatch/livepatch.txt
> index 2d7ed09dbd59..d849af312576 100644
> --- a/Documentation/livepatch/livepatch.txt
> +++ b/Documentation/livepatch/livepatch.txt
>
> [ ... snip ... ]
>
> +5.1. Loading
> +
>
> -5.1. Registration
> --
> +The only reasonable way is to enable the patch when the livepatch kernel
> +module is being loaded. For this, klp_enable_patch() has to be called
> +in module_init() callback. There are two main reasons:
s/in module_init()/in the module_init()
>
> [ ... snip ... ]
>
> +5.4. Removing
> +-
>
> -At this stage, all the relevant sys-fs entries are removed and the patch
> -is removed from the list of known patches.
> +Module removal is only safe when there are no users of the underlying
^^
Could a reader confuse "underlying functions" for functions in the
livepatching-core? Would "modified functions" or adding "(struct
klp_func) " make this more specific?
> +functions. This is the reason why the force feature permanently disables
> +the removal. The forced tasks entered the functions but we cannot say
^
Same ambiguity here.
> diff --git a/include/linux/livepatch.h b/include/linux/livepatch.h
> index b71892693da5..1366dbb159ab 100644
> --- a/include/linux/livepatch.h
> +++ b/include/linux/livepatch.h
> @@ -144,6 +144,7 @@ struct klp_object {
> * @kobj_alive: @kobj has been added and needs freeing
> * @enabled: the patch is enabled (but operation may be incomplete)
> * @forced: was involved in a forced transition
> + * @free_work: work freeing the patch that has to be done in another
> context
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, 5 Dec 2018 at 18:22, Nick Desaulniers wrote: > > On Wed, Dec 5, 2018 at 12:10 AM Ard Biesheuvel > wrote: > > > > (+ Arnd) > > > > On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor > > wrote: > > > > > > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote: > > > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor > > > > wrote: > > > > > > > > > > This flag is not supported by lld: > > > > > > > > > > ld.lld: error: unknown argument: --pic-veneer > > > > > > > > > > Signed-off-by: Nathan Chancellor > > > > > > > > Hi Nate, > > > > > > > > Does this mean ld.lld is guaranteed to produce position independent > > > > veneers if you build kernels that are bigger than the typical range of > > > > a relative branch? > > (+ Peter) who might be able to answer this. > LLD will only use position independent veneers when --shared or --pie is used, otherwise it will use veneers with absolute addresses. If the kernel must use position independent veneers even when the rest of the code isn't then we'll have to implement --pic-veneer. Peter > > > > > > > > > > Hi Ard, > > > > > > Honestly, I'm not quite sure. I saw your commit that introduced this > > > flag and I wasn't quite sure what to make of it for lld. What > > > configuration would I use to verify and what would I check for? > > > > > > > Try building allyesconfig, and check the resulting binary for veneers > > (which have 'veneer' in the symbol name, at least when ld.bfd emits > > them). These veneers should not take the [virtual] address of the > > branch target directly, but take a PC relative offset (as in the > > example in the commit log of that patch you are referring to) > > Not sure clang can compile an arm32 allyesconfig (haven't tried > personally, yet, TODO), but we could try gcc+lld. > > > > > > Additionally, I have filed an LLVM bug for the lld developers to > > > check and see if this is a flag they should support: > > > > > > https://bugs.llvm.org/show_bug.cgi?id=39886 > > > > > > Thanks for the quick reply, > > > Nathan > > > > > > > > --- > > > > > arch/arm/Makefile | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > > > > > index e2a0baf36766..4fab2aa29570 100644 > > > > > --- a/arch/arm/Makefile > > > > > +++ b/arch/arm/Makefile > > > > > @@ -10,7 +10,7 @@ > > > > > # > > > > > # Copyright (C) 1995-2001 by Russell King > > > > > > > > > > -LDFLAGS_vmlinux:= --no-undefined -X --pic-veneer > > > > > +LDFLAGS_vmlinux:= --no-undefined -X $(call > > > > > ld-option,--pic-veneer) > > > > > ifeq ($(CONFIG_CPU_ENDIAN_BE8),y) > > > > > LDFLAGS_vmlinux+= --be8 > > > > > KBUILD_LDFLAGS_MODULE += --be8 > > > > > -- > > > > > 2.20.0.rc1 > > > > > > > > > -- > Thanks, > ~Nick Desaulniers
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, 5 Dec 2018 at 18:22, Nick Desaulniers wrote: > > On Wed, Dec 5, 2018 at 12:10 AM Ard Biesheuvel > wrote: > > > > (+ Arnd) > > > > On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor > > wrote: > > > > > > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote: > > > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor > > > > wrote: > > > > > > > > > > This flag is not supported by lld: > > > > > > > > > > ld.lld: error: unknown argument: --pic-veneer > > > > > > > > > > Signed-off-by: Nathan Chancellor > > > > > > > > Hi Nate, > > > > > > > > Does this mean ld.lld is guaranteed to produce position independent > > > > veneers if you build kernels that are bigger than the typical range of > > > > a relative branch? > > (+ Peter) who might be able to answer this. > LLD will only use position independent veneers when --shared or --pie is used, otherwise it will use veneers with absolute addresses. If the kernel must use position independent veneers even when the rest of the code isn't then we'll have to implement --pic-veneer. Peter > > > > > > > > > > Hi Ard, > > > > > > Honestly, I'm not quite sure. I saw your commit that introduced this > > > flag and I wasn't quite sure what to make of it for lld. What > > > configuration would I use to verify and what would I check for? > > > > > > > Try building allyesconfig, and check the resulting binary for veneers > > (which have 'veneer' in the symbol name, at least when ld.bfd emits > > them). These veneers should not take the [virtual] address of the > > branch target directly, but take a PC relative offset (as in the > > example in the commit log of that patch you are referring to) > > Not sure clang can compile an arm32 allyesconfig (haven't tried > personally, yet, TODO), but we could try gcc+lld. > > > > > > Additionally, I have filed an LLVM bug for the lld developers to > > > check and see if this is a flag they should support: > > > > > > https://bugs.llvm.org/show_bug.cgi?id=39886 > > > > > > Thanks for the quick reply, > > > Nathan > > > > > > > > --- > > > > > arch/arm/Makefile | 2 +- > > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile > > > > > index e2a0baf36766..4fab2aa29570 100644 > > > > > --- a/arch/arm/Makefile > > > > > +++ b/arch/arm/Makefile > > > > > @@ -10,7 +10,7 @@ > > > > > # > > > > > # Copyright (C) 1995-2001 by Russell King > > > > > > > > > > -LDFLAGS_vmlinux:= --no-undefined -X --pic-veneer > > > > > +LDFLAGS_vmlinux:= --no-undefined -X $(call > > > > > ld-option,--pic-veneer) > > > > > ifeq ($(CONFIG_CPU_ENDIAN_BE8),y) > > > > > LDFLAGS_vmlinux+= --be8 > > > > > KBUILD_LDFLAGS_MODULE += --be8 > > > > > -- > > > > > 2.20.0.rc1 > > > > > > > > > -- > Thanks, > ~Nick Desaulniers
Re: [PATCH] x86/fault: Print "SUPERVISOR" and "READ" when decoding #PF oops
On 12/5/18 8:36 AM, Sean Christopherson wrote:
> ...instead of manually handling the case where error_code=0, e.g. to
> display "[SUPERVISOR] [READ]" instead of "normal kernel read fault".
>
> This makes the zero case consistent with all other messages and also
> provides additional information for other error code combinations,
> e.g. error_code==1 will display "[PROT] [SUPERVISOR] [READ]" instead
> of simply "[PROT]".
>
> Print unique names for the negative cases as opposed to e.g. "[!USER]"
> to avoid mixups due to users missing a single "!" character, and to be
> more concise for the !INSTR && !WRITE case.
>
> Print "SUPERVISOR" in favor of "KERNEL" to reduce the likelihood that
> the message is misinterpreted as a generic kernel/software error and
> to be consistent with the SDM's nomenclature.
>
> An alternative to passing a negated error code to err_str_append() would
> be to expand err_str_append() to take a second string for the negative
> test, but that approach complicates handling the "[READ]" case, which
> looks for !INSTR && !WRITE, e.g. it would require an extra call to
> err_str_append() and logic in err_str_append() to allow null messages
> for both the positive and negative tests. Printing "[INSTR] [READ]"
> wouldn't be the end of the world, but a little bit of trickery in the
> kernel is a relatively small price to pay in exchange for the ability
> to unequivocally know the access type by reading a single word.
>
> Now that all components of the message use the [] format,
> explicitly state that it's the error *code* that's being printed and
> group the err_str_append() calls by type so that the resulting print
> messages are consistent, e.g. the deciphered codes will always be:
>
> [PROT] [USER|SUPERVISOR] [WRITE|INSTR|READ] [RSDV] [PK]
RSVD
but it's correct in the patch.
BTW, what does PK mean?
thanks.
>
> Cc: Andy Lutomirski
> Cc: Borislav Petkov
> Cc: Dave Hansen
> Cc: H. Peter Anvin
> Cc: Linus Torvalds
> Cc: Peter Zijlstra
> Cc: Rik van Riel
> Cc: Thomas Gleixner
> Cc: Yu-cheng Yu
> Cc: linux-kernel@vger.kernel.org
> Cc: Ingo Molnar
> Signed-off-by: Sean Christopherson
> ---
> arch/x86/mm/fault.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index 2ff25ad33233..0b4ce5d2b461 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -609,7 +609,7 @@ static void show_ldttss(const struct desc_ptr *gdt, const
> char *name, u16 index)
> */
> static void err_str_append(unsigned long error_code, char *buf, unsigned
> long mask, const char *txt)
> {
> - if (error_code & mask) {
> + if ((error_code & mask) == mask) {
> if (buf[0])
> strcat(buf, " ");
> strcat(buf, txt);
> @@ -655,13 +655,16 @@ show_fault_oops(struct pt_regs *regs, unsigned long
> error_code, unsigned long ad
>* zero delimiter must fit into err_txt[].
>*/
> err_str_append(error_code, err_txt, X86_PF_PROT, "[PROT]" );
> - err_str_append(error_code, err_txt, X86_PF_WRITE, "[WRITE]");
> err_str_append(error_code, err_txt, X86_PF_USER, "[USER]" );
> - err_str_append(error_code, err_txt, X86_PF_RSVD, "[RSVD]" );
> + err_str_append(~error_code, err_txt, X86_PF_USER, "[SUPERVISOR]");
> + err_str_append(error_code, err_txt, X86_PF_WRITE, "[WRITE]");
> err_str_append(error_code, err_txt, X86_PF_INSTR, "[INSTR]");
> + err_str_append(~error_code, err_txt, X86_PF_WRITE | X86_PF_INSTR,
> + "[READ]");
> + err_str_append(error_code, err_txt, X86_PF_RSVD, "[RSVD]" );
> err_str_append(error_code, err_txt, X86_PF_PK,"[PK]" );
>
> - pr_alert("#PF error: %s\n", error_code ? err_txt : "[normal kernel read
> fault]");
> + pr_alert("#PF error code: %s\n", err_txt);
>
> if (!(error_code & X86_PF_USER) && user_mode(regs)) {
> struct desc_ptr idt, gdt;
>
--
~Randy
Re: [PATCH] x86/fault: Print "SUPERVISOR" and "READ" when decoding #PF oops
On 12/5/18 8:36 AM, Sean Christopherson wrote:
> ...instead of manually handling the case where error_code=0, e.g. to
> display "[SUPERVISOR] [READ]" instead of "normal kernel read fault".
>
> This makes the zero case consistent with all other messages and also
> provides additional information for other error code combinations,
> e.g. error_code==1 will display "[PROT] [SUPERVISOR] [READ]" instead
> of simply "[PROT]".
>
> Print unique names for the negative cases as opposed to e.g. "[!USER]"
> to avoid mixups due to users missing a single "!" character, and to be
> more concise for the !INSTR && !WRITE case.
>
> Print "SUPERVISOR" in favor of "KERNEL" to reduce the likelihood that
> the message is misinterpreted as a generic kernel/software error and
> to be consistent with the SDM's nomenclature.
>
> An alternative to passing a negated error code to err_str_append() would
> be to expand err_str_append() to take a second string for the negative
> test, but that approach complicates handling the "[READ]" case, which
> looks for !INSTR && !WRITE, e.g. it would require an extra call to
> err_str_append() and logic in err_str_append() to allow null messages
> for both the positive and negative tests. Printing "[INSTR] [READ]"
> wouldn't be the end of the world, but a little bit of trickery in the
> kernel is a relatively small price to pay in exchange for the ability
> to unequivocally know the access type by reading a single word.
>
> Now that all components of the message use the [] format,
> explicitly state that it's the error *code* that's being printed and
> group the err_str_append() calls by type so that the resulting print
> messages are consistent, e.g. the deciphered codes will always be:
>
> [PROT] [USER|SUPERVISOR] [WRITE|INSTR|READ] [RSDV] [PK]
RSVD
but it's correct in the patch.
BTW, what does PK mean?
thanks.
>
> Cc: Andy Lutomirski
> Cc: Borislav Petkov
> Cc: Dave Hansen
> Cc: H. Peter Anvin
> Cc: Linus Torvalds
> Cc: Peter Zijlstra
> Cc: Rik van Riel
> Cc: Thomas Gleixner
> Cc: Yu-cheng Yu
> Cc: linux-kernel@vger.kernel.org
> Cc: Ingo Molnar
> Signed-off-by: Sean Christopherson
> ---
> arch/x86/mm/fault.c | 11 +++
> 1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index 2ff25ad33233..0b4ce5d2b461 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -609,7 +609,7 @@ static void show_ldttss(const struct desc_ptr *gdt, const
> char *name, u16 index)
> */
> static void err_str_append(unsigned long error_code, char *buf, unsigned
> long mask, const char *txt)
> {
> - if (error_code & mask) {
> + if ((error_code & mask) == mask) {
> if (buf[0])
> strcat(buf, " ");
> strcat(buf, txt);
> @@ -655,13 +655,16 @@ show_fault_oops(struct pt_regs *regs, unsigned long
> error_code, unsigned long ad
>* zero delimiter must fit into err_txt[].
>*/
> err_str_append(error_code, err_txt, X86_PF_PROT, "[PROT]" );
> - err_str_append(error_code, err_txt, X86_PF_WRITE, "[WRITE]");
> err_str_append(error_code, err_txt, X86_PF_USER, "[USER]" );
> - err_str_append(error_code, err_txt, X86_PF_RSVD, "[RSVD]" );
> + err_str_append(~error_code, err_txt, X86_PF_USER, "[SUPERVISOR]");
> + err_str_append(error_code, err_txt, X86_PF_WRITE, "[WRITE]");
> err_str_append(error_code, err_txt, X86_PF_INSTR, "[INSTR]");
> + err_str_append(~error_code, err_txt, X86_PF_WRITE | X86_PF_INSTR,
> + "[READ]");
> + err_str_append(error_code, err_txt, X86_PF_RSVD, "[RSVD]" );
> err_str_append(error_code, err_txt, X86_PF_PK,"[PK]" );
>
> - pr_alert("#PF error: %s\n", error_code ? err_txt : "[normal kernel read
> fault]");
> + pr_alert("#PF error code: %s\n", err_txt);
>
> if (!(error_code & X86_PF_USER) && user_mode(regs)) {
> struct desc_ptr idt, gdt;
>
--
~Randy
Re: [patch 1/2 for-4.20] mm, thp: restore node-local hugepage allocations
On Wed, 5 Dec 2018, Michal Hocko wrote: > > > At minimum do not remove the cleanup part which consolidates the gfp > > > hadnling to a single place. There is no real reason to have the > > > __GFP_THISNODE ugliness outside of alloc_hugepage_direct_gfpmask. > > > > > > > The __GFP_THISNODE usage is still confined to > > alloc_hugepage_direct_gfpmask() for the thp fault path, we no longer set > > it in alloc_pages_vma() as done before the cleanup. > > Why should be new_page any different? > To match alloc_new_node_page() which does it correctly and does not change the behavior of mbind() that the cleanup did, which used alloc_hugepage_vma() to get the __GFP_THISNODE behavior. If there is a reason mbind() is somehow different wrt allocating hugepages locally, I think that should be a separate patch, but the goal of this patch is to revert all the behavioral change that caused hugepages to be allocated remotely.
Re: [patch 1/2 for-4.20] mm, thp: restore node-local hugepage allocations
On Wed, 5 Dec 2018, Michal Hocko wrote: > > > At minimum do not remove the cleanup part which consolidates the gfp > > > hadnling to a single place. There is no real reason to have the > > > __GFP_THISNODE ugliness outside of alloc_hugepage_direct_gfpmask. > > > > > > > The __GFP_THISNODE usage is still confined to > > alloc_hugepage_direct_gfpmask() for the thp fault path, we no longer set > > it in alloc_pages_vma() as done before the cleanup. > > Why should be new_page any different? > To match alloc_new_node_page() which does it correctly and does not change the behavior of mbind() that the cleanup did, which used alloc_hugepage_vma() to get the __GFP_THISNODE behavior. If there is a reason mbind() is somehow different wrt allocating hugepages locally, I think that should be a separate patch, but the goal of this patch is to revert all the behavioral change that caused hugepages to be allocated remotely.
[PATCH] kvm: x86: Report STIBP on GET_SUPPORTED_CPUID
Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL
to the guest, which makes STIBP available to guests. This was implemented
by commits d28b387fb74d ("KVM/VMX: Allow direct access to
MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to
MSR_IA32_SPEC_CTRL").
However, we never updated GET_SUPPORTED_CPUID to let userspace know that
STIBP can be enabled in CPUID. Fix that by updating
kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features.
Signed-off-by: Eduardo Habkost
---
arch/x86/kvm/cpuid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7bcfa61375c0..cc6dd65828a4 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -381,7 +381,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2
*entry, u32 function,
/* cpuid 0x8008.ebx */
const u32 kvm_cpuid_8000_0008_ebx_x86_features =
F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
- F(AMD_SSB_NO);
+ F(AMD_SSB_NO) | F(AMD_STIBP);
/* cpuid 0xC001.edx */
const u32 kvm_cpuid_C000_0001_edx_x86_features =
@@ -411,7 +411,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2
*entry, u32 function,
/* cpuid 7.0.edx*/
const u32 kvm_cpuid_7_0_edx_x86_features =
F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
- F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES);
+ F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP);
/* all calls to cpuid_count() should be made on the same cpu */
get_cpu();
--
2.18.0.rc1.1.g3f1ff2140
[PATCH] kvm: x86: Report STIBP on GET_SUPPORTED_CPUID
Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL
to the guest, which makes STIBP available to guests. This was implemented
by commits d28b387fb74d ("KVM/VMX: Allow direct access to
MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to
MSR_IA32_SPEC_CTRL").
However, we never updated GET_SUPPORTED_CPUID to let userspace know that
STIBP can be enabled in CPUID. Fix that by updating
kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features.
Signed-off-by: Eduardo Habkost
---
arch/x86/kvm/cpuid.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 7bcfa61375c0..cc6dd65828a4 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -381,7 +381,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2
*entry, u32 function,
/* cpuid 0x8008.ebx */
const u32 kvm_cpuid_8000_0008_ebx_x86_features =
F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) |
- F(AMD_SSB_NO);
+ F(AMD_SSB_NO) | F(AMD_STIBP);
/* cpuid 0xC001.edx */
const u32 kvm_cpuid_C000_0001_edx_x86_features =
@@ -411,7 +411,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2
*entry, u32 function,
/* cpuid 7.0.edx*/
const u32 kvm_cpuid_7_0_edx_x86_features =
F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
- F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES);
+ F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP);
/* all calls to cpuid_count() should be made on the same cpu */
get_cpu();
--
2.18.0.rc1.1.g3f1ff2140
Re: [PATCH v5 1/6] fieldbus_dev: add Fieldbus Device subsystem.
On Wed, Dec 05, 2018 at 10:39:56AM -0500, Sven Van Asbroeck wrote: > Hello Greg, thanks for the feedback! > > On Wed, Dec 5, 2018 at 5:17 AM Greg KH wrote: > > And why is this a class and not just a "normal" device and bus? Devices > > live on busses, not generally as a class. Can your devices live on > > different types of busses (USB, PCI, etc.)? > > This patchset can be a bit confusing, because it doesn't just add support for > a > single fieldbus device - it proposes a general fieldbus subsystem. Great, then call it a 'fieldbus' class, not "fieldbus_dev' class. Devices can belong to a bus, or a class, so you are fine here. > Fieldbus devices from different vendors can sit on the usb, i2c, pci, etc. > buse, but they all register as a fieldbus device, via fieldbus_dev_register(), > and show up as a fieldbus class member. ok, I'm just complaining about your name in sysfs, not your code :) I think what you did here is correct from a logic point of view. > Userspace can then enumerate all fieldbus devices connected to the system by > looking at the class. Without having to know which bus they happen to be > connected to. Sounds reasonable. nice work, greg k-h
Re: [PATCH v5 1/6] fieldbus_dev: add Fieldbus Device subsystem.
On Wed, Dec 05, 2018 at 10:39:56AM -0500, Sven Van Asbroeck wrote: > Hello Greg, thanks for the feedback! > > On Wed, Dec 5, 2018 at 5:17 AM Greg KH wrote: > > And why is this a class and not just a "normal" device and bus? Devices > > live on busses, not generally as a class. Can your devices live on > > different types of busses (USB, PCI, etc.)? > > This patchset can be a bit confusing, because it doesn't just add support for > a > single fieldbus device - it proposes a general fieldbus subsystem. Great, then call it a 'fieldbus' class, not "fieldbus_dev' class. Devices can belong to a bus, or a class, so you are fine here. > Fieldbus devices from different vendors can sit on the usb, i2c, pci, etc. > buse, but they all register as a fieldbus device, via fieldbus_dev_register(), > and show up as a fieldbus class member. ok, I'm just complaining about your name in sysfs, not your code :) I think what you did here is correct from a logic point of view. > Userspace can then enumerate all fieldbus devices connected to the system by > looking at the class. Without having to know which bus they happen to be > connected to. Sounds reasonable. nice work, greg k-h
Re: [LKP] [mm] ac5b2c1891: vm-scalability.throughput -61.3% regression
On Wed, 5 Dec 2018, Michal Hocko wrote: > > It isn't specific to MADV_HUGEPAGE, it is the policy for all transparent > > hugepage allocations, including defrag=always. We agree that > > MADV_HUGEPAGE is not exactly defined: does it mean try harder to allocate > > a hugepage locally, try compaction synchronous to the fault, allow remote > > fallback? It's undefined. > > Yeah, it is certainly underdefined. One thing is clear though. Using > MADV_HUGEPAGE implies that the specific mapping benefits from THPs and > is willing to pay associated init cost. This doesn't imply anything > regarding NUMA locality and as we have NUMA API it shouldn't even > attempt to do so because it would be conflating two things. This is exactly why we use MADV_HUGEPAGE when remapping our text segment to be backed by transparent hugepages, we want to pay the cost at startup to fault thp and that involves synchronous memory compaction rather than quickly falling back to remote memory. This is making the case for me. > > So to answer "what is so different about THP?", it's the performance data. > > The NUMA locality matters more than whether the pages are huge or not. We > > also have the added benefit of khugepaged being able to collapse pages > > locally if fragmentation improves rather than being stuck accessing a > > remote hugepage forever. > > Please back your claims by a variety of workloads. Including mentioned > KVMs one. You keep hand waving about access latency completely ignoring > all other aspects and that makes my suspicious that you do not really > appreciate all the complexity here even stronger. > I discussed the tradeoff of local hugepages vs local pages vs remote hugepages in https://marc.info/?l=linux-kernel=154077010828940 on Broadwell, Haswell, and Rome. When a single application does not fit on a single node, we obviously need to extend the API to allow it to fault remotely. We can do that without changing long-standing behavior that prefers to only fault locally and causing real-world users to regress. Your suggestions about how we can extend the API are all very logical. [ Note that is not the regression being addressed here, however, which is massive swap storms due to a fragmented local node, which is why the __GFP_COMPACT_ONLY patch was also proposed by Andrea. The ability to prefer faulting remotely is a worthwhile extension but it does no good whatsoever if we can encounter massive swap storms because we didn't set __GFP_NORETRY appropriately (which both of our patches do) both locally and now remotely. ]
Re: [LKP] [mm] ac5b2c1891: vm-scalability.throughput -61.3% regression
On Wed, 5 Dec 2018, Michal Hocko wrote: > > It isn't specific to MADV_HUGEPAGE, it is the policy for all transparent > > hugepage allocations, including defrag=always. We agree that > > MADV_HUGEPAGE is not exactly defined: does it mean try harder to allocate > > a hugepage locally, try compaction synchronous to the fault, allow remote > > fallback? It's undefined. > > Yeah, it is certainly underdefined. One thing is clear though. Using > MADV_HUGEPAGE implies that the specific mapping benefits from THPs and > is willing to pay associated init cost. This doesn't imply anything > regarding NUMA locality and as we have NUMA API it shouldn't even > attempt to do so because it would be conflating two things. This is exactly why we use MADV_HUGEPAGE when remapping our text segment to be backed by transparent hugepages, we want to pay the cost at startup to fault thp and that involves synchronous memory compaction rather than quickly falling back to remote memory. This is making the case for me. > > So to answer "what is so different about THP?", it's the performance data. > > The NUMA locality matters more than whether the pages are huge or not. We > > also have the added benefit of khugepaged being able to collapse pages > > locally if fragmentation improves rather than being stuck accessing a > > remote hugepage forever. > > Please back your claims by a variety of workloads. Including mentioned > KVMs one. You keep hand waving about access latency completely ignoring > all other aspects and that makes my suspicious that you do not really > appreciate all the complexity here even stronger. > I discussed the tradeoff of local hugepages vs local pages vs remote hugepages in https://marc.info/?l=linux-kernel=154077010828940 on Broadwell, Haswell, and Rome. When a single application does not fit on a single node, we obviously need to extend the API to allow it to fault remotely. We can do that without changing long-standing behavior that prefers to only fault locally and causing real-world users to regress. Your suggestions about how we can extend the API are all very logical. [ Note that is not the regression being addressed here, however, which is massive swap storms due to a fragmented local node, which is why the __GFP_COMPACT_ONLY patch was also proposed by Andrea. The ability to prefer faulting remotely is a worthwhile extension but it does no good whatsoever if we can encounter massive swap storms because we didn't set __GFP_NORETRY appropriately (which both of our patches do) both locally and now remotely. ]
Re: [PATCH v2] kmemleak: Turn kmemleak_lock to raw spinlock on RT
On 2018-12-05 21:53:37 [+0800], He Zhe wrote: > For call trace 1: … > Since kmemleak would most likely be used to debug in environments where > we would not expect as great performance as without it, and kfree() has raw > locks > in its main path and other debug function paths, I suppose it wouldn't hurt > that > we change to raw locks. okay. > >> >From what I reached above, this is RT-only and happens on v4.18 and v4.19. > >> > >> The call trace above is caused by grabbing kmemleak_lock and then getting > >> scheduled and then re-grabbing kmemleak_lock. Using raw lock can also solve > >> this problem. > > But this is a reader / writer lock. And if I understand the other part > > of the thread then it needs multiple readers. > > For call trace 2: > > I don't get what "it needs multiple readers" exactly means here. > > In this call trace, the kmemleak_lock is grabbed as write lock, and then > scheduled > away, and then grabbed again as write lock from another path. It's a > write->write locking, compared to the discussion in the other part of the > thread. > > This is essentially because kmemleak hooks on the very low level memory > allocation and free operations. After scheduled away, it can easily re-enter > itself. > We need raw locks to prevent this from happening. With raw locks you wouldn't have multiple readers at the same time. Maybe you wouldn't have recursion but since you can't have multiple readers you would add lock contention where was none (because you could have two readers at the same time). > > Couldn't we just get rid of that kfree() or move it somewhere else? > > I mean if the free() memory on CPU-down and allocate it again CPU-up > > then we could skip that, rigth? Just allocate it and don't free it > > because the CPU will likely get up again. > > For call trace 1: > > I went through the CPU hotplug code and found that the allocation of the > problematic data, cpuc->shared_regs, is done in intel_pmu_cpu_prepare. And > the free is done in intel_pmu_cpu_dying. They are handlers triggered by two > different perf events. > > It seems we can hardly form a convincing method that holds the data while > CPUs are off and then uses it again. raw locks would be easy and good enough. Why not allocate the memory in intel_pmu_cpu_prepare() if it is not already there (otherwise skip the allocation) and in intel_pmu_cpu_dying() not free it. It looks easy. > Thanks, > Zhe Sebastian
Re: [PATCH v2] kmemleak: Turn kmemleak_lock to raw spinlock on RT
On 2018-12-05 21:53:37 [+0800], He Zhe wrote: > For call trace 1: … > Since kmemleak would most likely be used to debug in environments where > we would not expect as great performance as without it, and kfree() has raw > locks > in its main path and other debug function paths, I suppose it wouldn't hurt > that > we change to raw locks. okay. > >> >From what I reached above, this is RT-only and happens on v4.18 and v4.19. > >> > >> The call trace above is caused by grabbing kmemleak_lock and then getting > >> scheduled and then re-grabbing kmemleak_lock. Using raw lock can also solve > >> this problem. > > But this is a reader / writer lock. And if I understand the other part > > of the thread then it needs multiple readers. > > For call trace 2: > > I don't get what "it needs multiple readers" exactly means here. > > In this call trace, the kmemleak_lock is grabbed as write lock, and then > scheduled > away, and then grabbed again as write lock from another path. It's a > write->write locking, compared to the discussion in the other part of the > thread. > > This is essentially because kmemleak hooks on the very low level memory > allocation and free operations. After scheduled away, it can easily re-enter > itself. > We need raw locks to prevent this from happening. With raw locks you wouldn't have multiple readers at the same time. Maybe you wouldn't have recursion but since you can't have multiple readers you would add lock contention where was none (because you could have two readers at the same time). > > Couldn't we just get rid of that kfree() or move it somewhere else? > > I mean if the free() memory on CPU-down and allocate it again CPU-up > > then we could skip that, rigth? Just allocate it and don't free it > > because the CPU will likely get up again. > > For call trace 1: > > I went through the CPU hotplug code and found that the allocation of the > problematic data, cpuc->shared_regs, is done in intel_pmu_cpu_prepare. And > the free is done in intel_pmu_cpu_dying. They are handlers triggered by two > different perf events. > > It seems we can hardly form a convincing method that holds the data while > CPUs are off and then uses it again. raw locks would be easy and good enough. Why not allocate the memory in intel_pmu_cpu_prepare() if it is not already there (otherwise skip the allocation) and in intel_pmu_cpu_dying() not free it. It looks easy. > Thanks, > Zhe Sebastian
Re: [RFC PATCH 02/14] mm/hms: heterogenenous memory system (HMS) documentation
On 2018-12-05 11:55 a.m., Jerome Glisse wrote: > So now once next type of device shows up with the exact same thing > let say FPGA, we have to create a new subsystem for them too. Also > this make the userspace life much much harder. Now userspace must > go parse PCIE, subsystem1, subsystem2, subsystemN, NUMA, ... and > merge all that different information together and rebuild the > representation i am putting forward in this patchset in userspace. Yes. But seeing such FPGA links aren't common yet and there isn't really much in terms of common FPGA infrastructure in the kernel (which are hard seeing the hardware is infinitely customization) you can let the people developing FPGA code worry about it and come up with their own solution. Buses between FPGAs may end up never being common enough for people to care, or they may end up being so weird that they need their own description independent of GPUS, or maybe when they become common they find a way to use the GPU link subsystem -- who knows. Don't try to design for use cases that don't exist yet. Yes, userspace will have to know about all the buses it cares to find links over. Sounds like a perfect thing for libhms to do. > There is no telling that kernel won't be able to provide quirk and > workaround because some merging is actually illegal on a given > platform (like some link from a subsystem is not accessible through > the PCI connection of one of the device connected to that link). These are all just different individual problems which need different solutions not grand new design concepts. > So it means userspace will have to grow its own database or work- > around and quirk and i am back in the situation i am in today. No, as I've said, quirks are firmly the responsibility of kernels. Userspace will need to know how to work with the different buses and CPU/node information but there really isn't that many of these to deal with and this is a much easier approach than trying to come up with a new API that can wrap the nuances of all existing and potential future bus types we may have to deal with. Logan
Re: [RFC PATCH 02/14] mm/hms: heterogenenous memory system (HMS) documentation
On 2018-12-05 11:55 a.m., Jerome Glisse wrote: > So now once next type of device shows up with the exact same thing > let say FPGA, we have to create a new subsystem for them too. Also > this make the userspace life much much harder. Now userspace must > go parse PCIE, subsystem1, subsystem2, subsystemN, NUMA, ... and > merge all that different information together and rebuild the > representation i am putting forward in this patchset in userspace. Yes. But seeing such FPGA links aren't common yet and there isn't really much in terms of common FPGA infrastructure in the kernel (which are hard seeing the hardware is infinitely customization) you can let the people developing FPGA code worry about it and come up with their own solution. Buses between FPGAs may end up never being common enough for people to care, or they may end up being so weird that they need their own description independent of GPUS, or maybe when they become common they find a way to use the GPU link subsystem -- who knows. Don't try to design for use cases that don't exist yet. Yes, userspace will have to know about all the buses it cares to find links over. Sounds like a perfect thing for libhms to do. > There is no telling that kernel won't be able to provide quirk and > workaround because some merging is actually illegal on a given > platform (like some link from a subsystem is not accessible through > the PCI connection of one of the device connected to that link). These are all just different individual problems which need different solutions not grand new design concepts. > So it means userspace will have to grow its own database or work- > around and quirk and i am back in the situation i am in today. No, as I've said, quirks are firmly the responsibility of kernels. Userspace will need to know how to work with the different buses and CPU/node information but there really isn't that many of these to deal with and this is a much easier approach than trying to come up with a new API that can wrap the nuances of all existing and potential future bus types we may have to deal with. Logan
Re: [PATCH v14 04/11] livepatch: Refuse to unload only livepatches available during a forced transition
On Thu, Nov 29, 2018 at 10:44:24AM +0100, Petr Mladek wrote: > module_put() is currently never called in klp_complete_transition() when > klp_force is set. As a result, we might keep the reference count even when > klp_enable_patch() fails and klp_cancel_transition() is called. > > This might make an assumption that a module might get blocked in some ^^ re-wording suggestion: "give the impression" > strange init state. Fortunately, it is not the case. The reference count > is ignored when mod->init fails and erroneous modules are always removed. > > Anyway, this might make some confusion. Instead, this patch moves ^^^ re-wording suggestions: "create confusion" or "be confusing" > the global klp_forced flag into struct klp_patch. As a result, > we block only modules that might still be in use after a forced > transition. Newly loaded livepatches might be eventually completely > removed later. > > It is not a big deal. But the code is at least consistent with > the reality. > > Signed-off-by: Petr Mladek Acked-by: Joe Lawrence -- Joe
Re: [PATCH v14 04/11] livepatch: Refuse to unload only livepatches available during a forced transition
On Thu, Nov 29, 2018 at 10:44:24AM +0100, Petr Mladek wrote: > module_put() is currently never called in klp_complete_transition() when > klp_force is set. As a result, we might keep the reference count even when > klp_enable_patch() fails and klp_cancel_transition() is called. > > This might make an assumption that a module might get blocked in some ^^ re-wording suggestion: "give the impression" > strange init state. Fortunately, it is not the case. The reference count > is ignored when mod->init fails and erroneous modules are always removed. > > Anyway, this might make some confusion. Instead, this patch moves ^^^ re-wording suggestions: "create confusion" or "be confusing" > the global klp_forced flag into struct klp_patch. As a result, > we block only modules that might still be in use after a forced > transition. Newly loaded livepatches might be eventually completely > removed later. > > It is not a big deal. But the code is at least consistent with > the reality. > > Signed-off-by: Petr Mladek Acked-by: Joe Lawrence -- Joe
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
On Wed, Dec 05, 2018 at 07:01:58PM +0100, Ingo Molnar wrote: > Oh - I thought we'd have arch/x86/mce/ or so? > > There's machine check events that are not tied to any particular CPU, > correct? So this would be the right conceptual level - and it would also > remove the somewhat redundant 'kernel' part. Well, all the MCE events reported are some way or the other tied to the CPU and they're reported in the CPU's MCA banks so I think we want arch/x86/cpu/mce/ -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH] x86/mce: Streamline MCE subsystem's naming
On Wed, Dec 05, 2018 at 07:01:58PM +0100, Ingo Molnar wrote: > Oh - I thought we'd have arch/x86/mce/ or so? > > There's machine check events that are not tied to any particular CPU, > correct? So this would be the right conceptual level - and it would also > remove the somewhat redundant 'kernel' part. Well, all the MCE events reported are some way or the other tied to the CPU and they're reported in the CPU's MCA banks so I think we want arch/x86/cpu/mce/ -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 4.14 121/146] x86/fpu: Disable bottom halves while loading FPU registers
On Wed, Dec 05, 2018 at 06:26:24PM +0200, Jari Ruusu wrote: > That same race appears to be present in older kernel branches also. > The context is sligthly different, so the patch for 4.14 does not > apply cleanly to older kernels. For 4.9 branch, this edit works: You could take the upstream one, amend it with your change, test it and send it to Greg - I believe he'll take the backport gladly. :-) -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH 4.14 121/146] x86/fpu: Disable bottom halves while loading FPU registers
On Wed, Dec 05, 2018 at 06:26:24PM +0200, Jari Ruusu wrote: > That same race appears to be present in older kernel branches also. > The context is sligthly different, so the patch for 4.14 does not > apply cleanly to older kernels. For 4.9 branch, this edit works: You could take the upstream one, amend it with your change, test it and send it to Greg - I believe he'll take the backport gladly. :-) -- Regards/Gruss, Boris. Good mailing practices for 400: avoid top-posting and trim the reply.
Re: [PATCH v14 03/11] livepatch: Consolidate klp_free functions
On Thu, Nov 29, 2018 at 10:44:23AM +0100, Petr Mladek wrote:
> The code for freeing livepatch structures is a bit scattered and tricky:
>
> + direct calls to klp_free_*_limited() and kobject_put() are
> used to release partially initialized objects
>
> + klp_free_patch() removes the patch from the public list
> and releases all objects except for patch->kobj
>
> + object_put(>kobj) and the related wait_for_completion()
> are called directly outside klp_mutex; this code is duplicated;
>
> Now, we are going to remove the registration stage to simplify the API
> and the code. This would require handling more situations in
> klp_enable_patch() error paths.
>
> More importantly, we are going to add a feature called atomic replace.
> It will need to dynamically create func and object structures. We will
> want to reuse the existing init() and free() functions. This would
> create even more error path scenarios.
>
> [*] We need our own flag. Note that kobject_put() cannot be called safely
> when kobj.state_initialized is set. This flag is true when kobject_add()
> part failed. And it is never cleared.
> [*] We need our own flag. Note that kobject_put() cannot be called safely
> when kobj.state_initialized is set. This flag is true when kobject_add()
> part failed. And it is never cleared.
> This patch implements a more clever free functions:
^
re-wording suggestions: "simpler", "clearer", "more straightforward"
>
> + checks kobj_alive flag instead of @limit[*]
>
> + initializes patch->list early so that the check for empty list
> always works
>
> + The action(s) that has to be done outside klp_mutex are done
> in separate klp_free_patch_finish() function. It waits only
> when patch->kobj was really released via the _start() part.
>
> The patch does not change the existing behavior.
>
> [*] We need our own flag. Note that kobject_put() cannot be called safely
> when kobj.state_initialized is set. This flag is true when kobject_add()
> part failed. And it is never cleared.
Isn't kobj.state_initialized also true in the ordinary kobject_put() case
where kobject_add() succeeded?
If so, this note could be modified slightly:
(minimal change)
[*] We need our own flag. Note that kobject_put() cannot be called safely
just because kobj.state_initialized is set. This flag is even true when
kobject_add()
part failed. And it is never cleared.
-- or --
(rewording)
[*] We need our own flag to track that the kobject was successfully
added to the hierarchy. Note that kobj.state_initialized only
indicates that kobject has been initialized, not whether is has been
added (and needs to be removed on cleanup).
>
> Signed-off-by: Petr Mladek
> Cc: Josh Poimboeuf
> Cc: Miroslav Benes
> Cc: Jessica Yu
> Cc: Jiri Kosina
> Cc: Jason Baron
> ---
Acked-by: Joe Lawrence
>
> [ ... snip ... ]
>
> +static int klp_init_patch(struct klp_patch *patch)
> +{
> + struct klp_object *obj;
> + int ret;
> +
> + mutex_lock(_mutex);
> +
> + ret = klp_init_patch_before_free(patch);
> if (ret) {
> mutex_unlock(_mutex);
> return ret;
> }
>
I believe klp_init_patch_before_free() accumulates more responsibilities
later in the patchset, but I'll ask here: does it really need the
klp_mutex since it looks to be operating only on the klp_patch, its
objects and functions?
-- Joe
Re: [PATCH v14 03/11] livepatch: Consolidate klp_free functions
On Thu, Nov 29, 2018 at 10:44:23AM +0100, Petr Mladek wrote:
> The code for freeing livepatch structures is a bit scattered and tricky:
>
> + direct calls to klp_free_*_limited() and kobject_put() are
> used to release partially initialized objects
>
> + klp_free_patch() removes the patch from the public list
> and releases all objects except for patch->kobj
>
> + object_put(>kobj) and the related wait_for_completion()
> are called directly outside klp_mutex; this code is duplicated;
>
> Now, we are going to remove the registration stage to simplify the API
> and the code. This would require handling more situations in
> klp_enable_patch() error paths.
>
> More importantly, we are going to add a feature called atomic replace.
> It will need to dynamically create func and object structures. We will
> want to reuse the existing init() and free() functions. This would
> create even more error path scenarios.
>
> [*] We need our own flag. Note that kobject_put() cannot be called safely
> when kobj.state_initialized is set. This flag is true when kobject_add()
> part failed. And it is never cleared.
> [*] We need our own flag. Note that kobject_put() cannot be called safely
> when kobj.state_initialized is set. This flag is true when kobject_add()
> part failed. And it is never cleared.
> This patch implements a more clever free functions:
^
re-wording suggestions: "simpler", "clearer", "more straightforward"
>
> + checks kobj_alive flag instead of @limit[*]
>
> + initializes patch->list early so that the check for empty list
> always works
>
> + The action(s) that has to be done outside klp_mutex are done
> in separate klp_free_patch_finish() function. It waits only
> when patch->kobj was really released via the _start() part.
>
> The patch does not change the existing behavior.
>
> [*] We need our own flag. Note that kobject_put() cannot be called safely
> when kobj.state_initialized is set. This flag is true when kobject_add()
> part failed. And it is never cleared.
Isn't kobj.state_initialized also true in the ordinary kobject_put() case
where kobject_add() succeeded?
If so, this note could be modified slightly:
(minimal change)
[*] We need our own flag. Note that kobject_put() cannot be called safely
just because kobj.state_initialized is set. This flag is even true when
kobject_add()
part failed. And it is never cleared.
-- or --
(rewording)
[*] We need our own flag to track that the kobject was successfully
added to the hierarchy. Note that kobj.state_initialized only
indicates that kobject has been initialized, not whether is has been
added (and needs to be removed on cleanup).
>
> Signed-off-by: Petr Mladek
> Cc: Josh Poimboeuf
> Cc: Miroslav Benes
> Cc: Jessica Yu
> Cc: Jiri Kosina
> Cc: Jason Baron
> ---
Acked-by: Joe Lawrence
>
> [ ... snip ... ]
>
> +static int klp_init_patch(struct klp_patch *patch)
> +{
> + struct klp_object *obj;
> + int ret;
> +
> + mutex_lock(_mutex);
> +
> + ret = klp_init_patch_before_free(patch);
> if (ret) {
> mutex_unlock(_mutex);
> return ret;
> }
>
I believe klp_init_patch_before_free() accumulates more responsibilities
later in the patchset, but I'll ask here: does it really need the
klp_mutex since it looks to be operating only on the klp_patch, its
objects and functions?
-- Joe
Re: [PATCH] mm/alloc: fallback to first node if the wanted node offline
On Wed, 5 Dec 2018, Pingfan Liu wrote: > > > And rather than using first_online_node, would next_online_node() work? > > > > > What is the gain? Is it for memory pressure on node0? > > > Maybe I got your point now. Do you try to give a cheap assumption on > nearest neigh of this node? > It's likely better than first_online_node, but probably going to be the same based on the node ids that you have reported since the nodemask will simply wrap around back to the first node.
Re: [PATCH] mm/alloc: fallback to first node if the wanted node offline
On Wed, 5 Dec 2018, Pingfan Liu wrote: > > > And rather than using first_online_node, would next_online_node() work? > > > > > What is the gain? Is it for memory pressure on node0? > > > Maybe I got your point now. Do you try to give a cheap assumption on > nearest neigh of this node? > It's likely better than first_online_node, but probably going to be the same based on the node ids that you have reported since the nodemask will simply wrap around back to the first node.
Re: [RFC PATCH 02/14] mm/hms: heterogenenous memory system (HMS) documentation
On Wed, Dec 05, 2018 at 11:48:37AM -0700, Logan Gunthorpe wrote: > > > On 2018-12-05 11:33 a.m., Jerome Glisse wrote: > > If i add a a fake driver for those what would i do ? under which > > sub-system i register them ? How i express the fact that they > > connect device X,Y and Z with some properties ? > > Yes this is exactly what I'm suggesting. I wouldn't call it a fake > driver, but a new struct device describing an actual device in the > system. It would be a feature of the GPU subsystem seeing this is a > feature of GPUs. Expressing that the new devices connect to a specific > set of GPUs is not a hard problem to solve. > > > This is not PCIE ... you can not discover bridges and child, it > > not a tree like structure, it is a random graph (which depends > > on how the OEM wire port on the chips). > > You must be able to discover that these links exist and register a > device with the system. Where else do you get the information currently? > The suggestion doesn't change anything to do with how you interact with > hardware, only how you describe the information within the kernel. > > > So i have not pre-existing driver, they are not in sysfs today and > > they do not need a driver. Hence why i proposed what i proposed > > a sysfs hierarchy where i can add those "virtual" object and shows > > how they connect existing device for which we have a sysfs directory > > to symlink. > > So add a new driver -- that's what I've been suggesting all along. > Having a driver not exist is no reason to not create one. I'd suggest > that if you want them to show up in the sysfs hierarchy then you do need > some kind of driver code to create a struct device. Just because the > kernel doesn't have to interact with them is no reason not to create a > struct device. It's *much* easier to create a new driver subsystem than > a whole new userspace API. So now once next type of device shows up with the exact same thing let say FPGA, we have to create a new subsystem for them too. Also this make the userspace life much much harder. Now userspace must go parse PCIE, subsystem1, subsystem2, subsystemN, NUMA, ... and merge all that different information together and rebuild the representation i am putting forward in this patchset in userspace. There is no telling that kernel won't be able to provide quirk and workaround because some merging is actually illegal on a given platform (like some link from a subsystem is not accessible through the PCI connection of one of the device connected to that link). So it means userspace will have to grow its own database or work- around and quirk and i am back in the situation i am in today. Not very convincing to me. What i am proposing here is a new common description provided by the kernel where we can reconciliate weird interaction. But i doubt i can convince you i will make progress on what i need today and keep working on sysfs. Cheers, Jérôme
Re: [RFC PATCH 02/14] mm/hms: heterogenenous memory system (HMS) documentation
On Wed, Dec 05, 2018 at 11:48:37AM -0700, Logan Gunthorpe wrote: > > > On 2018-12-05 11:33 a.m., Jerome Glisse wrote: > > If i add a a fake driver for those what would i do ? under which > > sub-system i register them ? How i express the fact that they > > connect device X,Y and Z with some properties ? > > Yes this is exactly what I'm suggesting. I wouldn't call it a fake > driver, but a new struct device describing an actual device in the > system. It would be a feature of the GPU subsystem seeing this is a > feature of GPUs. Expressing that the new devices connect to a specific > set of GPUs is not a hard problem to solve. > > > This is not PCIE ... you can not discover bridges and child, it > > not a tree like structure, it is a random graph (which depends > > on how the OEM wire port on the chips). > > You must be able to discover that these links exist and register a > device with the system. Where else do you get the information currently? > The suggestion doesn't change anything to do with how you interact with > hardware, only how you describe the information within the kernel. > > > So i have not pre-existing driver, they are not in sysfs today and > > they do not need a driver. Hence why i proposed what i proposed > > a sysfs hierarchy where i can add those "virtual" object and shows > > how they connect existing device for which we have a sysfs directory > > to symlink. > > So add a new driver -- that's what I've been suggesting all along. > Having a driver not exist is no reason to not create one. I'd suggest > that if you want them to show up in the sysfs hierarchy then you do need > some kind of driver code to create a struct device. Just because the > kernel doesn't have to interact with them is no reason not to create a > struct device. It's *much* easier to create a new driver subsystem than > a whole new userspace API. So now once next type of device shows up with the exact same thing let say FPGA, we have to create a new subsystem for them too. Also this make the userspace life much much harder. Now userspace must go parse PCIE, subsystem1, subsystem2, subsystemN, NUMA, ... and merge all that different information together and rebuild the representation i am putting forward in this patchset in userspace. There is no telling that kernel won't be able to provide quirk and workaround because some merging is actually illegal on a given platform (like some link from a subsystem is not accessible through the PCI connection of one of the device connected to that link). So it means userspace will have to grow its own database or work- around and quirk and i am back in the situation i am in today. Not very convincing to me. What i am proposing here is a new common description provided by the kernel where we can reconciliate weird interaction. But i doubt i can convince you i will make progress on what i need today and keep working on sysfs. Cheers, Jérôme
Re: [PATCH v2 0/4] add uart DMA function
On Wed, Dec 05, 2018 at 10:01:33PM +0530, Vinod Koul wrote: > Hi Greg, > > On 05-12-18, 11:03, Greg Kroah-Hartman wrote: > > On Wed, Dec 05, 2018 at 04:42:56PM +0800, Long Cheng wrote: > > > In Mediatek SOCs, the uart can support DMA function. > > > Base on DMA engine formwork, we add the DMA code to support uart. And put > > > the code under drivers/dma. > > > > > > This series contains document bindings, Kconfig to control the function > > > enable or not, > > > device tree including interrupt and dma device node, the code of UART DM > > > > I've queued up patches 1 and 3 from this series in my tty tree, thanks. > > Do you mind not taking patch 2, I would like that to go thru dmaengine > tree Like I said, I only took patches 1 and 3...
Re: [PATCH v2 0/4] add uart DMA function
On Wed, Dec 05, 2018 at 10:01:33PM +0530, Vinod Koul wrote: > Hi Greg, > > On 05-12-18, 11:03, Greg Kroah-Hartman wrote: > > On Wed, Dec 05, 2018 at 04:42:56PM +0800, Long Cheng wrote: > > > In Mediatek SOCs, the uart can support DMA function. > > > Base on DMA engine formwork, we add the DMA code to support uart. And put > > > the code under drivers/dma. > > > > > > This series contains document bindings, Kconfig to control the function > > > enable or not, > > > device tree including interrupt and dma device node, the code of UART DM > > > > I've queued up patches 1 and 3 from this series in my tty tree, thanks. > > Do you mind not taking patch 2, I would like that to go thru dmaengine > tree Like I said, I only took patches 1 and 3...
Re: [GIT PULL] x86: remove Intel MPX
On 12/5/18 10:42 AM, Konrad Rzeszutek Wilk wrote: > On Wed, Dec 05, 2018 at 08:44:43AM -0800, Dave Hansen wrote: >> Hi x86 maintainers, >> >> Please pull from: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-mpx.git >> mpx-remove >> >> There is only one commit, removing the Intel MPX implementation from the >> tree. The benefits of keeping the feature in the tree are not worth the >> ongoing maintenance cost. > Is there an replacement or another way to use this feature? There is no replacement for using the hardware. Similar functionality is available through software-only means like AddressSanitizer, though.
Re: [GIT PULL] x86: remove Intel MPX
On 12/5/18 10:42 AM, Konrad Rzeszutek Wilk wrote: > On Wed, Dec 05, 2018 at 08:44:43AM -0800, Dave Hansen wrote: >> Hi x86 maintainers, >> >> Please pull from: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-mpx.git >> mpx-remove >> >> There is only one commit, removing the Intel MPX implementation from the >> tree. The benefits of keeping the feature in the tree are not worth the >> ongoing maintenance cost. > Is there an replacement or another way to use this feature? There is no replacement for using the hardware. Similar functionality is available through software-only means like AddressSanitizer, though.
Re: [RFC PATCH 02/14] mm/hms: heterogenenous memory system (HMS) documentation
On 2018-12-05 11:33 a.m., Jerome Glisse wrote: > If i add a a fake driver for those what would i do ? under which > sub-system i register them ? How i express the fact that they > connect device X,Y and Z with some properties ? Yes this is exactly what I'm suggesting. I wouldn't call it a fake driver, but a new struct device describing an actual device in the system. It would be a feature of the GPU subsystem seeing this is a feature of GPUs. Expressing that the new devices connect to a specific set of GPUs is not a hard problem to solve. > This is not PCIE ... you can not discover bridges and child, it > not a tree like structure, it is a random graph (which depends > on how the OEM wire port on the chips). You must be able to discover that these links exist and register a device with the system. Where else do you get the information currently? The suggestion doesn't change anything to do with how you interact with hardware, only how you describe the information within the kernel. > So i have not pre-existing driver, they are not in sysfs today and > they do not need a driver. Hence why i proposed what i proposed > a sysfs hierarchy where i can add those "virtual" object and shows > how they connect existing device for which we have a sysfs directory > to symlink. So add a new driver -- that's what I've been suggesting all along. Having a driver not exist is no reason to not create one. I'd suggest that if you want them to show up in the sysfs hierarchy then you do need some kind of driver code to create a struct device. Just because the kernel doesn't have to interact with them is no reason not to create a struct device. It's *much* easier to create a new driver subsystem than a whole new userspace API. Logan
Re: [RFC PATCH 02/14] mm/hms: heterogenenous memory system (HMS) documentation
On 2018-12-05 11:33 a.m., Jerome Glisse wrote: > If i add a a fake driver for those what would i do ? under which > sub-system i register them ? How i express the fact that they > connect device X,Y and Z with some properties ? Yes this is exactly what I'm suggesting. I wouldn't call it a fake driver, but a new struct device describing an actual device in the system. It would be a feature of the GPU subsystem seeing this is a feature of GPUs. Expressing that the new devices connect to a specific set of GPUs is not a hard problem to solve. > This is not PCIE ... you can not discover bridges and child, it > not a tree like structure, it is a random graph (which depends > on how the OEM wire port on the chips). You must be able to discover that these links exist and register a device with the system. Where else do you get the information currently? The suggestion doesn't change anything to do with how you interact with hardware, only how you describe the information within the kernel. > So i have not pre-existing driver, they are not in sysfs today and > they do not need a driver. Hence why i proposed what i proposed > a sysfs hierarchy where i can add those "virtual" object and shows > how they connect existing device for which we have a sysfs directory > to symlink. So add a new driver -- that's what I've been suggesting all along. Having a driver not exist is no reason to not create one. I'd suggest that if you want them to show up in the sysfs hierarchy then you do need some kind of driver code to create a struct device. Just because the kernel doesn't have to interact with them is no reason not to create a struct device. It's *much* easier to create a new driver subsystem than a whole new userspace API. Logan
Re: [PATCH 0/2] tracing: arm64: Make ftrace_replace_code() schedulable for arm64
On Wed, 5 Dec 2018 at 19:33, Steven Rostedt wrote: > > > This is a little more involved, and I would like to push this through my > tree. Can I get a reviewed-by/ack for the second (arm64) patch? > > Anders, can you also test this to make sure that it fixes the issue you > see? Yes of course, I'll try them. Cheers, Anders > > Thanks! > > -- Steve > > > Steven Rostedt (VMware) (2): > ftrace: Allow ftrace_replace_code() to be schedulable > arm64: ftrace: Set FTRACE_SCHEDULABLE before ftrace_modify_all_code() > > > arch/arm64/kernel/ftrace.c | 1 + > include/linux/ftrace.h | 1 + > kernel/trace/ftrace.c | 19 --- > 3 files changed, 18 insertions(+), 3 deletions(-)
Re: [PATCH 0/2] tracing: arm64: Make ftrace_replace_code() schedulable for arm64
On Wed, 5 Dec 2018 at 19:33, Steven Rostedt wrote: > > > This is a little more involved, and I would like to push this through my > tree. Can I get a reviewed-by/ack for the second (arm64) patch? > > Anders, can you also test this to make sure that it fixes the issue you > see? Yes of course, I'll try them. Cheers, Anders > > Thanks! > > -- Steve > > > Steven Rostedt (VMware) (2): > ftrace: Allow ftrace_replace_code() to be schedulable > arm64: ftrace: Set FTRACE_SCHEDULABLE before ftrace_modify_all_code() > > > arch/arm64/kernel/ftrace.c | 1 + > include/linux/ftrace.h | 1 + > kernel/trace/ftrace.c | 19 --- > 3 files changed, 18 insertions(+), 3 deletions(-)
Re: [PATCH v14 02/11] livepatch: Shuffle klp_enable_patch()/klp_disable_patch() code
On Thu, Nov 29, 2018 at 10:44:22AM +0100, Petr Mladek wrote: > We are going to simplify the API and code by removing the registration > step. This would require calling init/free functions from enable/disable > ones. > > This patch just moves the code to prevent more forward declarations. > > This patch does not change the code except of two forward declarations. ^ s/except of/except for > > Signed-off-by: Petr Mladek > --- Acked-by: Joe Lawrence -- Joe
Re: [PATCH v14 02/11] livepatch: Shuffle klp_enable_patch()/klp_disable_patch() code
On Thu, Nov 29, 2018 at 10:44:22AM +0100, Petr Mladek wrote: > We are going to simplify the API and code by removing the registration > step. This would require calling init/free functions from enable/disable > ones. > > This patch just moves the code to prevent more forward declarations. > > This patch does not change the code except of two forward declarations. ^ s/except of/except for > > Signed-off-by: Petr Mladek > --- Acked-by: Joe Lawrence -- Joe
Re: [PATCH v14 01/11] livepatch: Change unsigned long old_addr -> void *old_func in struct klp_func
On Thu, Nov 29, 2018 at 10:44:21AM +0100, Petr Mladek wrote:
> The address of the to be patched function and new function is stored
> in struct klp_func as:
>
> void *new_func;
> unsigned long old_addr;
>
> The different naming scheme and type is derived from the way how
^^^
s/the way how/the way
> the addresses are set. @old_addr is assigned at runtime using
> kallsyms-based search. @new_func is statically initialized,
> for example:
>
> static struct klp_func funcs[] = {
> {
> .old_name = "cmdline_proc_show",
> .new_func = livepatch_cmdline_proc_show,
> }, { }
> };
>
> This patch changes unsigned log old_addr -> void *old_func. It removes
s/unsigned log/unsigned long
> some confusion when these address are later used in the code. It is
> motivated by a followup patch that adds special NOP struct klp_func
> where we want to assign func->new_func = func->old_addr respectively
> func->new_func = func->old_func.
>
> This patch does not modify the existing behavior.
>
> Suggested-by: Josh Poimboeuf
> Signed-off-by: Petr Mladek
> ---
Acked-by: Joe Lawrence
-- Joe
Re: [PATCH v14 01/11] livepatch: Change unsigned long old_addr -> void *old_func in struct klp_func
On Thu, Nov 29, 2018 at 10:44:21AM +0100, Petr Mladek wrote:
> The address of the to be patched function and new function is stored
> in struct klp_func as:
>
> void *new_func;
> unsigned long old_addr;
>
> The different naming scheme and type is derived from the way how
^^^
s/the way how/the way
> the addresses are set. @old_addr is assigned at runtime using
> kallsyms-based search. @new_func is statically initialized,
> for example:
>
> static struct klp_func funcs[] = {
> {
> .old_name = "cmdline_proc_show",
> .new_func = livepatch_cmdline_proc_show,
> }, { }
> };
>
> This patch changes unsigned log old_addr -> void *old_func. It removes
s/unsigned log/unsigned long
> some confusion when these address are later used in the code. It is
> motivated by a followup patch that adds special NOP struct klp_func
> where we want to assign func->new_func = func->old_addr respectively
> func->new_func = func->old_func.
>
> This patch does not modify the existing behavior.
>
> Suggested-by: Josh Poimboeuf
> Signed-off-by: Petr Mladek
> ---
Acked-by: Joe Lawrence
-- Joe
Re: [GIT PULL] x86: remove Intel MPX
On Wed, Dec 05, 2018 at 08:44:43AM -0800, Dave Hansen wrote: > Hi x86 maintainers, > > Please pull from: > > git://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-mpx.git > mpx-remove > > There is only one commit, removing the Intel MPX implementation from the > tree. The benefits of keeping the feature in the tree are not worth the > ongoing maintenance cost. Is there an replacement or another way to use this feature? Thanks.
Re: [GIT PULL] x86: remove Intel MPX
On Wed, Dec 05, 2018 at 08:44:43AM -0800, Dave Hansen wrote: > Hi x86 maintainers, > > Please pull from: > > git://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-mpx.git > mpx-remove > > There is only one commit, removing the Intel MPX implementation from the > tree. The benefits of keeping the feature in the tree are not worth the > ongoing maintenance cost. Is there an replacement or another way to use this feature? Thanks.
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, Dec 5, 2018 at 10:40 AM Ard Biesheuvel
wrote:
>
> On Wed, 5 Dec 2018 at 19:36, Nathan Chancellor
> wrote:
> >
> > On Wed, Dec 05, 2018 at 09:09:56AM +0100, Ard Biesheuvel wrote:
> > > (+ Arnd)
> > >
> > > On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor
> > > wrote:
> > > >
> > > > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote:
> > > > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor
> > > > > wrote:
> > > > > >
> > > > > > This flag is not supported by lld:
> > > > > >
> > > > > > ld.lld: error: unknown argument: --pic-veneer
> > > > > >
> > > > > > Signed-off-by: Nathan Chancellor
> > > > >
> > > > > Hi Nate,
> > > > >
> > > > > Does this mean ld.lld is guaranteed to produce position independent
> > > > > veneers if you build kernels that are bigger than the typical range of
> > > > > a relative branch?
> > > > >
> > > >
> > > > Hi Ard,
> > > >
> > > > Honestly, I'm not quite sure. I saw your commit that introduced this
> > > > flag and I wasn't quite sure what to make of it for lld. What
> > > > configuration would I use to verify and what would I check for?
> > > >
> > >
> > > Try building allyesconfig, and check the resulting binary for veneers
> > > (which have 'veneer' in the symbol name, at least when ld.bfd emits
> > > them). These veneers should not take the [virtual] address of the
> > > branch target directly, but take a PC relative offset (as in the
> > > example in the commit log of that patch you are referring to)
> > >
> >
> > Alright, compiling with allyesconfig is a little rough at the moment
> > (bug reports I will file in due time) but I was able to do it. Here's
> > the disassembly specifically for the functions you had in your commit,
> > my assembly knowledge is pretty much non-existent unfortunately so I am
> > not sure what to make of it (it doesn't look like there is a virtual
> > address for pc in that mix?). I am happy to provide any more information
> > that is needed.
> >
> > c03030cc <__enable_mmu>:
> > c03030cc: e3c2bic r0, r0, #2
> > c03030d0: e3c00b02bic r0, r0, #2048 ; 0x800
> > c03030d4: e3c00a01bic r0, r0, #4096 ; 0x1000
> > c03030d8: e3a05051mov r5, #81 ; 0x51
> > c03030dc: ee035f10mcr 15, 0, r5, cr3, cr0, {0}
> > c03030e0: ee024f10mcr 15, 0, r4, cr2, cr0, {0}
> > c03030e4: eafff3c5b c030 <__turn_mmu_on>
> > c03030e8: e320f000nop {0}
> > c03030ec: e320f000nop {0}
> > c03030f0: e320f000nop {0}
> > c03030f4: e320f000nop {0}
> > c03030f8: e320f000nop {0}
> > c03030fc: e320f000nop {0}
> >
> > c030 <__turn_mmu_on>:
> > c030: e1a0nop ; (mov r0, r0)
> > c034: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> > c038: ee010f10mcr 15, 0, r0, cr1, cr0, {0}
> > c03c: ee103f10mrc 15, 0, r3, cr0, cr0, {0}
> > c0300010: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> > c0300014: e1a03003mov r3, r3
> > c0300018: e1a0300dmov r3, sp
> > c030001c: e1a0f003mov pc, r3
> >
>
> Thanks Nate.
>
> So these functions no longer appear to reside far away from each
> other, so there no veneer has been emitted.
>
> What we're looking for are veneers, i.e., snippets inserted by the
> linker that serve as a trampoline so a branch target that is far away
> can be reached.
>
> If no symbols exist with 'veneer' in their name *, it might make sense
> to rebuild the kernel as Thumb2, which has a branching range of only 8
> MB (as opposed to 16 MB for ARM mode)
Heh, Arnd and I were just talking about this yesterday. Is there a
config that sets Thumb2 mode for the kernel?
>
> * I have no idea whether lld names its veneers like this, or even at all
--
Thanks,
~Nick Desaulniers
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, Dec 5, 2018 at 10:40 AM Ard Biesheuvel
wrote:
>
> On Wed, 5 Dec 2018 at 19:36, Nathan Chancellor
> wrote:
> >
> > On Wed, Dec 05, 2018 at 09:09:56AM +0100, Ard Biesheuvel wrote:
> > > (+ Arnd)
> > >
> > > On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor
> > > wrote:
> > > >
> > > > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote:
> > > > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor
> > > > > wrote:
> > > > > >
> > > > > > This flag is not supported by lld:
> > > > > >
> > > > > > ld.lld: error: unknown argument: --pic-veneer
> > > > > >
> > > > > > Signed-off-by: Nathan Chancellor
> > > > >
> > > > > Hi Nate,
> > > > >
> > > > > Does this mean ld.lld is guaranteed to produce position independent
> > > > > veneers if you build kernels that are bigger than the typical range of
> > > > > a relative branch?
> > > > >
> > > >
> > > > Hi Ard,
> > > >
> > > > Honestly, I'm not quite sure. I saw your commit that introduced this
> > > > flag and I wasn't quite sure what to make of it for lld. What
> > > > configuration would I use to verify and what would I check for?
> > > >
> > >
> > > Try building allyesconfig, and check the resulting binary for veneers
> > > (which have 'veneer' in the symbol name, at least when ld.bfd emits
> > > them). These veneers should not take the [virtual] address of the
> > > branch target directly, but take a PC relative offset (as in the
> > > example in the commit log of that patch you are referring to)
> > >
> >
> > Alright, compiling with allyesconfig is a little rough at the moment
> > (bug reports I will file in due time) but I was able to do it. Here's
> > the disassembly specifically for the functions you had in your commit,
> > my assembly knowledge is pretty much non-existent unfortunately so I am
> > not sure what to make of it (it doesn't look like there is a virtual
> > address for pc in that mix?). I am happy to provide any more information
> > that is needed.
> >
> > c03030cc <__enable_mmu>:
> > c03030cc: e3c2bic r0, r0, #2
> > c03030d0: e3c00b02bic r0, r0, #2048 ; 0x800
> > c03030d4: e3c00a01bic r0, r0, #4096 ; 0x1000
> > c03030d8: e3a05051mov r5, #81 ; 0x51
> > c03030dc: ee035f10mcr 15, 0, r5, cr3, cr0, {0}
> > c03030e0: ee024f10mcr 15, 0, r4, cr2, cr0, {0}
> > c03030e4: eafff3c5b c030 <__turn_mmu_on>
> > c03030e8: e320f000nop {0}
> > c03030ec: e320f000nop {0}
> > c03030f0: e320f000nop {0}
> > c03030f4: e320f000nop {0}
> > c03030f8: e320f000nop {0}
> > c03030fc: e320f000nop {0}
> >
> > c030 <__turn_mmu_on>:
> > c030: e1a0nop ; (mov r0, r0)
> > c034: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> > c038: ee010f10mcr 15, 0, r0, cr1, cr0, {0}
> > c03c: ee103f10mrc 15, 0, r3, cr0, cr0, {0}
> > c0300010: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> > c0300014: e1a03003mov r3, r3
> > c0300018: e1a0300dmov r3, sp
> > c030001c: e1a0f003mov pc, r3
> >
>
> Thanks Nate.
>
> So these functions no longer appear to reside far away from each
> other, so there no veneer has been emitted.
>
> What we're looking for are veneers, i.e., snippets inserted by the
> linker that serve as a trampoline so a branch target that is far away
> can be reached.
>
> If no symbols exist with 'veneer' in their name *, it might make sense
> to rebuild the kernel as Thumb2, which has a branching range of only 8
> MB (as opposed to 16 MB for ARM mode)
Heh, Arnd and I were just talking about this yesterday. Is there a
config that sets Thumb2 mode for the kernel?
>
> * I have no idea whether lld names its veneers like this, or even at all
--
Thanks,
~Nick Desaulniers
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, Dec 5, 2018 at 10:36 AM Nathan Chancellor
wrote:
>
> On Wed, Dec 05, 2018 at 09:09:56AM +0100, Ard Biesheuvel wrote:
> > (+ Arnd)
> >
> > On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor
> > wrote:
> > >
> > > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote:
> > > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor
> > > > wrote:
> > > > >
> > > > > This flag is not supported by lld:
> > > > >
> > > > > ld.lld: error: unknown argument: --pic-veneer
> > > > >
> > > > > Signed-off-by: Nathan Chancellor
> > > >
> > > > Hi Nate,
> > > >
> > > > Does this mean ld.lld is guaranteed to produce position independent
> > > > veneers if you build kernels that are bigger than the typical range of
> > > > a relative branch?
> > > >
> > >
> > > Hi Ard,
> > >
> > > Honestly, I'm not quite sure. I saw your commit that introduced this
> > > flag and I wasn't quite sure what to make of it for lld. What
> > > configuration would I use to verify and what would I check for?
> > >
> >
> > Try building allyesconfig, and check the resulting binary for veneers
> > (which have 'veneer' in the symbol name, at least when ld.bfd emits
> > them). These veneers should not take the [virtual] address of the
> > branch target directly, but take a PC relative offset (as in the
> > example in the commit log of that patch you are referring to)
> >
>
> Alright, compiling with allyesconfig is a little rough at the moment
> (bug reports I will file in due time) but I was able to do it. Here's
> the disassembly specifically for the functions you had in your commit,
> my assembly knowledge is pretty much non-existent unfortunately so I am
> not sure what to make of it (it doesn't look like there is a virtual
> address for pc in that mix?). I am happy to provide any more information
> that is needed.
Nathan,
Thanks for getting a build working! I think Ard is looking for the
presence of symbols with `veneer` in the name.
Something like `llvm-objdump -t vmlinux | grep veneer` or `llvm-nm
vmlinux | grep veneer` should tell you if such symbols exist.
If they do, Ard was then looking for the disassembly of those labels
(like you provided for __enable_mmu/__turn_mmu_on).
>
> c03030cc <__enable_mmu>:
> c03030cc: e3c2bic r0, r0, #2
> c03030d0: e3c00b02bic r0, r0, #2048 ; 0x800
> c03030d4: e3c00a01bic r0, r0, #4096 ; 0x1000
> c03030d8: e3a05051mov r5, #81 ; 0x51
> c03030dc: ee035f10mcr 15, 0, r5, cr3, cr0, {0}
> c03030e0: ee024f10mcr 15, 0, r4, cr2, cr0, {0}
> c03030e4: eafff3c5b c030 <__turn_mmu_on>
> c03030e8: e320f000nop {0}
> c03030ec: e320f000nop {0}
> c03030f0: e320f000nop {0}
> c03030f4: e320f000nop {0}
> c03030f8: e320f000nop {0}
> c03030fc: e320f000nop {0}
>
> c030 <__turn_mmu_on>:
> c030: e1a0nop ; (mov r0, r0)
> c034: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> c038: ee010f10mcr 15, 0, r0, cr1, cr0, {0}
> c03c: ee103f10mrc 15, 0, r3, cr0, cr0, {0}
> c0300010: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> c0300014: e1a03003mov r3, r3
> c0300018: e1a0300dmov r3, sp
> c030001c: e1a0f003mov pc, r3
>
> Thanks,
> Nathan
>
> > > Additionally, I have filed an LLVM bug for the lld developers to
> > > check and see if this is a flag they should support:
> > >
> > > https://bugs.llvm.org/show_bug.cgi?id=39886
> > >
> > > Thanks for the quick reply,
> > > Nathan
> > >
> > > > > ---
> > > > > arch/arm/Makefile | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > > > > index e2a0baf36766..4fab2aa29570 100644
> > > > > --- a/arch/arm/Makefile
> > > > > +++ b/arch/arm/Makefile
> > > > > @@ -10,7 +10,7 @@
> > > > > #
> > > > > # Copyright (C) 1995-2001 by Russell King
> > > > >
> > > > > -LDFLAGS_vmlinux:= --no-undefined -X --pic-veneer
> > > > > +LDFLAGS_vmlinux:= --no-undefined -X $(call
> > > > > ld-option,--pic-veneer)
> > > > > ifeq ($(CONFIG_CPU_ENDIAN_BE8),y)
> > > > > LDFLAGS_vmlinux+= --be8
> > > > > KBUILD_LDFLAGS_MODULE += --be8
> > > > > --
> > > > > 2.20.0.rc1
> > > > >
--
Thanks,
~Nick Desaulniers
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, Dec 5, 2018 at 10:36 AM Nathan Chancellor
wrote:
>
> On Wed, Dec 05, 2018 at 09:09:56AM +0100, Ard Biesheuvel wrote:
> > (+ Arnd)
> >
> > On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor
> > wrote:
> > >
> > > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote:
> > > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor
> > > > wrote:
> > > > >
> > > > > This flag is not supported by lld:
> > > > >
> > > > > ld.lld: error: unknown argument: --pic-veneer
> > > > >
> > > > > Signed-off-by: Nathan Chancellor
> > > >
> > > > Hi Nate,
> > > >
> > > > Does this mean ld.lld is guaranteed to produce position independent
> > > > veneers if you build kernels that are bigger than the typical range of
> > > > a relative branch?
> > > >
> > >
> > > Hi Ard,
> > >
> > > Honestly, I'm not quite sure. I saw your commit that introduced this
> > > flag and I wasn't quite sure what to make of it for lld. What
> > > configuration would I use to verify and what would I check for?
> > >
> >
> > Try building allyesconfig, and check the resulting binary for veneers
> > (which have 'veneer' in the symbol name, at least when ld.bfd emits
> > them). These veneers should not take the [virtual] address of the
> > branch target directly, but take a PC relative offset (as in the
> > example in the commit log of that patch you are referring to)
> >
>
> Alright, compiling with allyesconfig is a little rough at the moment
> (bug reports I will file in due time) but I was able to do it. Here's
> the disassembly specifically for the functions you had in your commit,
> my assembly knowledge is pretty much non-existent unfortunately so I am
> not sure what to make of it (it doesn't look like there is a virtual
> address for pc in that mix?). I am happy to provide any more information
> that is needed.
Nathan,
Thanks for getting a build working! I think Ard is looking for the
presence of symbols with `veneer` in the name.
Something like `llvm-objdump -t vmlinux | grep veneer` or `llvm-nm
vmlinux | grep veneer` should tell you if such symbols exist.
If they do, Ard was then looking for the disassembly of those labels
(like you provided for __enable_mmu/__turn_mmu_on).
>
> c03030cc <__enable_mmu>:
> c03030cc: e3c2bic r0, r0, #2
> c03030d0: e3c00b02bic r0, r0, #2048 ; 0x800
> c03030d4: e3c00a01bic r0, r0, #4096 ; 0x1000
> c03030d8: e3a05051mov r5, #81 ; 0x51
> c03030dc: ee035f10mcr 15, 0, r5, cr3, cr0, {0}
> c03030e0: ee024f10mcr 15, 0, r4, cr2, cr0, {0}
> c03030e4: eafff3c5b c030 <__turn_mmu_on>
> c03030e8: e320f000nop {0}
> c03030ec: e320f000nop {0}
> c03030f0: e320f000nop {0}
> c03030f4: e320f000nop {0}
> c03030f8: e320f000nop {0}
> c03030fc: e320f000nop {0}
>
> c030 <__turn_mmu_on>:
> c030: e1a0nop ; (mov r0, r0)
> c034: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> c038: ee010f10mcr 15, 0, r0, cr1, cr0, {0}
> c03c: ee103f10mrc 15, 0, r3, cr0, cr0, {0}
> c0300010: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> c0300014: e1a03003mov r3, r3
> c0300018: e1a0300dmov r3, sp
> c030001c: e1a0f003mov pc, r3
>
> Thanks,
> Nathan
>
> > > Additionally, I have filed an LLVM bug for the lld developers to
> > > check and see if this is a flag they should support:
> > >
> > > https://bugs.llvm.org/show_bug.cgi?id=39886
> > >
> > > Thanks for the quick reply,
> > > Nathan
> > >
> > > > > ---
> > > > > arch/arm/Makefile | 2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > > > > index e2a0baf36766..4fab2aa29570 100644
> > > > > --- a/arch/arm/Makefile
> > > > > +++ b/arch/arm/Makefile
> > > > > @@ -10,7 +10,7 @@
> > > > > #
> > > > > # Copyright (C) 1995-2001 by Russell King
> > > > >
> > > > > -LDFLAGS_vmlinux:= --no-undefined -X --pic-veneer
> > > > > +LDFLAGS_vmlinux:= --no-undefined -X $(call
> > > > > ld-option,--pic-veneer)
> > > > > ifeq ($(CONFIG_CPU_ENDIAN_BE8),y)
> > > > > LDFLAGS_vmlinux+= --be8
> > > > > KBUILD_LDFLAGS_MODULE += --be8
> > > > > --
> > > > > 2.20.0.rc1
> > > > >
--
Thanks,
~Nick Desaulniers
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, 5 Dec 2018 at 19:36, Nathan Chancellor wrote:
>
> On Wed, Dec 05, 2018 at 09:09:56AM +0100, Ard Biesheuvel wrote:
> > (+ Arnd)
> >
> > On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor
> > wrote:
> > >
> > > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote:
> > > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor
> > > > wrote:
> > > > >
> > > > > This flag is not supported by lld:
> > > > >
> > > > > ld.lld: error: unknown argument: --pic-veneer
> > > > >
> > > > > Signed-off-by: Nathan Chancellor
> > > >
> > > > Hi Nate,
> > > >
> > > > Does this mean ld.lld is guaranteed to produce position independent
> > > > veneers if you build kernels that are bigger than the typical range of
> > > > a relative branch?
> > > >
> > >
> > > Hi Ard,
> > >
> > > Honestly, I'm not quite sure. I saw your commit that introduced this
> > > flag and I wasn't quite sure what to make of it for lld. What
> > > configuration would I use to verify and what would I check for?
> > >
> >
> > Try building allyesconfig, and check the resulting binary for veneers
> > (which have 'veneer' in the symbol name, at least when ld.bfd emits
> > them). These veneers should not take the [virtual] address of the
> > branch target directly, but take a PC relative offset (as in the
> > example in the commit log of that patch you are referring to)
> >
>
> Alright, compiling with allyesconfig is a little rough at the moment
> (bug reports I will file in due time) but I was able to do it. Here's
> the disassembly specifically for the functions you had in your commit,
> my assembly knowledge is pretty much non-existent unfortunately so I am
> not sure what to make of it (it doesn't look like there is a virtual
> address for pc in that mix?). I am happy to provide any more information
> that is needed.
>
> c03030cc <__enable_mmu>:
> c03030cc: e3c2bic r0, r0, #2
> c03030d0: e3c00b02bic r0, r0, #2048 ; 0x800
> c03030d4: e3c00a01bic r0, r0, #4096 ; 0x1000
> c03030d8: e3a05051mov r5, #81 ; 0x51
> c03030dc: ee035f10mcr 15, 0, r5, cr3, cr0, {0}
> c03030e0: ee024f10mcr 15, 0, r4, cr2, cr0, {0}
> c03030e4: eafff3c5b c030 <__turn_mmu_on>
> c03030e8: e320f000nop {0}
> c03030ec: e320f000nop {0}
> c03030f0: e320f000nop {0}
> c03030f4: e320f000nop {0}
> c03030f8: e320f000nop {0}
> c03030fc: e320f000nop {0}
>
> c030 <__turn_mmu_on>:
> c030: e1a0nop ; (mov r0, r0)
> c034: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> c038: ee010f10mcr 15, 0, r0, cr1, cr0, {0}
> c03c: ee103f10mrc 15, 0, r3, cr0, cr0, {0}
> c0300010: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> c0300014: e1a03003mov r3, r3
> c0300018: e1a0300dmov r3, sp
> c030001c: e1a0f003mov pc, r3
>
Thanks Nate.
So these functions no longer appear to reside far away from each
other, so there no veneer has been emitted.
What we're looking for are veneers, i.e., snippets inserted by the
linker that serve as a trampoline so a branch target that is far away
can be reached.
If no symbols exist with 'veneer' in their name *, it might make sense
to rebuild the kernel as Thumb2, which has a branching range of only 8
MB (as opposed to 16 MB for ARM mode)
* I have no idea whether lld names its veneers like this, or even at all
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, 5 Dec 2018 at 19:36, Nathan Chancellor wrote:
>
> On Wed, Dec 05, 2018 at 09:09:56AM +0100, Ard Biesheuvel wrote:
> > (+ Arnd)
> >
> > On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor
> > wrote:
> > >
> > > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote:
> > > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor
> > > > wrote:
> > > > >
> > > > > This flag is not supported by lld:
> > > > >
> > > > > ld.lld: error: unknown argument: --pic-veneer
> > > > >
> > > > > Signed-off-by: Nathan Chancellor
> > > >
> > > > Hi Nate,
> > > >
> > > > Does this mean ld.lld is guaranteed to produce position independent
> > > > veneers if you build kernels that are bigger than the typical range of
> > > > a relative branch?
> > > >
> > >
> > > Hi Ard,
> > >
> > > Honestly, I'm not quite sure. I saw your commit that introduced this
> > > flag and I wasn't quite sure what to make of it for lld. What
> > > configuration would I use to verify and what would I check for?
> > >
> >
> > Try building allyesconfig, and check the resulting binary for veneers
> > (which have 'veneer' in the symbol name, at least when ld.bfd emits
> > them). These veneers should not take the [virtual] address of the
> > branch target directly, but take a PC relative offset (as in the
> > example in the commit log of that patch you are referring to)
> >
>
> Alright, compiling with allyesconfig is a little rough at the moment
> (bug reports I will file in due time) but I was able to do it. Here's
> the disassembly specifically for the functions you had in your commit,
> my assembly knowledge is pretty much non-existent unfortunately so I am
> not sure what to make of it (it doesn't look like there is a virtual
> address for pc in that mix?). I am happy to provide any more information
> that is needed.
>
> c03030cc <__enable_mmu>:
> c03030cc: e3c2bic r0, r0, #2
> c03030d0: e3c00b02bic r0, r0, #2048 ; 0x800
> c03030d4: e3c00a01bic r0, r0, #4096 ; 0x1000
> c03030d8: e3a05051mov r5, #81 ; 0x51
> c03030dc: ee035f10mcr 15, 0, r5, cr3, cr0, {0}
> c03030e0: ee024f10mcr 15, 0, r4, cr2, cr0, {0}
> c03030e4: eafff3c5b c030 <__turn_mmu_on>
> c03030e8: e320f000nop {0}
> c03030ec: e320f000nop {0}
> c03030f0: e320f000nop {0}
> c03030f4: e320f000nop {0}
> c03030f8: e320f000nop {0}
> c03030fc: e320f000nop {0}
>
> c030 <__turn_mmu_on>:
> c030: e1a0nop ; (mov r0, r0)
> c034: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> c038: ee010f10mcr 15, 0, r0, cr1, cr0, {0}
> c03c: ee103f10mrc 15, 0, r3, cr0, cr0, {0}
> c0300010: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
> c0300014: e1a03003mov r3, r3
> c0300018: e1a0300dmov r3, sp
> c030001c: e1a0f003mov pc, r3
>
Thanks Nate.
So these functions no longer appear to reside far away from each
other, so there no veneer has been emitted.
What we're looking for are veneers, i.e., snippets inserted by the
linker that serve as a trampoline so a branch target that is far away
can be reached.
If no symbols exist with 'veneer' in their name *, it might make sense
to rebuild the kernel as Thumb2, which has a branching range of only 8
MB (as opposed to 16 MB for ARM mode)
* I have no idea whether lld names its veneers like this, or even at all
Re: [PATCH 10/12] staging: rtl8188eu: correct indentation in update_wireless_mode()
On 12/5/18 6:37 PM, Joe Perches wrote: On Wed, 2018-12-05 at 18:02 +0100, Michael Straube wrote: Correct indentation in update_wireless_mode() to clear a checkpatch warning. WARNING: Statements should start on a tabstop [] diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c [] @@ -1442,7 +1442,7 @@ void update_wireless_mode(struct adapter *padapter) if (pmlmeext->cur_wireless_mode & WIRELESS_11B) update_mgnt_tx_rate(padapter, IEEE80211_CCK_RATE_1MB); -else + else update_mgnt_tx_rate(padapter, IEEE80211_OFDM_RATE_6MB); } . gcc generally emits smaller code by using a single call and a ternary. Good to know, thank you. $ size drivers/staging/rtl8188eu/core/rtw_wlan_util.o* text data bss dec hex filename 14974 36 0 150103aa2 drivers/staging/rtl8188eu/core/rtw_wlan_util.o.new 14990 36 0 150263ab2 drivers/staging/rtl8188eu/core/rtw_wlan_util.o.old --- drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c index 24918223499b..acad30f53f5d 100644 --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c @@ -1440,10 +1440,9 @@ void update_wireless_mode(struct adapter *padapter) rtw_hal_set_hwreg(padapter, HW_VAR_RESP_SIFS, (u8 *)_Timer); - if (pmlmeext->cur_wireless_mode & WIRELESS_11B) - update_mgnt_tx_rate(padapter, IEEE80211_CCK_RATE_1MB); -else - update_mgnt_tx_rate(padapter, IEEE80211_OFDM_RATE_6MB); + update_mgnt_tx_rate(padapter, + pmlmeext->cur_wireless_mode & WIRELESS_11B ? + IEEE80211_CCK_RATE_1MB : IEEE80211_OFDM_RATE_6MB); } void update_bmc_sta_support_rate(struct adapter *padapter, u32 mac_id)
Re: [PATCH 10/12] staging: rtl8188eu: correct indentation in update_wireless_mode()
On 12/5/18 6:37 PM, Joe Perches wrote: On Wed, 2018-12-05 at 18:02 +0100, Michael Straube wrote: Correct indentation in update_wireless_mode() to clear a checkpatch warning. WARNING: Statements should start on a tabstop [] diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c [] @@ -1442,7 +1442,7 @@ void update_wireless_mode(struct adapter *padapter) if (pmlmeext->cur_wireless_mode & WIRELESS_11B) update_mgnt_tx_rate(padapter, IEEE80211_CCK_RATE_1MB); -else + else update_mgnt_tx_rate(padapter, IEEE80211_OFDM_RATE_6MB); } . gcc generally emits smaller code by using a single call and a ternary. Good to know, thank you. $ size drivers/staging/rtl8188eu/core/rtw_wlan_util.o* text data bss dec hex filename 14974 36 0 150103aa2 drivers/staging/rtl8188eu/core/rtw_wlan_util.o.new 14990 36 0 150263ab2 drivers/staging/rtl8188eu/core/rtw_wlan_util.o.old --- drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c index 24918223499b..acad30f53f5d 100644 --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c @@ -1440,10 +1440,9 @@ void update_wireless_mode(struct adapter *padapter) rtw_hal_set_hwreg(padapter, HW_VAR_RESP_SIFS, (u8 *)_Timer); - if (pmlmeext->cur_wireless_mode & WIRELESS_11B) - update_mgnt_tx_rate(padapter, IEEE80211_CCK_RATE_1MB); -else - update_mgnt_tx_rate(padapter, IEEE80211_OFDM_RATE_6MB); + update_mgnt_tx_rate(padapter, + pmlmeext->cur_wireless_mode & WIRELESS_11B ? + IEEE80211_CCK_RATE_1MB : IEEE80211_OFDM_RATE_6MB); } void update_bmc_sta_support_rate(struct adapter *padapter, u32 mac_id)
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, Dec 05, 2018 at 09:09:56AM +0100, Ard Biesheuvel wrote:
> (+ Arnd)
>
> On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor
> wrote:
> >
> > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote:
> > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor
> > > wrote:
> > > >
> > > > This flag is not supported by lld:
> > > >
> > > > ld.lld: error: unknown argument: --pic-veneer
> > > >
> > > > Signed-off-by: Nathan Chancellor
> > >
> > > Hi Nate,
> > >
> > > Does this mean ld.lld is guaranteed to produce position independent
> > > veneers if you build kernels that are bigger than the typical range of
> > > a relative branch?
> > >
> >
> > Hi Ard,
> >
> > Honestly, I'm not quite sure. I saw your commit that introduced this
> > flag and I wasn't quite sure what to make of it for lld. What
> > configuration would I use to verify and what would I check for?
> >
>
> Try building allyesconfig, and check the resulting binary for veneers
> (which have 'veneer' in the symbol name, at least when ld.bfd emits
> them). These veneers should not take the [virtual] address of the
> branch target directly, but take a PC relative offset (as in the
> example in the commit log of that patch you are referring to)
>
Alright, compiling with allyesconfig is a little rough at the moment
(bug reports I will file in due time) but I was able to do it. Here's
the disassembly specifically for the functions you had in your commit,
my assembly knowledge is pretty much non-existent unfortunately so I am
not sure what to make of it (it doesn't look like there is a virtual
address for pc in that mix?). I am happy to provide any more information
that is needed.
c03030cc <__enable_mmu>:
c03030cc: e3c2bic r0, r0, #2
c03030d0: e3c00b02bic r0, r0, #2048 ; 0x800
c03030d4: e3c00a01bic r0, r0, #4096 ; 0x1000
c03030d8: e3a05051mov r5, #81 ; 0x51
c03030dc: ee035f10mcr 15, 0, r5, cr3, cr0, {0}
c03030e0: ee024f10mcr 15, 0, r4, cr2, cr0, {0}
c03030e4: eafff3c5b c030 <__turn_mmu_on>
c03030e8: e320f000nop {0}
c03030ec: e320f000nop {0}
c03030f0: e320f000nop {0}
c03030f4: e320f000nop {0}
c03030f8: e320f000nop {0}
c03030fc: e320f000nop {0}
c030 <__turn_mmu_on>:
c030: e1a0nop ; (mov r0, r0)
c034: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
c038: ee010f10mcr 15, 0, r0, cr1, cr0, {0}
c03c: ee103f10mrc 15, 0, r3, cr0, cr0, {0}
c0300010: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
c0300014: e1a03003mov r3, r3
c0300018: e1a0300dmov r3, sp
c030001c: e1a0f003mov pc, r3
Thanks,
Nathan
> > Additionally, I have filed an LLVM bug for the lld developers to
> > check and see if this is a flag they should support:
> >
> > https://bugs.llvm.org/show_bug.cgi?id=39886
> >
> > Thanks for the quick reply,
> > Nathan
> >
> > > > ---
> > > > arch/arm/Makefile | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > > > index e2a0baf36766..4fab2aa29570 100644
> > > > --- a/arch/arm/Makefile
> > > > +++ b/arch/arm/Makefile
> > > > @@ -10,7 +10,7 @@
> > > > #
> > > > # Copyright (C) 1995-2001 by Russell King
> > > >
> > > > -LDFLAGS_vmlinux:= --no-undefined -X --pic-veneer
> > > > +LDFLAGS_vmlinux:= --no-undefined -X $(call
> > > > ld-option,--pic-veneer)
> > > > ifeq ($(CONFIG_CPU_ENDIAN_BE8),y)
> > > > LDFLAGS_vmlinux+= --be8
> > > > KBUILD_LDFLAGS_MODULE += --be8
> > > > --
> > > > 2.20.0.rc1
> > > >
Re: [PATCH 2/2] ARM: Wrap '--pic-veneer' with ld-option
On Wed, Dec 05, 2018 at 09:09:56AM +0100, Ard Biesheuvel wrote:
> (+ Arnd)
>
> On Wed, 5 Dec 2018 at 09:06, Nathan Chancellor
> wrote:
> >
> > On Wed, Dec 05, 2018 at 08:37:05AM +0100, Ard Biesheuvel wrote:
> > > On Wed, 5 Dec 2018 at 02:42, Nathan Chancellor
> > > wrote:
> > > >
> > > > This flag is not supported by lld:
> > > >
> > > > ld.lld: error: unknown argument: --pic-veneer
> > > >
> > > > Signed-off-by: Nathan Chancellor
> > >
> > > Hi Nate,
> > >
> > > Does this mean ld.lld is guaranteed to produce position independent
> > > veneers if you build kernels that are bigger than the typical range of
> > > a relative branch?
> > >
> >
> > Hi Ard,
> >
> > Honestly, I'm not quite sure. I saw your commit that introduced this
> > flag and I wasn't quite sure what to make of it for lld. What
> > configuration would I use to verify and what would I check for?
> >
>
> Try building allyesconfig, and check the resulting binary for veneers
> (which have 'veneer' in the symbol name, at least when ld.bfd emits
> them). These veneers should not take the [virtual] address of the
> branch target directly, but take a PC relative offset (as in the
> example in the commit log of that patch you are referring to)
>
Alright, compiling with allyesconfig is a little rough at the moment
(bug reports I will file in due time) but I was able to do it. Here's
the disassembly specifically for the functions you had in your commit,
my assembly knowledge is pretty much non-existent unfortunately so I am
not sure what to make of it (it doesn't look like there is a virtual
address for pc in that mix?). I am happy to provide any more information
that is needed.
c03030cc <__enable_mmu>:
c03030cc: e3c2bic r0, r0, #2
c03030d0: e3c00b02bic r0, r0, #2048 ; 0x800
c03030d4: e3c00a01bic r0, r0, #4096 ; 0x1000
c03030d8: e3a05051mov r5, #81 ; 0x51
c03030dc: ee035f10mcr 15, 0, r5, cr3, cr0, {0}
c03030e0: ee024f10mcr 15, 0, r4, cr2, cr0, {0}
c03030e4: eafff3c5b c030 <__turn_mmu_on>
c03030e8: e320f000nop {0}
c03030ec: e320f000nop {0}
c03030f0: e320f000nop {0}
c03030f4: e320f000nop {0}
c03030f8: e320f000nop {0}
c03030fc: e320f000nop {0}
c030 <__turn_mmu_on>:
c030: e1a0nop ; (mov r0, r0)
c034: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
c038: ee010f10mcr 15, 0, r0, cr1, cr0, {0}
c03c: ee103f10mrc 15, 0, r3, cr0, cr0, {0}
c0300010: ee070f95mcr 15, 0, r0, cr7, cr5, {4}
c0300014: e1a03003mov r3, r3
c0300018: e1a0300dmov r3, sp
c030001c: e1a0f003mov pc, r3
Thanks,
Nathan
> > Additionally, I have filed an LLVM bug for the lld developers to
> > check and see if this is a flag they should support:
> >
> > https://bugs.llvm.org/show_bug.cgi?id=39886
> >
> > Thanks for the quick reply,
> > Nathan
> >
> > > > ---
> > > > arch/arm/Makefile | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> > > > index e2a0baf36766..4fab2aa29570 100644
> > > > --- a/arch/arm/Makefile
> > > > +++ b/arch/arm/Makefile
> > > > @@ -10,7 +10,7 @@
> > > > #
> > > > # Copyright (C) 1995-2001 by Russell King
> > > >
> > > > -LDFLAGS_vmlinux:= --no-undefined -X --pic-veneer
> > > > +LDFLAGS_vmlinux:= --no-undefined -X $(call
> > > > ld-option,--pic-veneer)
> > > > ifeq ($(CONFIG_CPU_ENDIAN_BE8),y)
> > > > LDFLAGS_vmlinux+= --be8
> > > > KBUILD_LDFLAGS_MODULE += --be8
> > > > --
> > > > 2.20.0.rc1
> > > >
[PATCH 0/2] tracing: arm64: Make ftrace_replace_code() schedulable for arm64
This is a little more involved, and I would like to push this through my tree. Can I get a reviewed-by/ack for the second (arm64) patch? Anders, can you also test this to make sure that it fixes the issue you see? Thanks! -- Steve Steven Rostedt (VMware) (2): ftrace: Allow ftrace_replace_code() to be schedulable arm64: ftrace: Set FTRACE_SCHEDULABLE before ftrace_modify_all_code() arch/arm64/kernel/ftrace.c | 1 + include/linux/ftrace.h | 1 + kernel/trace/ftrace.c | 19 --- 3 files changed, 18 insertions(+), 3 deletions(-)
[PATCH 0/2] tracing: arm64: Make ftrace_replace_code() schedulable for arm64
This is a little more involved, and I would like to push this through my tree. Can I get a reviewed-by/ack for the second (arm64) patch? Anders, can you also test this to make sure that it fixes the issue you see? Thanks! -- Steve Steven Rostedt (VMware) (2): ftrace: Allow ftrace_replace_code() to be schedulable arm64: ftrace: Set FTRACE_SCHEDULABLE before ftrace_modify_all_code() arch/arm64/kernel/ftrace.c | 1 + include/linux/ftrace.h | 1 + kernel/trace/ftrace.c | 19 --- 3 files changed, 18 insertions(+), 3 deletions(-)
Re: [PATCH v2] arm64: dts: qcom: sdm845: Add UART nodes
On Tue, Dec 04, 2018 at 12:37:18PM -0800, Matthias Kaehlcke wrote: > Hi Andy, > > can this be landed or are any more changes needed? > > Thanks I'll see about adding it to the second pull request. Thank, Andy
[PATCH v2 01/12] staging: rtl8188eu: refactor cckrates_included()
Refactor cckrates_included() to improve readability and slightly
reduce object file size. Also change the return type to bool.
Signed-off-by: Michael Straube
---
v1 -> v2
Changed patch 10/12 in the series.
v1: staging: rtl8188eu: correct indentation in update_wireless_mode()
v2: staging: rtl8188eu: replace if else with ternary
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 9 +
drivers/staging/rtl8188eu/include/rtw_mlme_ext.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 24918223499b..90160d5fc292 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -56,13 +56,14 @@ static u8 rtw_basic_rate_mix[7] = {
IEEE80211_OFDM_RATE_24MB | IEEE80211_BASIC_RATE_MASK
};
-int cckrates_included(unsigned char *rate, int ratelen)
+bool cckrates_included(unsigned char *rate, int ratelen)
{
- int i;
+ int i;
for (i = 0; i < ratelen; i++) {
- if rate[i]) & 0x7f) == 2) || (((rate[i]) & 0x7f) == 4) ||
-(((rate[i]) & 0x7f) == 11) || (((rate[i]) & 0x7f) == 22))
+ u8 r = rate[i] & 0x7f;
+
+ if (r == 2 || r == 4 || r == 11 || r == 22)
return true;
}
return false;
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
index 9526da3efcc4..c86dec12dec2 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
@@ -580,7 +580,7 @@ void addba_timer_hdl(struct timer_list *t);
mod_timer(>link_timer, jiffies + \
msecs_to_jiffies(ms))
-int cckrates_included(unsigned char *rate, int ratelen);
+bool cckrates_included(unsigned char *rate, int ratelen);
int cckratesonly_included(unsigned char *rate, int ratelen);
void process_addba_req(struct adapter *padapter, u8 *paddba_req, u8 *addr);
--
2.19.2
[PATCH 2/2] arm64: ftrace: Set FTRACE_SCHEDULABLE before ftrace_modify_all_code()
From: "Steven Rostedt (VMware)"
It has been reported that ftrace_replace_code() which is called by
ftrace_modify_all_code() can cause a soft lockup warning for an
allmodconfig kernel. This is because all the debug options enabled
causes the loop in ftrace_replace_code() (which loops over all the
functions being enabled where there can be 10s of thousands), is too
slow, and never schedules out.
To solve this, setting FTRACE_SCHEDULABLE to the command passed into
ftrace_replace_code() will make it call cond_resched() in the loop,
which prevents the soft lockup warning from triggering.
Link: http://lkml.kernel.org/r/20181204192903.8193-1-anders.rox...@linaro.org
Reported-by: Anders Roxell
Signed-off-by: Steven Rostedt (VMware)
---
arch/arm64/kernel/ftrace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 57e962290df3..9a8de0a79f97 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -193,6 +193,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace
*rec,
void arch_ftrace_update_code(int command)
{
+ command |= FTRACE_SCHEDULABLE;
ftrace_modify_all_code(command);
}
--
2.19.1
Re: perf: perf_fuzzer triggers GPF in perf_prepare_sample
On Wed, Dec 05, 2018 at 12:11:19PM -0500, Vince Weaver wrote:
> On Wed, 5 Dec 2018, Jiri Olsa wrote:
>
> > On Wed, Dec 05, 2018 at 01:45:38PM +0100, Jiri Olsa wrote:
> > > On Tue, Dec 04, 2018 at 10:54:55AM -0500, Vince Weaver wrote:
> > > > Hello,
> > > >
> > > > I was able to trigger another oops with the perf_fuzzer with current
> > > > git.
> > > >
> > > > This is 4.20-rc5 after the fix for the very similar oops I previously
> > > > reported got committed.
> > > >
> > > > It seems to be pointing to the same location in the source as
> > > > before, I guess maybe triggered a different way?
> > >
> > > nice.. yep, looks the same
> > >
> > > >
> > > > Unfortunately this crash is not easily reproducible like the last one
> > > > was.
> > >
> > > will check
> >
> > what model are hitting this on?
>
> Haswell. 6/60/3.
>
> While I can't deterministically trigger this, the fuzzer usually hits it
> within an hour or two. Is there any debug or printk messages I can
> add that would help figure out what's going on?
I can't see how we could end up with that config other than
some corruption.. the only way I see could be that we touch
cpu->events array without checking its active_mask bit
but that does not explain why the crash happened in the same
place as before
jirka
---
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index ecc3e34ca955..9a2fd5a68d87 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2404,7 +2404,7 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
struct cpu_hw_events *cpuc;
int loops;
u64 status;
- int handled;
+ int handled = 0;
int pmu_enabled;
cpuc = this_cpu_ptr(_hw_events);
@@ -2423,8 +2423,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
intel_bts_disable_local();
cpuc->enabled = 0;
__intel_pmu_disable_all();
- handled = intel_pmu_drain_bts_buffer();
- handled += intel_bts_interrupt();
+ if (test_bit(INTEL_PMC_IDX_FIXED_BTS, cpuc->active_mask)) {
+ handled += intel_pmu_drain_bts_buffer();
+ handled += intel_bts_interrupt();
+ }
status = intel_pmu_get_status();
if (!status)
goto done;
Re: [PATCH v2] arm64: dts: qcom: sdm845: Add UART nodes
On Tue, Dec 04, 2018 at 12:37:18PM -0800, Matthias Kaehlcke wrote: > Hi Andy, > > can this be landed or are any more changes needed? > > Thanks I'll see about adding it to the second pull request. Thank, Andy
[PATCH v2 01/12] staging: rtl8188eu: refactor cckrates_included()
Refactor cckrates_included() to improve readability and slightly
reduce object file size. Also change the return type to bool.
Signed-off-by: Michael Straube
---
v1 -> v2
Changed patch 10/12 in the series.
v1: staging: rtl8188eu: correct indentation in update_wireless_mode()
v2: staging: rtl8188eu: replace if else with ternary
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 9 +
drivers/staging/rtl8188eu/include/rtw_mlme_ext.h | 2 +-
2 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 24918223499b..90160d5fc292 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -56,13 +56,14 @@ static u8 rtw_basic_rate_mix[7] = {
IEEE80211_OFDM_RATE_24MB | IEEE80211_BASIC_RATE_MASK
};
-int cckrates_included(unsigned char *rate, int ratelen)
+bool cckrates_included(unsigned char *rate, int ratelen)
{
- int i;
+ int i;
for (i = 0; i < ratelen; i++) {
- if rate[i]) & 0x7f) == 2) || (((rate[i]) & 0x7f) == 4) ||
-(((rate[i]) & 0x7f) == 11) || (((rate[i]) & 0x7f) == 22))
+ u8 r = rate[i] & 0x7f;
+
+ if (r == 2 || r == 4 || r == 11 || r == 22)
return true;
}
return false;
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
index 9526da3efcc4..c86dec12dec2 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
@@ -580,7 +580,7 @@ void addba_timer_hdl(struct timer_list *t);
mod_timer(>link_timer, jiffies + \
msecs_to_jiffies(ms))
-int cckrates_included(unsigned char *rate, int ratelen);
+bool cckrates_included(unsigned char *rate, int ratelen);
int cckratesonly_included(unsigned char *rate, int ratelen);
void process_addba_req(struct adapter *padapter, u8 *paddba_req, u8 *addr);
--
2.19.2
[PATCH 2/2] arm64: ftrace: Set FTRACE_SCHEDULABLE before ftrace_modify_all_code()
From: "Steven Rostedt (VMware)"
It has been reported that ftrace_replace_code() which is called by
ftrace_modify_all_code() can cause a soft lockup warning for an
allmodconfig kernel. This is because all the debug options enabled
causes the loop in ftrace_replace_code() (which loops over all the
functions being enabled where there can be 10s of thousands), is too
slow, and never schedules out.
To solve this, setting FTRACE_SCHEDULABLE to the command passed into
ftrace_replace_code() will make it call cond_resched() in the loop,
which prevents the soft lockup warning from triggering.
Link: http://lkml.kernel.org/r/20181204192903.8193-1-anders.rox...@linaro.org
Reported-by: Anders Roxell
Signed-off-by: Steven Rostedt (VMware)
---
arch/arm64/kernel/ftrace.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm64/kernel/ftrace.c b/arch/arm64/kernel/ftrace.c
index 57e962290df3..9a8de0a79f97 100644
--- a/arch/arm64/kernel/ftrace.c
+++ b/arch/arm64/kernel/ftrace.c
@@ -193,6 +193,7 @@ int ftrace_make_nop(struct module *mod, struct dyn_ftrace
*rec,
void arch_ftrace_update_code(int command)
{
+ command |= FTRACE_SCHEDULABLE;
ftrace_modify_all_code(command);
}
--
2.19.1
Re: perf: perf_fuzzer triggers GPF in perf_prepare_sample
On Wed, Dec 05, 2018 at 12:11:19PM -0500, Vince Weaver wrote:
> On Wed, 5 Dec 2018, Jiri Olsa wrote:
>
> > On Wed, Dec 05, 2018 at 01:45:38PM +0100, Jiri Olsa wrote:
> > > On Tue, Dec 04, 2018 at 10:54:55AM -0500, Vince Weaver wrote:
> > > > Hello,
> > > >
> > > > I was able to trigger another oops with the perf_fuzzer with current
> > > > git.
> > > >
> > > > This is 4.20-rc5 after the fix for the very similar oops I previously
> > > > reported got committed.
> > > >
> > > > It seems to be pointing to the same location in the source as
> > > > before, I guess maybe triggered a different way?
> > >
> > > nice.. yep, looks the same
> > >
> > > >
> > > > Unfortunately this crash is not easily reproducible like the last one
> > > > was.
> > >
> > > will check
> >
> > what model are hitting this on?
>
> Haswell. 6/60/3.
>
> While I can't deterministically trigger this, the fuzzer usually hits it
> within an hour or two. Is there any debug or printk messages I can
> add that would help figure out what's going on?
I can't see how we could end up with that config other than
some corruption.. the only way I see could be that we touch
cpu->events array without checking its active_mask bit
but that does not explain why the crash happened in the same
place as before
jirka
---
diff --git a/arch/x86/events/intel/core.c b/arch/x86/events/intel/core.c
index ecc3e34ca955..9a2fd5a68d87 100644
--- a/arch/x86/events/intel/core.c
+++ b/arch/x86/events/intel/core.c
@@ -2404,7 +2404,7 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
struct cpu_hw_events *cpuc;
int loops;
u64 status;
- int handled;
+ int handled = 0;
int pmu_enabled;
cpuc = this_cpu_ptr(_hw_events);
@@ -2423,8 +2423,10 @@ static int intel_pmu_handle_irq(struct pt_regs *regs)
intel_bts_disable_local();
cpuc->enabled = 0;
__intel_pmu_disable_all();
- handled = intel_pmu_drain_bts_buffer();
- handled += intel_bts_interrupt();
+ if (test_bit(INTEL_PMC_IDX_FIXED_BTS, cpuc->active_mask)) {
+ handled += intel_pmu_drain_bts_buffer();
+ handled += intel_bts_interrupt();
+ }
status = intel_pmu_get_status();
if (!status)
goto done;
Re: [RFC PATCH 02/14] mm/hms: heterogenenous memory system (HMS) documentation
On Wed, Dec 05, 2018 at 11:20:30AM -0700, Logan Gunthorpe wrote: > > > On 2018-12-05 11:07 a.m., Jerome Glisse wrote: > >> Well multiple links are easy when you have a 'link' bus. Just add > >> another link device under the bus. > > > > So you are telling do what i am doing in this patchset but not under > > HMS directory ? > > No, it's completely different. I'm talking about creating a bus to > describe only the real hardware that links GPUs. Not creating a new > virtual tree containing a bunch of duplicate bus and device information > that already exists currently in sysfs. > > >> > >> Technically, the accessibility issue is already encoded in sysfs. For > >> example, through the PCI tree you can determine which ACS bits are set > >> and determine which devices are behind the same root bridge the same way > >> we do in the kernel p2pdma subsystem. This is all bus specific which is > >> fine, but if we want to change that, we should have a common way for > >> existing buses to describe these attributes in the existing tree. The > >> new 'link' bus devices would have to have some way to describe cases if > >> memory isn't accessible in some way across it. > > > > What i am looking at is much more complex than just access bit. It > > is a whole set of properties attach to each path (can it be cache > > coherent ? can it do atomic ? what is the access granularity ? what > > is the bandwidth ? is it dedicated link ? ...) > > I'm not talking about just an access bit. I'm talking about what you are > describing: standard ways for *existing* buses in the sysfs hierarchy to > describe things like cache coherency, atomics, granularity, etc without > creating a new hierarchy. > > > On top of that i argue that more people would use that information if it > > were available to them. I agree that i have no hard evidence to back that > > up and that it is just a feeling but you can not disprove me either as > > this is a chicken and egg problem, you can not prove people will not use > > an API if the API is not there to be use. > > And you miss my point that much of this information is already available > to them. And more can be added in the existing framework without > creating any brand new concepts. I haven't said anything about > chicken-and-egg problems -- I've given you a bunch of different > suggestions to split this up into more managable problems and address > many of them within the APIs and frameworks we have already. The thing is that what i am considering is not in sysfs, it does not even have linux kernel driver, it is just chips that connect device between them and there is nothing to do with those chips it is all hardware they do not need a driver. So there is nothing existing that address what i need to represent. If i add a a fake driver for those what would i do ? under which sub-system i register them ? How i express the fact that they connect device X,Y and Z with some properties ? This is not PCIE ... you can not discover bridges and child, it not a tree like structure, it is a random graph (which depends on how the OEM wire port on the chips). So i have not pre-existing driver, they are not in sysfs today and they do not need a driver. Hence why i proposed what i proposed a sysfs hierarchy where i can add those "virtual" object and shows how they connect existing device for which we have a sysfs directory to symlink. Cheers, Jérôme
Re: [RFC PATCH 02/14] mm/hms: heterogenenous memory system (HMS) documentation
On Wed, Dec 05, 2018 at 11:20:30AM -0700, Logan Gunthorpe wrote: > > > On 2018-12-05 11:07 a.m., Jerome Glisse wrote: > >> Well multiple links are easy when you have a 'link' bus. Just add > >> another link device under the bus. > > > > So you are telling do what i am doing in this patchset but not under > > HMS directory ? > > No, it's completely different. I'm talking about creating a bus to > describe only the real hardware that links GPUs. Not creating a new > virtual tree containing a bunch of duplicate bus and device information > that already exists currently in sysfs. > > >> > >> Technically, the accessibility issue is already encoded in sysfs. For > >> example, through the PCI tree you can determine which ACS bits are set > >> and determine which devices are behind the same root bridge the same way > >> we do in the kernel p2pdma subsystem. This is all bus specific which is > >> fine, but if we want to change that, we should have a common way for > >> existing buses to describe these attributes in the existing tree. The > >> new 'link' bus devices would have to have some way to describe cases if > >> memory isn't accessible in some way across it. > > > > What i am looking at is much more complex than just access bit. It > > is a whole set of properties attach to each path (can it be cache > > coherent ? can it do atomic ? what is the access granularity ? what > > is the bandwidth ? is it dedicated link ? ...) > > I'm not talking about just an access bit. I'm talking about what you are > describing: standard ways for *existing* buses in the sysfs hierarchy to > describe things like cache coherency, atomics, granularity, etc without > creating a new hierarchy. > > > On top of that i argue that more people would use that information if it > > were available to them. I agree that i have no hard evidence to back that > > up and that it is just a feeling but you can not disprove me either as > > this is a chicken and egg problem, you can not prove people will not use > > an API if the API is not there to be use. > > And you miss my point that much of this information is already available > to them. And more can be added in the existing framework without > creating any brand new concepts. I haven't said anything about > chicken-and-egg problems -- I've given you a bunch of different > suggestions to split this up into more managable problems and address > many of them within the APIs and frameworks we have already. The thing is that what i am considering is not in sysfs, it does not even have linux kernel driver, it is just chips that connect device between them and there is nothing to do with those chips it is all hardware they do not need a driver. So there is nothing existing that address what i need to represent. If i add a a fake driver for those what would i do ? under which sub-system i register them ? How i express the fact that they connect device X,Y and Z with some properties ? This is not PCIE ... you can not discover bridges and child, it not a tree like structure, it is a random graph (which depends on how the OEM wire port on the chips). So i have not pre-existing driver, they are not in sysfs today and they do not need a driver. Hence why i proposed what i proposed a sysfs hierarchy where i can add those "virtual" object and shows how they connect existing device for which we have a sysfs directory to symlink. Cheers, Jérôme
[PATCH 1/2] ftrace: Allow ftrace_replace_code() to be schedulable
From: "Steven Rostedt (VMware)"
The function ftrace_replace_code() is the ftrace engine that does the
work to modify all the nops into the calls to the function callback in
all the functions being traced.
The generic version which is normally called from stop machine, but an
architecture can implement a non stop machine version and still use the
generic ftrace_replace_code(). When an architecture does this,
ftrace_replace_code() may be called from a schedulable context, where
it can allow the code to be preemptible, and schedule out.
In order to allow an architecture to make ftrace_replace_code()
schedulable, a new command flag is added called:
FTRACE_SCHEDULABLE
Which can be or'd to the command that is passed to
ftrace_modify_all_code() that calls ftrace_replace_code() and will have
it call cond_resched() in the loop that modifies the nops into the
calls to the ftrace trampolines.
Link: http://lkml.kernel.org/r/20181204192903.8193-1-anders.rox...@linaro.org
Reported-by: Anders Roxell
Signed-off-by: Steven Rostedt (VMware)
---
include/linux/ftrace.h | 1 +
kernel/trace/ftrace.c | 19 ---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index dd16e8218db3..c281b16baef9 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -389,6 +389,7 @@ enum {
FTRACE_UPDATE_TRACE_FUNC= (1 << 2),
FTRACE_START_FUNC_RET = (1 << 3),
FTRACE_STOP_FUNC_RET= (1 << 4),
+ FTRACE_SCHEDULABLE = (1 << 5),
};
/*
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 77734451cb05..74fdcacba514 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -77,6 +77,11 @@
#define ASSIGN_OPS_HASH(opsname, val)
#endif
+enum {
+ FTRACE_MODIFY_ENABLE_FL = (1 << 0),
+ FTRACE_MODIFY_SCHEDULABLE_FL= (1 << 1),
+};
+
static struct ftrace_ops ftrace_list_end __read_mostly = {
.func = ftrace_stub,
.flags = FTRACE_OPS_FL_RECURSION_SAFE | FTRACE_OPS_FL_STUB,
@@ -2415,10 +2420,12 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int
enable)
return -1; /* unknow ftrace bug */
}
-void __weak ftrace_replace_code(int enable)
+void __weak ftrace_replace_code(int mod_flags)
{
struct dyn_ftrace *rec;
struct ftrace_page *pg;
+ int enable = mod_flags & FTRACE_MODIFY_ENABLE_FL;
+ int schedulable = mod_flags & FTRACE_MODIFY_SCHEDULABLE_FL;
int failed;
if (unlikely(ftrace_disabled))
@@ -2435,6 +2442,8 @@ void __weak ftrace_replace_code(int enable)
/* Stop processing */
return;
}
+ if (schedulable)
+ cond_resched();
} while_for_each_ftrace_rec();
}
@@ -2548,8 +2557,12 @@ int __weak ftrace_arch_code_modify_post_process(void)
void ftrace_modify_all_code(int command)
{
int update = command & FTRACE_UPDATE_TRACE_FUNC;
+ int mod_flags = 0;
int err = 0;
+ if (command & FTRACE_SCHEDULABLE)
+ mod_flags = FTRACE_MODIFY_SCHEDULABLE_FL;
+
/*
* If the ftrace_caller calls a ftrace_ops func directly,
* we need to make sure that it only traces functions it
@@ -2567,9 +2580,9 @@ void ftrace_modify_all_code(int command)
}
if (command & FTRACE_UPDATE_CALLS)
- ftrace_replace_code(1);
+ ftrace_replace_code(mod_flags | FTRACE_MODIFY_ENABLE_FL);
else if (command & FTRACE_DISABLE_CALLS)
- ftrace_replace_code(0);
+ ftrace_replace_code(mod_flags);
if (update && ftrace_trace_function != ftrace_ops_list_func) {
function_trace_op = set_function_trace_op;
--
2.19.1
[PATCH 1/2] ftrace: Allow ftrace_replace_code() to be schedulable
From: "Steven Rostedt (VMware)"
The function ftrace_replace_code() is the ftrace engine that does the
work to modify all the nops into the calls to the function callback in
all the functions being traced.
The generic version which is normally called from stop machine, but an
architecture can implement a non stop machine version and still use the
generic ftrace_replace_code(). When an architecture does this,
ftrace_replace_code() may be called from a schedulable context, where
it can allow the code to be preemptible, and schedule out.
In order to allow an architecture to make ftrace_replace_code()
schedulable, a new command flag is added called:
FTRACE_SCHEDULABLE
Which can be or'd to the command that is passed to
ftrace_modify_all_code() that calls ftrace_replace_code() and will have
it call cond_resched() in the loop that modifies the nops into the
calls to the ftrace trampolines.
Link: http://lkml.kernel.org/r/20181204192903.8193-1-anders.rox...@linaro.org
Reported-by: Anders Roxell
Signed-off-by: Steven Rostedt (VMware)
---
include/linux/ftrace.h | 1 +
kernel/trace/ftrace.c | 19 ---
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index dd16e8218db3..c281b16baef9 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -389,6 +389,7 @@ enum {
FTRACE_UPDATE_TRACE_FUNC= (1 << 2),
FTRACE_START_FUNC_RET = (1 << 3),
FTRACE_STOP_FUNC_RET= (1 << 4),
+ FTRACE_SCHEDULABLE = (1 << 5),
};
/*
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 77734451cb05..74fdcacba514 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -77,6 +77,11 @@
#define ASSIGN_OPS_HASH(opsname, val)
#endif
+enum {
+ FTRACE_MODIFY_ENABLE_FL = (1 << 0),
+ FTRACE_MODIFY_SCHEDULABLE_FL= (1 << 1),
+};
+
static struct ftrace_ops ftrace_list_end __read_mostly = {
.func = ftrace_stub,
.flags = FTRACE_OPS_FL_RECURSION_SAFE | FTRACE_OPS_FL_STUB,
@@ -2415,10 +2420,12 @@ __ftrace_replace_code(struct dyn_ftrace *rec, int
enable)
return -1; /* unknow ftrace bug */
}
-void __weak ftrace_replace_code(int enable)
+void __weak ftrace_replace_code(int mod_flags)
{
struct dyn_ftrace *rec;
struct ftrace_page *pg;
+ int enable = mod_flags & FTRACE_MODIFY_ENABLE_FL;
+ int schedulable = mod_flags & FTRACE_MODIFY_SCHEDULABLE_FL;
int failed;
if (unlikely(ftrace_disabled))
@@ -2435,6 +2442,8 @@ void __weak ftrace_replace_code(int enable)
/* Stop processing */
return;
}
+ if (schedulable)
+ cond_resched();
} while_for_each_ftrace_rec();
}
@@ -2548,8 +2557,12 @@ int __weak ftrace_arch_code_modify_post_process(void)
void ftrace_modify_all_code(int command)
{
int update = command & FTRACE_UPDATE_TRACE_FUNC;
+ int mod_flags = 0;
int err = 0;
+ if (command & FTRACE_SCHEDULABLE)
+ mod_flags = FTRACE_MODIFY_SCHEDULABLE_FL;
+
/*
* If the ftrace_caller calls a ftrace_ops func directly,
* we need to make sure that it only traces functions it
@@ -2567,9 +2580,9 @@ void ftrace_modify_all_code(int command)
}
if (command & FTRACE_UPDATE_CALLS)
- ftrace_replace_code(1);
+ ftrace_replace_code(mod_flags | FTRACE_MODIFY_ENABLE_FL);
else if (command & FTRACE_DISABLE_CALLS)
- ftrace_replace_code(0);
+ ftrace_replace_code(mod_flags);
if (update && ftrace_trace_function != ftrace_ops_list_func) {
function_trace_op = set_function_trace_op;
--
2.19.1
[PATCH v2 06/12] staging: rtl8188eu: cleanup block comment in rtw_wlan_util.c
Cleanup a block comment to clear a checkpatch warning. WARNING: Block comments use * on subsequent lines Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c index e61fc7ce65e8..956a331beda2 100644 --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c @@ -760,11 +760,11 @@ void HTOnAssocRsp(struct adapter *padapter) return; } - /* handle A-MPDU parameter field */ - /* - AMPDU_para [1:0]:Max AMPDU Len => 0:8k , 1:16k, 2:32k, 3:64k - AMPDU_para [4:2]:Min MPDU Start Spacing - */ + /* handle A-MPDU parameter field +* +* AMPDU_para [1:0]:Max AMPDU Len => 0:8k , 1:16k, 2:32k, 3:64k +* AMPDU_para [4:2]:Min MPDU Start Spacing +*/ max_AMPDU_len = pmlmeinfo->HT_caps.ampdu_params_info & 0x03; min_MPDU_spacing = (pmlmeinfo->HT_caps.ampdu_params_info & 0x1c) >> 2; -- 2.19.2
[PATCH v2 04/12] staging: rtl8188eu: remove unnecessary parentheses in rtw_wlan_util.c
Remove unnecessary parentheses reported by checkpatch.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_wlan_util.c| 64 +--
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 76c7010c3a5c..0e2653a68f4f 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -377,7 +377,7 @@ int is_client_associated_to_ap(struct adapter *padapter)
return _FAIL;
pmlmeext = >mlmeextpriv;
- pmlmeinfo = &(pmlmeext->mlmext_info);
+ pmlmeinfo = >mlmext_info;
if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_STATION_STATE))
return true;
@@ -388,7 +388,7 @@ int is_client_associated_to_ap(struct adapter *padapter)
int is_client_associated_to_ibss(struct adapter *padapter)
{
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_ADHOC_STATE))
return true;
@@ -400,7 +400,7 @@ int is_IBSS_empty(struct adapter *padapter)
{
unsigned int i;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
for (i = IBSS_START_MAC_ID; i < NUM_STA; i++) {
if (pmlmeinfo->FW_sta_info[i].status == 1)
@@ -465,7 +465,7 @@ int allocate_fw_sta_entry(struct adapter *padapter)
{
unsigned int mac_id;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
for (mac_id = IBSS_START_MAC_ID; mac_id < NUM_STA; mac_id++) {
if (pmlmeinfo->FW_sta_info[mac_id].status == 0) {
@@ -481,7 +481,7 @@ int allocate_fw_sta_entry(struct adapter *padapter)
void flush_all_cam_entry(struct adapter *padapter)
{
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
rtw_hal_set_hwreg(padapter, HW_VAR_CAM_INVALID_ALL, NULL);
@@ -491,9 +491,9 @@ void flush_all_cam_entry(struct adapter *padapter)
int WMM_param_handler(struct adapter *padapter, struct ndis_802_11_var_ie *pIE)
{
/* struct registry_priv *pregpriv = >registrypriv; */
- struct mlme_priv*pmlmepriv = &(padapter->mlmepriv);
+ struct mlme_priv*pmlmepriv = >mlmepriv;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
if (pmlmepriv->qospriv.qos_option == 0) {
pmlmeinfo->WMM_enable = 0;
@@ -501,7 +501,7 @@ int WMM_param_handler(struct adapter *padapter, struct
ndis_802_11_var_ie *pIE)
}
pmlmeinfo->WMM_enable = 1;
- memcpy(&(pmlmeinfo->WMM_param), (pIE->data + 6), sizeof(struct
WMM_para_element));
+ memcpy(>WMM_param, pIE->data + 6, sizeof(struct
WMM_para_element));
return true;
}
@@ -513,7 +513,7 @@ void WMMOnAssocRsp(struct adapter *padapter)
u32 acParm, i;
u32 edca[4], inx[4];
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
struct xmit_priv*pxmitpriv = >xmitpriv;
struct registry_priv*pregpriv = >registrypriv;
@@ -609,9 +609,9 @@ static void bwmode_update_check(struct adapter *padapter,
struct ndis_802_11_var
unsigned charnew_bwmode;
unsigned char new_ch_offset;
struct HT_info_element *pHT_info;
- struct mlme_priv*pmlmepriv = &(padapter->mlmepriv);
+ struct mlme_priv*pmlmepriv = >mlmepriv;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
struct registry_priv *pregistrypriv = >registrypriv;
struct ht_priv *phtpriv = >htpriv;
@@ -660,7 +660,7 @@ static void bwmode_update_check(struct adapter *padapter,
struct ndis_802_11_var
if (pmlmeinfo->bwmode_updated) {
struct sta_info *psta;
- struct wlan_bssid_ex*cur_network = &(pmlmeinfo->network);
+ struct wlan_bssid_ex*cur_network = >network;
struct sta_priv *pstapriv = >stapriv;
/*
[PATCH v2 06/12] staging: rtl8188eu: cleanup block comment in rtw_wlan_util.c
Cleanup a block comment to clear a checkpatch warning. WARNING: Block comments use * on subsequent lines Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c index e61fc7ce65e8..956a331beda2 100644 --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c @@ -760,11 +760,11 @@ void HTOnAssocRsp(struct adapter *padapter) return; } - /* handle A-MPDU parameter field */ - /* - AMPDU_para [1:0]:Max AMPDU Len => 0:8k , 1:16k, 2:32k, 3:64k - AMPDU_para [4:2]:Min MPDU Start Spacing - */ + /* handle A-MPDU parameter field +* +* AMPDU_para [1:0]:Max AMPDU Len => 0:8k , 1:16k, 2:32k, 3:64k +* AMPDU_para [4:2]:Min MPDU Start Spacing +*/ max_AMPDU_len = pmlmeinfo->HT_caps.ampdu_params_info & 0x03; min_MPDU_spacing = (pmlmeinfo->HT_caps.ampdu_params_info & 0x1c) >> 2; -- 2.19.2
[PATCH v2 04/12] staging: rtl8188eu: remove unnecessary parentheses in rtw_wlan_util.c
Remove unnecessary parentheses reported by checkpatch.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_wlan_util.c| 64 +--
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 76c7010c3a5c..0e2653a68f4f 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -377,7 +377,7 @@ int is_client_associated_to_ap(struct adapter *padapter)
return _FAIL;
pmlmeext = >mlmeextpriv;
- pmlmeinfo = &(pmlmeext->mlmext_info);
+ pmlmeinfo = >mlmext_info;
if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_STATION_STATE))
return true;
@@ -388,7 +388,7 @@ int is_client_associated_to_ap(struct adapter *padapter)
int is_client_associated_to_ibss(struct adapter *padapter)
{
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_ADHOC_STATE))
return true;
@@ -400,7 +400,7 @@ int is_IBSS_empty(struct adapter *padapter)
{
unsigned int i;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
for (i = IBSS_START_MAC_ID; i < NUM_STA; i++) {
if (pmlmeinfo->FW_sta_info[i].status == 1)
@@ -465,7 +465,7 @@ int allocate_fw_sta_entry(struct adapter *padapter)
{
unsigned int mac_id;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
for (mac_id = IBSS_START_MAC_ID; mac_id < NUM_STA; mac_id++) {
if (pmlmeinfo->FW_sta_info[mac_id].status == 0) {
@@ -481,7 +481,7 @@ int allocate_fw_sta_entry(struct adapter *padapter)
void flush_all_cam_entry(struct adapter *padapter)
{
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
rtw_hal_set_hwreg(padapter, HW_VAR_CAM_INVALID_ALL, NULL);
@@ -491,9 +491,9 @@ void flush_all_cam_entry(struct adapter *padapter)
int WMM_param_handler(struct adapter *padapter, struct ndis_802_11_var_ie *pIE)
{
/* struct registry_priv *pregpriv = >registrypriv; */
- struct mlme_priv*pmlmepriv = &(padapter->mlmepriv);
+ struct mlme_priv*pmlmepriv = >mlmepriv;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
if (pmlmepriv->qospriv.qos_option == 0) {
pmlmeinfo->WMM_enable = 0;
@@ -501,7 +501,7 @@ int WMM_param_handler(struct adapter *padapter, struct
ndis_802_11_var_ie *pIE)
}
pmlmeinfo->WMM_enable = 1;
- memcpy(&(pmlmeinfo->WMM_param), (pIE->data + 6), sizeof(struct
WMM_para_element));
+ memcpy(>WMM_param, pIE->data + 6, sizeof(struct
WMM_para_element));
return true;
}
@@ -513,7 +513,7 @@ void WMMOnAssocRsp(struct adapter *padapter)
u32 acParm, i;
u32 edca[4], inx[4];
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
struct xmit_priv*pxmitpriv = >xmitpriv;
struct registry_priv*pregpriv = >registrypriv;
@@ -609,9 +609,9 @@ static void bwmode_update_check(struct adapter *padapter,
struct ndis_802_11_var
unsigned charnew_bwmode;
unsigned char new_ch_offset;
struct HT_info_element *pHT_info;
- struct mlme_priv*pmlmepriv = &(padapter->mlmepriv);
+ struct mlme_priv*pmlmepriv = >mlmepriv;
struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = &(pmlmeext->mlmext_info);
+ struct mlme_ext_info*pmlmeinfo = >mlmext_info;
struct registry_priv *pregistrypriv = >registrypriv;
struct ht_priv *phtpriv = >htpriv;
@@ -660,7 +660,7 @@ static void bwmode_update_check(struct adapter *padapter,
struct ndis_802_11_var
if (pmlmeinfo->bwmode_updated) {
struct sta_info *psta;
- struct wlan_bssid_ex*cur_network = &(pmlmeinfo->network);
+ struct wlan_bssid_ex*cur_network = >network;
struct sta_priv *pstapriv = >stapriv;
/*
[PATCH v2 09/12] staging: rtl8188eu: write out multiplying in wifirate2_ratetbl_inx()
Write out multiplying in wifirate2_ratetbl_inx() to improve
readabilitiy and clear checkpatch issues with missing spaces
around '*' operator.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_wlan_util.c| 22 +--
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 0eb904317118..7af3dd910c93 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -1123,29 +1123,29 @@ static int wifirate2_ratetbl_inx(unsigned char rate)
rate = rate & 0x7f;
switch (rate) {
- case 54*2:
+ case 108:
return 11;
- case 48*2:
+ case 96:
return 10;
- case 36*2:
+ case 72:
return 9;
- case 24*2:
+ case 48:
return 8;
- case 18*2:
+ case 36:
return 7;
- case 12*2:
+ case 24:
return 6;
- case 9*2:
+ case 18:
return 5;
- case 6*2:
+ case 12:
return 4;
- case 11*2:
+ case 22:
return 3;
case 11:
return 2;
- case 2*2:
+ case 4:
return 1;
- case 1*2:
+ case 2:
return 0;
default:
return 0;
--
2.19.2
[PATCH v2 09/12] staging: rtl8188eu: write out multiplying in wifirate2_ratetbl_inx()
Write out multiplying in wifirate2_ratetbl_inx() to improve
readabilitiy and clear checkpatch issues with missing spaces
around '*' operator.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_wlan_util.c| 22 +--
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 0eb904317118..7af3dd910c93 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -1123,29 +1123,29 @@ static int wifirate2_ratetbl_inx(unsigned char rate)
rate = rate & 0x7f;
switch (rate) {
- case 54*2:
+ case 108:
return 11;
- case 48*2:
+ case 96:
return 10;
- case 36*2:
+ case 72:
return 9;
- case 24*2:
+ case 48:
return 8;
- case 18*2:
+ case 36:
return 7;
- case 12*2:
+ case 24:
return 6;
- case 9*2:
+ case 18:
return 5;
- case 6*2:
+ case 12:
return 4;
- case 11*2:
+ case 22:
return 3;
case 11:
return 2;
- case 2*2:
+ case 4:
return 1;
- case 1*2:
+ case 2:
return 0;
default:
return 0;
--
2.19.2
[PATCH v2 05/12] staging: rtl8188eu: cleanup declarations in rtw_wlan_util.c
Replace tabs with spaces and/or remove spaces in declarations
to cleanup whitespace. Remove unused/commented declarations.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_wlan_util.c| 175 +-
1 file changed, 86 insertions(+), 89 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 0e2653a68f4f..e61fc7ce65e8 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -177,7 +177,7 @@ static unsigned int ratetbl2rateset(struct adapter
*padapter, unsigned char *rat
{
int i;
unsigned char rate;
- unsigned intlen = 0;
+ unsigned int len = 0;
struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
for (i = 0; i < NumRates; i++) {
@@ -213,8 +213,8 @@ void get_rate_set(struct adapter *padapter, unsigned char
*pbssrate, int *bssrat
void UpdateBrateTbl(struct adapter *Adapter, u8 *mbrate)
{
- u8 i;
- u8 rate;
+ u8 i;
+ u8 rate;
/* 1M, 2M, 5.5M, 11M, 6M, 12M, 24M are mandatory. */
for (i = 0; i < NDIS_802_11_LENGTH_RATES_EX; i++) {
@@ -235,8 +235,8 @@ void UpdateBrateTbl(struct adapter *Adapter, u8 *mbrate)
void UpdateBrateTblForSoftAP(u8 *bssrateset, u32 bssratelen)
{
- u8 i;
- u8 rate;
+ u8 i;
+ u8 rate;
for (i = 0; i < bssratelen; i++) {
rate = bssrateset[i] & 0x7f;
@@ -253,14 +253,14 @@ void UpdateBrateTblForSoftAP(u8 *bssrateset, u32
bssratelen)
void Save_DM_Func_Flag(struct adapter *padapter)
{
- u8 saveflag = true;
+ u8 saveflag = true;
rtw_hal_set_hwreg(padapter, HW_VAR_DM_FUNC_OP, (u8 *)());
}
void Restore_DM_Func_Flag(struct adapter *padapter)
{
- u8 saveflag = false;
+ u8 saveflag = false;
rtw_hal_set_hwreg(padapter, HW_VAR_DM_FUNC_OP, (u8 *)());
}
@@ -370,8 +370,8 @@ u16 get_beacon_interval(struct wlan_bssid_ex *bss)
int is_client_associated_to_ap(struct adapter *padapter)
{
- struct mlme_ext_priv*pmlmeext;
- struct mlme_ext_info*pmlmeinfo;
+ struct mlme_ext_priv *pmlmeext;
+ struct mlme_ext_info *pmlmeinfo;
if (!padapter)
return _FAIL;
@@ -387,8 +387,8 @@ int is_client_associated_to_ap(struct adapter *padapter)
int is_client_associated_to_ibss(struct adapter *padapter)
{
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_ADHOC_STATE))
return true;
@@ -399,8 +399,8 @@ int is_client_associated_to_ibss(struct adapter *padapter)
int is_IBSS_empty(struct adapter *padapter)
{
unsigned int i;
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
for (i = IBSS_START_MAC_ID; i < NUM_STA; i++) {
if (pmlmeinfo->FW_sta_info[i].status == 1)
@@ -426,9 +426,9 @@ void invalidate_cam_all(struct adapter *padapter)
void write_cam(struct adapter *padapter, u8 entry, u16 ctrl, u8 *mac, u8 *key)
{
- unsigned inti, val, addr;
+ unsigned int i, val, addr;
int j;
- u32 cam_val[2];
+ u32 cam_val[2];
addr = entry << 3;
@@ -464,8 +464,8 @@ void clear_cam_entry(struct adapter *padapter, u8 entry)
int allocate_fw_sta_entry(struct adapter *padapter)
{
unsigned int mac_id;
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
for (mac_id = IBSS_START_MAC_ID; mac_id < NUM_STA; mac_id++) {
if (pmlmeinfo->FW_sta_info[mac_id].status == 0) {
@@ -480,8 +480,8 @@ int allocate_fw_sta_entry(struct adapter *padapter)
void flush_all_cam_entry(struct adapter *padapter)
{
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
rtw_hal_set_hwreg(padapter, HW_VAR_CAM_INVALID_ALL, NULL);
@@ -490,10 +490,9 @@ void flush_all_cam_entry(struct adapter *padapter)
int WMM_param_handler(struct adapter *padapter, struct ndis_802_11_var_ie *pIE)
{
- /* struct registry_priv *pregpriv = >registrypriv; */
- struct mlme_priv*pmlmepriv = >mlmepriv;
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info
[PATCH v2 05/12] staging: rtl8188eu: cleanup declarations in rtw_wlan_util.c
Replace tabs with spaces and/or remove spaces in declarations
to cleanup whitespace. Remove unused/commented declarations.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_wlan_util.c| 175 +-
1 file changed, 86 insertions(+), 89 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 0e2653a68f4f..e61fc7ce65e8 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -177,7 +177,7 @@ static unsigned int ratetbl2rateset(struct adapter
*padapter, unsigned char *rat
{
int i;
unsigned char rate;
- unsigned intlen = 0;
+ unsigned int len = 0;
struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
for (i = 0; i < NumRates; i++) {
@@ -213,8 +213,8 @@ void get_rate_set(struct adapter *padapter, unsigned char
*pbssrate, int *bssrat
void UpdateBrateTbl(struct adapter *Adapter, u8 *mbrate)
{
- u8 i;
- u8 rate;
+ u8 i;
+ u8 rate;
/* 1M, 2M, 5.5M, 11M, 6M, 12M, 24M are mandatory. */
for (i = 0; i < NDIS_802_11_LENGTH_RATES_EX; i++) {
@@ -235,8 +235,8 @@ void UpdateBrateTbl(struct adapter *Adapter, u8 *mbrate)
void UpdateBrateTblForSoftAP(u8 *bssrateset, u32 bssratelen)
{
- u8 i;
- u8 rate;
+ u8 i;
+ u8 rate;
for (i = 0; i < bssratelen; i++) {
rate = bssrateset[i] & 0x7f;
@@ -253,14 +253,14 @@ void UpdateBrateTblForSoftAP(u8 *bssrateset, u32
bssratelen)
void Save_DM_Func_Flag(struct adapter *padapter)
{
- u8 saveflag = true;
+ u8 saveflag = true;
rtw_hal_set_hwreg(padapter, HW_VAR_DM_FUNC_OP, (u8 *)());
}
void Restore_DM_Func_Flag(struct adapter *padapter)
{
- u8 saveflag = false;
+ u8 saveflag = false;
rtw_hal_set_hwreg(padapter, HW_VAR_DM_FUNC_OP, (u8 *)());
}
@@ -370,8 +370,8 @@ u16 get_beacon_interval(struct wlan_bssid_ex *bss)
int is_client_associated_to_ap(struct adapter *padapter)
{
- struct mlme_ext_priv*pmlmeext;
- struct mlme_ext_info*pmlmeinfo;
+ struct mlme_ext_priv *pmlmeext;
+ struct mlme_ext_info *pmlmeinfo;
if (!padapter)
return _FAIL;
@@ -387,8 +387,8 @@ int is_client_associated_to_ap(struct adapter *padapter)
int is_client_associated_to_ibss(struct adapter *padapter)
{
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_ADHOC_STATE))
return true;
@@ -399,8 +399,8 @@ int is_client_associated_to_ibss(struct adapter *padapter)
int is_IBSS_empty(struct adapter *padapter)
{
unsigned int i;
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
for (i = IBSS_START_MAC_ID; i < NUM_STA; i++) {
if (pmlmeinfo->FW_sta_info[i].status == 1)
@@ -426,9 +426,9 @@ void invalidate_cam_all(struct adapter *padapter)
void write_cam(struct adapter *padapter, u8 entry, u16 ctrl, u8 *mac, u8 *key)
{
- unsigned inti, val, addr;
+ unsigned int i, val, addr;
int j;
- u32 cam_val[2];
+ u32 cam_val[2];
addr = entry << 3;
@@ -464,8 +464,8 @@ void clear_cam_entry(struct adapter *padapter, u8 entry)
int allocate_fw_sta_entry(struct adapter *padapter)
{
unsigned int mac_id;
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
for (mac_id = IBSS_START_MAC_ID; mac_id < NUM_STA; mac_id++) {
if (pmlmeinfo->FW_sta_info[mac_id].status == 0) {
@@ -480,8 +480,8 @@ int allocate_fw_sta_entry(struct adapter *padapter)
void flush_all_cam_entry(struct adapter *padapter)
{
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info*pmlmeinfo = >mlmext_info;
+ struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
+ struct mlme_ext_info *pmlmeinfo = >mlmext_info;
rtw_hal_set_hwreg(padapter, HW_VAR_CAM_INVALID_ALL, NULL);
@@ -490,10 +490,9 @@ void flush_all_cam_entry(struct adapter *padapter)
int WMM_param_handler(struct adapter *padapter, struct ndis_802_11_var_ie *pIE)
{
- /* struct registry_priv *pregpriv = >registrypriv; */
- struct mlme_priv*pmlmepriv = >mlmepriv;
- struct mlme_ext_priv*pmlmeext = >mlmeextpriv;
- struct mlme_ext_info
Re: [PATCH v2 22/34] dt-bindings: arm: Convert QCom board/soc bindings to json-schema
On Mon, Dec 03, 2018 at 03:32:11PM -0600, Rob Herring wrote: > Convert QCom SoC bindings to DT schema format using json-schema. > > Cc: Andy Gross > Cc: David Brown > Cc: Mark Rutland > Cc: devicet...@vger.kernel.org > Signed-off-by: Rob Herring Acked-by: Andy Gross
Re: [PATCH v2 22/34] dt-bindings: arm: Convert QCom board/soc bindings to json-schema
On Mon, Dec 03, 2018 at 03:32:11PM -0600, Rob Herring wrote: > Convert QCom SoC bindings to DT schema format using json-schema. > > Cc: Andy Gross > Cc: David Brown > Cc: Mark Rutland > Cc: devicet...@vger.kernel.org > Signed-off-by: Rob Herring Acked-by: Andy Gross
[PATCH v2 10/12] staging: rtl8188eu: replace if else with ternary operator
Replace if else with a single call and ternary operator to slightly reduce object file size. Also clears a checkpatch warning: WARNING: Statements should start on a tabstop Suggested-by: Joe Perches Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c index 7af3dd910c93..9a20faad96c9 100644 --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c @@ -1440,10 +1440,9 @@ void update_wireless_mode(struct adapter *padapter) rtw_hal_set_hwreg(padapter, HW_VAR_RESP_SIFS, (u8 *)_Timer); - if (pmlmeext->cur_wireless_mode & WIRELESS_11B) - update_mgnt_tx_rate(padapter, IEEE80211_CCK_RATE_1MB); -else - update_mgnt_tx_rate(padapter, IEEE80211_OFDM_RATE_6MB); + update_mgnt_tx_rate(padapter, + pmlmeext->cur_wireless_mode & WIRELESS_11B ? + IEEE80211_CCK_RATE_1MB : IEEE80211_OFDM_RATE_6MB); } void update_bmc_sta_support_rate(struct adapter *padapter, u32 mac_id) -- 2.19.2
[PATCH v2 08/12] staging: rtl8188eu: add spaces around operators in rtw_wlan_util.c
Add spaces around '+', '-', '&' and '>>' to follow kernel coding
style. Reported by checkpatch.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_wlan_util.c| 21 +++
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 2ffa0332bf32..0eb904317118 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -444,7 +444,8 @@ void write_cam(struct adapter *padapter, u8 entry, u16
ctrl, u8 *mac, u8 *key)
break;
default:
i = (j - 2) << 2;
- val = key[i] | (key[i+1] << 8) | (key[i+2] << 16) |
(key[i+3] << 24);
+ val = key[i] | (key[i + 1] << 8) | (key[i + 2] << 16) |
+ (key[i + 3] << 24);
break;
}
@@ -580,7 +581,7 @@ void WMMOnAssocRsp(struct adapter *padapter)
/* entry indx: 0->vo, 1->vi, 2->be, 3->bk. */
for (i = 0; i < 4; i++) {
- for (j = i+1; j < 4; j++) {
+ for (j = i + 1; j < 4; j++) {
/* compare CW and AIFS */
if ((edca[j] & 0x) < (edca[i] & 0x)) {
change_inx = true;
@@ -894,7 +895,7 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8
*pframe, u32 packet_len)
ht_info_infos_0 = 0;
}
if (ht_cap_info != cur_network->BcnInfo.ht_cap_info ||
- ((ht_info_infos_0&0x03) !=
(cur_network->BcnInfo.ht_info_infos_0&0x03))) {
+ ((ht_info_infos_0 & 0x03) != (cur_network->BcnInfo.ht_info_infos_0
& 0x03))) {
DBG_88E("%s bcn now: ht_cap_info:%x
ht_info_infos_0:%x\n", __func__,
ht_cap_info, ht_info_infos_0);
DBG_88E("%s bcn link: ht_cap_info:%x
ht_info_infos_0:%x\n", __func__,
@@ -983,18 +984,20 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8
*pframe, u32 packet_len)
}
if (encryp_protocol == ENCRYP_PROTOCOL_WPA || encryp_protocol ==
ENCRYP_PROTOCOL_WPA2) {
- pbuf = rtw_get_wpa_ie(>ies[12], _ielen,
bssid->ie_length-12);
+ pbuf = rtw_get_wpa_ie(>ies[12], _ielen,
+ bssid->ie_length - 12);
if (pbuf && (wpa_ielen > 0)) {
- if (_SUCCESS == rtw_parse_wpa_ie(pbuf, wpa_ielen+2,
_cipher, _cipher, _8021x)) {
+ if (_SUCCESS == rtw_parse_wpa_ie(pbuf, wpa_ielen + 2,
_cipher, _cipher, _8021x)) {
RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_,
("%s pnetwork->pairwise_cipher: %d,
group_cipher is %d, is_8021x is %d\n", __func__,
pairwise_cipher, group_cipher,
is_8021x));
}
} else {
- pbuf = rtw_get_wpa2_ie(>ies[12], _ielen,
bssid->ie_length-12);
+ pbuf = rtw_get_wpa2_ie(>ies[12], _ielen,
+ bssid->ie_length - 12);
if (pbuf && (wpa_ielen > 0)) {
- if (_SUCCESS == rtw_parse_wpa2_ie(pbuf,
wpa_ielen+2, _cipher, _cipher, _8021x)) {
+ if (_SUCCESS == rtw_parse_wpa2_ie(pbuf,
wpa_ielen + 2, _cipher, _cipher, _8021x)) {
RT_TRACE(_module_rtl871x_mlme_c_,
_drv_info_,
("%s
pnetwork->pairwise_cipher: %d, pnetwork->group_cipher is %d, is_802x is %d\n",
__func__, pairwise_cipher,
group_cipher, is_8021x));
@@ -1498,7 +1501,7 @@ void process_addba_req(struct adapter *padapter, u8
*paddba_req, u8 *addr)
if (psta) {
param = le16_to_cpu(preq->BA_para_set);
- tid = (param>>2)&0x0f;
+ tid = (param >> 2) & 0x0f;
preorder_ctrl = >recvreorder_ctrl[tid];
preorder_ctrl->indicate_seq = 0x;
preorder_ctrl->enable = pmlmeinfo->accept_addba_req;
@@ -1513,7 +1516,7 @@ void update_TSF(struct mlme_ext_priv *pmlmeext, u8
*pframe, uint len)
pIE = pframe + sizeof(struct ieee80211_hdr_3addr);
pbuf = (__le32 *)pIE;
- pmlmeext->TSFValue = le32_to_cpu(*(pbuf+1));
+ pmlmeext->TSFValue = le32_to_cpu(*(pbuf + 1));
pmlmeext->TSFValue = pmlmeext->TSFValue << 32;
--
2.19.2
[PATCH v2 10/12] staging: rtl8188eu: replace if else with ternary operator
Replace if else with a single call and ternary operator to slightly reduce object file size. Also clears a checkpatch warning: WARNING: Statements should start on a tabstop Suggested-by: Joe Perches Signed-off-by: Michael Straube --- drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c index 7af3dd910c93..9a20faad96c9 100644 --- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c +++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c @@ -1440,10 +1440,9 @@ void update_wireless_mode(struct adapter *padapter) rtw_hal_set_hwreg(padapter, HW_VAR_RESP_SIFS, (u8 *)_Timer); - if (pmlmeext->cur_wireless_mode & WIRELESS_11B) - update_mgnt_tx_rate(padapter, IEEE80211_CCK_RATE_1MB); -else - update_mgnt_tx_rate(padapter, IEEE80211_OFDM_RATE_6MB); + update_mgnt_tx_rate(padapter, + pmlmeext->cur_wireless_mode & WIRELESS_11B ? + IEEE80211_CCK_RATE_1MB : IEEE80211_OFDM_RATE_6MB); } void update_bmc_sta_support_rate(struct adapter *padapter, u32 mac_id) -- 2.19.2
[PATCH v2 08/12] staging: rtl8188eu: add spaces around operators in rtw_wlan_util.c
Add spaces around '+', '-', '&' and '>>' to follow kernel coding
style. Reported by checkpatch.
Signed-off-by: Michael Straube
---
.../staging/rtl8188eu/core/rtw_wlan_util.c| 21 +++
1 file changed, 12 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 2ffa0332bf32..0eb904317118 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -444,7 +444,8 @@ void write_cam(struct adapter *padapter, u8 entry, u16
ctrl, u8 *mac, u8 *key)
break;
default:
i = (j - 2) << 2;
- val = key[i] | (key[i+1] << 8) | (key[i+2] << 16) |
(key[i+3] << 24);
+ val = key[i] | (key[i + 1] << 8) | (key[i + 2] << 16) |
+ (key[i + 3] << 24);
break;
}
@@ -580,7 +581,7 @@ void WMMOnAssocRsp(struct adapter *padapter)
/* entry indx: 0->vo, 1->vi, 2->be, 3->bk. */
for (i = 0; i < 4; i++) {
- for (j = i+1; j < 4; j++) {
+ for (j = i + 1; j < 4; j++) {
/* compare CW and AIFS */
if ((edca[j] & 0x) < (edca[i] & 0x)) {
change_inx = true;
@@ -894,7 +895,7 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8
*pframe, u32 packet_len)
ht_info_infos_0 = 0;
}
if (ht_cap_info != cur_network->BcnInfo.ht_cap_info ||
- ((ht_info_infos_0&0x03) !=
(cur_network->BcnInfo.ht_info_infos_0&0x03))) {
+ ((ht_info_infos_0 & 0x03) != (cur_network->BcnInfo.ht_info_infos_0
& 0x03))) {
DBG_88E("%s bcn now: ht_cap_info:%x
ht_info_infos_0:%x\n", __func__,
ht_cap_info, ht_info_infos_0);
DBG_88E("%s bcn link: ht_cap_info:%x
ht_info_infos_0:%x\n", __func__,
@@ -983,18 +984,20 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8
*pframe, u32 packet_len)
}
if (encryp_protocol == ENCRYP_PROTOCOL_WPA || encryp_protocol ==
ENCRYP_PROTOCOL_WPA2) {
- pbuf = rtw_get_wpa_ie(>ies[12], _ielen,
bssid->ie_length-12);
+ pbuf = rtw_get_wpa_ie(>ies[12], _ielen,
+ bssid->ie_length - 12);
if (pbuf && (wpa_ielen > 0)) {
- if (_SUCCESS == rtw_parse_wpa_ie(pbuf, wpa_ielen+2,
_cipher, _cipher, _8021x)) {
+ if (_SUCCESS == rtw_parse_wpa_ie(pbuf, wpa_ielen + 2,
_cipher, _cipher, _8021x)) {
RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_,
("%s pnetwork->pairwise_cipher: %d,
group_cipher is %d, is_8021x is %d\n", __func__,
pairwise_cipher, group_cipher,
is_8021x));
}
} else {
- pbuf = rtw_get_wpa2_ie(>ies[12], _ielen,
bssid->ie_length-12);
+ pbuf = rtw_get_wpa2_ie(>ies[12], _ielen,
+ bssid->ie_length - 12);
if (pbuf && (wpa_ielen > 0)) {
- if (_SUCCESS == rtw_parse_wpa2_ie(pbuf,
wpa_ielen+2, _cipher, _cipher, _8021x)) {
+ if (_SUCCESS == rtw_parse_wpa2_ie(pbuf,
wpa_ielen + 2, _cipher, _cipher, _8021x)) {
RT_TRACE(_module_rtl871x_mlme_c_,
_drv_info_,
("%s
pnetwork->pairwise_cipher: %d, pnetwork->group_cipher is %d, is_802x is %d\n",
__func__, pairwise_cipher,
group_cipher, is_8021x));
@@ -1498,7 +1501,7 @@ void process_addba_req(struct adapter *padapter, u8
*paddba_req, u8 *addr)
if (psta) {
param = le16_to_cpu(preq->BA_para_set);
- tid = (param>>2)&0x0f;
+ tid = (param >> 2) & 0x0f;
preorder_ctrl = >recvreorder_ctrl[tid];
preorder_ctrl->indicate_seq = 0x;
preorder_ctrl->enable = pmlmeinfo->accept_addba_req;
@@ -1513,7 +1516,7 @@ void update_TSF(struct mlme_ext_priv *pmlmeext, u8
*pframe, uint len)
pIE = pframe + sizeof(struct ieee80211_hdr_3addr);
pbuf = (__le32 *)pIE;
- pmlmeext->TSFValue = le32_to_cpu(*(pbuf+1));
+ pmlmeext->TSFValue = le32_to_cpu(*(pbuf + 1));
pmlmeext->TSFValue = pmlmeext->TSFValue << 32;
--
2.19.2
[PATCH v2 02/12] staging: rtl8188eu: refactor cckratesonly_included()
Refactor cckratesonly_included() to improve readability and slightly
reduce object file size. Also change the return type to bool.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 10 +-
drivers/staging/rtl8188eu/include/rtw_mlme_ext.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 90160d5fc292..fac1c1c20b2f 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -69,16 +69,16 @@ bool cckrates_included(unsigned char *rate, int ratelen)
return false;
}
-int cckratesonly_included(unsigned char *rate, int ratelen)
+bool cckratesonly_included(unsigned char *rate, int ratelen)
{
- int i;
+ int i;
for (i = 0; i < ratelen; i++) {
- if rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) &&
- (((rate[i]) & 0x7f) != 11) && (((rate[i]) & 0x7f)
!= 22))
+ u8 r = rate[i] & 0x7f;
+
+ if (r != 2 && r != 4 && r != 11 && r != 22)
return false;
}
-
return true;
}
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
index c86dec12dec2..0ade33df16d2 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
@@ -581,7 +581,7 @@ void addba_timer_hdl(struct timer_list *t);
msecs_to_jiffies(ms))
bool cckrates_included(unsigned char *rate, int ratelen);
-int cckratesonly_included(unsigned char *rate, int ratelen);
+bool cckratesonly_included(unsigned char *rate, int ratelen);
void process_addba_req(struct adapter *padapter, u8 *paddba_req, u8 *addr);
--
2.19.2
[PATCH v2 07/12] staging: rtl8188eu: cleanup long lines in rtw_wlan_util.c
Cleanup some lines over 80 characters by adding appropriate
line breaks and removing commented code.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 956a331beda2..2ffa0332bf32 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -379,7 +379,8 @@ int is_client_associated_to_ap(struct adapter *padapter)
pmlmeext = >mlmeextpriv;
pmlmeinfo = >mlmext_info;
- if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_STATION_STATE))
+ if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
+ (pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE)
return true;
else
return _FAIL;
@@ -390,7 +391,8 @@ int is_client_associated_to_ibss(struct adapter *padapter)
struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
struct mlme_ext_info *pmlmeinfo = >mlmext_info;
- if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_ADHOC_STATE))
+ if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
+ (pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE)
return true;
else
return _FAIL;
@@ -662,8 +664,6 @@ static void bwmode_update_check(struct adapter *padapter,
struct ndis_802_11_var
struct wlan_bssid_ex*cur_network = >network;
struct sta_priv *pstapriv = >stapriv;
- /* set_channel_bwmode(padapter, pmlmeext->cur_channel,
pmlmeext->cur_ch_offset, pmlmeext->cur_bwmode); */
-
/* update ap's stainfo */
psta = rtw_get_stainfo(pstapriv, cur_network->MacAddress);
if (psta) {
--
2.19.2
[PATCH v2 02/12] staging: rtl8188eu: refactor cckratesonly_included()
Refactor cckratesonly_included() to improve readability and slightly
reduce object file size. Also change the return type to bool.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 10 +-
drivers/staging/rtl8188eu/include/rtw_mlme_ext.h | 2 +-
2 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 90160d5fc292..fac1c1c20b2f 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -69,16 +69,16 @@ bool cckrates_included(unsigned char *rate, int ratelen)
return false;
}
-int cckratesonly_included(unsigned char *rate, int ratelen)
+bool cckratesonly_included(unsigned char *rate, int ratelen)
{
- int i;
+ int i;
for (i = 0; i < ratelen; i++) {
- if rate[i]) & 0x7f) != 2) && (((rate[i]) & 0x7f) != 4) &&
- (((rate[i]) & 0x7f) != 11) && (((rate[i]) & 0x7f)
!= 22))
+ u8 r = rate[i] & 0x7f;
+
+ if (r != 2 && r != 4 && r != 11 && r != 22)
return false;
}
-
return true;
}
diff --git a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
index c86dec12dec2..0ade33df16d2 100644
--- a/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
+++ b/drivers/staging/rtl8188eu/include/rtw_mlme_ext.h
@@ -581,7 +581,7 @@ void addba_timer_hdl(struct timer_list *t);
msecs_to_jiffies(ms))
bool cckrates_included(unsigned char *rate, int ratelen);
-int cckratesonly_included(unsigned char *rate, int ratelen);
+bool cckratesonly_included(unsigned char *rate, int ratelen);
void process_addba_req(struct adapter *padapter, u8 *paddba_req, u8 *addr);
--
2.19.2
[PATCH v2 07/12] staging: rtl8188eu: cleanup long lines in rtw_wlan_util.c
Cleanup some lines over 80 characters by adding appropriate
line breaks and removing commented code.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index 956a331beda2..2ffa0332bf32 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -379,7 +379,8 @@ int is_client_associated_to_ap(struct adapter *padapter)
pmlmeext = >mlmeextpriv;
pmlmeinfo = >mlmext_info;
- if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_STATION_STATE))
+ if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
+ (pmlmeinfo->state & 0x03) == WIFI_FW_STATION_STATE)
return true;
else
return _FAIL;
@@ -390,7 +391,8 @@ int is_client_associated_to_ibss(struct adapter *padapter)
struct mlme_ext_priv *pmlmeext = >mlmeextpriv;
struct mlme_ext_info *pmlmeinfo = >mlmext_info;
- if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
((pmlmeinfo->state&0x03) == WIFI_FW_ADHOC_STATE))
+ if ((pmlmeinfo->state & WIFI_FW_ASSOC_SUCCESS) &&
+ (pmlmeinfo->state & 0x03) == WIFI_FW_ADHOC_STATE)
return true;
else
return _FAIL;
@@ -662,8 +664,6 @@ static void bwmode_update_check(struct adapter *padapter,
struct ndis_802_11_var
struct wlan_bssid_ex*cur_network = >network;
struct sta_priv *pstapriv = >stapriv;
- /* set_channel_bwmode(padapter, pmlmeext->cur_channel,
pmlmeext->cur_ch_offset, pmlmeext->cur_bwmode); */
-
/* update ap's stainfo */
psta = rtw_get_stainfo(pstapriv, cur_network->MacAddress);
if (psta) {
--
2.19.2
[PATCH v2 11/12] staging: rtl8188eu: rename struct field Wifi_Error_Status
Rename struct field Wifi_Error_Status to avoid CamelCase.
Wifi_Error_Status -> wifi_error_status
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_sreset.c | 4 ++--
drivers/staging/rtl8188eu/include/rtw_sreset.h | 2 +-
drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_sreset.c
b/drivers/staging/rtl8188eu/core/rtw_sreset.c
index fb5adaf4a42c..a8397b132002 100644
--- a/drivers/staging/rtl8188eu/core/rtw_sreset.c
+++ b/drivers/staging/rtl8188eu/core/rtw_sreset.c
@@ -12,10 +12,10 @@ void rtw_hal_sreset_init(struct adapter *padapter)
{
struct sreset_priv *psrtpriv = >HalData->srestpriv;
- psrtpriv->Wifi_Error_Status = WIFI_STATUS_SUCCESS;
+ psrtpriv->wifi_error_status = WIFI_STATUS_SUCCESS;
}
void sreset_set_wifi_error_status(struct adapter *padapter, u32 status)
{
- padapter->HalData->srestpriv.Wifi_Error_Status = status;
+ padapter->HalData->srestpriv.wifi_error_status = status;
}
diff --git a/drivers/staging/rtl8188eu/include/rtw_sreset.h
b/drivers/staging/rtl8188eu/include/rtw_sreset.h
index 3ee6a4a7847d..ea3c0d93bf0b 100644
--- a/drivers/staging/rtl8188eu/include/rtw_sreset.h
+++ b/drivers/staging/rtl8188eu/include/rtw_sreset.h
@@ -11,7 +11,7 @@
#include
struct sreset_priv {
- u8 Wifi_Error_Status;
+ u8 wifi_error_status;
};
#include
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index 7dc7028c1cd5..e4f2af2974ed 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -263,7 +263,7 @@ static int usbctrl_vendorreq(struct adapter *adapt, u8
request, u16 value, u16 i
if (status == (-ESHUTDOWN) || status == -ENODEV)
adapt->bSurpriseRemoved = true;
else
-
adapt->HalData->srestpriv.Wifi_Error_Status = USB_VEN_REQ_CMD_FAIL;
+
adapt->HalData->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
} else { /* status != len && status >= 0 */
if (status > 0) {
if (requesttype == 0x01) {
@@ -410,7 +410,7 @@ static void usb_read_port_complete(struct urb *purb, struct
pt_regs *regs)
break;
case -EPROTO:
case -EOVERFLOW:
- adapt->HalData->srestpriv.Wifi_Error_Status =
USB_READ_PORT_FAIL;
+ adapt->HalData->srestpriv.wifi_error_status =
USB_READ_PORT_FAIL;
precvbuf->reuse = true;
usb_read_port(adapt, RECV_BULK_IN_ADDR, precvbuf);
break;
--
2.19.2
[PATCH v2 12/12] staging: rtl8188eu: remove unused code in rtw_cmd.c
Remove unused/commented code in rtw_cmd.c.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_cmd.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_cmd.c
b/drivers/staging/rtl8188eu/core/rtw_cmd.c
index e3e46f7ac834..f9cdd1da8add 100644
--- a/drivers/staging/rtl8188eu/core/rtw_cmd.c
+++ b/drivers/staging/rtl8188eu/core/rtw_cmd.c
@@ -215,8 +215,6 @@ int rtw_cmd_thread(void *context)
/* free all cmd_obj resources */
while ((pcmd = rtw_dequeue_cmd(>cmd_queue))) {
- /* DBG_88E("%s: leaving... drop cmdcode:%u\n", __func__,
pcmd->cmdcode); */
-
rtw_free_cmd_obj(pcmd);
}
@@ -259,7 +257,6 @@ u8 rtw_sitesurvey_cmd(struct adapter *padapter, struct
ndis_802_11_ssid *ssid,
init_h2fwcmd_w_parm_no_rsp(ph2c, psurveyPara, _SiteSurvey_CMD_);
- /* psurveyPara->bsslimit = 48; */
psurveyPara->scan_mode = pmlmepriv->scan_mode;
/* prepare ssid list */
@@ -660,9 +657,6 @@ u8 rtw_addbareq_cmd(struct adapter *padapter, u8 tid, u8
*addr)
init_h2fwcmd_w_parm_no_rsp(ph2c, paddbareq_parm, _AddBAReq_CMD_);
- /* DBG_88E("rtw_addbareq_cmd, tid =%d\n", tid); */
-
- /* rtw_enqueue_cmd(pcmdpriv, ph2c); */
res = rtw_enqueue_cmd(pcmdpriv, ph2c);
exit:
@@ -696,7 +690,6 @@ u8 rtw_dynamic_chk_wk_cmd(struct adapter *padapter)
init_h2fwcmd_w_parm_no_rsp(ph2c, pdrvextra_cmd_parm,
_Set_Drv_Extra_CMD_);
- /* rtw_enqueue_cmd(pcmdpriv, ph2c); */
res = rtw_enqueue_cmd(pcmdpriv, ph2c);
exit:
return res;
@@ -862,7 +855,6 @@ static void lps_ctrl_wk_hdl(struct adapter *padapter, u8
lps_ctrl_type)
rtw_hal_set_hwreg(padapter, HW_VAR_H2C_FW_JOINBSSRPT, (u8
*)());
break;
case LPS_CTRL_SPECIAL_PACKET:
- /* DBG_88E("LPS_CTRL_SPECIAL_PACKET\n"); */
pwrpriv->DelayLPSLastTimeStamp = jiffies;
LPS_Leave(padapter);
break;
@@ -879,7 +871,6 @@ u8 rtw_lps_ctrl_wk_cmd(struct adapter *padapter, u8
lps_ctrl_type, u8 enqueue)
struct cmd_obj *ph2c;
struct drvextra_cmd_parm*pdrvextra_cmd_parm;
struct cmd_priv *pcmdpriv = >cmdpriv;
- /* struct pwrctrl_priv *pwrctrlpriv = >pwrctrlpriv; */
u8 res = _SUCCESS;
if (enqueue) {
@@ -1029,9 +1020,6 @@ static void rtw_chk_hi_queue_hdl(struct adapter *padapter)
if (psta_bmc->sleepq_len == 0) {
u8 val = 0;
- /* while ((rtw_read32(padapter, 0x414)&0x0000)!= 0) */
- /* while ((rtw_read32(padapter, 0x414)&0xff00)!= 0) */
-
rtw_hal_get_hwreg(padapter, HW_VAR_CHK_HI_QUEUE_EMPTY, );
while (!val) {
--
2.19.2
[PATCH v2 11/12] staging: rtl8188eu: rename struct field Wifi_Error_Status
Rename struct field Wifi_Error_Status to avoid CamelCase.
Wifi_Error_Status -> wifi_error_status
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_sreset.c | 4 ++--
drivers/staging/rtl8188eu/include/rtw_sreset.h | 2 +-
drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c | 4 ++--
3 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_sreset.c
b/drivers/staging/rtl8188eu/core/rtw_sreset.c
index fb5adaf4a42c..a8397b132002 100644
--- a/drivers/staging/rtl8188eu/core/rtw_sreset.c
+++ b/drivers/staging/rtl8188eu/core/rtw_sreset.c
@@ -12,10 +12,10 @@ void rtw_hal_sreset_init(struct adapter *padapter)
{
struct sreset_priv *psrtpriv = >HalData->srestpriv;
- psrtpriv->Wifi_Error_Status = WIFI_STATUS_SUCCESS;
+ psrtpriv->wifi_error_status = WIFI_STATUS_SUCCESS;
}
void sreset_set_wifi_error_status(struct adapter *padapter, u32 status)
{
- padapter->HalData->srestpriv.Wifi_Error_Status = status;
+ padapter->HalData->srestpriv.wifi_error_status = status;
}
diff --git a/drivers/staging/rtl8188eu/include/rtw_sreset.h
b/drivers/staging/rtl8188eu/include/rtw_sreset.h
index 3ee6a4a7847d..ea3c0d93bf0b 100644
--- a/drivers/staging/rtl8188eu/include/rtw_sreset.h
+++ b/drivers/staging/rtl8188eu/include/rtw_sreset.h
@@ -11,7 +11,7 @@
#include
struct sreset_priv {
- u8 Wifi_Error_Status;
+ u8 wifi_error_status;
};
#include
diff --git a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
index 7dc7028c1cd5..e4f2af2974ed 100644
--- a/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/usb_ops_linux.c
@@ -263,7 +263,7 @@ static int usbctrl_vendorreq(struct adapter *adapt, u8
request, u16 value, u16 i
if (status == (-ESHUTDOWN) || status == -ENODEV)
adapt->bSurpriseRemoved = true;
else
-
adapt->HalData->srestpriv.Wifi_Error_Status = USB_VEN_REQ_CMD_FAIL;
+
adapt->HalData->srestpriv.wifi_error_status = USB_VEN_REQ_CMD_FAIL;
} else { /* status != len && status >= 0 */
if (status > 0) {
if (requesttype == 0x01) {
@@ -410,7 +410,7 @@ static void usb_read_port_complete(struct urb *purb, struct
pt_regs *regs)
break;
case -EPROTO:
case -EOVERFLOW:
- adapt->HalData->srestpriv.Wifi_Error_Status =
USB_READ_PORT_FAIL;
+ adapt->HalData->srestpriv.wifi_error_status =
USB_READ_PORT_FAIL;
precvbuf->reuse = true;
usb_read_port(adapt, RECV_BULK_IN_ADDR, precvbuf);
break;
--
2.19.2
[PATCH v2 12/12] staging: rtl8188eu: remove unused code in rtw_cmd.c
Remove unused/commented code in rtw_cmd.c.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_cmd.c | 12
1 file changed, 12 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_cmd.c
b/drivers/staging/rtl8188eu/core/rtw_cmd.c
index e3e46f7ac834..f9cdd1da8add 100644
--- a/drivers/staging/rtl8188eu/core/rtw_cmd.c
+++ b/drivers/staging/rtl8188eu/core/rtw_cmd.c
@@ -215,8 +215,6 @@ int rtw_cmd_thread(void *context)
/* free all cmd_obj resources */
while ((pcmd = rtw_dequeue_cmd(>cmd_queue))) {
- /* DBG_88E("%s: leaving... drop cmdcode:%u\n", __func__,
pcmd->cmdcode); */
-
rtw_free_cmd_obj(pcmd);
}
@@ -259,7 +257,6 @@ u8 rtw_sitesurvey_cmd(struct adapter *padapter, struct
ndis_802_11_ssid *ssid,
init_h2fwcmd_w_parm_no_rsp(ph2c, psurveyPara, _SiteSurvey_CMD_);
- /* psurveyPara->bsslimit = 48; */
psurveyPara->scan_mode = pmlmepriv->scan_mode;
/* prepare ssid list */
@@ -660,9 +657,6 @@ u8 rtw_addbareq_cmd(struct adapter *padapter, u8 tid, u8
*addr)
init_h2fwcmd_w_parm_no_rsp(ph2c, paddbareq_parm, _AddBAReq_CMD_);
- /* DBG_88E("rtw_addbareq_cmd, tid =%d\n", tid); */
-
- /* rtw_enqueue_cmd(pcmdpriv, ph2c); */
res = rtw_enqueue_cmd(pcmdpriv, ph2c);
exit:
@@ -696,7 +690,6 @@ u8 rtw_dynamic_chk_wk_cmd(struct adapter *padapter)
init_h2fwcmd_w_parm_no_rsp(ph2c, pdrvextra_cmd_parm,
_Set_Drv_Extra_CMD_);
- /* rtw_enqueue_cmd(pcmdpriv, ph2c); */
res = rtw_enqueue_cmd(pcmdpriv, ph2c);
exit:
return res;
@@ -862,7 +855,6 @@ static void lps_ctrl_wk_hdl(struct adapter *padapter, u8
lps_ctrl_type)
rtw_hal_set_hwreg(padapter, HW_VAR_H2C_FW_JOINBSSRPT, (u8
*)());
break;
case LPS_CTRL_SPECIAL_PACKET:
- /* DBG_88E("LPS_CTRL_SPECIAL_PACKET\n"); */
pwrpriv->DelayLPSLastTimeStamp = jiffies;
LPS_Leave(padapter);
break;
@@ -879,7 +871,6 @@ u8 rtw_lps_ctrl_wk_cmd(struct adapter *padapter, u8
lps_ctrl_type, u8 enqueue)
struct cmd_obj *ph2c;
struct drvextra_cmd_parm*pdrvextra_cmd_parm;
struct cmd_priv *pcmdpriv = >cmdpriv;
- /* struct pwrctrl_priv *pwrctrlpriv = >pwrctrlpriv; */
u8 res = _SUCCESS;
if (enqueue) {
@@ -1029,9 +1020,6 @@ static void rtw_chk_hi_queue_hdl(struct adapter *padapter)
if (psta_bmc->sleepq_len == 0) {
u8 val = 0;
- /* while ((rtw_read32(padapter, 0x414)&0x0000)!= 0) */
- /* while ((rtw_read32(padapter, 0x414)&0xff00)!= 0) */
-
rtw_hal_get_hwreg(padapter, HW_VAR_CHK_HI_QUEUE_EMPTY, );
while (!val) {
--
2.19.2
[PATCH v2 03/12] staging: rtl8188eu: simplify array initializations
Simplfy initialization of arrays with zero only values
to improve readability and save a line.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index fac1c1c20b2f..76c7010c3a5c 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -455,9 +455,8 @@ void write_cam(struct adapter *padapter, u8 entry, u16
ctrl, u8 *mac, u8 *key)
void clear_cam_entry(struct adapter *padapter, u8 entry)
{
- u8 null_sta[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
- u8 null_key[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ u8 null_sta[ETH_ALEN] = {};
+ u8 null_key[16] = {};
write_cam(padapter, entry, 0, null_sta, null_key);
}
--
2.19.2
[PATCH v2 03/12] staging: rtl8188eu: simplify array initializations
Simplfy initialization of arrays with zero only values
to improve readability and save a line.
Signed-off-by: Michael Straube
---
drivers/staging/rtl8188eu/core/rtw_wlan_util.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
index fac1c1c20b2f..76c7010c3a5c 100644
--- a/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8188eu/core/rtw_wlan_util.c
@@ -455,9 +455,8 @@ void write_cam(struct adapter *padapter, u8 entry, u16
ctrl, u8 *mac, u8 *key)
void clear_cam_entry(struct adapter *padapter, u8 entry)
{
- u8 null_sta[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
- u8 null_key[] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
-0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ u8 null_sta[ETH_ALEN] = {};
+ u8 null_key[16] = {};
write_cam(padapter, entry, 0, null_sta, null_key);
}
--
2.19.2
Re: [PATCH v6 10/24] arm64: irqflags: Use ICC_PMR_EL1 for interrupt masking
On Wed, Dec 05, 2018 at 04:55:54PM +, Julien Thierry wrote:
> On 04/12/18 17:36, Catalin Marinas wrote:
> > On Mon, Nov 12, 2018 at 11:57:01AM +, Julien Thierry wrote:
> >> diff --git a/arch/arm64/include/asm/irqflags.h
> >> b/arch/arm64/include/asm/irqflags.h
> >> index 24692ed..e0a32e4 100644
> >> --- a/arch/arm64/include/asm/irqflags.h
> >> +++ b/arch/arm64/include/asm/irqflags.h
> >> @@ -18,7 +18,27 @@
> >>
> >> #ifdef __KERNEL__
> >>
> >> +#include
> >> +#include
> >> #include
> >> +#include
> >> +
> >> +
> >> +/*
> >> + * When ICC_PMR_EL1 is used for interrupt masking, only the bit indicating
> >> + * whether the normal interrupts are masked is kept along with the daif
> >> + * flags.
> >> + */
> >> +#define ARCH_FLAG_PMR_EN 0x1
> >> +
> >> +#define MAKE_ARCH_FLAGS(daif, pmr)
> >> \
> >> + ((daif) | (((pmr) >> GIC_PRIO_STATUS_SHIFT) & ARCH_FLAG_PMR_EN))
> >> +
> >> +#define ARCH_FLAGS_GET_PMR(flags) \
> >> + flags) & ARCH_FLAG_PMR_EN) << GIC_PRIO_STATUS_SHIFT) \
> >> + | GIC_PRIO_IRQOFF)
> >> +
> >> +#define ARCH_FLAGS_GET_DAIF(flags) ((flags) & ~ARCH_FLAG_PMR_EN)
> >
> > I wonder whether we could just use the PSR_I_BIT here to decide whether
> > to set the GIC_PRIO_IRQ{ON,OFF}. We could clear the PSR_I_BIT in
> > _restore_daif() with an alternative.
>
> So, the issue with it is that some contexts might be using PSR.I to
> disable interrupts (any contexts with async errors or debug exceptions
> disabled, kvm guest entry paths, pseudo-NMIs, ...).
>
> If any of these contexts calls local_irq_save()/local_irq_restore() or
> local_daif_save()/local_daif_restore(), by only relying on PSR_I_BIT to
> represent the PMR status, we might end up clearing PSR.I when we shouldn't.
>
> I'm not sure whether there are no callers of these functions in those
> context. But if that is the case, we could simplify things, yes.
There are callers of local_daif_save() (3) and local_daif_mask() (7) but
do they all need to disable the pseudo-NMIs?
At a brief look at x86, it seems that they have something like
stop_nmi() and restart_nmi(). These don't have save/restore semantics,
so we could do something similar on arm64 that only deals with the
PSTATE.I bit directly and keep the software (flags) PSR.I as the PMR
bit. But we'd have to go through the 10 local_daif_* cases above to see
which actually need the stop_nmi() semantics.
--
Catalin
Re: [PATCH v6 10/24] arm64: irqflags: Use ICC_PMR_EL1 for interrupt masking
On Wed, Dec 05, 2018 at 04:55:54PM +, Julien Thierry wrote:
> On 04/12/18 17:36, Catalin Marinas wrote:
> > On Mon, Nov 12, 2018 at 11:57:01AM +, Julien Thierry wrote:
> >> diff --git a/arch/arm64/include/asm/irqflags.h
> >> b/arch/arm64/include/asm/irqflags.h
> >> index 24692ed..e0a32e4 100644
> >> --- a/arch/arm64/include/asm/irqflags.h
> >> +++ b/arch/arm64/include/asm/irqflags.h
> >> @@ -18,7 +18,27 @@
> >>
> >> #ifdef __KERNEL__
> >>
> >> +#include
> >> +#include
> >> #include
> >> +#include
> >> +
> >> +
> >> +/*
> >> + * When ICC_PMR_EL1 is used for interrupt masking, only the bit indicating
> >> + * whether the normal interrupts are masked is kept along with the daif
> >> + * flags.
> >> + */
> >> +#define ARCH_FLAG_PMR_EN 0x1
> >> +
> >> +#define MAKE_ARCH_FLAGS(daif, pmr)
> >> \
> >> + ((daif) | (((pmr) >> GIC_PRIO_STATUS_SHIFT) & ARCH_FLAG_PMR_EN))
> >> +
> >> +#define ARCH_FLAGS_GET_PMR(flags) \
> >> + flags) & ARCH_FLAG_PMR_EN) << GIC_PRIO_STATUS_SHIFT) \
> >> + | GIC_PRIO_IRQOFF)
> >> +
> >> +#define ARCH_FLAGS_GET_DAIF(flags) ((flags) & ~ARCH_FLAG_PMR_EN)
> >
> > I wonder whether we could just use the PSR_I_BIT here to decide whether
> > to set the GIC_PRIO_IRQ{ON,OFF}. We could clear the PSR_I_BIT in
> > _restore_daif() with an alternative.
>
> So, the issue with it is that some contexts might be using PSR.I to
> disable interrupts (any contexts with async errors or debug exceptions
> disabled, kvm guest entry paths, pseudo-NMIs, ...).
>
> If any of these contexts calls local_irq_save()/local_irq_restore() or
> local_daif_save()/local_daif_restore(), by only relying on PSR_I_BIT to
> represent the PMR status, we might end up clearing PSR.I when we shouldn't.
>
> I'm not sure whether there are no callers of these functions in those
> context. But if that is the case, we could simplify things, yes.
There are callers of local_daif_save() (3) and local_daif_mask() (7) but
do they all need to disable the pseudo-NMIs?
At a brief look at x86, it seems that they have something like
stop_nmi() and restart_nmi(). These don't have save/restore semantics,
so we could do something similar on arm64 that only deals with the
PSTATE.I bit directly and keep the software (flags) PSR.I as the PMR
bit. But we'd have to go through the 10 local_daif_* cases above to see
which actually need the stop_nmi() semantics.
--
Catalin
Re: [PATCH 4.19 000/139] 4.19.7-stable review
On Wed, Dec 05, 2018 at 07:51:28PM +0530, Harsh Shandilya wrote: > On 4 December 2018 4:18:01 PM IST, Greg Kroah-Hartman > wrote: > >This is the start of the stable review cycle for the 4.19.7 release. > >There are 139 patches in this series, all will be posted as a response > >to this one. If anyone has any issues with these being applied, please > >let me know. > > > >Responses should be made by Thu Dec 6 10:36:22 UTC 2018. > >Anything received after that time might be too late. > > > >The whole patch series can be found in one patch at: > > > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.7-rc1.gz > >or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > >linux-4.19.y > >and the diffstat can be found below. > > > >thanks, > > > >greg k-h > Peachy as usual on the Lenovo IdeaPad 330-15ARR, no dmesg regressions. Great, thanks for testing and letting me know. greg k-h
Re: [PATCH 4.19 000/139] 4.19.7-stable review
On Wed, Dec 05, 2018 at 07:51:28PM +0530, Harsh Shandilya wrote: > On 4 December 2018 4:18:01 PM IST, Greg Kroah-Hartman > wrote: > >This is the start of the stable review cycle for the 4.19.7 release. > >There are 139 patches in this series, all will be posted as a response > >to this one. If anyone has any issues with these being applied, please > >let me know. > > > >Responses should be made by Thu Dec 6 10:36:22 UTC 2018. > >Anything received after that time might be too late. > > > >The whole patch series can be found in one patch at: > > > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.7-rc1.gz > >or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > >linux-4.19.y > >and the diffstat can be found below. > > > >thanks, > > > >greg k-h > Peachy as usual on the Lenovo IdeaPad 330-15ARR, no dmesg regressions. Great, thanks for testing and letting me know. greg k-h
