Hi!
On Mon 2017-12-18 03:20:11, Alexandru Chirvasitu wrote:
> Short description of the problem: latest rc kernel results in seemingly APIC-caused hard lockups, whereas latest stable kernel works fine.
>
> I have an old ASUS F5RL laptop with an Intel Core 2 Duo CPU T5450 @1.66GHz. It is currently running Debian 9.3 stable 32 bit (by default on a 4.9-series kernel), but I have been compiling and installing the latest kernels.
>
Thanks for doing that.
> The latest rc kernel at the time of this writing (4.15.0-rc3) boots but then results in hard lockups on both CPUs after login. Starting in recovery mode returns the error
>
> "spurious APIC interrupt through vector ff on CPU#0, should never happen"
>
> before lockip up the CPUs again. A hard reboot is necessary.
>
> Starting with kernel option noapic logs me in uneventfully, but for some reason has the effect of rendering my ethrenet card inoperable. It is a Qualcomm Atheros Attansic L2 Fast Ethernet (rev a0), handled by kernel module atl2. In noapic mode the card is still seen by the system, can be brought up / down, etc., but dhclient never manages to acquire a lease.
>
> Starting with kernel option nolapic instead brings up the network and logs me in, but only sees one CPU instead of two, as usual.
>
> The latest kernel that exhibits none of these issues is the latest stable one as of this writing: 4.14.7.
>
> ---
>
> As this seems to be APIC-related, I am sending the message to the maintainers mentioned in arch/x86/kernel/apic/apic.c. I am unsure whether this is the correct procedure however.
>
Good enough procedure. You want to always copy linux-kernel mailing
list, and you should probably look for X86 maintainers in MAINTAINERS
file, and cc them, too.
If you run out of other options, you can always do "git bisect"...
Best regards, Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Thank you!
On Mon, Dec 18, 2017 at 11:11:31AM +0100, Pavel Machek wrote:
> Hi!
> On Mon 2017-12-18 03:20:11, Alexandru Chirvasitu wrote:
> > Short description of the problem: latest rc kernel results in seemingly APIC-caused hard lockups, whereas latest stable kernel works fine.
> >
> > I have an old ASUS F5RL laptop with an Intel Core 2 Duo CPU T5450 @1.66GHz. It is currently running Debian 9.3 stable 32 bit (by default on a 4.9-series kernel), but I have been compiling and installing the latest kernels.
> >
>
> Thanks for doing that.
>
> > The latest rc kernel at the time of this writing (4.15.0-rc3) boots but then results in hard lockups on both CPUs after login. Starting in recovery mode returns the error
> >
> > "spurious APIC interrupt through vector ff on CPU#0, should never happen"
> >
> > before lockip up the CPUs again. A hard reboot is necessary.
> >
> > Starting with kernel option noapic logs me in uneventfully, but for some reason has the effect of rendering my ethrenet card inoperable. It is a Qualcomm Atheros Attansic L2 Fast Ethernet (rev a0), handled by kernel module atl2. In noapic mode the card is still seen by the system, can be brought up / down, etc., but dhclient never manages to acquire a lease.
> >
> > Starting with kernel option nolapic instead brings up the network and logs me in, but only sees one CPU instead of two, as usual.
> >
> > The latest kernel that exhibits none of these issues is the latest stable one as of this writing: 4.14.7.
> >
> > ---
> >
> > As this seems to be APIC-related, I am sending the message to the maintainers mentioned in arch/x86/kernel/apic/apic.c. I am unsure whether this is the correct procedure however.
> >
>
> Good enough procedure. You want to always copy linux-kernel mailing
> list, and you should probably look for X86 maintainers in MAINTAINERS
> file, and cc them, too.
>
> If you run out of other options, you can always do "git bisect"...
>
I had never heard of 'bisect' before this casual mention (you might tell I am a bit out of my depth). I've since applied it to Linus' tree between
bebc608 Linux 4.14 (good)
and
4fbd8d1 Linux 4.15-rc1 (bad)
It took about 13 attempts (I had access to a faster machine to compile on, and ccache helped once the cache built up some momentum). The result is (as presented by 'git bisect' at the end of the process, between the --- dividers added by me for clarity):
--- start of output ---
2b5175c4fa974b6aa05bbd2ee8d443a8036a1714 is the first bad commit
commit 2b5175c4fa974b6aa05bbd2ee8d443a8036a1714
Author: Thomas Gleixner <[email protected]>
Date: Tue Oct 17 09:54:57 2017 +0200
genirq: Add config option for reservation mode
The interrupt reservation mode requires reactivation of PCI/MSI
interrupts. Create a config option, so the PCI code can set the
corresponding flag when required.
Signed-off-by: Thomas Gleixner <[email protected]>
Cc: Josh Poulson <[email protected]>
Cc: Mihai Costache <[email protected]>
Cc: Stephen Hemminger <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: [email protected]
Cc: Haiyang Zhang <[email protected]>
Cc: Dexuan Cui <[email protected]>
Cc: Simon Xiao <[email protected]>
Cc: Saeed Mahameed <[email protected]>
Cc: Jork Loeser <[email protected]>
Cc: Bjorn Helgaas <[email protected]>
Cc: [email protected]
Cc: KY Srinivasan <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
:040000 040000 5e73031cc0c8411a20722cce7876ab7b82ed3858 dcf98e7a6b7d5f7c5353b7ccab02125e6d332ec8 M kernel
--- end of output ---
Consequently, I am cc-ing in the listed addresses.
Thank you,
Alex Chirvasitu
> Best regards, Pavel
> --
> (english) http://www.livejournal.com/~pavelmachek
> (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On Tue, 19 Dec 2017, Alexandru Chirvasitu wrote:
> I had never heard of 'bisect' before this casual mention (you might tell
> I am a bit out of my depth). I've since applied it to Linus' tree between
> bebc608 Linux 4.14 (good)
>
> and
>
> 4fbd8d1 Linux 4.15-rc1 (bad)
Is Linus current head 4.15-rc4 bad as well?
> It took about 13 attempts (I had access to a faster machine to compile
> on, and ccache helped once the cache built up some momentum). The result
> is (as presented by 'git bisect' at the end of the process, between the
> --- dividers added by me for clarity):
> --- start of output ---
>
> 2b5175c4fa974b6aa05bbd2ee8d443a8036a1714 is the first bad commit
> commit 2b5175c4fa974b6aa05bbd2ee8d443a8036a1714
> Author: Thomas Gleixner <[email protected]>
> Date: Tue Oct 17 09:54:57 2017 +0200
>
> genirq: Add config option for reservation mode
>
> The interrupt reservation mode requires reactivation of PCI/MSI
> interrupts. Create a config option, so the PCI code can set the
> corresponding flag when required.
>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Cc: Josh Poulson <[email protected]>
> Cc: Mihai Costache <[email protected]>
> Cc: Stephen Hemminger <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: [email protected]
> Cc: Haiyang Zhang <[email protected]>
> Cc: Dexuan Cui <[email protected]>
> Cc: Simon Xiao <[email protected]>
> Cc: Saeed Mahameed <[email protected]>
> Cc: Jork Loeser <[email protected]>
> Cc: Bjorn Helgaas <[email protected]>
> Cc: [email protected]
> Cc: KY Srinivasan <[email protected]>
> Link: https://lkml.kernel.org/r/[email protected]
>
> :040000 040000 5e73031cc0c8411a20722cce7876ab7b82ed3858 dcf98e7a6b7d5f7c5353b7ccab02125e6d332ec8 M kernel
>
> --- end of output ---
>
> Consequently, I am cc-ing in the listed addresses.
Thanks for doing that bisect, but unfortunately this commit cannot be the
problematic one, It merily adds a config symbol, but it does not change any
code at all. It has no effect whatsoever. So something might have gone
wrong in your bisecting.
I CC'ed Dou Liyang. He has changed the early APIC setup code and there has
been an issue reported already. Though I lost track of that. Dou, any
pointers?
Thanks,
tglx
Hi Thomas,
At 12/20/2017 08:31 AM, Thomas Gleixner wrote:
> On Tue, 19 Dec 2017, Alexandru Chirvasitu wrote:
>
>> I had never heard of 'bisect' before this casual mention (you might tell
>> I am a bit out of my depth). I've since applied it to Linus' tree between
>
>> bebc608 Linux 4.14 (good)
>>
>> and
>>
>> 4fbd8d1 Linux 4.15-rc1 (bad)
>
> Is Linus current head 4.15-rc4 bad as well?
>
[...]
>
> Thanks for doing that bisect, but unfortunately this commit cannot be the
> problematic one, It merily adds a config symbol, but it does not change any
> code at all. It has no effect whatsoever. So something might have gone
> wrong in your bisecting.
>
Agree.
> I CC'ed Dou Liyang. He has changed the early APIC setup code and there has
> been an issue reported already. Though I lost track of that. Dou, any
^^^^^^^^ Is it this one?
https://marc.info/?l=linux-kernel&m=151188084018443
> pointers?
>
Not sure, but seems the APIC failed to start in that 32-bit system.
I will look into it.
Alex,
Could you give me your .config file and the dmesg-log of 4.15.0-rc3.
Thanks,
dou
On Wed, Dec 20, 2017 at 11:58:57AM +0800, Dou Liyang wrote:
> Hi Thomas,
>
> At 12/20/2017 08:31 AM, Thomas Gleixner wrote:
> > On Tue, 19 Dec 2017, Alexandru Chirvasitu wrote:
> >
> > > I had never heard of 'bisect' before this casual mention (you might tell
> > > I am a bit out of my depth). I've since applied it to Linus' tree between
> >
> > > bebc608 Linux 4.14 (good)
> > >
> > > and
> > >
> > > 4fbd8d1 Linux 4.15-rc1 (bad)
> >
> > Is Linus current head 4.15-rc4 bad as well?
> >
> [...]
Yes. Exactly the same symptoms on
1291a0d5 Linux 4.15-rc4
compiled just now from Linus' tree.
> >
> > Thanks for doing that bisect, but unfortunately this commit cannot be the
> > problematic one, It merily adds a config symbol, but it does not change any
> > code at all. It has no effect whatsoever. So something might have gone
> > wrong in your bisecting.
> >
>
> Agree.
I agree too, I think.. For what it's worth,
0696d05
which is that commit's parent, *also* exhibits the symptoms (behaves
identically). But then I'm at a bit of a loss as to how to use git
bisect to pin this down.
I'll look into this some more; I'm sure I must have misunderstood
something in the documentation.
>
> > I CC'ed Dou Liyang. He has changed the early APIC setup code and there has
> > been an issue reported already. Though I lost track of that. Dou, any
> ^^^^^^^^ Is it this one?
> ? ? ? ? ? ? https://marc.info/?l=linux-kernel&m=151188084018443
> > pointers?
> >
>
> Not sure, but seems the APIC failed to start in that 32-bit system.
>
> I will look into it.
>
> Alex,
>
> Could you give me your .config file and the dmesg-log of 4.15.0-rc3.
I am attaching the config file I used, as well as the output of
'journalctl --boot=-1 -k' (boot=-1 because that specific boot fails; I
need to reboot with a working kernel).
Unfortunately, the logs don't show the lockup trace and I don't know
how to get them to: I've run
grep -r -i "lockup"
in /var/log/ with no results.
---
Merging the contents of another exchange spawned from the original
thread:
On Wed, Dec 20, 2017 at 02:12:05AM +0000, Dexuan Cui wrote:
> Hi Alexandru,
> Is there any chance this could be a build issue somehow? :-)
>
> I would suggest you check out 4.15.0-rc3 cleanly, run "make mrproper", and generate a proper .config a\
nd build & install the kernel, and double check the grub.cfg to make the correct kernel is used to boot \
the machine.
>
Hmm.. Perhaps, but I do make clean && make mrproper after every single
compile, so I am quite confident that's not it.
The config file I used to build 4.15.0-rc3 from Linus' tree is
attached. I produced it by taking the current one working for me in
4.14.7 on that same machine and issuing
yes "" | make oldconfig
> I tried the 4.15.0-rc3 kernel in my Linux VM running on Hyper-V, and everything worked fine.
> Unluckily I didn't have a bare metal machine to test the kernel.
>
> For Linux VM running on Hyper-V, we did get "spurious APIC interrupt through vector " and a patchset, \
which included the patch you identifed ("genirq: Add config option for reservation mode"), was made to f\
ix the issue. But since you're using a physical machine rathter than a VM, I suspect it should be a diff\
erent issue.
>
> When you have the issue with 4.15.0-rc3, can you try if the kernel pararmeter pci=nomsi would help? If\
it works, we'll have more confidenct to say there is a kernel bug.
>
It did work! Passing that kernel parameter through the relevant line
of grub.cfg corrects 4.15.0-rc3's misbehaviour (that machine is now
booted into it and logged in with no issue).
> Good luck!
>
> Thanks,
> -- Dexuan
On Wed, 20 Dec 2017, Alexandru Chirvasitu wrote:
> On Wed, Dec 20, 2017 at 11:58:57AM +0800, Dou Liyang wrote:
> > At 12/20/2017 08:31 AM, Thomas Gleixner wrote:
> > > > I had never heard of 'bisect' before this casual mention (you might tell
> > > > I am a bit out of my depth). I've since applied it to Linus' tree between
> > >
> > > > bebc608 Linux 4.14 (good)
> > > >
> > > > and
> > > >
> > > > 4fbd8d1 Linux 4.15-rc1 (bad)
> > >
> > > Is Linus current head 4.15-rc4 bad as well?
> > >
> > [...]
>
> Yes. Exactly the same symptoms on
>
> 1291a0d5 Linux 4.15-rc4
>
> compiled just now from Linus' tree.
Ok, lets take a step back. The bisect/kexec attempts led us away from the
initial problem which is the machine locking up after login, right?
Could you try the patch below on top of Linus tree (rc5+)?
Thanks,
tglx
8<---------------
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -151,7 +151,7 @@ static struct apic apic_flat __ro_after_
.apic_id_valid = default_apic_id_valid,
.apic_id_registered = flat_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
.irq_dest_mode = 1, /* logical */
.disable_esr = 0,
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -105,7 +105,7 @@ static struct apic apic_default __ro_aft
.apic_id_valid = default_apic_id_valid,
.apic_id_registered = default_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
/* logical delivery broadcast to all CPUs: */
.irq_dest_mode = 1,
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -184,7 +184,7 @@ static struct apic apic_x2apic_cluster _
.apic_id_valid = x2apic_apic_id_valid,
.apic_id_registered = x2apic_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
.irq_dest_mode = 1, /* logical */
.disable_esr = 0,
On Wed, 20 Dec 2017, Alexandru Chirvasitu wrote:
> Merging the contents of another exchange spawned from the original
> > On Wed, Dec 20, 2017 at 02:12:05AM +0000, Dexuan Cui wrote:
> > For Linux VM running on Hyper-V, we did get "spurious APIC interrupt
> > through vector " and a patchset, which included the patch you identifed
> > ("genirq: Add config option for reservation mode"), was made to fix the
> > issue. But since you're using a physical machine rathter than a VM, I
> > suspect it should be a different issue.
Aaargh! Why was this never reported and where is that magic patchset?
Thanks,
tglx
Thanks for all of this!
On Thu, Dec 28, 2017 at 12:00:47PM +0100, Thomas Gleixner wrote:
> On Wed, 20 Dec 2017, Alexandru Chirvasitu wrote:
> > On Wed, Dec 20, 2017 at 11:58:57AM +0800, Dou Liyang wrote:
> > > At 12/20/2017 08:31 AM, Thomas Gleixner wrote:
> > > > > I had never heard of 'bisect' before this casual mention (you might tell
> > > > > I am a bit out of my depth). I've since applied it to Linus' tree between
> > > >
> > > > > bebc608 Linux 4.14 (good)
> > > > >
> > > > > and
> > > > >
> > > > > 4fbd8d1 Linux 4.15-rc1 (bad)
> > > >
> > > > Is Linus current head 4.15-rc4 bad as well?
> > > >
> > > [...]
> >
> > Yes. Exactly the same symptoms on
> >
> > 1291a0d5 Linux 4.15-rc4
> >
> > compiled just now from Linus' tree.
>
> Ok, lets take a step back. The bisect/kexec attempts led us away from the
> initial problem which is the machine locking up after login, right?
>
Yes; sorry about that..
> Could you try the patch below on top of Linus tree (rc5+)?
>
> Thanks,
>
> tglx
>
> 8<---------------
> --- a/arch/x86/kernel/apic/apic_flat_64.c
> +++ b/arch/x86/kernel/apic/apic_flat_64.c
> @@ -151,7 +151,7 @@ static struct apic apic_flat __ro_after_
> .apic_id_valid = default_apic_id_valid,
> .apic_id_registered = flat_apic_id_registered,
>
> - .irq_delivery_mode = dest_LowestPrio,
> + .irq_delivery_mode = dest_Fixed,
> .irq_dest_mode = 1, /* logical */
>
> .disable_esr = 0,
> --- a/arch/x86/kernel/apic/probe_32.c
> +++ b/arch/x86/kernel/apic/probe_32.c
> @@ -105,7 +105,7 @@ static struct apic apic_default __ro_aft
> .apic_id_valid = default_apic_id_valid,
> .apic_id_registered = default_apic_id_registered,
>
> - .irq_delivery_mode = dest_LowestPrio,
> + .irq_delivery_mode = dest_Fixed,
> /* logical delivery broadcast to all CPUs: */
> .irq_dest_mode = 1,
>
> --- a/arch/x86/kernel/apic/x2apic_cluster.c
> +++ b/arch/x86/kernel/apic/x2apic_cluster.c
> @@ -184,7 +184,7 @@ static struct apic apic_x2apic_cluster _
> .apic_id_valid = x2apic_apic_id_valid,
> .apic_id_registered = x2apic_apic_id_registered,
>
> - .irq_delivery_mode = dest_LowestPrio,
> + .irq_delivery_mode = dest_Fixed,
> .irq_dest_mode = 1, /* logical */
>
> .disable_esr = 0,
>
I tried both patches that you guys sent in the last couple of
messages. I applied them separately to the last 4.15-rc5 kernel I had
(the one for which I sent Dou the journalctl output). The diffs are
both to that version.
Results follow.
(1)
Dou's patch:
------------------------------------------------------------
x86/vector: Replace the raw_spin_lock() with
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 7504491..e5bab02 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -726,6 +726,7 @@ static int apic_set_affinity(struct irq_data *irqd,
const struct cpumask *dest, bool force)
{
struct apic_chip_data *apicd = apic_chip_data(irqd);
+ unsigned long flags;
int err;
/*
@@ -740,13 +741,13 @@ static int apic_set_affinity(struct irq_data *irqd,
(apicd->is_managed || apicd->can_reserve))
return IRQ_SET_MASK_OK;
- raw_spin_lock(&vector_lock);
+ raw_spin_lock_irqsave(&vector_lock, flags);
cpumask_and(vector_searchmask, dest, cpu_online_mask);
if (irqd_affinity_is_managed(irqd))
err = assign_managed_vector(irqd, vector_searchmask);
else
err = assign_vector_locked(irqd, vector_searchmask);
- raw_spin_unlock(&vector_lock);
+ raw_spin_unlock_irqrestore(&vector_lock, flags);
return err ? err : IRQ_SET_MASK_OK;
}
------------------------------------------------------------
With this, I still get the lockup messages after login, but not the
freezes!
The lockups register in the log, which I am attaching (see below for
attachment naming conventions).
The computer's still clearly impaired (ethernet won't connect again
for instance, and the CPU distress messages happen periodically
throughout the tty session), but at least it's logged now.
---
(2)
Thomas' patch:
------------------------------------------------------------
apic patch from tglx
diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c
index aa85690..1f734d4 100644
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -151,7 +151,7 @@ static struct apic apic_flat __ro_after_init = {
.apic_id_valid = default_apic_id_valid,
.apic_id_registered = flat_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
.irq_dest_mode = 1, /* logical */
.disable_esr = 0,
diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
index fa22017..765cded 100644
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -105,7 +105,7 @@ static struct apic apic_default __ro_after_init = {
.apic_id_valid = default_apic_id_valid,
.apic_id_registered = default_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
/* logical delivery broadcast to all CPUs: */
.irq_dest_mode = 1,
diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 622f13c..39568bd 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -184,7 +184,7 @@ static struct apic apic_x2apic_cluster __ro_after_init = {
.apic_id_valid = x2apic_apic_id_valid,
.apic_id_registered = x2apic_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
.irq_dest_mode = 1, /* logical */
.disable_esr = 0,
------------------------------------------------------------
This gives me the same disabling lockups as before, i.e. I have to
reboot. Correspondingly, the log I'm attaching for this kernel won't
be of much use, because it's called with `journalctl --boot=-1` after
the fact.
Might still be of some use..
---
The log files I'm attaching comply with the following naming pattern:
'dou' means the log comes from the kernel patched with Dou's patch;
'thms' refers to Thomas' patch;
'jrnl' means it came from journalctl with various boot=? options;
'dmesg' means it came from calling dmesg;
'noparams' means the kernel was called with no additional parameters;
'debug' means it was called with 'apic=debug'.
---
P.S.
It was very considerate to send the attachment Dou, but that shouldn't
be necessary anymore; the issues I had with 'git apply' were
blank-space-related, and I've managed to resolve them.
So anything copy-pastable directly in the message body should do if I
try more patches.
Thank you!
On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> On Thu, Dec 28, 2017 at 12:00:47PM +0100, Thomas Gleixner wrote:
> > Ok, lets take a step back. The bisect/kexec attempts led us away from the
> > initial problem which is the machine locking up after login, right?
> >
>
> Yes; sorry about that..
Nothing to be sorry about.
> x86/vector: Replace the raw_spin_lock() with
>
> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> index 7504491..e5bab02 100644
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -726,6 +726,7 @@ static int apic_set_affinity(struct irq_data *irqd,
> const struct cpumask *dest, bool force)
> {
> struct apic_chip_data *apicd = apic_chip_data(irqd);
> + unsigned long flags;
> int err;
>
> /*
> @@ -740,13 +741,13 @@ static int apic_set_affinity(struct irq_data *irqd,
> (apicd->is_managed || apicd->can_reserve))
> return IRQ_SET_MASK_OK;
>
> - raw_spin_lock(&vector_lock);
> + raw_spin_lock_irqsave(&vector_lock, flags);
> cpumask_and(vector_searchmask, dest, cpu_online_mask);
> if (irqd_affinity_is_managed(irqd))
> err = assign_managed_vector(irqd, vector_searchmask);
> else
> err = assign_vector_locked(irqd, vector_searchmask);
> - raw_spin_unlock(&vector_lock);
> + raw_spin_unlock_irqrestore(&vector_lock, flags);
> return err ? err : IRQ_SET_MASK_OK;
> }
>
> With this, I still get the lockup messages after login, but not the
> freezes!
That's really interesting. There should be no code path which calls into
that with interrupts enabled. I assume you never ran that kernel with
CONFIG_PROVE_LOCKING=y.
Find below a debug patch which should show us the call chain for that
case. Please apply that on top of Dou's patch so the machine stays
accessible. Plain output from dmesg is sufficient.
> The lockups register in the log, which I am attaching (see below for
> attachment naming conventions).
Hmm. That's RCU lockups and that backtrace on the CPU which gets the stall
looks very familiar. I'd like to see the above result first and then I'll
send you another pile of patches which might cure that RCU issue.
Thanks,
tglx
8<-------------------
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -729,6 +729,8 @@ static int apic_set_affinity(struct irq_
unsigned long flags;
int err;
+ WARN_ON_ONCE(!irqs_disabled());
+
/*
* Core code can call here for inactive interrupts. For inactive
* interrupts which use managed or reservation mode there is no
On Thu, Dec 28, 2017 at 03:48:15PM +0100, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > On Thu, Dec 28, 2017 at 12:00:47PM +0100, Thomas Gleixner wrote:
> > > Ok, lets take a step back. The bisect/kexec attempts led us away from the
> > > initial problem which is the machine locking up after login, right?
> > >
> >
> > Yes; sorry about that..
>
> Nothing to be sorry about.
>
> > x86/vector: Replace the raw_spin_lock() with
> >
> > diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> > index 7504491..e5bab02 100644
> > --- a/arch/x86/kernel/apic/vector.c
> > +++ b/arch/x86/kernel/apic/vector.c
> > @@ -726,6 +726,7 @@ static int apic_set_affinity(struct irq_data *irqd,
> > const struct cpumask *dest, bool force)
> > {
> > struct apic_chip_data *apicd = apic_chip_data(irqd);
> > + unsigned long flags;
> > int err;
> >
> > /*
> > @@ -740,13 +741,13 @@ static int apic_set_affinity(struct irq_data *irqd,
> > (apicd->is_managed || apicd->can_reserve))
> > return IRQ_SET_MASK_OK;
> >
> > - raw_spin_lock(&vector_lock);
> > + raw_spin_lock_irqsave(&vector_lock, flags);
> > cpumask_and(vector_searchmask, dest, cpu_online_mask);
> > if (irqd_affinity_is_managed(irqd))
> > err = assign_managed_vector(irqd, vector_searchmask);
> > else
> > err = assign_vector_locked(irqd, vector_searchmask);
> > - raw_spin_unlock(&vector_lock);
> > + raw_spin_unlock_irqrestore(&vector_lock, flags);
> > return err ? err : IRQ_SET_MASK_OK;
> > }
> >
> > With this, I still get the lockup messages after login, but not the
> > freezes!
>
> That's really interesting. There should be no code path which calls into
> that with interrupts enabled. I assume you never ran that kernel with
> CONFIG_PROVE_LOCKING=y.
>
Correct. That option is not set in .config.
> Find below a debug patch which should show us the call chain for that
> case. Please apply that on top of Dou's patch so the machine stays
> accessible. Plain output from dmesg is sufficient.
>
> > The lockups register in the log, which I am attaching (see below for
> > attachment naming conventions).
>
> Hmm. That's RCU lockups and that backtrace on the CPU which gets the stall
> looks very familiar. I'd like to see the above result first and then I'll
> send you another pile of patches which might cure that RCU issue.
>
> Thanks,
>
> tglx
>
> 8<-------------------
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -729,6 +729,8 @@ static int apic_set_affinity(struct irq_
> unsigned long flags;
> int err;
>
> + WARN_ON_ONCE(!irqs_disabled());
> +
> /*
> * Core code can call here for inactive interrupts. For inactive
> * interrupts which use managed or reservation mode there is no
>
>
>
Bit of a step back here: the kernel treated with Dou's patch no longer
logs me in reliably as before, with or without this newest patch on
top..
So now I sometimes get immediate lockups and freezes upon trying to
log in, and other times I get logged in but get a freeze seconds
later.
In no case can I roam around long nough to get a dmesg, and I no
longer get the non-freezing lockups from before. I can't imagine what
I could possibly have changed..
Here's the output of `git log --pretty=oneline -5` on the branch I'm
working in.
--------------------
f2c02af5cc1d620c039b21fab0ca5948a06daf90 2nd tglx patch
7715575170bacf3566d400b9f2210a10ce152880 x86/vector: Replace the raw_spin_lock() with raw_spin_lock_irqsave()
8d9d56caf33d78bfe6b6087767b1b84acee58458 x86-32: fix kexec with stack canary (CONFIG_CC_STACKPROTECTOR)
a197e9dea4ccb72e1a6457fac15329bd5319e719 irq/matrix: Remove the overused BUGON() in irq_matrix_assign_system()
464e1d5f23cca236b930ef068c328a64cab78fb1 Linux 4.15-rc5
--------------------
7715575170bacf3566d400b9f2210a10ce152880, which is the kernel with
Dou's patch, logged me in and allowed me to produce the dmesg from
before. I did this a couple of times back then. I no longer can, for
some reason, as it's reverted back to the no-go lockups from before.
And the next one, f2c02af5cc1d620c039b21fab0ca5948a06daf90, where I
applied the patch you just sent, behaves identically.
On Thu, Dec 28, 2017 at 10:48:35AM -0500, Alexandru Chirvasitu wrote:
> On Thu, Dec 28, 2017 at 03:48:15PM +0100, Thomas Gleixner wrote:
> > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > > On Thu, Dec 28, 2017 at 12:00:47PM +0100, Thomas Gleixner wrote:
> > > > Ok, lets take a step back. The bisect/kexec attempts led us away from the
> > > > initial problem which is the machine locking up after login, right?
> > > >
> > >
> > > Yes; sorry about that..
> >
> > Nothing to be sorry about.
> >
> > > x86/vector: Replace the raw_spin_lock() with
> > >
> > > diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> > > index 7504491..e5bab02 100644
> > > --- a/arch/x86/kernel/apic/vector.c
> > > +++ b/arch/x86/kernel/apic/vector.c
> > > @@ -726,6 +726,7 @@ static int apic_set_affinity(struct irq_data *irqd,
> > > const struct cpumask *dest, bool force)
> > > {
> > > struct apic_chip_data *apicd = apic_chip_data(irqd);
> > > + unsigned long flags;
> > > int err;
> > >
> > > /*
> > > @@ -740,13 +741,13 @@ static int apic_set_affinity(struct irq_data *irqd,
> > > (apicd->is_managed || apicd->can_reserve))
> > > return IRQ_SET_MASK_OK;
> > >
> > > - raw_spin_lock(&vector_lock);
> > > + raw_spin_lock_irqsave(&vector_lock, flags);
> > > cpumask_and(vector_searchmask, dest, cpu_online_mask);
> > > if (irqd_affinity_is_managed(irqd))
> > > err = assign_managed_vector(irqd, vector_searchmask);
> > > else
> > > err = assign_vector_locked(irqd, vector_searchmask);
> > > - raw_spin_unlock(&vector_lock);
> > > + raw_spin_unlock_irqrestore(&vector_lock, flags);
> > > return err ? err : IRQ_SET_MASK_OK;
> > > }
> > >
> > > With this, I still get the lockup messages after login, but not the
> > > freezes!
> >
> > That's really interesting. There should be no code path which calls into
> > that with interrupts enabled. I assume you never ran that kernel with
> > CONFIG_PROVE_LOCKING=y.
> >
>
> Correct. That option is not set in .config.
>
> > Find below a debug patch which should show us the call chain for that
> > case. Please apply that on top of Dou's patch so the machine stays
> > accessible. Plain output from dmesg is sufficient.
> >
> > > The lockups register in the log, which I am attaching (see below for
> > > attachment naming conventions).
> >
> > Hmm. That's RCU lockups and that backtrace on the CPU which gets the stall
> > looks very familiar. I'd like to see the above result first and then I'll
> > send you another pile of patches which might cure that RCU issue.
> >
> > Thanks,
> >
> > tglx
> >
> > 8<-------------------
> > --- a/arch/x86/kernel/apic/vector.c
> > +++ b/arch/x86/kernel/apic/vector.c
> > @@ -729,6 +729,8 @@ static int apic_set_affinity(struct irq_
> > unsigned long flags;
> > int err;
> >
> > + WARN_ON_ONCE(!irqs_disabled());
> > +
> > /*
> > * Core code can call here for inactive interrupts. For inactive
> > * interrupts which use managed or reservation mode there is no
> >
> >
> >
>
> Bit of a step back here: the kernel treated with Dou's patch no longer
> logs me in reliably as before, with or without this newest patch on
> top..
>
> So now I sometimes get immediate lockups and freezes upon trying to
> log in, and other times I get logged in but get a freeze seconds
> later.
>
> In no case can I roam around long nough to get a dmesg, and I no
> longer get the non-freezing lockups from before. I can't imagine what
> I could possibly have changed..
>
> Here's the output of `git log --pretty=oneline -5` on the branch I'm
> working in.
>
> --------------------
>
> f2c02af5cc1d620c039b21fab0ca5948a06daf90 2nd tglx patch
> 7715575170bacf3566d400b9f2210a10ce152880 x86/vector: Replace the raw_spin_lock() with raw_spin_lock_irqsave()
> 8d9d56caf33d78bfe6b6087767b1b84acee58458 x86-32: fix kexec with stack canary (CONFIG_CC_STACKPROTECTOR)
> a197e9dea4ccb72e1a6457fac15329bd5319e719 irq/matrix: Remove the overused BUGON() in irq_matrix_assign_system()
> 464e1d5f23cca236b930ef068c328a64cab78fb1 Linux 4.15-rc5
>
> --------------------
>
> 7715575170bacf3566d400b9f2210a10ce152880, which is the kernel with
> Dou's patch, logged me in and allowed me to produce the dmesg from
> before. I did this a couple of times back then. I no longer can, for
> some reason, as it's reverted back to the no-go lockups from before.
>
> And the next one, f2c02af5cc1d620c039b21fab0ca5948a06daf90, where I
> applied the patch you just sent, behaves identically.
>
>
Actually, it decided to cooperate for just long enough for me to get
the dmesg out. Attached.
This is from the kernel you asked about: Dou's patch + yours, i.e. the
latest one in that git log I just sent, booted up with 'apic=debug'.
On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> Actually, it decided to cooperate for just long enough for me to get
> the dmesg out. Attached.
>
> This is from the kernel you asked about: Dou's patch + yours, i.e. the
> latest one in that git log I just sent, booted up with 'apic=debug'.
Ok. As I suspected that warning does not trigger. I would have been
massively surprised if that happened. So Dou's patch is just a red herring
and just might change the timing enough to make the problem 'hide'.
Can you try something completely different please?
Just use plain Linus tree without any additional patches on top and disable
CONFIG_NO_HZ_IDLE, i.e. select CONFIG_HZ_PERIODIC.
If that works, then reenable it and add 'nohz=off' to the kernel command
line.
Thanks,
tglx
On Thu, Dec 28, 2017 at 05:10:28PM +0100, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > Actually, it decided to cooperate for just long enough for me to get
> > the dmesg out. Attached.
> >
> > This is from the kernel you asked about: Dou's patch + yours, i.e. the
> > latest one in that git log I just sent, booted up with 'apic=debug'.
>
> Ok. As I suspected that warning does not trigger. I would have been
> massively surprised if that happened. So Dou's patch is just a red herring
> and just might change the timing enough to make the problem 'hide'.
>
> Can you try something completely different please?
>
> Just use plain Linus tree without any additional patches on top and disable
> CONFIG_NO_HZ_IDLE, i.e. select CONFIG_HZ_PERIODIC.
>
> If that works, then reenable it and add 'nohz=off' to the kernel command
> line.
>
No go here I'm afraid:
Linus' clean 4.15-rc5 compiled with CONFIG_HZ_PERIODIC exhibits the
familiar behaviour: lockups, sometimes instant upon trying to log in,
sometimes logging me in and freaking out seconds later.
Attached config for verification.
> Thanks,
>
> tglx
>
On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> On Thu, Dec 28, 2017 at 05:10:28PM +0100, Thomas Gleixner wrote:
> > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > > Actually, it decided to cooperate for just long enough for me to get
> > > the dmesg out. Attached.
> > >
> > > This is from the kernel you asked about: Dou's patch + yours, i.e. the
> > > latest one in that git log I just sent, booted up with 'apic=debug'.
> >
> > Ok. As I suspected that warning does not trigger. I would have been
> > massively surprised if that happened. So Dou's patch is just a red herring
> > and just might change the timing enough to make the problem 'hide'.
> >
> > Can you try something completely different please?
> >
> > Just use plain Linus tree without any additional patches on top and disable
> > CONFIG_NO_HZ_IDLE, i.e. select CONFIG_HZ_PERIODIC.
> >
> > If that works, then reenable it and add 'nohz=off' to the kernel command
> > line.
> >
>
> No go here I'm afraid:
>
> Linus' clean 4.15-rc5 compiled with CONFIG_HZ_PERIODIC exhibits the
> familiar behaviour: lockups, sometimes instant upon trying to log in,
> sometimes logging me in and freaking out seconds later.
Ok. So it's not the issue I had in mind.
Back to some of the interesting bits in the logs:
[ 36.017942] spurious APIC interrupt through vector ff on CPU#0, should never happen.
Does that message ever show up in 4.14 or 4.9?
Thanks,
tglx
No; it seems to be tied to this specific issue, and I was seeing even
before getting logs just now, whenever I'd start one of the bad
kernels in recovery mode.
But no, I've never seen that in any other logs, or on any other
screens outside of those popping up in relation to this problem.
On Thu, Dec 28, 2017 at 06:29:05PM +0100, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > On Thu, Dec 28, 2017 at 05:10:28PM +0100, Thomas Gleixner wrote:
> > > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > > > Actually, it decided to cooperate for just long enough for me to get
> > > > the dmesg out. Attached.
> > > >
> > > > This is from the kernel you asked about: Dou's patch + yours, i.e. the
> > > > latest one in that git log I just sent, booted up with 'apic=debug'.
> > >
> > > Ok. As I suspected that warning does not trigger. I would have been
> > > massively surprised if that happened. So Dou's patch is just a red herring
> > > and just might change the timing enough to make the problem 'hide'.
> > >
> > > Can you try something completely different please?
> > >
> > > Just use plain Linus tree without any additional patches on top and disable
> > > CONFIG_NO_HZ_IDLE, i.e. select CONFIG_HZ_PERIODIC.
> > >
> > > If that works, then reenable it and add 'nohz=off' to the kernel command
> > > line.
> > >
> >
> > No go here I'm afraid:
> >
> > Linus' clean 4.15-rc5 compiled with CONFIG_HZ_PERIODIC exhibits the
> > familiar behaviour: lockups, sometimes instant upon trying to log in,
> > sometimes logging me in and freaking out seconds later.
>
> Ok. So it's not the issue I had in mind.
>
> Back to some of the interesting bits in the logs:
>
> [ 36.017942] spurious APIC interrupt through vector ff on CPU#0, should never happen.
>
> Does that message ever show up in 4.14 or 4.9?
>
> Thanks,
>
> tglx
On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> No; it seems to be tied to this specific issue, and I was seeing even
> before getting logs just now, whenever I'd start one of the bad
> kernels in recovery mode.
>
> But no, I've never seen that in any other logs, or on any other
> screens outside of those popping up in relation to this problem.
Ok. I'll dig into it and we have a 100% reproducer reported by someone else
now, which might give us simpler insight into that issue. I'll let you know
once I have something to test. Might be a couple of days though.
Thanks,
tglx
> From: Thomas Gleixner [mailto:[email protected]]
> Sent: Thursday, December 28, 2017 03:03
> > > On Wed, Dec 20, 2017 at 02:12:05AM +0000, Dexuan Cui wrote:
>
> > > For Linux VM running on Hyper-V, we did get "spurious APIC interrupt
> > > through vector " and a patchset, which included the patch you identifed
> > > ("genirq: Add config option for reservation mode"), was made to fix the
> > > issue. But since you're using a physical machine rathter than a VM, I
> > > suspect it should be a different issue.
>
> Aaargh! Why was this never reported and where is that magic patchset?
> tglx
Hi Thomas,
The Hyper-V specific issue was reported and you made a patchset to fix it:
https://patchwork.kernel.org/patch/10006171/
https://lkml.org/lkml/2017/10/17/120
Thanks,
-- Dexuan
On Thu, 28 Dec 2017, Dexuan Cui wrote:
> > From: Thomas Gleixner [mailto:[email protected]]
> > Sent: Thursday, December 28, 2017 03:03
> > > > On Wed, Dec 20, 2017 at 02:12:05AM +0000, Dexuan Cui wrote:
> >
> > > > For Linux VM running on Hyper-V, we did get "spurious APIC interrupt
> > > > through vector " and a patchset, which included the patch you identifed
> > > > ("genirq: Add config option for reservation mode"), was made to fix the
> > > > issue. But since you're using a physical machine rathter than a VM, I
> > > > suspect it should be a different issue.
> >
> > Aaargh! Why was this never reported and where is that magic patchset?
> > tglx
>
> Hi Thomas,
> The Hyper-V specific issue was reported and you made a patchset to fix it:
> https://patchwork.kernel.org/patch/10006171/
> https://lkml.org/lkml/2017/10/17/120
Ah, ok. I did not make the connection. I'll have a scan through the tree to
figure out whether there is some other weird place which is missing that.
Thanks,
tglx
On Thu, 28 Dec 2017, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
>
> > No; it seems to be tied to this specific issue, and I was seeing even
> > before getting logs just now, whenever I'd start one of the bad
> > kernels in recovery mode.
> >
> > But no, I've never seen that in any other logs, or on any other
> > screens outside of those popping up in relation to this problem.
>
> Ok. I'll dig into it and we have a 100% reproducer reported by someone else
> now, which might give us simpler insight into that issue. I'll let you know
> once I have something to test. Might be a couple of days though.
Can you please provide the output of
lspci -vvv
from a working kernel, preferrably 4.14.y
Thanks,
tglx
Attached.
I don't have a 4.14 family kernel available at the moment on that
machine. What I'm attaching comes from the 4.13 one I was playing with
yesterday, what with kexec and all.
On Thu, Dec 28, 2017 at 10:54:25PM +0100, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Thomas Gleixner wrote:
> > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> >
> > > No; it seems to be tied to this specific issue, and I was seeing even
> > > before getting logs just now, whenever I'd start one of the bad
> > > kernels in recovery mode.
> > >
> > > But no, I've never seen that in any other logs, or on any other
> > > screens outside of those popping up in relation to this problem.
> >
> > Ok. I'll dig into it and we have a 100% reproducer reported by someone else
> > now, which might give us simpler insight into that issue. I'll let you know
> > once I have something to test. Might be a couple of days though.
>
> Can you please provide the output of
>
> lspci -vvv
>
> from a working kernel, preferrably 4.14.y
>
> Thanks,
>
> tglx
On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> Attached.
>
> I don't have a 4.14 family kernel available at the moment on that
> machine. What I'm attaching comes from the 4.13 one I was playing with
> yesterday, what with kexec and all.
Good enough. Thanks !
On Thu, 28 Dec 2017, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
>
> > Attached.
> >
> > I don't have a 4.14 family kernel available at the moment on that
> > machine. What I'm attaching comes from the 4.13 one I was playing with
> > yesterday, what with kexec and all.
>
> Good enough. Thanks !
Spoke too fast. Could you please run that command as root?
And while at it please provide the output of
cat /proc/interrupts
as well.
Thanks,
tglx
Attached, but heads up on this: when redirecting the output of lspci
-vvv to a text file as root I get
pcilib: sysfs_read_vpd: read failed: Input/output error
I can find bugs filed for various distros to this same effect, but
haven't tracked down any explanations.
On Fri, Dec 29, 2017 at 12:19:19AM +0100, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Thomas Gleixner wrote:
>
> > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> >
> > > Attached.
> > >
> > > I don't have a 4.14 family kernel available at the moment on that
> > > machine. What I'm attaching comes from the 4.13 one I was playing with
> > > yesterday, what with kexec and all.
> >
> > Good enough. Thanks !
>
> Spoke too fast. Could you please run that command as root?
>
> And while at it please provide the output of
>
> cat /proc/interrupts
>
> as well.
>
> Thanks,
>
> tglx
>
On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> Attached, but heads up on this: when redirecting the output of lspci
> -vvv to a text file as root I get
>
> pcilib: sysfs_read_vpd: read failed: Input/output error
>
> I can find bugs filed for various distros to this same effect, but
> haven't tracked down any explanations.
Weird, but the info looks complete.
Can you please add 'pci=nomsi' to the 4.15 kernel command line and see
whether that works?
Thanks,
tglx
On Fri, Dec 29, 2017 at 12:36:37AM +0100, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
>
> > Attached, but heads up on this: when redirecting the output of lspci
> > -vvv to a text file as root I get
> >
> > pcilib: sysfs_read_vpd: read failed: Input/output error
> >
> > I can find bugs filed for various distros to this same effect, but
> > haven't tracked down any explanations.
>
> Weird, but the info looks complete.
>
> Can you please add 'pci=nomsi' to the 4.15 kernel command line and see
> whether that works?
It does (emailing from that successful boot as we speak). I'm on a
clean 4.15-rc5 (as in no patches, etc.).
This was also suggested way at the top of this thread by Dexuan Cui
for 4.15-rc3 (where this exchange started), and it worked back then
too.
>
> Thanks,
>
> tglx
On Thu, Dec 28, 2017 at 06:30:58PM -0500, Alexandru Chirvasitu wrote:
> Attached, but heads up on this: when redirecting the output of lspci
> -vvv to a text file as root I get
>
> pcilib: sysfs_read_vpd: read failed: Input/output error
>
> I can find bugs filed for various distros to this same effect, but
> haven't tracked down any explanations.
This is a tangent, but I think you should *always* see "Input/output
error" on this system when running "lspci -vvv" as root, regardless of
whether you redirect the output (the error probably goes to stderr,
not stdout, so it's probably easy to miss when not redirecting the
output).
I think this is the -EIO return from pci_vpd_read(), which probably
means pci_vpd_size() returned 0 for one of your devices, which means
the VPD data provided by the device wasn't formatted correctly. If
this happens, you should see a warning in dmesg about it ("invalid VPD
tag" or similar) -- could you verify that?
It's possible we should return something other than -EIO, or maybe
pcilib should do something other than emitting the warning. In
pcilib, sysfs_read_vpd() emits the warning [1], and it would seem sort
of ugly to special-case EIO, so maybe we should change this in the
kernel.
It looks like your Qualcomm Atheros Attansic NIC at 06:00.0 is the
only device with VPD, so that's probably the one:
06:00.0 Ethernet controller: Qualcomm Atheros Attansic L2 Fast Ethernet
Capabilities: [6c] Vital Product Data
Not readable
I think lspci would still print "Not readable" if we just made the
kernel return 0 instead of -EIO [2].
Bjorn
[1] https://git.kernel.org/pub/scm/utils/pciutils/pciutils.git/tree/lib/sysfs.c#n410
[2] https://git.kernel.org/pub/scm/utils/pciutils/pciutils.git/tree/ls-vpd.c#n87
On Thu, Dec 28, 2017 at 06:15:19PM -0600, Bjorn Helgaas wrote:
> On Thu, Dec 28, 2017 at 06:30:58PM -0500, Alexandru Chirvasitu wrote:
> > Attached, but heads up on this: when redirecting the output of lspci
> > -vvv to a text file as root I get
> >
> > pcilib: sysfs_read_vpd: read failed: Input/output error
> >
> > I can find bugs filed for various distros to this same effect, but
> > haven't tracked down any explanations.
>
> This is a tangent, but I think you should *always* see "Input/output
> error" on this system when running "lspci -vvv" as root, regardless of
> whether you redirect the output (the error probably goes to stderr,
> not stdout, so it's probably easy to miss when not redirecting the
> output).
>
> I think this is the -EIO return from pci_vpd_read(), which probably
> means pci_vpd_size() returned 0 for one of your devices, which means
> the VPD data provided by the device wasn't formatted correctly. If
> this happens, you should see a warning in dmesg about it ("invalid VPD
> tag" or similar) -- could you verify that?
>
This in dmesg:
pci 0000:06:00.0: [Firmware Bug]: disabling VPD access (can't
determine size of non-standard VPD format)
So yes, looks like you pinned it down good. No other VPD instances in
dmesg.
And yes, the error does seem to always be present. I see it with
lspci -vvv 2>&1 | grep pcilib
so it was there in stderr all along.
> It's possible we should return something other than -EIO, or maybe
> pcilib should do something other than emitting the warning. In
> pcilib, sysfs_read_vpd() emits the warning [1], and it would seem sort
> of ugly to special-case EIO, so maybe we should change this in the
> kernel.
>
> It looks like your Qualcomm Atheros Attansic NIC at 06:00.0 is the
> only device with VPD, so that's probably the one:
>
> 06:00.0 Ethernet controller: Qualcomm Atheros Attansic L2 Fast Ethernet
> Capabilities: [6c] Vital Product Data
> Not readable
>
> I think lspci would still print "Not readable" if we just made the
> kernel return 0 instead of -EIO [2].
>
> Bjorn
>
> [1] https://git.kernel.org/pub/scm/utils/pciutils/pciutils.git/tree/lib/sysfs.c#n410
> [2] https://git.kernel.org/pub/scm/utils/pciutils/pciutils.git/tree/ls-vpd.c#n87
On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> On Fri, Dec 29, 2017 at 12:36:37AM +0100, Thomas Gleixner wrote:
> > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> >
> > > Attached, but heads up on this: when redirecting the output of lspci
> > > -vvv to a text file as root I get
> > >
> > > pcilib: sysfs_read_vpd: read failed: Input/output error
> > >
> > > I can find bugs filed for various distros to this same effect, but
> > > haven't tracked down any explanations.
> >
> > Weird, but the info looks complete.
> >
> > Can you please add 'pci=nomsi' to the 4.15 kernel command line and see
> > whether that works?
>
> It does (emailing from that successful boot as we speak). I'm on a
> clean 4.15-rc5 (as in no patches, etc.).
>
> This was also suggested way at the top of this thread by Dexuan Cui
> for 4.15-rc3 (where this exchange started), and it worked back then
> too.
I missed that part of the conversation. Let me stare into the MSI code
again.
Thanks,
tglx
All right, I tried to do some more digging around, in the hope of
getting as close to the source of the problem as I can.
I went back to the very first commit that went astray for me, 2db1f95
(which is the only one actually panicking), and tried to move from its
parent 90ad9e2 (that boots fine) to it gradually, altering the code in
small chunks.
I tried to ignore the stuff that clearly shouldn't make a difference,
such as definitions. So in the end I get defined-but-unused-function
errors in my compilations, but I'm ignoring those for now. Some
results:
(1) When I move from the good commit 90ad9e2 according to the attached
bad-diff (which moves partly towards 2db1f95), I get a panic.
(2) On the other hand, when I further change this last panicking
commit by simply doing
----------------------------------------------------------------
removed activate / deactivate from x86_vector_domain_ops
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 7317ba5a..063594d 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -514,8 +514,6 @@ void x86_vector_debug_show(struct seq_file *m, struct irq_domain *d,
static const struct irq_domain_ops x86_vector_domain_ops = {
.alloc = x86_vector_alloc_irqs,
.free = x86_vector_free_irqs,
- .activate = x86_vector_activate,
- .deactivate = x86_vector_deactivate,
#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
.debug_show = x86_vector_debug_show,
#endif
----------------------------------------------------------------
all is well.
On Fri, Dec 29, 2017 at 09:07:45AM +0100, Thomas Gleixner wrote:
> On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > On Fri, Dec 29, 2017 at 12:36:37AM +0100, Thomas Gleixner wrote:
> > > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > >
> > > > Attached, but heads up on this: when redirecting the output of lspci
> > > > -vvv to a text file as root I get
> > > >
> > > > pcilib: sysfs_read_vpd: read failed: Input/output error
> > > >
> > > > I can find bugs filed for various distros to this same effect, but
> > > > haven't tracked down any explanations.
> > >
> > > Weird, but the info looks complete.
> > >
> > > Can you please add 'pci=nomsi' to the 4.15 kernel command line and see
> > > whether that works?
> >
> > It does (emailing from that successful boot as we speak). I'm on a
> > clean 4.15-rc5 (as in no patches, etc.).
> >
> > This was also suggested way at the top of this thread by Dexuan Cui
> > for 4.15-rc3 (where this exchange started), and it worked back then
> > too.
>
> I missed that part of the conversation. Let me stare into the MSI code
> again.
>
> Thanks,
>
> tglx
On Fri, Dec 29, 2017 at 06:49:15AM -0500, Alexandru Chirvasitu wrote:
> All right, I tried to do some more digging around, in the hope of
> getting as close to the source of the problem as I can.
>
> I went back to the very first commit that went astray for me, 2db1f95
> (which is the only one actually panicking), and tried to move from its
> parent 90ad9e2 (that boots fine) to it gradually, altering the code in
> small chunks.
>
> I tried to ignore the stuff that clearly shouldn't make a difference,
> such as definitions. So in the end I get defined-but-unused-function
> errors in my compilations, but I'm ignoring those for now. Some
> results:
>
> (1) When I move from the good commit 90ad9e2 according to the attached
> bad-diff (which moves partly towards 2db1f95), I get a panic.
>
> (2) On the other hand, when I further change this last panicking
> commit by simply doing
>
>
> ----------------------------------------------------------------
> removed activate / deactivate from x86_vector_domain_ops
>
> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> index 7317ba5a..063594d 100644
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -514,8 +514,6 @@ void x86_vector_debug_show(struct seq_file *m, struct irq_domain *d,
> static const struct irq_domain_ops x86_vector_domain_ops = {
> .alloc = x86_vector_alloc_irqs,
> .free = x86_vector_free_irqs,
> - .activate = x86_vector_activate,
> - .deactivate = x86_vector_deactivate,
> #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
> .debug_show = x86_vector_debug_show,
> #endif
> ----------------------------------------------------------------
>
> all is well.
>
And sure enough, simply diffing
----------------------------------------------------------------
removed activate / deactivate from x86_vector_domain_ops
diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
index 3f53572..e6cb55d 100644
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -511,8 +511,6 @@ void x86_vector_debug_show(struct seq_file *m, struct irq_domain *d,
static const struct irq_domain_ops x86_vector_domain_ops = {
.alloc = x86_vector_alloc_irqs,
.free = x86_vector_free_irqs,
- .activate = x86_vector_activate,
- .deactivate = x86_vector_deactivate,
#ifdef CONFIG_GENERIC_IRQ_DEBUGFS
.debug_show = x86_vector_debug_show,
#endif
----------------------------------------------------------------
directly against 2db1f95 fixes the issues (no freezes, lockups, or
panics).
>
>
>
> On Fri, Dec 29, 2017 at 09:07:45AM +0100, Thomas Gleixner wrote:
> > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > > On Fri, Dec 29, 2017 at 12:36:37AM +0100, Thomas Gleixner wrote:
> > > > On Thu, 28 Dec 2017, Alexandru Chirvasitu wrote:
> > > >
> > > > > Attached, but heads up on this: when redirecting the output of lspci
> > > > > -vvv to a text file as root I get
> > > > >
> > > > > pcilib: sysfs_read_vpd: read failed: Input/output error
> > > > >
> > > > > I can find bugs filed for various distros to this same effect, but
> > > > > haven't tracked down any explanations.
> > > >
> > > > Weird, but the info looks complete.
> > > >
> > > > Can you please add 'pci=nomsi' to the 4.15 kernel command line and see
> > > > whether that works?
> > >
> > > It does (emailing from that successful boot as we speak). I'm on a
> > > clean 4.15-rc5 (as in no patches, etc.).
> > >
> > > This was also suggested way at the top of this thread by Dexuan Cui
> > > for 4.15-rc3 (where this exchange started), and it worked back then
> > > too.
> >
> > I missed that part of the conversation. Let me stare into the MSI code
> > again.
> >
> > Thanks,
> >
> > tglx
> diff --git a/arch/x86/include/asm/irq_vectors.h b/arch/x86/include/asm/irq_vectors.h
> index aaf8d28..1e9bd28 100644
> --- a/arch/x86/include/asm/irq_vectors.h
> +++ b/arch/x86/include/asm/irq_vectors.h
> @@ -101,12 +101,8 @@
> #define POSTED_INTR_NESTED_VECTOR 0xf0
> #endif
>
> -/*
> - * Local APIC timer IRQ vector is on a different priority level,
> - * to work around the 'lost local interrupt if more than 2 IRQ
> - * sources per level' errata.
> - */
> -#define LOCAL_TIMER_VECTOR 0xef
> +#define MANAGED_IRQ_SHUTDOWN_VECTOR 0xef
> +#define LOCAL_TIMER_VECTOR 0xee
>
> #define NR_VECTORS 256
>
> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> index f08d44f..7317ba5a 100644
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -32,7 +32,8 @@ struct apic_chip_data {
> unsigned int prev_cpu;
> unsigned int irq;
> struct hlist_node clist;
> - u8 move_in_progress : 1;
> + unsigned int move_in_progress : 1,
> + is_managed : 1;
> };
>
> struct irq_domain *x86_vector_domain;
> @@ -152,6 +153,28 @@ static void apic_update_vector(struct irq_data *irqd, unsigned int newvec,
> per_cpu(vector_irq, newcpu)[newvec] = desc;
> }
>
> +static void vector_assign_managed_shutdown(struct irq_data *irqd)
> +{
> + unsigned int cpu = cpumask_first(cpu_online_mask);
> +
> + apic_update_irq_cfg(irqd, MANAGED_IRQ_SHUTDOWN_VECTOR, cpu);
> +}
> +
> +static int reserve_managed_vector(struct irq_data *irqd)
> +{
> + const struct cpumask *affmsk = irq_data_get_affinity_mask(irqd);
> + struct apic_chip_data *apicd = apic_chip_data(irqd);
> + unsigned long flags;
> + int ret;
> +
> + raw_spin_lock_irqsave(&vector_lock, flags);
> + apicd->is_managed = true;
> + ret = irq_matrix_reserve_managed(vector_matrix, affmsk);
> + raw_spin_unlock_irqrestore(&vector_lock, flags);
> + trace_vector_reserve_managed(irqd->irq, ret);
> + return ret;
> +}
> +
> static int allocate_vector(struct irq_data *irqd, const struct cpumask *dest)
> {
> struct apic_chip_data *apicd = apic_chip_data(irqd);
> @@ -211,9 +234,58 @@ static int assign_irq_vector_policy(struct irq_data *irqd,
> return assign_irq_vector(irqd, cpu_online_mask);
> }
>
> +
> +static int assign_irq_vector_any_locked(struct irq_data *irqd)
> +{
> + int node = irq_data_get_node(irqd);
> +
> + if (node != NUMA_NO_NODE) {
> + if (!assign_vector_locked(irqd, cpumask_of_node(node)))
> + return 0;
> + }
> + return assign_vector_locked(irqd, cpu_online_mask);
> +}
> +
> +static int assign_irq_vector_any(struct irq_data *irqd)
> +{
> + unsigned long flags;
> + int ret;
> +
> + raw_spin_lock_irqsave(&vector_lock, flags);
> + ret = assign_irq_vector_any_locked(irqd);
> + raw_spin_unlock_irqrestore(&vector_lock, flags);
> + return ret;
> +}
> +
> +
> +static int assign_managed_vector(struct irq_data *irqd, const struct cpumask *dest)
> +{
> + const struct cpumask *affmsk = irq_data_get_affinity_mask(irqd);
> + struct apic_chip_data *apicd = apic_chip_data(irqd);
> + int vector, cpu;
> +
> + cpumask_and(vector_searchmask, vector_searchmask, affmsk);
> + cpu = cpumask_first(vector_searchmask);
> + if (cpu >= nr_cpu_ids)
> + return -EINVAL;
> + /* set_affinity might call here for nothing */
> + if (apicd->vector && cpumask_test_cpu(apicd->cpu, vector_searchmask))
> + return 0;
> + vector = irq_matrix_alloc_managed(vector_matrix, cpu);
> + trace_vector_alloc_managed(irqd->irq, vector, vector);
> + if (vector < 0)
> + return vector;
> + apic_update_vector(irqd, vector, cpu);
> + apic_update_irq_cfg(irqd, vector, cpu);
> + return 0;
> +}
> +
> +
> +
> static void clear_irq_vector(struct irq_data *irqd)
> {
> struct apic_chip_data *apicd = apic_chip_data(irqd);
> + bool managed = irqd_affinity_is_managed(irqd);
> unsigned int vector = apicd->vector;
>
> lockdep_assert_held(&vector_lock);
> @@ -240,6 +312,80 @@ static void clear_irq_vector(struct irq_data *irqd)
> hlist_del_init(&apicd->clist);
> }
>
> +static void x86_vector_deactivate(struct irq_domain *dom, struct irq_data *irqd)
> +{
> + struct apic_chip_data *apicd = apic_chip_data(irqd);
> + unsigned long flags;
> +
> + trace_vector_deactivate(irqd->irq, apicd->is_managed,
> + false, false);
> +
> + if (apicd->is_managed)
> + return;
> +
> + raw_spin_lock_irqsave(&vector_lock, flags);
> + clear_irq_vector(irqd);
> + vector_assign_managed_shutdown(irqd);
> + raw_spin_unlock_irqrestore(&vector_lock, flags);
> +}
> +
> +static int activate_managed(struct irq_data *irqd)
> +{
> + const struct cpumask *dest = irq_data_get_affinity_mask(irqd);
> + int ret;
> +
> + cpumask_and(vector_searchmask, dest, cpu_online_mask);
> + if (WARN_ON_ONCE(cpumask_empty(vector_searchmask))) {
> + /* Something in the core code broke! Survive gracefully */
> + pr_err("Managed startup for irq %u, but no CPU\n", irqd->irq);
> + return EINVAL;
> + }
> +
> + ret = assign_managed_vector(irqd, vector_searchmask);
> + /*
> + * This should not happen. The vector reservation got buggered. Handle
> + * it gracefully.
> + */
> + if (WARN_ON_ONCE(ret < 0)) {
> + pr_err("Managed startup irq %u, no vector available\n",
> + irqd->irq);
> + }
> + return ret;
> +}
> +
> +static int x86_vector_activate(struct irq_domain *dom, struct irq_data *irqd,
> + bool early)
> +{
> + struct apic_chip_data *apicd = apic_chip_data(irqd);
> + unsigned long flags;
> + int ret = 0;
> +
> + trace_vector_activate(irqd->irq, apicd->is_managed,
> + false, early);
> +
> + if (!apicd->is_managed)
> + return 0;
> +
> + raw_spin_lock_irqsave(&vector_lock, flags);
> + if (early || irqd_is_managed_and_shutdown(irqd))
> + vector_assign_managed_shutdown(irqd);
> + else
> + ret = activate_managed(irqd);
> + raw_spin_unlock_irqrestore(&vector_lock, flags);
> + return ret;
> +}
> +
> +static void vector_free_reserved_and_managed(struct irq_data *irqd)
> +{
> + const struct cpumask *dest = irq_data_get_affinity_mask(irqd);
> + struct apic_chip_data *apicd = apic_chip_data(irqd);
> +
> + trace_vector_teardown(irqd->irq, apicd->is_managed, false);
> +
> + if (apicd->is_managed)
> + irq_matrix_remove_managed(vector_matrix, dest);
> +}
> +
> static void x86_vector_free_irqs(struct irq_domain *domain,
> unsigned int virq, unsigned int nr_irqs)
> {
> @@ -368,6 +514,8 @@ void x86_vector_debug_show(struct seq_file *m, struct irq_domain *d,
> static const struct irq_domain_ops x86_vector_domain_ops = {
> .alloc = x86_vector_alloc_irqs,
> .free = x86_vector_free_irqs,
> + .activate = x86_vector_activate,
> + .deactivate = x86_vector_deactivate,
> #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
> .debug_show = x86_vector_debug_show,
> #endif
> @@ -577,6 +725,15 @@ static void free_moved_vector(struct apic_chip_data *apicd)
> {
> unsigned int vector = apicd->prev_vector;
> unsigned int cpu = apicd->prev_cpu;
> + bool managed = apicd->is_managed;
> +
> + /*
> + * This should never happen. Managed interrupts are not
> + * migrated except on CPU down, which does not involve the
> + * cleanup vector. But try to keep the accounting correct
> + * nevertheless.
> + */
> + WARN_ON_ONCE(managed);
>
> trace_vector_free_moved(apicd->irq, vector, false);
> irq_matrix_free(vector_matrix, cpu, vector, false);
On Fri, 29 Dec 2017, Alexandru Chirvasitu wrote:
> All right, I tried to do some more digging around, in the hope of
> getting as close to the source of the problem as I can.
>
> I went back to the very first commit that went astray for me, 2db1f95
> (which is the only one actually panicking), and tried to move from its
> parent 90ad9e2 (that boots fine) to it gradually, altering the code in
> small chunks.
>
> I tried to ignore the stuff that clearly shouldn't make a difference,
> such as definitions. So in the end I get defined-but-unused-function
> errors in my compilations, but I'm ignoring those for now. Some
> results:
>
> (1) When I move from the good commit 90ad9e2 according to the attached
> bad-diff (which moves partly towards 2db1f95), I get a panic.
>
> (2) On the other hand, when I further change this last panicking
> commit by simply doing
>
>
> ----------------------------------------------------------------
> removed activate / deactivate from x86_vector_domain_ops
>
> diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> index 7317ba5a..063594d 100644
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -514,8 +514,6 @@ void x86_vector_debug_show(struct seq_file *m, struct irq_domain *d,
> static const struct irq_domain_ops x86_vector_domain_ops = {
> .alloc = x86_vector_alloc_irqs,
> .free = x86_vector_free_irqs,
> - .activate = x86_vector_activate,
> - .deactivate = x86_vector_deactivate,
> #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
> .debug_show = x86_vector_debug_show,
> #endif
> ----------------------------------------------------------------
>
> all is well.
Nice detective work. Unfortunately that's not a real solution ...
Can you try the patch below on top of Linus tree, please?
Thanks,
tglx
8<---------------------
--- a/kernel/irq/msi.c
+++ b/kernel/irq/msi.c
@@ -339,6 +339,40 @@ int msi_domain_populate_irqs(struct irq_
return ret;
}
+/*
+ * Carefully check whether the device can use reservation mode. If
+ * reservation mode is enabled then the early activation will assign a
+ * dummy vector to the device. If the PCI/MSI device does not support
+ * masking of the entry then this can result in spurious interrupts when
+ * the device driver is not absolutely careful. But even then a malfunction
+ * of the hardware could result in a spurious interrupt on the dummy vector
+ * and render the device unusable. If the entry can be masked then the core
+ * logic will prevent the spurious interrupt and reservation mode can be
+ * used. For now reservation mode is restricted to PCI/MSI.
+ */
+static bool msi_check_reservation_mode(struct irq_domain *domain,
+ struct msi_domain_info *info,
+ struct device *dev)
+{
+ struct msi_desc *desc;
+
+ if (domain->bus_token != DOMAIN_BUS_PCI_MSI)
+ return false;
+
+ if (!(info->flags & MSI_FLAG_MUST_REACTIVATE))
+ return false;
+
+ if (IS_ENABLED(CONFIG_PCI_MSI) && pci_msi_ignore_mask)
+ return false;
+
+ /*
+ * Checking the first MSI descriptor is sufficient. MSIX supports
+ * masking and MSI does so when the maskbit is set.
+ */
+ desc = first_msi_entry(dev);
+ return desc->msi_attrib.is_msix || desc->msi_attrib.maskbit;
+}
+
/**
* msi_domain_alloc_irqs - Allocate interrupts from a MSI interrupt domain
* @domain: The domain to allocate from
@@ -353,9 +387,11 @@ int msi_domain_alloc_irqs(struct irq_dom
{
struct msi_domain_info *info = domain->host_data;
struct msi_domain_ops *ops = info->ops;
- msi_alloc_info_t arg;
+ struct irq_data *irq_data;
struct msi_desc *desc;
+ msi_alloc_info_t arg;
int i, ret, virq;
+ bool can_reserve;
ret = msi_domain_prepare_irqs(domain, dev, nvec, &arg);
if (ret)
@@ -385,6 +421,8 @@ int msi_domain_alloc_irqs(struct irq_dom
if (ops->msi_finish)
ops->msi_finish(&arg, 0);
+ can_reserve = msi_check_reservation_mode(domain, info, dev);
+
for_each_msi_entry(desc, dev) {
virq = desc->irq;
if (desc->nvec_used == 1)
@@ -397,17 +435,28 @@ int msi_domain_alloc_irqs(struct irq_dom
* the MSI entries before the PCI layer enables MSI in the
* card. Otherwise the card latches a random msi message.
*/
- if (info->flags & MSI_FLAG_ACTIVATE_EARLY) {
- struct irq_data *irq_data;
+ if (!(info->flags & MSI_FLAG_ACTIVATE_EARLY))
+ continue;
+ irq_data = irq_domain_get_irq_data(domain, desc->irq);
+ if (!can_reserve)
+ irqd_clr_can_reserve(irq_data);
+ ret = irq_domain_activate_irq(irq_data, can_reserve);
+ if (ret)
+ goto cleanup;
+ }
+
+ /*
+ * If these interrupts use reservation mode clear the activated bit
+ * so request_irq() will assign the final vector.
+ */
+ if (can_reserve) {
+ for_each_msi_entry(desc, dev) {
irq_data = irq_domain_get_irq_data(domain, desc->irq);
- ret = irq_domain_activate_irq(irq_data, true);
- if (ret)
- goto cleanup;
- if (info->flags & MSI_FLAG_MUST_REACTIVATE)
- irqd_clr_activated(irq_data);
+ irqd_clr_activated(irq_data);
}
}
+
return 0;
cleanup:
--- a/arch/x86/kernel/apic/vector.c
+++ b/arch/x86/kernel/apic/vector.c
@@ -184,6 +184,7 @@ static void reserve_irq_vector_locked(st
irq_matrix_reserve(vector_matrix);
apicd->can_reserve = true;
apicd->has_reserved = true;
+ irqd_set_can_reserve(irqd);
trace_vector_reserve(irqd->irq, 0);
vector_assign_managed_shutdown(irqd);
}
@@ -368,8 +369,18 @@ static int activate_reserved(struct irq_
int ret;
ret = assign_irq_vector_any_locked(irqd);
- if (!ret)
+ if (!ret) {
apicd->has_reserved = false;
+ /*
+ * Core might have disabled reservation mode after
+ * allocating the irq descriptor. Ideally this should
+ * happen before allocation time, but that would require
+ * completely convoluted ways of transporting that
+ * information.
+ */
+ if (!irqd_can_reserve(irqd))
+ apicd->can_reserve = false;
+ }
return ret;
}
@@ -478,6 +489,7 @@ static bool vector_configure_legacy(unsi
} else {
/* Release the vector */
apicd->can_reserve = true;
+ irqd_set_can_reserve(irqd);
clear_irq_vector(irqd);
realloc = true;
}
--- a/include/linux/irq.h
+++ b/include/linux/irq.h
@@ -212,6 +212,7 @@ struct irq_data {
* mask. Applies only to affinity managed irqs.
* IRQD_SINGLE_TARGET - IRQ allows only a single affinity target
* IRQD_DEFAULT_TRIGGER_SET - Expected trigger already been set
+ * IRQD_CAN_RESERVE - Can use reservation mode
*/
enum {
IRQD_TRIGGER_MASK = 0xf,
@@ -233,6 +234,7 @@ enum {
IRQD_MANAGED_SHUTDOWN = (1 << 23),
IRQD_SINGLE_TARGET = (1 << 24),
IRQD_DEFAULT_TRIGGER_SET = (1 << 25),
+ IRQD_CAN_RESERVE = (1 << 26),
};
#define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
@@ -377,6 +379,21 @@ static inline bool irqd_is_managed_and_s
return __irqd_to_state(d) & IRQD_MANAGED_SHUTDOWN;
}
+static inline void irqd_set_can_reserve(struct irq_data *d)
+{
+ __irqd_to_state(d) |= IRQD_CAN_RESERVE;
+}
+
+static inline void irqd_clr_can_reserve(struct irq_data *d)
+{
+ __irqd_to_state(d) &= ~IRQD_CAN_RESERVE;
+}
+
+static inline bool irqd_can_reserve(struct irq_data *d)
+{
+ return __irqd_to_state(d) & IRQD_CAN_RESERVE;
+}
+
#undef __irqd_to_state
static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
--- a/kernel/irq/debugfs.c
+++ b/kernel/irq/debugfs.c
@@ -113,6 +113,7 @@ static const struct irq_bit_descr irqdat
BIT_MASK_DESCR(IRQD_SETAFFINITY_PENDING),
BIT_MASK_DESCR(IRQD_AFFINITY_MANAGED),
BIT_MASK_DESCR(IRQD_MANAGED_SHUTDOWN),
+ BIT_MASK_DESCR(IRQD_CAN_RESERVE),
BIT_MASK_DESCR(IRQD_FORWARDED_TO_VCPU),
--- a/arch/x86/kernel/apic/apic_flat_64.c
+++ b/arch/x86/kernel/apic/apic_flat_64.c
@@ -151,7 +151,7 @@ static struct apic apic_flat __ro_after_
.apic_id_valid = default_apic_id_valid,
.apic_id_registered = flat_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
.irq_dest_mode = 1, /* logical */
.disable_esr = 0,
--- a/arch/x86/kernel/apic/apic_noop.c
+++ b/arch/x86/kernel/apic/apic_noop.c
@@ -110,7 +110,7 @@ struct apic apic_noop __ro_after_init =
.apic_id_valid = default_apic_id_valid,
.apic_id_registered = noop_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
/* logical delivery broadcast to all CPUs: */
.irq_dest_mode = 1,
--- a/arch/x86/kernel/apic/msi.c
+++ b/arch/x86/kernel/apic/msi.c
@@ -39,17 +39,13 @@ static void irq_msi_compose_msg(struct i
((apic->irq_dest_mode == 0) ?
MSI_ADDR_DEST_MODE_PHYSICAL :
MSI_ADDR_DEST_MODE_LOGICAL) |
- ((apic->irq_delivery_mode != dest_LowestPrio) ?
- MSI_ADDR_REDIRECTION_CPU :
- MSI_ADDR_REDIRECTION_LOWPRI) |
+ MSI_ADDR_REDIRECTION_CPU |
MSI_ADDR_DEST_ID(cfg->dest_apicid);
msg->data =
MSI_DATA_TRIGGER_EDGE |
MSI_DATA_LEVEL_ASSERT |
- ((apic->irq_delivery_mode != dest_LowestPrio) ?
- MSI_DATA_DELIVERY_FIXED :
- MSI_DATA_DELIVERY_LOWPRI) |
+ MSI_DATA_DELIVERY_FIXED |
MSI_DATA_VECTOR(cfg->vector);
}
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -105,7 +105,7 @@ static struct apic apic_default __ro_aft
.apic_id_valid = default_apic_id_valid,
.apic_id_registered = default_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
/* logical delivery broadcast to all CPUs: */
.irq_dest_mode = 1,
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -184,7 +184,7 @@ static struct apic apic_x2apic_cluster _
.apic_id_valid = x2apic_apic_id_valid,
.apic_id_registered = x2apic_apic_id_registered,
- .irq_delivery_mode = dest_LowestPrio,
+ .irq_delivery_mode = dest_Fixed,
.irq_dest_mode = 1, /* logical */
.disable_esr = 0,
--- a/drivers/pci/host/pci-hyperv.c
+++ b/drivers/pci/host/pci-hyperv.c
@@ -985,9 +985,7 @@ static u32 hv_compose_msi_req_v1(
int_pkt->wslot.slot = slot;
int_pkt->int_desc.vector = vector;
int_pkt->int_desc.vector_count = 1;
- int_pkt->int_desc.delivery_mode =
- (apic->irq_delivery_mode == dest_LowestPrio) ?
- dest_LowestPrio : dest_Fixed;
+ int_pkt->int_desc.delivery_mode = dest_Fixed;
/*
* Create MSI w/ dummy vCPU set, overwritten by subsequent retarget in
@@ -1008,9 +1006,7 @@ static u32 hv_compose_msi_req_v2(
int_pkt->wslot.slot = slot;
int_pkt->int_desc.vector = vector;
int_pkt->int_desc.vector_count = 1;
- int_pkt->int_desc.delivery_mode =
- (apic->irq_delivery_mode == dest_LowestPrio) ?
- dest_LowestPrio : dest_Fixed;
+ int_pkt->int_desc.delivery_mode = dest_Fixed;
/*
* Create MSI w/ dummy vCPU set targeting just one vCPU, overwritten
On Fri, Dec 29, 2017 at 02:09:40PM +0100, Thomas Gleixner wrote:
> On Fri, 29 Dec 2017, Alexandru Chirvasitu wrote:
> > All right, I tried to do some more digging around, in the hope of
> > getting as close to the source of the problem as I can.
> >
> > I went back to the very first commit that went astray for me, 2db1f95
> > (which is the only one actually panicking), and tried to move from its
> > parent 90ad9e2 (that boots fine) to it gradually, altering the code in
> > small chunks.
> >
> > I tried to ignore the stuff that clearly shouldn't make a difference,
> > such as definitions. So in the end I get defined-but-unused-function
> > errors in my compilations, but I'm ignoring those for now. Some
> > results:
> >
> > (1) When I move from the good commit 90ad9e2 according to the attached
> > bad-diff (which moves partly towards 2db1f95), I get a panic.
> >
> > (2) On the other hand, when I further change this last panicking
> > commit by simply doing
> >
> >
> > ----------------------------------------------------------------
> > removed activate / deactivate from x86_vector_domain_ops
> >
> > diff --git a/arch/x86/kernel/apic/vector.c b/arch/x86/kernel/apic/vector.c
> > index 7317ba5a..063594d 100644
> > --- a/arch/x86/kernel/apic/vector.c
> > +++ b/arch/x86/kernel/apic/vector.c
> > @@ -514,8 +514,6 @@ void x86_vector_debug_show(struct seq_file *m, struct irq_domain *d,
> > static const struct irq_domain_ops x86_vector_domain_ops = {
> > .alloc = x86_vector_alloc_irqs,
> > .free = x86_vector_free_irqs,
> > - .activate = x86_vector_activate,
> > - .deactivate = x86_vector_deactivate,
> > #ifdef CONFIG_GENERIC_IRQ_DEBUGFS
> > .debug_show = x86_vector_debug_show,
> > #endif
> > ----------------------------------------------------------------
> >
> > all is well.
>
> Nice detective work. Unfortunately that's not a real solution ...
>
Oh, of course. It was never intended as a solution, only as
information perhaps enabling someone who knows what they're doing
(unlike myself :) ) to find one.
> Can you try the patch below on top of Linus tree, please?
>
> Thanks,
>
Applied it to 464e1d5 4.15-rc5 just now. it appears to be
trouble-free: booted, logged me in fine, the works.
> tglx
>
> 8<---------------------
> --- a/kernel/irq/msi.c
> +++ b/kernel/irq/msi.c
> @@ -339,6 +339,40 @@ int msi_domain_populate_irqs(struct irq_
> return ret;
> }
>
> +/*
> + * Carefully check whether the device can use reservation mode. If
> + * reservation mode is enabled then the early activation will assign a
> + * dummy vector to the device. If the PCI/MSI device does not support
> + * masking of the entry then this can result in spurious interrupts when
> + * the device driver is not absolutely careful. But even then a malfunction
> + * of the hardware could result in a spurious interrupt on the dummy vector
> + * and render the device unusable. If the entry can be masked then the core
> + * logic will prevent the spurious interrupt and reservation mode can be
> + * used. For now reservation mode is restricted to PCI/MSI.
> + */
> +static bool msi_check_reservation_mode(struct irq_domain *domain,
> + struct msi_domain_info *info,
> + struct device *dev)
> +{
> + struct msi_desc *desc;
> +
> + if (domain->bus_token != DOMAIN_BUS_PCI_MSI)
> + return false;
> +
> + if (!(info->flags & MSI_FLAG_MUST_REACTIVATE))
> + return false;
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI) && pci_msi_ignore_mask)
> + return false;
> +
> + /*
> + * Checking the first MSI descriptor is sufficient. MSIX supports
> + * masking and MSI does so when the maskbit is set.
> + */
> + desc = first_msi_entry(dev);
> + return desc->msi_attrib.is_msix || desc->msi_attrib.maskbit;
> +}
> +
> /**
> * msi_domain_alloc_irqs - Allocate interrupts from a MSI interrupt domain
> * @domain: The domain to allocate from
> @@ -353,9 +387,11 @@ int msi_domain_alloc_irqs(struct irq_dom
> {
> struct msi_domain_info *info = domain->host_data;
> struct msi_domain_ops *ops = info->ops;
> - msi_alloc_info_t arg;
> + struct irq_data *irq_data;
> struct msi_desc *desc;
> + msi_alloc_info_t arg;
> int i, ret, virq;
> + bool can_reserve;
>
> ret = msi_domain_prepare_irqs(domain, dev, nvec, &arg);
> if (ret)
> @@ -385,6 +421,8 @@ int msi_domain_alloc_irqs(struct irq_dom
> if (ops->msi_finish)
> ops->msi_finish(&arg, 0);
>
> + can_reserve = msi_check_reservation_mode(domain, info, dev);
> +
> for_each_msi_entry(desc, dev) {
> virq = desc->irq;
> if (desc->nvec_used == 1)
> @@ -397,17 +435,28 @@ int msi_domain_alloc_irqs(struct irq_dom
> * the MSI entries before the PCI layer enables MSI in the
> * card. Otherwise the card latches a random msi message.
> */
> - if (info->flags & MSI_FLAG_ACTIVATE_EARLY) {
> - struct irq_data *irq_data;
> + if (!(info->flags & MSI_FLAG_ACTIVATE_EARLY))
> + continue;
>
> + irq_data = irq_domain_get_irq_data(domain, desc->irq);
> + if (!can_reserve)
> + irqd_clr_can_reserve(irq_data);
> + ret = irq_domain_activate_irq(irq_data, can_reserve);
> + if (ret)
> + goto cleanup;
> + }
> +
> + /*
> + * If these interrupts use reservation mode clear the activated bit
> + * so request_irq() will assign the final vector.
> + */
> + if (can_reserve) {
> + for_each_msi_entry(desc, dev) {
> irq_data = irq_domain_get_irq_data(domain, desc->irq);
> - ret = irq_domain_activate_irq(irq_data, true);
> - if (ret)
> - goto cleanup;
> - if (info->flags & MSI_FLAG_MUST_REACTIVATE)
> - irqd_clr_activated(irq_data);
> + irqd_clr_activated(irq_data);
> }
> }
> +
> return 0;
>
> cleanup:
> --- a/arch/x86/kernel/apic/vector.c
> +++ b/arch/x86/kernel/apic/vector.c
> @@ -184,6 +184,7 @@ static void reserve_irq_vector_locked(st
> irq_matrix_reserve(vector_matrix);
> apicd->can_reserve = true;
> apicd->has_reserved = true;
> + irqd_set_can_reserve(irqd);
> trace_vector_reserve(irqd->irq, 0);
> vector_assign_managed_shutdown(irqd);
> }
> @@ -368,8 +369,18 @@ static int activate_reserved(struct irq_
> int ret;
>
> ret = assign_irq_vector_any_locked(irqd);
> - if (!ret)
> + if (!ret) {
> apicd->has_reserved = false;
> + /*
> + * Core might have disabled reservation mode after
> + * allocating the irq descriptor. Ideally this should
> + * happen before allocation time, but that would require
> + * completely convoluted ways of transporting that
> + * information.
> + */
> + if (!irqd_can_reserve(irqd))
> + apicd->can_reserve = false;
> + }
> return ret;
> }
>
> @@ -478,6 +489,7 @@ static bool vector_configure_legacy(unsi
> } else {
> /* Release the vector */
> apicd->can_reserve = true;
> + irqd_set_can_reserve(irqd);
> clear_irq_vector(irqd);
> realloc = true;
> }
> --- a/include/linux/irq.h
> +++ b/include/linux/irq.h
> @@ -212,6 +212,7 @@ struct irq_data {
> * mask. Applies only to affinity managed irqs.
> * IRQD_SINGLE_TARGET - IRQ allows only a single affinity target
> * IRQD_DEFAULT_TRIGGER_SET - Expected trigger already been set
> + * IRQD_CAN_RESERVE - Can use reservation mode
> */
> enum {
> IRQD_TRIGGER_MASK = 0xf,
> @@ -233,6 +234,7 @@ enum {
> IRQD_MANAGED_SHUTDOWN = (1 << 23),
> IRQD_SINGLE_TARGET = (1 << 24),
> IRQD_DEFAULT_TRIGGER_SET = (1 << 25),
> + IRQD_CAN_RESERVE = (1 << 26),
> };
>
> #define __irqd_to_state(d) ACCESS_PRIVATE((d)->common, state_use_accessors)
> @@ -377,6 +379,21 @@ static inline bool irqd_is_managed_and_s
> return __irqd_to_state(d) & IRQD_MANAGED_SHUTDOWN;
> }
>
> +static inline void irqd_set_can_reserve(struct irq_data *d)
> +{
> + __irqd_to_state(d) |= IRQD_CAN_RESERVE;
> +}
> +
> +static inline void irqd_clr_can_reserve(struct irq_data *d)
> +{
> + __irqd_to_state(d) &= ~IRQD_CAN_RESERVE;
> +}
> +
> +static inline bool irqd_can_reserve(struct irq_data *d)
> +{
> + return __irqd_to_state(d) & IRQD_CAN_RESERVE;
> +}
> +
> #undef __irqd_to_state
>
> static inline irq_hw_number_t irqd_to_hwirq(struct irq_data *d)
> --- a/kernel/irq/debugfs.c
> +++ b/kernel/irq/debugfs.c
> @@ -113,6 +113,7 @@ static const struct irq_bit_descr irqdat
> BIT_MASK_DESCR(IRQD_SETAFFINITY_PENDING),
> BIT_MASK_DESCR(IRQD_AFFINITY_MANAGED),
> BIT_MASK_DESCR(IRQD_MANAGED_SHUTDOWN),
> + BIT_MASK_DESCR(IRQD_CAN_RESERVE),
>
> BIT_MASK_DESCR(IRQD_FORWARDED_TO_VCPU),
>
> --- a/arch/x86/kernel/apic/apic_flat_64.c
> +++ b/arch/x86/kernel/apic/apic_flat_64.c
> @@ -151,7 +151,7 @@ static struct apic apic_flat __ro_after_
> .apic_id_valid = default_apic_id_valid,
> .apic_id_registered = flat_apic_id_registered,
>
> - .irq_delivery_mode = dest_LowestPrio,
> + .irq_delivery_mode = dest_Fixed,
> .irq_dest_mode = 1, /* logical */
>
> .disable_esr = 0,
> --- a/arch/x86/kernel/apic/apic_noop.c
> +++ b/arch/x86/kernel/apic/apic_noop.c
> @@ -110,7 +110,7 @@ struct apic apic_noop __ro_after_init =
> .apic_id_valid = default_apic_id_valid,
> .apic_id_registered = noop_apic_id_registered,
>
> - .irq_delivery_mode = dest_LowestPrio,
> + .irq_delivery_mode = dest_Fixed,
> /* logical delivery broadcast to all CPUs: */
> .irq_dest_mode = 1,
>
> --- a/arch/x86/kernel/apic/msi.c
> +++ b/arch/x86/kernel/apic/msi.c
> @@ -39,17 +39,13 @@ static void irq_msi_compose_msg(struct i
> ((apic->irq_dest_mode == 0) ?
> MSI_ADDR_DEST_MODE_PHYSICAL :
> MSI_ADDR_DEST_MODE_LOGICAL) |
> - ((apic->irq_delivery_mode != dest_LowestPrio) ?
> - MSI_ADDR_REDIRECTION_CPU :
> - MSI_ADDR_REDIRECTION_LOWPRI) |
> + MSI_ADDR_REDIRECTION_CPU |
> MSI_ADDR_DEST_ID(cfg->dest_apicid);
>
> msg->data =
> MSI_DATA_TRIGGER_EDGE |
> MSI_DATA_LEVEL_ASSERT |
> - ((apic->irq_delivery_mode != dest_LowestPrio) ?
> - MSI_DATA_DELIVERY_FIXED :
> - MSI_DATA_DELIVERY_LOWPRI) |
> + MSI_DATA_DELIVERY_FIXED |
> MSI_DATA_VECTOR(cfg->vector);
> }
>
> --- a/arch/x86/kernel/apic/probe_32.c
> +++ b/arch/x86/kernel/apic/probe_32.c
> @@ -105,7 +105,7 @@ static struct apic apic_default __ro_aft
> .apic_id_valid = default_apic_id_valid,
> .apic_id_registered = default_apic_id_registered,
>
> - .irq_delivery_mode = dest_LowestPrio,
> + .irq_delivery_mode = dest_Fixed,
> /* logical delivery broadcast to all CPUs: */
> .irq_dest_mode = 1,
>
> --- a/arch/x86/kernel/apic/x2apic_cluster.c
> +++ b/arch/x86/kernel/apic/x2apic_cluster.c
> @@ -184,7 +184,7 @@ static struct apic apic_x2apic_cluster _
> .apic_id_valid = x2apic_apic_id_valid,
> .apic_id_registered = x2apic_apic_id_registered,
>
> - .irq_delivery_mode = dest_LowestPrio,
> + .irq_delivery_mode = dest_Fixed,
> .irq_dest_mode = 1, /* logical */
>
> .disable_esr = 0,
> --- a/drivers/pci/host/pci-hyperv.c
> +++ b/drivers/pci/host/pci-hyperv.c
> @@ -985,9 +985,7 @@ static u32 hv_compose_msi_req_v1(
> int_pkt->wslot.slot = slot;
> int_pkt->int_desc.vector = vector;
> int_pkt->int_desc.vector_count = 1;
> - int_pkt->int_desc.delivery_mode =
> - (apic->irq_delivery_mode == dest_LowestPrio) ?
> - dest_LowestPrio : dest_Fixed;
> + int_pkt->int_desc.delivery_mode = dest_Fixed;
>
> /*
> * Create MSI w/ dummy vCPU set, overwritten by subsequent retarget in
> @@ -1008,9 +1006,7 @@ static u32 hv_compose_msi_req_v2(
> int_pkt->wslot.slot = slot;
> int_pkt->int_desc.vector = vector;
> int_pkt->int_desc.vector_count = 1;
> - int_pkt->int_desc.delivery_mode =
> - (apic->irq_delivery_mode == dest_LowestPrio) ?
> - dest_LowestPrio : dest_Fixed;
> + int_pkt->int_desc.delivery_mode = dest_Fixed;
>
> /*
> * Create MSI w/ dummy vCPU set targeting just one vCPU, overwritten