Linux kernel staging patches
 help / color / mirror / Atom feed
From: Stefan Wahren <wahrenst@gmx.net>
To: Umang Jain <umang.jain@ideasonboard.com>, linux-staging@lists.linux.dev
Cc: Dan Carpenter <error27@gmail.com>,
	Kieran Bingham <kieran.bingham@ideasonboard.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Dave Stevenson <dave.stevenson@raspberrypi.com>,
	Phil Elwell <phil@raspberrypi.com>, Greg KH <greg@kroah.com>
Subject: Re: [PATCH v5 03/11] staging: vc04_services: vchiq_arm: Drop g_cache_line_size
Date: Fri, 19 Apr 2024 16:04:27 +0200	[thread overview]
Message-ID: <837906bb-0e40-444a-803f-f5ed5d693c2f@gmx.net> (raw)
In-Reply-To: <20240412075743.60712-4-umang.jain@ideasonboard.com>

Hi Umang,

Am 12.04.24 um 09:57 schrieb Umang Jain:
> The cache-line-size is cached in struct vchiq_platform_info.
> There is no need to cache this again via g_cache_line_size
> and use it. Instead use the value from vchiq_platform_info directly.
>
> While at it, move the comment about L2 cache line size in the kerneldoc
> block of struct vchiq_platform_info.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> ---
>   .../interface/vchiq_arm/vchiq_arm.c           | 34 ++++++++-----------
>   .../interface/vchiq_arm/vchiq_arm.h           | 11 ++++++
>   2 files changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> index 7cf38f8581fa..af74d7268717 100644
> --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> @@ -131,17 +131,6 @@ struct vchiq_pagelist_info {
>   };
>
>   static void __iomem *g_regs;
> -/* This value is the size of the L2 cache lines as understood by the
> - * VPU firmware, which determines the required alignment of the
> - * offsets/sizes in pagelists.
> - *
> - * Modern VPU firmware looks for a DT "cache-line-size" property in
> - * the VCHIQ node and will overwrite it with the actual L2 cache size,
> - * which the kernel must then respect.  That property was rejected
> - * upstream, so we have to use the VPU firmware's compatibility value
> - * of 32.
> - */
> -static unsigned int g_cache_line_size = 32;
>   static unsigned int g_fragments_size;
>   static char *g_fragments_base;
>   static char *g_free_fragments;
> @@ -212,6 +201,7 @@ static struct vchiq_pagelist_info *
>   create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
>   		size_t count, unsigned short type)
>   {
> +	struct vchiq_drv_mgmt *drv_mgmt;
>   	struct pagelist *pagelist;
>   	struct vchiq_pagelist_info *pagelistinfo;
>   	struct page **pages;
> @@ -226,6 +216,8 @@ create_pagelist(struct vchiq_instance *instance, char *buf, char __user *ubuf,
>   	if (count >= INT_MAX - PAGE_SIZE)
>   		return NULL;
>
> +	drv_mgmt = dev_get_drvdata(instance->state->dev->parent);
this patch is wrong and cause a null pointer dereference:

[  232.297800] Unable to handle kernel NULL pointer dereference at
virtual address 00000004 when read
[  232.317576] [00000004] *pgd=02963831, *pte=00000000, *ppte=00000000
[  232.334240] Internal error: Oops: 17 [#1] ARM
[  232.348423] Modules linked in: bcm2835_v4l2(C) videobuf2_vmalloc
videobuf2_memops bcm2835_mmal_vchiq(C) videobuf2_v4l2 videobuf2_common
snd_bcm2835(C) raspberrypi_hwmon bcm2835_rng rng_core vchiq(C) configfs
[  232.377176] CPU: 0 PID: 504 Comm: VCHIQ completio Tainted: G        
C         6.9.0-rc2+ #1
[  232.395690] Hardware name: BCM2835
[  232.409044] PC is at vchiq_prepare_bulk_data+0x26c/0x528 [vchiq]
[  232.425441] LR is at vchiq_prepare_bulk_data+0x4ec/0x528 [vchiq]
[  232.441815] pc : [<bf0154f8>]    lr : [<bf015778>] psr: 60000013
[  232.458395] sp : ccc29d80  ip : 00000000  fp : c7c43000
[  232.473982] r10: bf022fc8  r9 : 43cd7000  r8 : c64a4000
[  232.489690] r7 : 00000001  r6 : 00000000  r5 : c7c43020  r4 : 43cd7000
[  232.506316] r3 : 00000000  r2 : 0000028c  r1 : c3cd7a80  r0 : 00000000
[  232.522865] Flags: nZCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM
Segment none
[  232.540123] Control: 00c5387d  Table: 02828008  DAC: 00000051
[  232.556061] Register r0 information: NULL pointer
[  232.570951] Register r1 information: non-slab/vmalloc memory
[  232.586845] Register r2 information: non-paged memory
[  232.602036] Register r3 information: NULL pointer
[  232.616689] Register r4 information: non-paged memory
[  232.631750] Register r5 information: 0-page vmalloc region starting
at 0xc7c00000 allocated at iotable_init+0x0/0xf8
[  232.652526] Register r6 information: NULL pointer
[  232.667439] Register r7 information: non-paged memory
[  232.682558] Register r8 information: slab kmalloc-4k start c64a4000
pointer offset 0 size 4096
[  232.701421] Register r9 information: non-paged memory
[  232.716545] Register r10 information: 4-page vmalloc region starting
at 0xbf021000 allocated at load_module+0x73c/0x16e0
[  232.737816] Register r11 information: 0-page vmalloc region starting
at 0xc7c00000 allocated at iotable_init+0x0/0xf8
[  232.758991] Register r12 information: NULL pointer
[  232.774079] Process VCHIQ completio (pid: 504, stack limit = 0xe510e83c)
[  232.791136] Stack: (0xccc29d80 to 0xccc2a000)
[  232.805806] 9d80: 00000000 00000000 00000036 00000000 00000001
00000001 c7c43008 00000001
[  232.824634] 9da0: c1d610e8 0002b28c 00000001 00000010 47c43000
10a55ba2 c2291744 c1d61000
[  232.843077] 9dc0: c1d610d4 00000000 00000000 00001002 00000000
c1d610e8 c1d61170 bf012bd8
[  232.861553] 9de0: 00000800 00000001 6c6b6a69 706f6e6d c8209748
ccc29e28 74737271 c236006c
[  232.880103] 9e00: 00000000 00000000 00000072 0002b28c 00000006
c64a4000 00000051 c2c9e580
[  232.898623] 9e20: ccc29e90 10a55ba2 00000000 c22b2dc0 c64a4000
c014c406 b6ceaca8 00000000
[  232.917119] 9e40: c2c9e580 00000036 c1d61000 bf0173a0 00000800
00001002 00000000 00000001
[  232.935668] 9e60: 00000002 c8200194 00000007 00000003 c1d61000
c64a4020 00000002 c2c9e580
[  232.954296] 9e80: 00000003 00000000 b6eaf22c 00001001 0000d001
0002b28c 00000800 00001002
[  232.973059] 9ea0: 00000000 c2643420 cbfc8160 c0112bec 00000000
c644fc18 cbfc8160 00000255
[  232.991818] 9ec0: 0b98b34f c0227c9c 00000001 c1c6d200 00000255
00001020 b6308000 ccc29fb0
[  233.010716] 9ee0: c1c6d204 c644fc18 00000cc0 10a55ba2 b6308000
c22b2dc0 c22dd600 b6ceaca8
[  233.029603] 9f00: c014c406 c2c9e580 00000003 00000036 b6eaf0d8
c026d4d0 c22b2dc0 c026de50
[  233.048517] 9f20: b6321000 ccc29fb0 00000817 b6308734 c2c9e580
c22dd601 00000255 c096a730
[  233.067479] 9f40: ccc29fb0 c096a730 00000000 ccc29f50 00000000
c010271c 40000000 ee08aa10
[  233.086503] 9f60: ccc29fb0 c010271c ccc29fb0 b6e9af40 c0102648
c2c9e580 00c5387d 10a55ba2
[  233.105615] 9f80: b6cead48 b6eaf130 b6ceaca8 c014c406 00000036
c01002c0 c2c9e580 00000036
[  233.124926] 9fa0: b6eaf0d8 c0100060 b6eaf130 b6ceaca8 00000003
c014c406 b6ceaca8 00000000
[  233.144213] 9fc0: b6eaf130 b6ceaca8 c014c406 00000036 00000800
00001002 0002b28c b6eaf0d8
[  233.163431] 9fe0: b6eaf060 b6ceac9c b6e9c0c8 b6dd353c 80000010
00000003 00000000 00000000
[  233.182630] Call trace:
[  233.182680]  vchiq_prepare_bulk_data [vchiq] from
vchiq_bulk_transfer+0x1e4/0x3d4 [vchiq]
[  233.215907]  vchiq_bulk_transfer [vchiq] from
vchiq_ioctl+0x1d4/0x1114 [vchiq]
[  233.234685]  vchiq_ioctl [vchiq] from vfs_ioctl+0x28/0x3c
[  233.251519]  vfs_ioctl from sys_ioctl+0xc4/0x928
[  233.267303]  sys_ioctl from ret_fast_syscall+0x0/0x54
[  233.283472] Exception stack(0xccc29fa8 to 0xccc29ff0)
[  233.299605] 9fa0:                   b6eaf130 b6ceaca8 00000003
c014c406 b6ceaca8 00000000
[  233.318897] 9fc0: b6eaf130 b6ceaca8 c014c406 00000036 00000800
00001002 0002b28c b6eaf0d8
[  233.338126] 9fe0: b6eaf060 b6ceac9c b6e9c0c8 b6dd353c
[  233.354204] Code: e3530001 1a000032 e59d300c e1db20b6 (e5933004)

Please always test your changes as documented in interface/TESTING

I will send a follow-up fix soon

  parent reply	other threads:[~2024-04-19 14:04 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-12  7:57 [PATCH v5 00/11] staging: vc04_services: Drop non-essential global members Umang Jain
2024-04-12  7:57 ` [PATCH v5 01/11] staging: vc04_services: Drop g_once_init global variable Umang Jain
2024-04-14 10:23   ` Stefan Wahren
2024-04-12  7:57 ` [PATCH v5 02/11] staging: vc04_services: vchiq_arm: Split driver static and runtime data Umang Jain
2024-04-14 12:04   ` Stefan Wahren
2024-04-19 13:58   ` Stefan Wahren
2024-04-12  7:57 ` [PATCH v5 03/11] staging: vc04_services: vchiq_arm: Drop g_cache_line_size Umang Jain
2024-04-14 12:09   ` Stefan Wahren
2024-04-19 14:04   ` Stefan Wahren [this message]
2024-04-21  8:58     ` Umang Jain
2024-04-12  7:57 ` [PATCH v5 04/11] staging: vc04_services: Move variables for tracking connections Umang Jain
2024-04-14 10:41   ` Stefan Wahren
2024-04-12  7:57 ` [PATCH v5 05/11] staging: vc04_services: Drop vchiq_connected.[ch] files Umang Jain
2024-04-12  7:57 ` [PATCH v5 06/11] staging: vc04_services: Move global variables tracking allocated pages Umang Jain
2024-04-14 11:45   ` Stefan Wahren
2024-04-12  7:57 ` [PATCH v5 07/11] staging: vc04_services: Move global memory mapped pointer Umang Jain
2024-04-14 11:58   ` Stefan Wahren
2024-04-16  6:42     ` Laurent Pinchart
2024-04-12  7:57 ` [PATCH v5 08/11] staging: vc04_services: Move spinlocks to vchiq_state Umang Jain
2024-04-14 11:59   ` Stefan Wahren
2024-04-12  7:57 ` [PATCH v5 09/11] staging: vc04_services: vchiq_mmal: Rename service_callback() Umang Jain
2024-04-14 11:06   ` Stefan Wahren
2024-04-12  7:57 ` [PATCH v5 10/11] staging: vc04_services: Move global g_state to vchiq_state Umang Jain
2024-04-14 11:26   ` Stefan Wahren
2024-04-12  7:57 ` [PATCH v5 11/11] staging: vc04_services: Drop completed TODO item Umang Jain

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=837906bb-0e40-444a-803f-f5ed5d693c2f@gmx.net \
    --to=wahrenst@gmx.net \
    --cc=dave.stevenson@raspberrypi.com \
    --cc=error27@gmail.com \
    --cc=greg@kroah.com \
    --cc=kieran.bingham@ideasonboard.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-staging@lists.linux.dev \
    --cc=phil@raspberrypi.com \
    --cc=umang.jain@ideasonboard.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).