Re: [PATCH 4/4] crypto: caamhash: replace kmalloc with kzalloc

[yjin]

On 2015年03月02日 19:03, Horia Geantă wrote:

On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote:

From: Yanjiang Jin yanjiang@windriver.com

This can make sure we get a clean memory, else system would report
the below warning:

I'd avoid using kzalloc, it's an overhead on the hot path. kmalloc can
be used with a bit of attention to detail, i.e. what params to
explicitly initialize.

Got it. Just zeroing edesc-sec4_sg_bytes in V2.

Thanks!
Yanjiang


I see that the stack trace reports using WR Linux and a modified caam
driver - it uses NAPI (net_rx_action) instead of tasklet.

Have you actually reproduced the problem on upstream linux?
There are some fixes that already address similar (if not exact) problem:
76b99080ccc9 crypto: caam - fix uninitialized edesc-dst_dma fiel
45e9af78b1ab crypto: caam - fix uninitialized S/G table size in
ahash_digest

Thanks,
Horia


caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has 
not allocated [device address=0xdeadbeefdeadbeef] [size=18446744073150512879 
bytes]
[ cut here ]
WARNING: at lib/dma-debug.c:877
Modules linked in:
CPU: 1 PID: 98 Comm: cryptomgr_test Not tainted 3.10.62-ltsi-WR6.0.0.0_standard 
#175
task: c000f74bc400 ti: c000fffd task.ti: c000f775c000
NIP: c04f5ed8 LR: c04f5ed4 CTR: c055a160
REGS: c000fffd3650 TRAP: 0700   Not tainted  
(3.10.62-ltsi-WR6.0.0.0_standard)
MSR: 80029000 CE,EE,ME  CR: 24a48e84  XER: 
SOFTE: 1

004f5ed4 c000fffd38d0 c12af348 00a0
24a48e84  c125f1c8 01eb
0060 0001 10187373 0020
01eb c1fff780 c11ac928 c0007f003028
0097 0098 0098 c000f7758800
f7098c00 0001 0001 003f
f7098c00 0014 c0007f003000 c11b0e98
 c1565b80 c000fffd39e0 c000f72f2410
NIP [c04f5ed8] .check_unmap+0x848/0x9c0
LR [c04f5ed4] .check_unmap+0x844/0x9c0
Call Trace:
[c000fffd38d0] [c04f5ed4] .check_unmap+0x844/0x9c0 (unreliable)
[c000fffd3970] [c04f60d4] .debug_dma_unmap_page+0x84/0xb0
[c000fffd3aa0] [c08295cc] .ahash_done+0x1dc/0x360
[c000fffd3ca0] [c081b7ec] .caam_jr_dequeue+0x26c/0x3a0
[c000fffd3da0] [c08be50c] .net_rx_action+0x1cc/0x330
[c000fffd3e80] [c007276c] .__do_softirq+0x19c/0x3d0
[c000fffd3f90] [c0017054] .call_do_softirq+0x14/0x24
[c000f775ef10] [c0005fe8] .do_softirq+0x118/0x150
  sda: sda1 sda2 sda3
[c000f775efa0] [c0072c54] .irq_exit+0x124/0x140
[c000f775f020] [c0005ac4] .do_IRQ+0x184/0x370
[c000f775f0d0] [c001b93c] exc_0x500_common+0xfc/0x100
--- Exception: 501 at .rcu_note_context_switch+0x0/0x370
edule+0xbc/0x7f0
[c000f775f3c0] [c0a29944] .__schedule+0xa4/0x7f0 (unreliable)
[c000f775f620] [c0a277f4] .schedule_timeout+0x1b4/0x2e0
[c000f775f700] [c0a29428] .wait_for_common+0xf8/0x1d0
[c000f775f7c0] [c0a295ac] 
.wait_for_completion_interruptible+0x2c/0x50
[c000f775f840] [c0494b64] 
.do_one_async_hash_op.isra.1.part.2+0x24/0x50
[c000f775f8c0] [c04951a8] .test_hash+0x618/0x7d0
[c000f775fb30] [c0495424] .alg_test_hash+0xc4/0xf0
[c000f775fbc0] [c0494928] .alg_test+0xa8/0x2c0
[c000f775fcb0] [c0491164] .cryptomgr_test+0x64/0x80
[c000f775fd30] [c009a8d0] .kthread+0xf0/0x100
[c000f775fe30] [ca08] .ret_from_kernel_thread+0x5c/0xd4
Instruction dump:
7c641b78 419e0160 e8a90050 2fa5 409e0008 e8a90010 e8de0028 e8fe0030
3c62ff90 38638320 48546b69 6000 0fe0 4b34 e87e0010 2fa3
---[ end trace 52825d316d569f00 ]---

Signed-off-by: Yanjiang Jin yanjiang@windriver.com
---






--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] crypto: caamhash: add two missed dma_mapping_error

[yjin]

On 2015年03月02日 19:53, Horia Geantă wrote:

On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote:

From: Yanjiang Jin yanjiang@windriver.com

Add two missed dma_mapping_error() after dma_map_single().

Signed-off-by: Yanjiang Jin yanjiang@windriver.com
---
  drivers/crypto/caam/caamhash.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index f347ab7..f6ad322 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -160,6 +160,10 @@ static inline dma_addr_t map_seq_out_ptr_result(u32 *desc, 
struct device *jrdev,
dma_addr_t dst_dma;
  
  	dst_dma = dma_map_single(jrdev, result, digestsize, DMA_FROM_DEVICE);

+   if (dma_mapping_error(jrdev, dst_dma)) {
+   dev_err(jrdev, unable to map dst dma\n);
+   return -ENOMEM;
+   }
append_seq_out_ptr(desc, dst_dma, digestsize, 0);
  
  	return dst_dma;

Value returned by map_seq_out_ptr_result() - dst_dma - is always fed to
dma_mapping_error().
Note that using an invalid dst_dma in append_seq_out_ptr() doesn't break
anything, so it's ok to check dst_dma later.


@@ -173,6 +177,10 @@ static inline dma_addr_t buf_map_to_sec4_sg(struct device 
*jrdev,
dma_addr_t buf_dma;
  
  	buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE);

+   if (dma_mapping_error(jrdev, buf_dma)) {
+   dev_err(jrdev, unable to map buf dma\n);
+   return 0;
+   }
dma_to_sec4_sg_one(sec4_sg, buf_dma, buflen, 0);
  
  	return buf_dma;



These functions are expected to return dma_addr_t, not an error code.
If dma_mapping_error() is needed within their scope, the return type
will have to change. And return value will need to be checked by their
callers.

System run well without the above changes, so abandoned this patch in 
V2. Will do more test in the future.


Thanks!
Yanjiang





--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem

[yjin]

On 2015年03月05日 02:36, Kim Phillips wrote:

On Wed, 4 Mar 2015 13:33:22 +0800
yjin yanjiang@windriver.com wrote:


On 2015年03月04日 03:31, Kim Phillips wrote:

On Tue, 3 Mar 2015 14:50:52 +0800
yanjiang@windriver.com wrote:


-   dma_unmap_single(jrdev, ctx-sh_desc_dma, DESC_RNG_LEN,
-DMA_TO_DEVICE);
+   dma_unmap_single(jrdev, ctx-sh_desc_dma,
+   desc_bytes(ctx-sh_desc), DMA_TO_DEVICE);

alignment: the 'd' in desc_bytes should fall directly under the 'j'
in jrdev.

Also, DESC_RNG_LEN should be corrected to (4 * CAAM_CMD_SZ).

Hi Kim,

I can't find the obvious limitation for the RNG descriptor length in
Freescale documents, could you point out it?

?  rng_create_sh_desc() creates a fixed descriptor of exactly 4
command-lengths.


Hi Kim,

Do you mean that the code itself limits the descriptor length? Not a 
hardware limitation.
If so, I prefer to dma_unmap with desc_bytes(ctx-sh_desc) as my 
previous patch, and correct DESC_RNG_LEN to (4 * CAAM_CMD_SZ).


Thanks!
Yanjiang



Kim


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] crypto: caam: fix some compile warnings

[yjin]

On 2015年03月04日 10:32, yjin wrote:


On 2015年03月04日 02:59, Kim Phillips wrote:

