linux-kernel - Re: [PATCH] usb: typec: fusb302: fix scheduling while atomic when using virtio-gpio

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <m7n22g5fsfvpjz4s5d6zfcfddrzrj3ixgaqehrjkg7mcbufvjc@s4omshvxtkaf>
Date: Mon, 4 Aug 2025 20:00:56 +0200
From: Sebastian Reichel <sebastian.reichel@...labora.com>
To: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Cc: Yongbo Zhang <giraffesnn123@...il.com>, gregkh@...uxfoundation.org, 
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org, Hans de Goede <hansg@...nel.org>
Subject: Re: [PATCH] usb: typec: fusb302: fix scheduling while atomic when
 using virtio-gpio

+CC Hans de Goede

Hi,

On Wed, May 28, 2025 at 01:23:01PM +0300, Heikki Krogerus wrote:
> On Mon, May 26, 2025 at 12:34:33PM +0800, Yongbo Zhang wrote:
> > When the gpio irqchip connected to a slow bus(e.g., i2c bus or virtio
> > bus), calling disable_irq_nosync() in top-half ISR handler will trigger
> > the following kernel BUG:
> > 
> > BUG: scheduling while atomic: RenderEngine/253/0x00010002
> > ...
> > Call trace:
> >  dump_backtrace+0x0/0x1c8
> >  show_stack+0x1c/0x2c
> >  dump_stack_lvl+0xdc/0x12c
> >  dump_stack+0x1c/0x64
> >  __schedule_bug+0x64/0x80
> >  schedule_debug+0x98/0x118
> >  __schedule+0x68/0x704
> >  schedule+0xa0/0xe8
> >  schedule_timeout+0x38/0x124
> >  wait_for_common+0xa4/0x134
> >  wait_for_completion+0x1c/0x2c
> >  _virtio_gpio_req+0xf8/0x198
> >  virtio_gpio_irq_bus_sync_unlock+0x94/0xf0
> >  __irq_put_desc_unlock+0x50/0x54
> >  disable_irq_nosync+0x64/0x94
> >  fusb302_irq_intn+0x24/0x84
> >  __handle_irq_event_percpu+0x84/0x278
> >  handle_irq_event+0x64/0x14c
> >  handle_level_irq+0x134/0x1d4
> >  generic_handle_domain_irq+0x40/0x68
> >  virtio_gpio_event_vq+0xb0/0x130
> >  vring_interrupt+0x7c/0x90
> >  vm_interrupt+0x88/0xd8
> >  __handle_irq_event_percpu+0x84/0x278
> >  handle_irq_event+0x64/0x14c
> >  handle_fasteoi_irq+0x110/0x210
> >  __handle_domain_irq+0x80/0xd0
> >  gic_handle_irq+0x78/0x154
> >  el0_irq_naked+0x60/0x6c
> > 
> > This patch replaces request_irq() with devm_request_threaded_irq() to
> > avoid the use of disable_irq_nosync().
> > 
> > Signed-off-by: Yongbo Zhang <giraffesnn123@...il.com>
> 
> Reviewed-by: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
> 
> > ---

I'm currently investigating a potential "regression" (quotes,
because USB-C support is not yet enabled in the upstream board
devicetree) with the Radxa ROCK 5B USB-C support after rebasing
to latest master branch. I'm not yet sure, if this patch is at
fault or totally unrelated, but please be aware that it undoes
previous work from Hans de Goede to NOT use threaded IRQs:

207338ec5a27 ("usb: typec: fusb302: Improve suspend/resume handling")

At the same time the fix from Yongbo Zhang misses cleaning up the
now useless fusb302_irq_work() split, which had been introduced by
Hans patch to have the hard IRQ as short as possible. With the
interrupt handler being a thread itself, the code can just be called
directly.

Greetings,

-- Sebastian

> >  drivers/usb/typec/tcpm/fusb302.c | 12 ++++--------
> >  1 file changed, 4 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/usb/typec/tcpm/fusb302.c b/drivers/usb/typec/tcpm/fusb302.c
> > index f15c63d3a8f4..f2801279c4b5 100644
> > --- a/drivers/usb/typec/tcpm/fusb302.c
> > +++ b/drivers/usb/typec/tcpm/fusb302.c
> > @@ -1477,9 +1477,6 @@ static irqreturn_t fusb302_irq_intn(int irq, void *dev_id)
> >  	struct fusb302_chip *chip = dev_id;
> >  	unsigned long flags;
> > 
> > -	/* Disable our level triggered IRQ until our irq_work has cleared it */
> > -	disable_irq_nosync(chip->gpio_int_n_irq);
> > -
> >  	spin_lock_irqsave(&chip->irq_lock, flags);
> >  	if (chip->irq_suspended)
> >  		chip->irq_while_suspended = true;
> > @@ -1622,7 +1619,6 @@ static void fusb302_irq_work(struct work_struct *work)
> >  	}
> >  done:
> >  	mutex_unlock(&chip->lock);
> > -	enable_irq(chip->gpio_int_n_irq);
> >  }
> > 
> >  static int init_gpio(struct fusb302_chip *chip)
> > @@ -1747,9 +1743,10 @@ static int fusb302_probe(struct i2c_client *client)
> >  		goto destroy_workqueue;
> >  	}
> > 
> > -	ret = request_irq(chip->gpio_int_n_irq, fusb302_irq_intn,
> > -			  IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> > -			  "fsc_interrupt_int_n", chip);
> > +	ret = devm_request_threaded_irq(dev, chip->gpio_int_n_irq,
> > +					NULL, fusb302_irq_intn,
> > +					IRQF_ONESHOT | IRQF_TRIGGER_LOW,
> > +					"fsc_interrupt_int_n", chip);
> >  	if (ret < 0) {
> >  		dev_err(dev, "cannot request IRQ for GPIO Int_N, ret=%d", ret);
> >  		goto tcpm_unregister_port;
> > @@ -1774,7 +1771,6 @@ static void fusb302_remove(struct i2c_client *client)
> >  	struct fusb302_chip *chip = i2c_get_clientdata(client);
> > 
> >  	disable_irq_wake(chip->gpio_int_n_irq);
> > -	free_irq(chip->gpio_int_n_irq, chip);
> >  	cancel_work_sync(&chip->irq_work);
> >  	cancel_delayed_work_sync(&chip->bc_lvl_handler);
> >  	tcpm_unregister_port(chip->tcpm_port);
> > --
> > 2.49.0
> 
> -- 
> heikki
> 

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)