From: Aili Yao <yaoaili@xxxxxxxxxxxx> Hi! Thanks for your reply. Got you! And for x86 platform, NMI is not only for hw errors, it does have some other functions like watchdog, and maybe others i don't know. when CPU is in heavy workload, the NMI watchdog will be triggered repeatily, and it will come to ghes_notify_nmi, the atomic raw_spin_lock may lock the memory bus which may have little performance inpact to other coresi i think. I think you may modify it. Thanks Aili Yao > -----Original Message----- > From: James Morse [mailto:james.morse@xxxxxxx] > Sent: Friday, October 30, 2020 8:40 PM > To: yaoaili126@xxxxxxx > Cc: rjw@xxxxxxxxxxxxx; lenb@xxxxxxxxxx; tony.luck@xxxxxxxxx; bp@xxxxxxxxx; > linux-acpi@xxxxxxxxxxxxxxx; YANGFENG1 > <YANGFENG1@xxxxxxxxxxxx>; yaoaili<yaoaili@xxxxxxxxxxxx> > Subject: Re: One question about ghes_notify_nmi > > Hello, > > On 30/10/2020 02:41, yaoaili126@xxxxxxx wrote: > > From: Aili Yao <yaoaili@xxxxxxxxxxxx> > > Sorry for my ignorance，When I look in to this code, I am totally condused. > > No worries - this code is pretty confusing! > > > > The Line 1136 has guarranted that Only one NMI will enter following > > code I think, Is this right? if so, what is ghes_notify_lock_nmi going to > pretect? > > Looking at one of the others like ghes_notify_sea() might be simpler. > The lock protects the fixmap slot, in case the notification occurs on multiple > CPUs. > > notify_nmi is weird as it has this atomic_add_unless() which seems to throw > away some of the CPUs if they arrive ~together. It was added by commit > 6fe9e7c26a971 ("GHES: Make NMI handler have a single reader"), which > describes the motivation. > > I'm not familiar with how x86 CPUs trigger NMI. From the commit message > I've assumed this means there is some broadcast source of NMI, that is never > firmware-first. I thought the trip via SMM for firmware-first did 'something' > to hold the other CPUs so only one CPU takes the NMI - but I couldn't find it > last time I went looking. > > As I've no idea how this works, I decided not to change it. I added the > spinlock so that the the fixmap slot provided to > ghes_in_nmi_spool_from_list() is always protected by a spinlock, but the > atomic means that for notify_nmi, the lock will never be contended. > > > > Thanks, > > James