On Tue, 3 Mar 2015 14:50:51 +0800
yanjiang@windriver.com wrote:


This commit is to avoid the below warnings:

drivers/crypto/caam/sg_sw_sec4.h:88:12: warning:
'dma_map_sg_chained' defined but not used [-Wunused-function]
  static int dma_map_sg_chained(struct device *dev, struct 
scatterlist *sg,

 ^
drivers/crypto/caam/sg_sw_sec4.h:104:12: warning:
'dma_unmap_sg_chained' defined but not used [-Wunused-function]
  static int dma_unmap_sg_chained(struct device *dev,
 ^

I'm not seeing these warnings - both caamalg.c and caamhash.c use
those functions fine.


As you said, both caamalg.c and caamhash.c use those functions, so no 
warning reported.


But if a new file just wants to include sg_sw_sec4.h, doesn't want 
to use these functions, the above warnings will appear.


We can find an example in Freescale SDK 1.6:
caampkc.c includes pkc_desc.h, pkc_desc.h includes sg_sw_sec4.h, but 
caampkc.c doesn't call those functions.


Without my patch, every file which includes sg_sw_sec4.h must call 
these two functions in the future, I don't think it is a good idea.


Thanks!
Yanjiang


-static int dma_map_sg_chained(struct device *dev, struct 
scatterlist *sg,
+static inline int dma_map_sg_chained(struct device *dev, struct 
scatterlist *sg,

unsigned int nents, enum dma_data_direction dir,
bool chained)

not to mention this isn't how to fix a defined but not used warning:
marking the functions inline results in different compiler output.

NACK from me.
An alternative is moving the definitions to a .c file, but I don't 
think it will be fundamental different.
I know I am fixing a potential error which doesn't exist now, it seems 
useless for the current upstream version, we can abandon my patch. But I 
still think the current implementation adds unnecessary restrictions for 
its users.


Thanks!
Yanjiang


Kim




--
To unsubscribe from this list: send the line unsubscribe 
linux-kernel in

the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/




--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem

[yjin]

On 2015年03月04日 03:31, Kim Phillips wrote:

On Tue, 3 Mar 2015 14:50:52 +0800
yanjiang@windriver.com wrote:


-   dma_unmap_single(jrdev, ctx-sh_desc_dma, DESC_RNG_LEN,
-DMA_TO_DEVICE);
+   dma_unmap_single(jrdev, ctx-sh_desc_dma,
+   desc_bytes(ctx-sh_desc), DMA_TO_DEVICE);

alignment: the 'd' in desc_bytes should fall directly under the 'j'
in jrdev.

Also, DESC_RNG_LEN should be corrected to (4 * CAAM_CMD_SZ).


Hi Kim,

I can't find the obvious limitation for the RNG descriptor length in 
Freescale documents, could you point out it?


Thanks!
Yanjiang


Kim




--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] crypto: caam: fix some compile warnings

[yjin]

On 2015年03月04日 02:59, Kim Phillips wrote:

On Tue, 3 Mar 2015 14:50:51 +0800
yanjiang@windriver.com wrote:


This commit is to avoid the below warnings:

drivers/crypto/caam/sg_sw_sec4.h:88:12: warning:
'dma_map_sg_chained' defined but not used [-Wunused-function]
  static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
 ^
drivers/crypto/caam/sg_sw_sec4.h:104:12: warning:
'dma_unmap_sg_chained' defined but not used [-Wunused-function]
  static int dma_unmap_sg_chained(struct device *dev,
 ^

I'm not seeing these warnings - both caamalg.c and caamhash.c use
those functions fine.


As you said, both caamalg.c and caamhash.c use those functions, so no 
warning reported.


But if a new file just wants to include sg_sw_sec4.h, doesn't want to 
use these functions, the above warnings will appear.


We can find an example in Freescale SDK 1.6:
caampkc.c includes pkc_desc.h, pkc_desc.h includes sg_sw_sec4.h, but 
caampkc.c doesn't call those functions.


Without my patch, every file which includes sg_sw_sec4.h must call these 
two functions in the future, I don't think it is a good idea.


Thanks!
Yanjiang



-static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
+static inline int dma_map_sg_chained(struct device *dev, struct scatterlist 
*sg,
  unsigned int nents, enum dma_data_direction dir,
  bool chained)

not to mention this isn't how to fix a defined but not used warning:
marking the functions inline results in different compiler output.

NACK from me.

Kim




--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RT PATCH] sched: rt: fix two possible deadlocks in push_irq_work_func

[yjin]

On 2015年11月14日 12:25, Steven Rostedt wrote:

On Sat, 14 Nov 2015 10:53:18 +0800
 wrote:


From: Yanjiang Jin 

This can only happen in RT kernel due to run_timer_softirq() calls
irq_work_tick() when CONFIG_PREEMPT_RT_FULL is enabled as below:

static void run_timer_softirq(struct softirq_action *h)
{

if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)
 irq_work_tick();
endif

}

Use raw_spin_{un,}lock_irq{save,restore} in push_irq_work_func() to
prevent following potentially deadlock scenario:

Ug. No, the real fix is that the irq work is to be run from hard
interrupt context.

But if so, we shouldn't call irq_work_tick() in run_timer_softirq(), right?

Thanks!
Yanjiang

Moving the scheduling of high priority real-time
tasks to ksoftirqd defeats the purpose. The question is, why is that
irq work being run from thread context when it has the
IRQ_WORK_HARD_IRQ flag set?

-- Steve



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RT PATCH] sched: rt: fix two possible deadlocks in push_irq_work_func

[yjin]

Hi Thomas,

Thanks for your help.
I have tested your patch this morning, it seems well.
Could you tell me when will you send your formal patch? And please CC me 
when you send your patch.


Regards!
Yanjiang

On 2015年11月15日 19:05, Thomas Gleixner wrote:

On Sun, 15 Nov 2015, Thomas Gleixner wrote:

Which does not happen on MIPS as it uses the generic
arch_irq_work_has_interrupt() implementation which returns 'false'.

So the proper fix is to ensure that the irq safe irq work actually
happens in interrupt context. Patch below.

Thanks,

tglx

8<-

diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
index 0e427a9997f3..2543aab05daa 100644
--- a/include/linux/irq_work.h
+++ b/include/linux/irq_work.h
@@ -52,4 +52,10 @@ static inline bool irq_work_needs_cpu(void) { return false; }
  static inline void irq_work_run(void) { }
  #endif
  
+#if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)

+void irq_work_tick_soft(void);
+#else
+static inline void irq_work_tick_soft(void) { }
+#endif
+
  #endif /* _LINUX_IRQ_WORK_H */
diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 5a0f4525139c..58cf46638ca0 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -200,8 +200,17 @@ void irq_work_tick(void)
  
  	if (!llist_empty(raised) && !arch_irq_work_has_interrupt())

irq_work_run_list(raised);
+
+   if (!IS_ENABLED(CONFIG_PREEMPT_RT_FULL))
+   irq_work_run_list(this_cpu_ptr(_list));
+}
+
+#if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)
+void irq_work_tick_soft(void)
+{
irq_work_run_list(this_cpu_ptr(_list));
  }
