Re: Keystone 2 boards boot failure

[Grygorii Strashko <grygorii.strashko@ti.com>]

hi Arnd,

On 02/05/2016 06:18 PM, Arnd Bergmann wrote:
> On Thursday 04 February 2016 18:25:08 Grygorii Strashko wrote:
>>>
>>> I have another version for testing below. That removes the logic that
>>> splits and reassembles the 64-bit values, but leaves the other changes
>>> in place. Can you try this?
>>>
>>
>> Nop. It crashes kernel
> 
> Ah. too bad.
> 
>>     50.244448] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
>> [   50.266219] Unable to handle kernel NULL pointer dereference at virtual address 00000001
>> [   50.274287] pgd = c0003000
>> [   50.277007] [00000001] *pgd=80000800004003, *pmd=00000000
>> [   50.282412] Internal error: Oops: a07 [#1] PREEMPT SMP ARM
>> [   50.287881] Modules linked in:
>> [   50.290938] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G        W       4.5.0-rc2-00179-gad2f022-dirty #30
>> [   50.300214] Hardware name: Keystone
>> [   50.303693] task: c07476c0 ti: c0742000 task.ti: c0742000
>> [   50.309082] PC is at _test_and_set_bit+0x4/0x4c
>> [   50.313607] LR is at __netif_schedule+0x1c/0x60
>> [   50.318127] pc : [<c028415c>]    lr : [<c04294f0>]    psr: 20000113
>> [   50.318127] sp : c0743d68  ip : 00000001  fp : c0743d7c
>> [   50.329568] r10: c0743e00  r9 : c0744100  r8 : ffff9e75
>> [   50.334775] r7 : 00000000  r6 : 00000040  r5 : de495b00  r4 : 6d3cdb51
>> [   50.341282] r3 : 00000001  r2 : c07476c0  r1 : 6d3cdba9  r0 : 00000000
>> [   50.347790] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment kernel
>> [   50.355077] Control: 30c5387d  Table: 1878abc0  DAC: fffffffd
>> [   50.360803] Process swapper/0 (pid: 0, stack limit = 0xc0742210)
>> [   50.366790] Stack: (0xc0743d68 to 0xc0744000)
>> [   50.371137] 3d60:                   d9a16a00 de495b00 c0743d94 c0743d80 c04295a0 c04294e0
>> [   50.379291] 3d80: de7a9cc0 de495b00 c0743dbc c0743d98 c037396c c0429570 c0061d34 00000010
>> [   50.387445] 3da0: de7a9d80 de7a9d80 00000040 0000012c c0743ddc c0743dc0 c0375f58 c0373848
>> [   50.395599] 3dc0: de7a9d80 c0375f3c 00000040 0000012c c0743e3c c0743de0 c042a0cc c0375f48
>> [   50.403752] 3de0: c0743e9c c06635f8 c0744b1c c0744b1c c0773c46 1e46b000 c0741000 debac000
>> [   50.411907] 3e00: c0743e00 c0743e00 c0743e08 c0743e08 de408000 00000000 c074408c c0742000
>> [   50.420061] 3e20: 00000101 00000003 40000003 c0744080 c0743e9c c0743e40 c0026670 c0429ed8
>> [   50.428215] 3e40: d8722360 c0744808 00200000 c0744100 ffff9e74 c054088c 0000000a c07779c0
>> [   50.436369] 3e60: c073d2c8 c0744080 c0743e40 c0742000 c0744808 c073dddc 0000004e 00000000
>> [   50.444522] 3e80: 00000000 de408000 c0773aa4 c07444fc c0743eb4 c0743ea0 c0026a38 c0026548
>> [   50.452675] 3ea0: c073dddc 0000004e c0743edc c0743eb8 c0061d34 c00269c4 c0744808 e080400c
>> [   50.460828] 3ec0: c0743f08 e0804000 e0805000 c0773aa4 c0743f04 c0743ee0 c0009438 c0061cd8
>> [   50.468981] 3ee0: c0010314 60000013 ffffffff c0743f3c c0773aa4 c0773aa4 c0743f64 c0743f08
>> [   50.477136] 3f00: c0013a80 c0009404 00000000 deba8348 000070c8 c001f880 c0742000 c07444b0
>> [   50.485289] 3f20: c073d324 c0743f78 c0773aa4 c0773aa4 c07444fc c0743f64 c0743f68 c0743f58
>> [   50.493442] 3f40: c0010310 c0010314 60000013 ffffffff c006d624 c006a15c c0743f74 c0743f68
>> [   50.501595] 3f60: c00598c0 c00102e0 c0743f8c c0743f78 c00599dc c00598a4 00000002 00000000
>> [   50.509749] 3f80: c0743fa4 c0743f90 c0538468 c00598d8 c0777050 00000000 c0743ff4 c0743fa8
>> [   50.517902] 3fa0: c06fad60 c05383e4 ffffffff ffffffff 00000000 c06fa6d8 ffffffff 00000000
>> [   50.526056] 3fc0: 00000000 c0731a30 00000000 c0777294 c0744484 c0731a2c c0748878 80007000
>> [   50.534210] 3fe0: 412fc0f4 00000000 00000000 c0743ff8 80008090 c06fa964 00000000 00000000
>> [   50.542357] Backtrace:
>> [   50.544816] [<c04294d4>] (__netif_schedule) from [<c04295a0>] (netif_wake_subqueue+0x3c/0x44)
>> [   50.553312]  r5:de495b00 r4:d9a16a00
>> [   50.556909] [<c0429564>] (netif_wake_subqueue) from [<c037396c>] (netcp_process_tx_compl_packets+0x130/0x134)
>> [   50.566789]  r5:de495b00 r4:de7a9cc0
>> [   50.570381] [<c037383c>] (netcp_process_tx_compl_packets) from [<c0375f58>] (netcp_tx_poll+0x1c/0x4c)
>> [   50.579570]  r7:0000012c r6:00000040 r5:de7a9d80 r4:de7a9d80
>> [   50.585258] [<c0375f3c>] (netcp_tx_poll) from [<c042a0cc>] (net_rx_action+0x200/0x2f8)
>> [   50.593148]  r7:0000012c r6:00000040 r5:c0375f3c r4:de7a9d80
>> [   50.598833] [<c0429ecc>] (net_rx_action) from [<c0026670>] (__do_softirq+0x134/0x258)
>> [   50.606637]  r10:c0744080 r9:40000003 r8:00000003 r7:00000101 r6:c0742000 r5:c074408c
>> [   50.614486]  r4:00000000
>> [   50.617023] [<c002653c>] (__do_softirq) from [<c0026a38>] (irq_exit+0x80/0xb8)
>> [   50.624221]  r10:c07444fc r9:c0773aa4 r8:de408000 r7:00000000 r6:00000000 r5:0000004e
>> [   50.632069]  r4:c073dddc
>> [   50.634608] [<c00269b8>] (irq_exit) from [<c0061d34>] (__handle_domain_irq+0x68/0xbc)
>> [   50.642410]  r5:0000004e r4:c073dddc
>> [   50.645996] [<c0061ccc>] (__handle_domain_irq) from [<c0009438>] (gic_handle_irq+0x40/0x78)
>>
> 
> This is a different bug now, something is corrupting the skb pointer, probably as a
> result of the patch below (which is a subset of what is now applied compared
> to the last working version):
> 
> diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
> index 7e291c04a81a..cda19f2401c1 100644
> --- a/drivers/net/ethernet/ti/netcp_core.c
> +++ b/drivers/net/ethernet/ti/netcp_core.c
> @@ -117,10 +117,18 @@ static void get_pkt_info(dma_addr_t *buff, u32 *buff_len, dma_addr_t *ndesc,
> +
> +static void get_pad_ptr(void **padptr, struct knav_dma_desc *desc)
> +{
> +	u64 pad64;
> +
> +	pad64 = le32_to_cpu(desc->pad[0]);
> +	*padptr = (void *)(uintptr_t)pad64;
>   }
>   
>   static void get_org_pkt_info(dma_addr_t *buff, u32 *buff_len,
> 
> @@ -953,11 +966,11 @@ static int netcp_process_tx_compl_packets(struct netcp_intf *netcp,
>   					  unsigned int budget)
>   {
>   	struct knav_dma_desc *desc;
> +	void *ptr;
>   	struct sk_buff *skb;
>   	unsigned int dma_sz;
>   	dma_addr_t dma;
>   	int pkts = 0;
> -	u32 tmp;
>   
>   	while (budget--) {
>   		dma = knav_queue_pop(netcp->tx_compl_q, &dma_sz);
> @@ -970,7 +983,8 @@ static int netcp_process_tx_compl_packets(struct netcp_intf *netcp,
>   			continue;
>   		}
>   
> -		get_pad_info((u32 *)&skb, &tmp, desc);
> +		get_pad_ptr(&ptr, desc);
> +		skb = ptr;
>   		netcp_free_tx_desc_chain(netcp, desc, dma_sz);
>   		if (!skb) {
>   			dev_err(netcp->ndev_dev, "No skb in Tx desc\n");
> @@ -1173,7 +1189,8 @@ static int netcp_tx_submit_skb(struct netcp_intf *netcp,
>   	}
>   
>   	set_words(&tmp, 1, &desc->packet_info);
> -	set_words((u32 *)&skb, 1, &desc->pad[0]);
> +	tmp = (uintptr_t)&skb;
> +	set_words(&tmp, 1, &desc->pad[0]);

&skb is virt address and its size is 32bit even when LPAE=y (phys/dma 64 bit)
so  this is excess conversion to/from u64 ;)
This is from the first look.

>   
>   	if (tx_pipe->flags & SWITCH_TO_PORT_IN_TAGINFO) {
>   		tmp = tx_pipe->switch_to_port;
> 
> 
> I'm sure it's something obvious and stupid in there, but I just can't
> see it and that is very unsatisfying. Do you see where I am going wrong?
> Most of all, I want to know it so I don't make the same mistake again
> when I patch another driver.
> 

I'm very sorry, but I'll not be able to test it in the nearest future :(
What I could do now is update your/my patch as i mentioned in [1]
and re-send it at the weekend (with your authorship and my signoff).
Do you agree?


[1] https://www.mail-archive.com/netdev@vger.kernel.org/msg95831.html

-- 
regards,
-grygorii