linux-kernel - Re: [PATCH] pci: Add quirk for still enabled interrupts on Intel Sandy Bridge GPUs

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 for Android: free password hash cracker in your pocket

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

Date:	Thu, 8 Dec 2011 09:27:06 +0100
From:	Thomas Jarosch <thomas.jarosch@...ra2net.com>
To:	Jesse Barnes <jbarnes@...tuousgeek.org>
Cc:	linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
	Charlie Suffin <charlie.suffin@...atus.com>, stable@...nel.org
Subject: Re: [PATCH] pci: Add quirk for still enabled interrupts on Intel Sandy Bridge GPUs

Hi Jesse,

On Wednesday, 7. December 2011 22:30:00 Jesse Barnes wrote:
> > +	/* Check if any interrupt line is still enabled */
> > +	if (readl(regs + I915_DEIER_REG) != 0) {
> > +		dev_warn(&dev->dev, "BIOS left Intel GPU interrupts enabled; "
> > +			"disabling\n");
> >  {
> 
> Could really be dev_dbg, I think this will be fairly common. 

Thanks for your review.

I prefer dev_warn() or dev_info() for two reasons:

1. This is new code. If a system crashes in the unlikely event
because of this change, the user will have a better chance
to see the message before we tweak around the registers.

2. All other quirks either use dev_warn() (like the Intel e100 quirk
which is similar to this fix, see quirk_e100_interrupt()) or dev_info().

If there is something wrong and we activate a workaround,
let the user know about it, at least for now.

User impact is minimal and users with graphical boot up don't see
kernel messages anyway.

> I think we also need to ack any outstanding interrupts after disabling
> them in IER by writing IIR back on itself. Can you test that?

Can you go into more detail why we need this?

Looking at the logic diagram at page 23 of this document
http://intellinuxgraphics.org/documentation/SNB/IHD_OS_Vol3_Part2.pdf

tells me we don't necessarily need this, no interrupt should slip through.

Also the i915 driver first disables IER and then clears IIR
in drivers/gpu/drm/i915/i915_irq.c: ivybridge_irq_handler()
or ironlake_irq_handler(). So it always initializes the GPU to
a sane state before enabling the interrupts.

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