+#endif
  
  /*

   * Synchronize against the irq_work @entry, ensures the entry is not
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index adb1d82d6631..c68ba873da3c 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1455,7 +1455,7 @@ void update_process_times(int user_tick)
scheduler_tick();
run_local_timers();
rcu_check_callbacks(user_tick);
-#if defined(CONFIG_IRQ_WORK) && !defined(CONFIG_PREEMPT_RT_FULL)
+#if defined(CONFIG_IRQ_WORK)
if (in_irq())
irq_work_tick();
  #endif
@@ -1471,9 +1471,7 @@ static void run_timer_softirq(struct softirq_action *h)
  
  	hrtimer_run_pending();
  
-#if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)

-   irq_work_tick();
-#endif
+   irq_work_tick_soft();
  
  	if (time_after_eq(jiffies, base->timer_jiffies))

__run_timers(base);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] vmcore: replace Elf64_Ehdr/Elf32_Ehdr with elfhdr

[yjin]

On 2015年09月16日 18:39, Minfei Huang wrote:

On 09/16/15 at 10:58am, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

Function parse_crash_elf_headers() reads e_ident[EI_CLASS] then decides to
call parse_crash_elf64_headers() or parse_crash_elf32_headers().
But this happens in run time, not compile time. So compiler will report
the below warning:

In file included from include/linux/elf.h:4:0,
  from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf32_headers':
arch/mips/include/asm/elf.h:258:23: warning: initializatio
n from incompatible pointer type
   struct elfhdr *__h = (hdr); \
^

How about converting the hdr to type elfhdr in above sentence, like
following.

struct elfhdr *__h = (struct elfhdr *)(hdr);


Yes, this is a replacement, and it seems more safe because it just 
affects MIPS arch.

But I also can't see any obvious impact if modifying common vmcore.c:-)
Anyway, if you stick to your opinion, I can send a V2 patch to update 
mips' elf.h rather than vmcore.c.


Thanks!
Yanjiang


Thanks
Minfei


fs/proc/vmcore.c:1071:4: note: in expansion of macro 'elf_
check_arch'
!elf_check_arch() ||
 ^

Signed-off-by: Yanjiang Jin 
---
  fs/proc/vmcore.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 4e61388..576bb26 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -999,7 +999,7 @@ static void free_elfcorebuf(void)
  static int __init parse_crash_elf64_headers(void)
  {
int rc=0;
-   Elf64_Ehdr ehdr;
+   struct elfhdr ehdr;
u64 addr;
  
  	addr = elfcorehdr_addr;

@@ -1055,7 +1055,7 @@ fail:
  static int __init parse_crash_elf32_headers(void)
  {
int rc=0;
-   Elf32_Ehdr ehdr;
+   struct elfhdr ehdr;
u64 addr;
  
  	addr = elfcorehdr_addr;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] mips: vmcore: forced convert 'hdr' in elf_check_arch()

[yjin]

The new version patch only modifies mips/elf.h, so add Ralf Baechle and 
cc linux-m...@linux-mips.org.

This is a V2 patch, attach the V1 patch for reference.

Thanks!
Yanjiang

On 2015年09月18日 15:42, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

elf_check_arch() will be called both in parse_crash_elf64_headers()
and parse_crash_elf32_headers(). But in these two functions, the type of
the parameter ehdr is different: Elf32_Ehdr and Elf64_Ehdr.

Function parse_crash_elf_headers() reads e_ident[EI_CLASS] then decides to
call parse_crash_elf64_headers() or parse_crash_elf32_headers().
This happens in run time, not compile time. So compiler will report
the below warning:

In file included from include/linux/elf.h:4:0,
  from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf32_headers':
arch/mips/include/asm/elf.h:258:23: warning: initializatio
n from incompatible pointer type
   struct elfhdr *__h = (hdr); \
^
fs/proc/vmcore.c:1071:4: note: in expansion of macro 'elf_
check_arch'
!elf_check_arch() ||
 ^

Signed-off-by: Yanjiang Jin 
---
  arch/mips/include/asm/elf.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index f19e890..ece490d 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -224,7 +224,7 @@ struct mips_elf_abiflags_v0 {
  #define elf_check_arch(hdr)   \
  ({\
int __res = 1;  \
-   struct elfhdr *__h = (hdr); \
+   struct elfhdr *__h = (struct elfhdr *)(hdr);\
\
if (__h->e_machine != EM_MIPS)   \
__res = 0;  \
@@ -255,7 +255,7 @@ struct mips_elf_abiflags_v0 {
  #define elf_check_arch(hdr)   \
  ({\
int __res = 1;  \
-   struct elfhdr *__h = (hdr); \
+   struct elfhdr *__h = (struct elfhdr *)(hdr);\
\
if (__h->e_machine != EM_MIPS)   \
__res = 0;  \




Re: [PATCH] vmcore: replace Elf64_Ehdr_Elf32_Ehdr with elfhdr.eml
Description: application/extension-eml

Re: [PATCH] mips: vmcore: forced convert 'hdr' in elf_check_arch()

[yjin]

It seems the last mail has been blocked, resend it.

On 2015年09月21日 10:16, yjin wrote:
The new version patch only modifies mips/elf.h, so add Ralf Baechle 
and cc linux-m...@linux-mips.org.

This is a V2 patch, attach the V1 patch for reference.

Thanks!
Yanjiang

On 2015年09月18日 15:42, yanjiang@windriver.com wrote:

From: Yanjiang Jin <yanjiang@windriver.com>

elf_check_arch() will be called both in parse_crash_elf64_headers()
and parse_crash_elf32_headers(). But in these two functions, the type of
the parameter ehdr is different: Elf32_Ehdr and Elf64_Ehdr.

Function parse_crash_elf_headers() reads e_ident[EI_CLASS] then 
decides to

call parse_crash_elf64_headers() or parse_crash_elf32_headers().
This happens in run time, not compile time. So compiler will report
the below warning:

In file included from include/linux/elf.h:4:0,
  from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf32_headers':
arch/mips/include/asm/elf.h:258:23: warning: initializatio
n from incompatible pointer type
   struct elfhdr *__h = (hdr); \
^
fs/proc/vmcore.c:1071:4: note: in expansion of macro 'elf_
check_arch'
!elf_check_arch() ||
 ^

Signed-off-by: Yanjiang Jin <yanjiang@windriver.com>
---
  arch/mips/include/asm/elf.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index f19e890..ece490d 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -224,7 +224,7 @@ struct mips_elf_abiflags_v0 {
  #define elf_check_arch(hdr)\
  ({\
  int __res = 1;\
-struct elfhdr *__h = (hdr);\
+struct elfhdr *__h = (struct elfhdr *)(hdr);\
  \
  if (__h->e_machine != EM_MIPS)\
  __res = 0;\
@@ -255,7 +255,7 @@ struct mips_elf_abiflags_v0 {
  #define elf_check_arch(hdr)\
  ({\
  int __res = 1;\
-struct elfhdr *__h = (hdr);\
+struct elfhdr *__h = (struct elfhdr *)(hdr);\
  \
  if (__h->e_machine != EM_MIPS)\
  __res = 0;\




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] crypto: rsa: rename two rsa key files

[yjin]

Thanks for Herbert's reminder.
I have drop this patch in a previous mail.

Regards!
Yanjiang

On 2016年11月08日 20:09, Herbert Xu wrote:

yanjiang@windriver.com wrote:

From: Yanjiang Jin 

This is to eliminate the below compile error:

crypto/rsa_helper.c:19:29: fatal error: rsaprivkey-asn1.h: No such file or 
directory
#include "rsaprivkey-asn1.h"
 ^
compilation terminated.

Signed-off-by: Yanjiang Jin 

This patch is bogus.  The header files are meant to be generated.
Please find out why they are not being generated in your case.

Thanks,

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

Hi Scott,

Thanks for your reminder!
I rephrased it as below:

-#define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct)   \
+   (u64)(cputime_to_clock_t((cputime_t)(ct)))


and rebuilt the kernel, no warnings found.
New patch is attached, could you also help me to review it?

Regards!
Yanjiang

On 2016年11月30日 22:40, Scott Wood wrote:

On Mon, 2016-11-21 at 12:56 +0800, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

This patch is to avoid the below warning:

kernel/sched/cpuacct.c:298:25: warning:
format '%lld' expects argument of type 'long long int',
but argument 4 has type 'long unsigned int' [-Wformat=]

Signed-off-by: Yanjiang Jin 
---
  arch/powerpc/include/asm/cputime.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputime.h
b/arch/powerpc/include/asm/cputime.h
index 4f60db0..4423e97 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const
unsigned long clk)
return (__force cputime_t) ct;
  }
  
-#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))

+#define cputime64_to_clock_t(ct)   \
+   (__force u64)(cputime_to_clock_t((cputime_t)(ct)))

Why is __force needed?

-Scott



>From c32bc6ffe070e3d79a420e6adcbed5709079dc23 Mon Sep 17 00:00:00 2001
From: Yanjiang Jin 
Date: Sat, 19 Nov 2016 05:04:53 +
Subject: [PATCH] powerpc: cputime: fix a compile warning

This patch is to avoid the below warning:

kernel/sched/cpuacct.c:298:25: warning:
format '%lld' expects argument of type 'long long int',
but argument 4 has type 'long unsigned int' [-Wformat=]

Signed-off-by: Yanjiang Jin 
---
 arch/powerpc/include/asm/cputime.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 4f60db0..a25082b 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned long clk)
 	return (__force cputime_t) ct;
 }
 
-#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct)	\
+	(u64)(cputime_to_clock_t((cputime_t)(ct)))
 
 /*
  * PPC64 uses PACA which is task independent for storing accounting data while
-- 
1.9.1

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

On 2016年12月02日 12:22, Balbir Singh wrote:

On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman  wrote:

yanjiang@windriver.com writes:


diff --git a/arch/powerpc/include/asm/cputime.h 
b/arch/powerpc/include/asm/cputime.h
index 4f60db0..4423e97 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned 
long clk)
   return (__force cputime_t) ct;
  }

-#define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct) \
+ (__force u64)(cputime_to_clock_t((cputime_t)(ct)))

Given the name of the function is "cputime64 to clock_t", surely we
should be returning a clock_t ?

Please fix it in cpuacct.c  Also check out git commit
527b0a76f41d062381adbb55c8eb61e32cb0bfc9
sched/cpuacct: Avoid %lld seq_printf warning


Hi Balbir,

Where can I find this commit?

Thanks!
Yanjiang


Balbir

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

On 2016年12月02日 13:15, Pan Xinhui wrote:



在 2016/12/2 12:35, yjin 写道:


On 2016年12月02日 12:22, Balbir Singh wrote:
On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman 
<m...@ellerman.id.au> wrote:

yanjiang@windriver.com writes:

diff --git a/arch/powerpc/include/asm/cputime.h 
b/arch/powerpc/include/asm/cputime.h

index 4f60db0..4423e97 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t 
clock_t_to_cputime(const unsigned long clk)

   return (__force cputime_t) ct;
  }

-#define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct) \
+ (__force u64)(cputime_to_clock_t((cputime_t)(ct)))

Given the name of the function is "cputime64 to clock_t", surely we
should be returning a clock_t ?

Please fix it in cpuacct.c  Also check out git commit
527b0a76f41d062381adbb55c8eb61e32cb0bfc9
sched/cpuacct: Avoid %lld seq_printf warning


Hi Balbir,

Where can I find this commit?


hello,
it is in next tree. :)


Got it. Thanks!

Regards!
Yanjiang



commit 527b0a76f41d062381adbb55c8eb61e32cb0bfc9
Author: Martin Schwidefsky <schwidef...@de.ibm.com>
Date:   Fri Nov 11 15:27:49 2016 +0100

sched/cpuacct: Avoid %lld seq_printf warning
For s390 kernel builds I keep getting this warning:
 kernel/sched/cpuacct.c: In function 'cpuacct_stats_show':
 kernel/sched/cpuacct.c:298:25: warning: format '%lld' expects 
argument of type 'long long int', but argument 4 has type 'clock_t 
{aka long int}' [-Wformat=]

   seq_printf(sf, "%s %lld\n",
Silence the warning by adding an explicit cast.
Signed-off-by: Martin Schwidefsky <schwidef...@de.ibm.com>
Signed-off-by: Peter Zijlstra (Intel) <pet...@infradead.org>
Cc: Linus Torvalds <torva...@linux-foundation.org>
Cc: Peter Zijlstra <pet...@infradead.org>
Cc: Thomas Gleixner <t...@linutronix.de>
Link: 
http://lkml.kernel.org/r/2016142749.6545-1-schwidef...@de.ibm.com

Signed-off-by: Ingo Molnar <mi...@kernel.org>

diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index bc0b309c..9add206 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -297,7 +297,7 @@ static int cpuacct_stats_show(struct seq_file *sf, 
void *v)

for (stat = 0; stat < CPUACCT_STAT_NSTATS; stat++) {
seq_printf(sf, "%s %lld\n",
   cpuacct_stat_desc[stat],
-  cputime64_to_clock_t(val[stat]));
+  (long long)cputime64_to_clock_t(val[stat]));
}

return 0;


Thanks!
Yanjiang


Balbir

Re: [PATCH] watchdog: imx2_wdt: use new_timeout value to override the old

[yjin]

On 2017年07月20日 11:18, Guenter Roeck wrote:

On 07/19/2017 07:45 PM, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

Without this patch we couldn't change the timeout value of imx2_wdt.

Signed-off-by: Yanjiang Jin 
---
  drivers/watchdog/imx2_wdt.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 4874b0f..6cfeee6 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -178,6 +178,9 @@ static int imx2_wdt_set_timeout(struct 
watchdog_device *wdog,

regmap_update_bits(wdev->regmap, IMX2_WDT_WCR, IMX2_WDT_WCR_WT,
 WDOG_SEC_TO_COUNT(new_timeout));
+
+wdog->timeout = new_timeout;
+


I must be missing something. That very same code is already in line 177,
added with commit 30dd4a8f08b57 ("watchdog: imx2_wdt: also set 
wdog->timeout

to new_timeout") back in 2015.

Did you see this in an older kernel, maybe ?


Oh, I missed this. I did use the older kernel to validate it. Thanks for 
your reminder, please ignore my patch.

Thanks again!

Regards!
Yanjiang


Guenter

Re: [PATCH] socfpga_a10: reset CPU1 in socfpga_cpu_kill()

[yjin]

On 2017年05月12日 22:36, Mark Rutland wrote:

On Wed, May 10, 2017 at 01:13:04AM -0400, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

Kexec's second kernel would hang if CPU1 isn't reset.

Signed-off-by: Yanjiang Jin 
---
  arch/arm/mach-socfpga/platsmp.c | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-socfpga/platsmp.c b/arch/arm/mach-socfpga/platsmp.c
index 0ee7677..db3940e 100644
--- a/arch/arm/mach-socfpga/platsmp.c
+++ b/arch/arm/mach-socfpga/platsmp.c
@@ -117,6 +117,16 @@ static int socfpga_cpu_kill(unsigned int cpu)
  {
return 1;
  }
+
+static int socfpga_a10_cpu_kill(unsigned int cpu)
+{
+   /* This will put CPU #1 into reset. */
+   if (socfpga_cpu1start_addr)
+   writel(RSTMGR_MPUMODRST_CPU1, rst_manager_base_addr +
+   SOCFPGA_A10_RSTMGR_MODMPURST);
+
+   return 1;
+}
  #endif

