linux-kernel - Re: [PATCH v5 0/7] Add AMD ISP4 driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aQsYJhbGifdXhjCJ@sultan-box>
Date: Wed, 5 Nov 2025 01:25:58 -0800
From: Sultan Alsawaf <sultan@...neltoast.com>
To: Bin Du <Bin.Du@....com>
Cc: mchehab@...nel.org, hverkuil@...all.nl,
	laurent.pinchart+renesas@...asonboard.com,
	bryan.odonoghue@...aro.org, sakari.ailus@...ux.intel.com,
	prabhakar.mahadev-lad.rj@...renesas.com,
	linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
	pratap.nirujogi@....com, benjamin.chan@....com, king.li@....com,
	gjorgji.rosikopulos@....com, Phil.Jawich@....com,
	Dominic.Antony@....com, mario.limonciello@....com,
	richard.gong@....com, anson.tsao@....com
Subject: Re: [PATCH v5 0/7] Add AMD ISP4 driver

Hi Bin,

To expedite review, I've attached a patch containing a bunch of fixes I've made
on top of v5. Most of my changes should be self-explanatory; feel free to ask
further about specific changes if you have any questions.

I haven't had a chance to review all of the v4 -> v5 changes yet, but I figured
I should send what I've got so far.

FYI, there is a regression in isp4if_dequeue_buffer() where the bufq lock isn't
protecting the list_del() anymore. I also checked the compiler output when using
guard() versus scoped_guard() in that function and there is no difference:

  sha1sum:
  5652a0306da22ea741d80a9e03a787d0f71758a8  isp4_interface.o // guard()
  5652a0306da22ea741d80a9e03a787d0f71758a8  isp4_interface.o // scoped_guard()

So guard() should be used there again, which I've done in my patch.

I also have a few questions:

1. Does ISP FW provide a register interface to disable the IRQ? If so, it is
   faster to use that than using disable_irq_nosync() to disable the IRQ from
   the CPU's side.

2. When the IRQ is re-enabled in isp4sd_fw_resp_func(), is there anything
   beforehand to mask all pending interrupts from the ISP so that there isn't a
   bunch of stale interrupts firing as soon the IRQ is re-enabled?

3. Is there any risk of deadlock due to the stream kthread racing with the ISP
   when the ISP posts a new response _after_ the kthread determines there are no
   more new responses but _before_ the enable_irq() in isp4sd_fw_resp_func()?

4. Why are some lines much longer than before? It seems inconsistent that now
   there is a mix of several lines wrapped to 80 cols and many lines going
   beyond 80 cols. And there are multiple places where code is wrapped before
   reaching 80 cols even with lots of room left, specifically for cases where it
   wouldn't hurt readability to put more characters onto each line.

Sultan

View attachment "amd-isp4-v5-fixes-round-1.patch" of type "text/plain" (22372 bytes)