I agree that currently, socfpga_cpu_die is completely bogus, as the CPU is just
sat in WFI with the MMU, caches, etc enabled, and not actually off:

static void socfpga_cpu_die(unsigned int cpu)
{
 /* Do WFI. If we wake up early, go back into WFI */
 while (1)
 cpu_do_idle();
}

... so that goes wrong as soon as the kerenl text gets ovewritten.

However, AFAICT, this patch forcibly resets is without any teardown
having happened. That will surely result in data being lost from the
caches, for example.

So I think this is incomplete.

Hi Mark,

I think I got what you mean. So far, flush_cache is the only thing that 
I can think of.
Can we just add flush_cache_all in cpu_die() now, and add other missing 
parts(if have) in the future.


Thanks!
Yanjiang


Thanks,
Mark.

  
  static const struct smp_operations socfpga_smp_ops __initconst = {

@@ -133,7 +143,7 @@ static int socfpga_cpu_kill(unsigned int cpu)
.smp_boot_secondary = socfpga_a10_boot_secondary,
  #ifdef CONFIG_HOTPLUG_CPU
.cpu_die= socfpga_cpu_die,
-   .cpu_kill   = socfpga_cpu_kill,
+   .cpu_kill   = socfpga_a10_cpu_kill,
  #endif
  };
  
--

1.9.1


___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] watchdog: imx2_wdt: use new_timeout value to override the old

[yjin]

On 2017年07月20日 11:18, Guenter Roeck wrote:

On 07/19/2017 07:45 PM, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

Without this patch we couldn't change the timeout value of imx2_wdt.

Signed-off-by: Yanjiang Jin 
---
  drivers/watchdog/imx2_wdt.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/drivers/watchdog/imx2_wdt.c b/drivers/watchdog/imx2_wdt.c
index 4874b0f..6cfeee6 100644
--- a/drivers/watchdog/imx2_wdt.c
+++ b/drivers/watchdog/imx2_wdt.c
@@ -178,6 +178,9 @@ static int imx2_wdt_set_timeout(struct 
watchdog_device *wdog,

regmap_update_bits(wdev->regmap, IMX2_WDT_WCR, IMX2_WDT_WCR_WT,
 WDOG_SEC_TO_COUNT(new_timeout));
+
+wdog->timeout = new_timeout;
+


I must be missing something. That very same code is already in line 177,
added with commit 30dd4a8f08b57 ("watchdog: imx2_wdt: also set 
wdog->timeout

to new_timeout") back in 2015.

Did you see this in an older kernel, maybe ?


Oh, I missed this. I did use the older kernel to validate it. Thanks for 
your reminder, please ignore my patch.

Thanks again!

Regards!
Yanjiang


Guenter

Re: [PATCH 4/4] crypto: caamhash: replace kmalloc with kzalloc

[yjin]

On 2015年03月02日 19:03, Horia Geantă wrote:

On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

This can make sure we get a clean memory, else system would report
the below warning:

I'd avoid using kzalloc, it's an overhead on the hot path. kmalloc can
be used with a bit of attention to detail, i.e. what params to
explicitly initialize.

Got it. Just zeroing edesc->sec4_sg_bytes in V2.

Thanks!
Yanjiang


I see that the stack trace reports using WR Linux and a modified caam
driver - it uses NAPI (net_rx_action) instead of tasklet.

Have you actually reproduced the problem on upstream linux?
There are some fixes that already address similar (if not exact) problem:
76b99080ccc9 "crypto: caam - fix uninitialized edesc->dst_dma fiel"
45e9af78b1ab "crypto: caam - fix uninitialized S/G table size in
ahash_digest"

Thanks,
Horia


caam_jr ffe301000.jr: DMA-API: device driver tries to free DMA memory it has 
not allocated [device address=0xdeadbeefdeadbeef] [size=18446744073150512879 
bytes]
[ cut here ]
WARNING: at lib/dma-debug.c:877
Modules linked in:
CPU: 1 PID: 98 Comm: cryptomgr_test Not tainted 3.10.62-ltsi-WR6.0.0.0_standard 
#175
task: c000f74bc400 ti: c000fffd task.ti: c000f775c000
NIP: c04f5ed8 LR: c04f5ed4 CTR: c055a160
REGS: c000fffd3650 TRAP: 0700   Not tainted  
(3.10.62-ltsi-WR6.0.0.0_standard)
MSR: 80029000   CR: 24a48e84  XER: 
SOFTE: 1

004f5ed4 c000fffd38d0 c12af348 00a0
24a48e84  c125f1c8 01eb
0060 0001 10187373 0020
01eb c1fff780 c11ac928 c0007f003028
0097 0098 0098 c000f7758800
f7098c00 0001 0001 003f
f7098c00 0014 c0007f003000 c11b0e98
 c1565b80 c000fffd39e0 c000f72f2410
NIP [c04f5ed8] .check_unmap+0x848/0x9c0
LR [c04f5ed4] .check_unmap+0x844/0x9c0
Call Trace:
[c000fffd38d0] [c04f5ed4] .check_unmap+0x844/0x9c0 (unreliable)
[c000fffd3970] [c04f60d4] .debug_dma_unmap_page+0x84/0xb0
[c000fffd3aa0] [c08295cc] .ahash_done+0x1dc/0x360
[c000fffd3ca0] [c081b7ec] .caam_jr_dequeue+0x26c/0x3a0
[c000fffd3da0] [c08be50c] .net_rx_action+0x1cc/0x330
[c000fffd3e80] [c007276c] .__do_softirq+0x19c/0x3d0
[c000fffd3f90] [c0017054] .call_do_softirq+0x14/0x24
[c000f775ef10] [c0005fe8] .do_softirq+0x118/0x150
  sda: sda1 sda2 sda3
[c000f775efa0] [c0072c54] .irq_exit+0x124/0x140
[c000f775f020] [c0005ac4] .do_IRQ+0x184/0x370
[c000f775f0d0] [c001b93c] exc_0x500_common+0xfc/0x100
--- Exception: 501 at .rcu_note_context_switch+0x0/0x370
edule+0xbc/0x7f0
[c000f775f3c0] [c0a29944] .__schedule+0xa4/0x7f0 (unreliable)
[c000f775f620] [c0a277f4] .schedule_timeout+0x1b4/0x2e0
[c000f775f700] [c0a29428] .wait_for_common+0xf8/0x1d0
[c000f775f7c0] [c0a295ac] 
.wait_for_completion_interruptible+0x2c/0x50
[c000f775f840] [c0494b64] 
.do_one_async_hash_op.isra.1.part.2+0x24/0x50
[c000f775f8c0] [c04951a8] .test_hash+0x618/0x7d0
[c000f775fb30] [c0495424] .alg_test_hash+0xc4/0xf0
[c000f775fbc0] [c0494928] .alg_test+0xa8/0x2c0
[c000f775fcb0] [c0491164] .cryptomgr_test+0x64/0x80
[c000f775fd30] [c009a8d0] .kthread+0xf0/0x100
[c000f775fe30] [ca08] .ret_from_kernel_thread+0x5c/0xd4
Instruction dump:
7c641b78 419e0160 e8a90050 2fa5 409e0008 e8a90010 e8de0028 e8fe0030
3c62ff90 38638320 48546b69 6000 <0fe0> 4b34 e87e0010 2fa3
---[ end trace 52825d316d569f00 ]---

Signed-off-by: Yanjiang Jin 
---






--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 3/4] crypto: caamhash: add two missed dma_mapping_error

[yjin]

On 2015年03月02日 19:53, Horia Geantă wrote:

On 2/28/2015 8:00 AM, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

Add two missed dma_mapping_error() after dma_map_single().

Signed-off-by: Yanjiang Jin 
---
  drivers/crypto/caam/caamhash.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/drivers/crypto/caam/caamhash.c b/drivers/crypto/caam/caamhash.c
index f347ab7..f6ad322 100644
--- a/drivers/crypto/caam/caamhash.c
+++ b/drivers/crypto/caam/caamhash.c
@@ -160,6 +160,10 @@ static inline dma_addr_t map_seq_out_ptr_result(u32 *desc, 
struct device *jrdev,
dma_addr_t dst_dma;
  
  	dst_dma = dma_map_single(jrdev, result, digestsize, DMA_FROM_DEVICE);

+   if (dma_mapping_error(jrdev, dst_dma)) {
+   dev_err(jrdev, "unable to map dst dma\n");
+   return -ENOMEM;
+   }
append_seq_out_ptr(desc, dst_dma, digestsize, 0);
  
  	return dst_dma;

Value returned by map_seq_out_ptr_result() - dst_dma - is always fed to
dma_mapping_error().
Note that using an invalid dst_dma in append_seq_out_ptr() doesn't break
anything, so it's ok to check dst_dma later.


@@ -173,6 +177,10 @@ static inline dma_addr_t buf_map_to_sec4_sg(struct device 
*jrdev,
dma_addr_t buf_dma;
  
  	buf_dma = dma_map_single(jrdev, buf, buflen, DMA_TO_DEVICE);

+   if (dma_mapping_error(jrdev, buf_dma)) {
+   dev_err(jrdev, "unable to map buf dma\n");
+   return 0;
+   }
dma_to_sec4_sg_one(sec4_sg, buf_dma, buflen, 0);
  
  	return buf_dma;



These functions are expected to return dma_addr_t, not an error code.
If dma_mapping_error() is needed within their scope, the return type
will have to change. And return value will need to be checked by their
callers.

System run well without the above changes, so abandoned this patch in 
V2. Will do more test in the future.


Thanks!
Yanjiang





--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] crypto: caam: fix some compile warnings

[yjin]

On 2015年03月04日 02:59, Kim Phillips wrote:

On Tue, 3 Mar 2015 14:50:51 +0800
 wrote:


This commit is to avoid the below warnings:

drivers/crypto/caam/sg_sw_sec4.h:88:12: warning:
'dma_map_sg_chained' defined but not used [-Wunused-function]
  static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
 ^
drivers/crypto/caam/sg_sw_sec4.h:104:12: warning:
'dma_unmap_sg_chained' defined but not used [-Wunused-function]
  static int dma_unmap_sg_chained(struct device *dev,
 ^

I'm not seeing these warnings - both caamalg.c and caamhash.c use
those functions fine.


As you said, both caamalg.c and caamhash.c use those functions, so no 
warning reported.


But if a new file just wants to include "sg_sw_sec4.h", doesn't want to 
use these functions, the above warnings will appear.


We can find an example in Freescale SDK 1.6:
caampkc.c includes pkc_desc.h, pkc_desc.h includes sg_sw_sec4.h, but 
caampkc.c doesn't call those functions.


Without my patch, every file which includes sg_sw_sec4.h must call these 
two functions in the future, I don't think it is a good idea.


Thanks!
Yanjiang



-static int dma_map_sg_chained(struct device *dev, struct scatterlist *sg,
+static inline int dma_map_sg_chained(struct device *dev, struct scatterlist 
*sg,
  unsigned int nents, enum dma_data_direction dir,
  bool chained)

not to mention this isn't how to fix a defined but not used warning:
marking the functions inline results in different compiler output.

NACK from me.

Kim




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/3] crypto: caam: fix some compile warnings

[yjin]

On 2015年03月04日 10:32, yjin wrote:


On 2015年03月04日 02:59, Kim Phillips wrote:

On Tue, 3 Mar 2015 14:50:51 +0800
 wrote:


This commit is to avoid the below warnings:

drivers/crypto/caam/sg_sw_sec4.h:88:12: warning:
'dma_map_sg_chained' defined but not used [-Wunused-function]
  static int dma_map_sg_chained(struct device *dev, struct 
scatterlist *sg,

 ^
drivers/crypto/caam/sg_sw_sec4.h:104:12: warning:
'dma_unmap_sg_chained' defined but not used [-Wunused-function]
  static int dma_unmap_sg_chained(struct device *dev,
 ^

I'm not seeing these warnings - both caamalg.c and caamhash.c use
those functions fine.


As you said, both caamalg.c and caamhash.c use those functions, so no 
warning reported.


But if a new file just wants to include "sg_sw_sec4.h", doesn't want 
to use these functions, the above warnings will appear.


We can find an example in Freescale SDK 1.6:
caampkc.c includes pkc_desc.h, pkc_desc.h includes sg_sw_sec4.h, but 
caampkc.c doesn't call those functions.


Without my patch, every file which includes sg_sw_sec4.h must call 
these two functions in the future, I don't think it is a good idea.


Thanks!
Yanjiang


-static int dma_map_sg_chained(struct device *dev, struct 
scatterlist *sg,
+static inline int dma_map_sg_chained(struct device *dev, struct 
scatterlist *sg,

unsigned int nents, enum dma_data_direction dir,
bool chained)

not to mention this isn't how to fix a defined but not used warning:
marking the functions inline results in different compiler output.

NACK from me.
An alternative is moving the definitions to a ".c" file, but I don't 
think it will be fundamental different.
I know I am fixing a potential error which doesn't exist now, it seems 
useless for the current upstream version, we can abandon my patch. But I 
still think the current implementation adds unnecessary restrictions for 
its users.


Thanks!
Yanjiang


Kim




--
To unsubscribe from this list: send the line "unsubscribe 
linux-kernel" in

the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem

[yjin]

On 2015年03月04日 03:31, Kim Phillips wrote:

On Tue, 3 Mar 2015 14:50:52 +0800
 wrote:


-   dma_unmap_single(jrdev, ctx->sh_desc_dma, DESC_RNG_LEN,
-DMA_TO_DEVICE);
+   dma_unmap_single(jrdev, ctx->sh_desc_dma,
+   desc_bytes(ctx->sh_desc), DMA_TO_DEVICE);

alignment: the 'd' in desc_bytes should fall directly under the 'j'
in jrdev.

Also, DESC_RNG_LEN should be corrected to (4 * CAAM_CMD_SZ).


Hi Kim,

I can't find the obvious limitation for the RNG descriptor length in 
Freescale documents, could you point out it?


Thanks!
Yanjiang


Kim




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 2/3] crypto: caam_rng: fix rng_unmap_ctx's DMA_UNMAP size problem

[yjin]

On 2015年03月05日 02:36, Kim Phillips wrote:

On Wed, 4 Mar 2015 13:33:22 +0800
yjin  wrote:


On 2015年03月04日 03:31, Kim Phillips wrote:

On Tue, 3 Mar 2015 14:50:52 +0800
 wrote:


-   dma_unmap_single(jrdev, ctx->sh_desc_dma, DESC_RNG_LEN,
-DMA_TO_DEVICE);
+   dma_unmap_single(jrdev, ctx->sh_desc_dma,
+   desc_bytes(ctx->sh_desc), DMA_TO_DEVICE);

alignment: the 'd' in desc_bytes should fall directly under the 'j'
in jrdev.

Also, DESC_RNG_LEN should be corrected to (4 * CAAM_CMD_SZ).

Hi Kim,

I can't find the obvious limitation for the RNG descriptor length in
Freescale documents, could you point out it?

?  rng_create_sh_desc() creates a fixed descriptor of exactly 4
command-lengths.


Hi Kim,

Do you mean that the code itself limits the descriptor length? Not a 
hardware limitation.
If so, I prefer to dma_unmap with desc_bytes(ctx->sh_desc) as my 
previous patch, and correct DESC_RNG_LEN to (4 * CAAM_CMD_SZ).


Thanks!
Yanjiang



Kim


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] socfpga_a10: reset CPU1 in socfpga_cpu_kill()

[yjin]

On 2017年05月12日 22:36, Mark Rutland wrote:

On Wed, May 10, 2017 at 01:13:04AM -0400, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

Kexec's second kernel would hang if CPU1 isn't reset.

Signed-off-by: Yanjiang Jin 
---
  arch/arm/mach-socfpga/platsmp.c | 12 +++-
  1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-socfpga/platsmp.c b/arch/arm/mach-socfpga/platsmp.c
index 0ee7677..db3940e 100644
--- a/arch/arm/mach-socfpga/platsmp.c
+++ b/arch/arm/mach-socfpga/platsmp.c
@@ -117,6 +117,16 @@ static int socfpga_cpu_kill(unsigned int cpu)
  {
return 1;
  }
+
+static int socfpga_a10_cpu_kill(unsigned int cpu)
+{
+   /* This will put CPU #1 into reset. */
+   if (socfpga_cpu1start_addr)
+   writel(RSTMGR_MPUMODRST_CPU1, rst_manager_base_addr +
+   SOCFPGA_A10_RSTMGR_MODMPURST);
+
+   return 1;
+}
  #endif

I agree that currently, socfpga_cpu_die is completely bogus, as the CPU is just
sat in WFI with the MMU, caches, etc enabled, and not actually off:

static void socfpga_cpu_die(unsigned int cpu)
{
 /* Do WFI. If we wake up early, go back into WFI */
 while (1)
 cpu_do_idle();
}

... so that goes wrong as soon as the kerenl text gets ovewritten.

However, AFAICT, this patch forcibly resets is without any teardown
having happened. That will surely result in data being lost from the
caches, for example.

So I think this is incomplete.

Hi Mark,

I think I got what you mean. So far, flush_cache is the only thing that 
I can think of.
Can we just add flush_cache_all in cpu_die() now, and add other missing 
parts(if have) in the future.


Thanks!
Yanjiang


Thanks,
Mark.

  
  static const struct smp_operations socfpga_smp_ops __initconst = {

@@ -133,7 +143,7 @@ static int socfpga_cpu_kill(unsigned int cpu)
.smp_boot_secondary = socfpga_a10_boot_secondary,
  #ifdef CONFIG_HOTPLUG_CPU
.cpu_die= socfpga_cpu_die,
-   .cpu_kill   = socfpga_cpu_kill,
+   .cpu_kill   = socfpga_a10_cpu_kill,
  #endif
  };
  
--

1.9.1


___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] crypto: rsa: rename two rsa key files

[yjin]

Thanks for Herbert's reminder.
I have drop this patch in a previous mail.

Regards!
Yanjiang

On 2016年11月08日 20:09, Herbert Xu wrote:

yanjiang@windriver.com wrote:

From: Yanjiang Jin 

This is to eliminate the below compile error:

crypto/rsa_helper.c:19:29: fatal error: rsaprivkey-asn1.h: No such file or 
directory
#include "rsaprivkey-asn1.h"
 ^
compilation terminated.

Signed-off-by: Yanjiang Jin 

This patch is bogus.  The header files are meant to be generated.
Please find out why they are not being generated in your case.

Thanks,

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

On 2016年12月02日 12:22, Balbir Singh wrote:

On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman  wrote:

yanjiang@windriver.com writes:


diff --git a/arch/powerpc/include/asm/cputime.h 
b/arch/powerpc/include/asm/cputime.h
index 4f60db0..4423e97 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned 
long clk)
   return (__force cputime_t) ct;
  }

-#define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct) \
+ (__force u64)(cputime_to_clock_t((cputime_t)(ct)))

Given the name of the function is "cputime64 to clock_t", surely we
should be returning a clock_t ?

Please fix it in cpuacct.c  Also check out git commit
527b0a76f41d062381adbb55c8eb61e32cb0bfc9
sched/cpuacct: Avoid %lld seq_printf warning


Hi Balbir,

Where can I find this commit?

Thanks!
Yanjiang


Balbir

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

On 2016年12月02日 13:15, Pan Xinhui wrote:



在 2016/12/2 12:35, yjin 写道:


On 2016年12月02日 12:22, Balbir Singh wrote:
On Fri, Dec 2, 2016 at 3:15 PM, Michael Ellerman 
 wrote:

yanjiang@windriver.com writes:

diff --git a/arch/powerpc/include/asm/cputime.h 
b/arch/powerpc/include/asm/cputime.h

index 4f60db0..4423e97 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t 
clock_t_to_cputime(const unsigned long clk)

   return (__force cputime_t) ct;
  }

-#define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct) \
+ (__force u64)(cputime_to_clock_t((cputime_t)(ct)))

Given the name of the function is "cputime64 to clock_t", surely we
should be returning a clock_t ?

Please fix it in cpuacct.c  Also check out git commit
527b0a76f41d062381adbb55c8eb61e32cb0bfc9
sched/cpuacct: Avoid %lld seq_printf warning


Hi Balbir,

Where can I find this commit?


hello,
it is in next tree. :)


Got it. Thanks!

Regards!
Yanjiang



commit 527b0a76f41d062381adbb55c8eb61e32cb0bfc9
Author: Martin Schwidefsky 
Date:   Fri Nov 11 15:27:49 2016 +0100

sched/cpuacct: Avoid %lld seq_printf warning
For s390 kernel builds I keep getting this warning:
 kernel/sched/cpuacct.c: In function 'cpuacct_stats_show':
 kernel/sched/cpuacct.c:298:25: warning: format '%lld' expects 
argument of type 'long long int', but argument 4 has type 'clock_t 
{aka long int}' [-Wformat=]

   seq_printf(sf, "%s %lld\n",
Silence the warning by adding an explicit cast.
Signed-off-by: Martin Schwidefsky 
Signed-off-by: Peter Zijlstra (Intel) 
Cc: Linus Torvalds 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Link: 
http://lkml.kernel.org/r/2016142749.6545-1-schwidef...@de.ibm.com

Signed-off-by: Ingo Molnar 

diff --git a/kernel/sched/cpuacct.c b/kernel/sched/cpuacct.c
index bc0b309c..9add206 100644
--- a/kernel/sched/cpuacct.c
+++ b/kernel/sched/cpuacct.c
@@ -297,7 +297,7 @@ static int cpuacct_stats_show(struct seq_file *sf, 
void *v)

for (stat = 0; stat < CPUACCT_STAT_NSTATS; stat++) {
seq_printf(sf, "%s %lld\n",
   cpuacct_stat_desc[stat],
-  cputime64_to_clock_t(val[stat]));
+  (long long)cputime64_to_clock_t(val[stat]));
}

return 0;


Thanks!
Yanjiang


Balbir

Re: [PATCH] powerpc: cputime: fix a compile warning

[yjin]

Hi Scott,

Thanks for your reminder!
I rephrased it as below:

-#define cputime64_to_clock_t(ct) cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct)   \
+   (u64)(cputime_to_clock_t((cputime_t)(ct)))


and rebuilt the kernel, no warnings found.
New patch is attached, could you also help me to review it?

Regards!
Yanjiang

On 2016年11月30日 22:40, Scott Wood wrote:

On Mon, 2016-11-21 at 12:56 +0800, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

This patch is to avoid the below warning:

kernel/sched/cpuacct.c:298:25: warning:
format '%lld' expects argument of type 'long long int',
but argument 4 has type 'long unsigned int' [-Wformat=]

Signed-off-by: Yanjiang Jin 
---
  arch/powerpc/include/asm/cputime.h | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputime.h
b/arch/powerpc/include/asm/cputime.h
index 4f60db0..4423e97 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const
unsigned long clk)
return (__force cputime_t) ct;
  }
  
-#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))

+#define cputime64_to_clock_t(ct)   \
+   (__force u64)(cputime_to_clock_t((cputime_t)(ct)))

Why is __force needed?

-Scott



>From c32bc6ffe070e3d79a420e6adcbed5709079dc23 Mon Sep 17 00:00:00 2001
From: Yanjiang Jin 
Date: Sat, 19 Nov 2016 05:04:53 +
Subject: [PATCH] powerpc: cputime: fix a compile warning

This patch is to avoid the below warning:

kernel/sched/cpuacct.c:298:25: warning:
format '%lld' expects argument of type 'long long int',
but argument 4 has type 'long unsigned int' [-Wformat=]

Signed-off-by: Yanjiang Jin 
---
 arch/powerpc/include/asm/cputime.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/cputime.h b/arch/powerpc/include/asm/cputime.h
index 4f60db0..a25082b 100644
--- a/arch/powerpc/include/asm/cputime.h
+++ b/arch/powerpc/include/asm/cputime.h
@@ -228,7 +228,8 @@ static inline cputime_t clock_t_to_cputime(const unsigned long clk)
 	return (__force cputime_t) ct;
 }
 
-#define cputime64_to_clock_t(ct)	cputime_to_clock_t((cputime_t)(ct))
+#define cputime64_to_clock_t(ct)	\
+	(u64)(cputime_to_clock_t((cputime_t)(ct)))
 
 /*
  * PPC64 uses PACA which is task independent for storing accounting data while
-- 
1.9.1

Re: [RT PATCH] sched: rt: fix two possible deadlocks in push_irq_work_func

[yjin]

On 2015年11月14日 12:25, Steven Rostedt wrote:

On Sat, 14 Nov 2015 10:53:18 +0800
 wrote:


From: Yanjiang Jin 

This can only happen in RT kernel due to run_timer_softirq() calls
irq_work_tick() when CONFIG_PREEMPT_RT_FULL is enabled as below:

static void run_timer_softirq(struct softirq_action *h)
{

if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)
 irq_work_tick();
endif

}

Use raw_spin_{un,}lock_irq{save,restore} in push_irq_work_func() to
prevent following potentially deadlock scenario:

Ug. No, the real fix is that the irq work is to be run from hard
interrupt context.

But if so, we shouldn't call irq_work_tick() in run_timer_softirq(), right?

Thanks!
Yanjiang

Moving the scheduling of high priority real-time
tasks to ksoftirqd defeats the purpose. The question is, why is that
irq work being run from thread context when it has the
IRQ_WORK_HARD_IRQ flag set?

-- Steve



--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [RT PATCH] sched: rt: fix two possible deadlocks in push_irq_work_func

[yjin]

Hi Thomas,

Thanks for your help.
I have tested your patch this morning, it seems well.
Could you tell me when will you send your formal patch? And please CC me 
when you send your patch.


Regards!
Yanjiang

On 2015年11月15日 19:05, Thomas Gleixner wrote:

On Sun, 15 Nov 2015, Thomas Gleixner wrote:

Which does not happen on MIPS as it uses the generic
arch_irq_work_has_interrupt() implementation which returns 'false'.

So the proper fix is to ensure that the irq safe irq work actually
happens in interrupt context. Patch below.

Thanks,

tglx

8<-

diff --git a/include/linux/irq_work.h b/include/linux/irq_work.h
index 0e427a9997f3..2543aab05daa 100644
--- a/include/linux/irq_work.h
+++ b/include/linux/irq_work.h
@@ -52,4 +52,10 @@ static inline bool irq_work_needs_cpu(void) { return false; }
  static inline void irq_work_run(void) { }
  #endif
  
+#if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)

+void irq_work_tick_soft(void);
+#else
+static inline void irq_work_tick_soft(void) { }
+#endif
+
  #endif /* _LINUX_IRQ_WORK_H */
diff --git a/kernel/irq_work.c b/kernel/irq_work.c
index 5a0f4525139c..58cf46638ca0 100644
--- a/kernel/irq_work.c
+++ b/kernel/irq_work.c
@@ -200,8 +200,17 @@ void irq_work_tick(void)
  
  	if (!llist_empty(raised) && !arch_irq_work_has_interrupt())

irq_work_run_list(raised);
+
+   if (!IS_ENABLED(CONFIG_PREEMPT_RT_FULL))
+   irq_work_run_list(this_cpu_ptr(_list));
+}
+
+#if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)
+void irq_work_tick_soft(void)
+{
irq_work_run_list(this_cpu_ptr(_list));
  }
+#endif
  
  /*

   * Synchronize against the irq_work @entry, ensures the entry is not
diff --git a/kernel/time/timer.c b/kernel/time/timer.c
index adb1d82d6631..c68ba873da3c 100644
--- a/kernel/time/timer.c
+++ b/kernel/time/timer.c
@@ -1455,7 +1455,7 @@ void update_process_times(int user_tick)
scheduler_tick();
run_local_timers();
rcu_check_callbacks(user_tick);
-#if defined(CONFIG_IRQ_WORK) && !defined(CONFIG_PREEMPT_RT_FULL)
+#if defined(CONFIG_IRQ_WORK)
if (in_irq())
irq_work_tick();
  #endif
@@ -1471,9 +1471,7 @@ static void run_timer_softirq(struct softirq_action *h)
  
  	hrtimer_run_pending();
  
-#if defined(CONFIG_IRQ_WORK) && defined(CONFIG_PREEMPT_RT_FULL)

-   irq_work_tick();
-#endif
+   irq_work_tick_soft();
  
  	if (time_after_eq(jiffies, base->timer_jiffies))

__run_timers(base);


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] vmcore: replace Elf64_Ehdr/Elf32_Ehdr with elfhdr

[yjin]

On 2015年09月16日 18:39, Minfei Huang wrote:

On 09/16/15 at 10:58am, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

Function parse_crash_elf_headers() reads e_ident[EI_CLASS] then decides to
call parse_crash_elf64_headers() or parse_crash_elf32_headers().
But this happens in run time, not compile time. So compiler will report
the below warning:

In file included from include/linux/elf.h:4:0,
  from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf32_headers':
arch/mips/include/asm/elf.h:258:23: warning: initializatio
n from incompatible pointer type
   struct elfhdr *__h = (hdr); \
^

How about converting the hdr to type elfhdr in above sentence, like
following.

struct elfhdr *__h = (struct elfhdr *)(hdr);


Yes, this is a replacement, and it seems more safe because it just 
affects MIPS arch.

But I also can't see any obvious impact if modifying common vmcore.c:-)
Anyway, if you stick to your opinion, I can send a V2 patch to update 
mips' elf.h rather than vmcore.c.


Thanks!
Yanjiang


Thanks
Minfei


fs/proc/vmcore.c:1071:4: note: in expansion of macro 'elf_
check_arch'
!elf_check_arch() ||
 ^

Signed-off-by: Yanjiang Jin 
---
  fs/proc/vmcore.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index 4e61388..576bb26 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -999,7 +999,7 @@ static void free_elfcorebuf(void)
  static int __init parse_crash_elf64_headers(void)
  {
int rc=0;
-   Elf64_Ehdr ehdr;
+   struct elfhdr ehdr;
u64 addr;
  
  	addr = elfcorehdr_addr;

@@ -1055,7 +1055,7 @@ fail:
  static int __init parse_crash_elf32_headers(void)
  {
int rc=0;
-   Elf32_Ehdr ehdr;
+   struct elfhdr ehdr;
u64 addr;
  
  	addr = elfcorehdr_addr;


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] mips: vmcore: forced convert 'hdr' in elf_check_arch()

[yjin]

The new version patch only modifies mips/elf.h, so add Ralf Baechle and 
cc linux-m...@linux-mips.org.

This is a V2 patch, attach the V1 patch for reference.

Thanks!
Yanjiang

On 2015年09月18日 15:42, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

elf_check_arch() will be called both in parse_crash_elf64_headers()
and parse_crash_elf32_headers(). But in these two functions, the type of
the parameter ehdr is different: Elf32_Ehdr and Elf64_Ehdr.

Function parse_crash_elf_headers() reads e_ident[EI_CLASS] then decides to
call parse_crash_elf64_headers() or parse_crash_elf32_headers().
This happens in run time, not compile time. So compiler will report
the below warning:

In file included from include/linux/elf.h:4:0,
  from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf32_headers':
arch/mips/include/asm/elf.h:258:23: warning: initializatio
n from incompatible pointer type
   struct elfhdr *__h = (hdr); \
^
fs/proc/vmcore.c:1071:4: note: in expansion of macro 'elf_
check_arch'
!elf_check_arch() ||
 ^

Signed-off-by: Yanjiang Jin 
---
  arch/mips/include/asm/elf.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index f19e890..ece490d 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -224,7 +224,7 @@ struct mips_elf_abiflags_v0 {
  #define elf_check_arch(hdr)   \
  ({\
int __res = 1;  \
-   struct elfhdr *__h = (hdr); \
+   struct elfhdr *__h = (struct elfhdr *)(hdr);\
\
if (__h->e_machine != EM_MIPS)   \
__res = 0;  \
@@ -255,7 +255,7 @@ struct mips_elf_abiflags_v0 {
  #define elf_check_arch(hdr)   \
  ({\
int __res = 1;  \
-   struct elfhdr *__h = (hdr); \
+   struct elfhdr *__h = (struct elfhdr *)(hdr);\
\
if (__h->e_machine != EM_MIPS)   \
__res = 0;  \




Re: [PATCH] vmcore: replace Elf64_Ehdr_Elf32_Ehdr with elfhdr.eml
Description: application/extension-eml

Re: [PATCH] mips: vmcore: forced convert 'hdr' in elf_check_arch()

[yjin]

It seems the last mail has been blocked, resend it.

On 2015年09月21日 10:16, yjin wrote:
The new version patch only modifies mips/elf.h, so add Ralf Baechle 
and cc linux-m...@linux-mips.org.

This is a V2 patch, attach the V1 patch for reference.

Thanks!
Yanjiang

On 2015年09月18日 15:42, yanjiang@windriver.com wrote:

From: Yanjiang Jin 

elf_check_arch() will be called both in parse_crash_elf64_headers()
and parse_crash_elf32_headers(). But in these two functions, the type of
the parameter ehdr is different: Elf32_Ehdr and Elf64_Ehdr.

Function parse_crash_elf_headers() reads e_ident[EI_CLASS] then 
decides to

call parse_crash_elf64_headers() or parse_crash_elf32_headers().
This happens in run time, not compile time. So compiler will report
the below warning:

In file included from include/linux/elf.h:4:0,
  from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf32_headers':
arch/mips/include/asm/elf.h:258:23: warning: initializatio
n from incompatible pointer type
   struct elfhdr *__h = (hdr); \
^
fs/proc/vmcore.c:1071:4: note: in expansion of macro 'elf_
check_arch'
!elf_check_arch() ||
 ^

Signed-off-by: Yanjiang Jin 
---
  arch/mips/include/asm/elf.h | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index f19e890..ece490d 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -224,7 +224,7 @@ struct mips_elf_abiflags_v0 {
  #define elf_check_arch(hdr)\
  ({\
  int __res = 1;\
-struct elfhdr *__h = (hdr);\
+struct elfhdr *__h = (struct elfhdr *)(hdr);\
  \
  if (__h->e_machine != EM_MIPS)\
  __res = 0;\
@@ -255,7 +255,7 @@ struct mips_elf_abiflags_v0 {
  #define elf_check_arch(hdr)\
  ({\
  int __res = 1;\
-struct elfhdr *__h = (hdr);\
+struct elfhdr *__h = (struct elfhdr *)(hdr);\
  \
  if (__h->e_machine != EM_MIPS)\
  __res = 0;\




--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/