* microcode loading got really slow. @ 2013-05-21 23:03 Dave Jones 2013-05-22 19:03 ` H. Peter Anvin 0 siblings, 1 reply; 25+ messages in thread From: Dave Jones @ 2013-05-21 23:03 UTC (permalink / raw) To: Linux Kernel; +Cc: x86 In 3.10-rc2 I see this happening.. [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba For some reason the events for udev seem to be getting delayed 60s for each core. Dave ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-21 23:03 microcode loading got really slow Dave Jones @ 2013-05-22 19:03 ` H. Peter Anvin 2013-05-22 20:00 ` Dave Jones 0 siblings, 1 reply; 25+ messages in thread From: H. Peter Anvin @ 2013-05-22 19:03 UTC (permalink / raw) To: Dave Jones, Linux Kernel, x86 On 05/21/2013 04:03 PM, Dave Jones wrote: > In 3.10-rc2 I see this happening.. > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba > > For some reason the events for udev seem to be getting delayed 60s for each core. > > Dave > Is this a regression from -rc1? -hpa ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-22 19:03 ` H. Peter Anvin @ 2013-05-22 20:00 ` Dave Jones 2013-05-22 20:09 ` H. Peter Anvin 0 siblings, 1 reply; 25+ messages in thread From: Dave Jones @ 2013-05-22 20:00 UTC (permalink / raw) To: H. Peter Anvin; +Cc: Linux Kernel, x86 On Wed, May 22, 2013 at 12:03:04PM -0700, H. Peter Anvin wrote: > On 05/21/2013 04:03 PM, Dave Jones wrote: > > In 3.10-rc2 I see this happening.. > > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba > > > > For some reason the events for udev seem to be getting delayed 60s for each core. > > > > Dave > > > > Is this a regression from -rc1? No. Looks like it was introduced between 3.9 and 3.10-rc1 I see bisecting in my future. Dave ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-22 20:00 ` Dave Jones @ 2013-05-22 20:09 ` H. Peter Anvin 2013-05-23 0:19 ` Dave Jones 2013-05-23 3:39 ` Dave Jones 0 siblings, 2 replies; 25+ messages in thread From: H. Peter Anvin @ 2013-05-22 20:09 UTC (permalink / raw) To: Dave Jones; +Cc: Linux Kernel, x86, fenghua.yu A bisect would be appreciated. Cc Fenghua. Dave Jones <davej@redhat.com> wrote: >On Wed, May 22, 2013 at 12:03:04PM -0700, H. Peter Anvin wrote: > > On 05/21/2013 04:03 PM, Dave Jones wrote: > > > In 3.10-rc2 I see this happening.. > > > > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 >> > [ 252.702055] microcode: Microcode Update Driver: v2.00 ><tigran@aivazian.fsnet.co.uk>, Peter Oruba > > > >> > For some reason the events for udev seem to be getting delayed 60s >for each core. > > > > > > Dave > > > > > > > Is this a regression from -rc1? > >No. Looks like it was introduced between 3.9 and 3.10-rc1 > >I see bisecting in my future. > > Dave -- Sent from my mobile phone. Please excuse brevity and lack of formatting. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-22 20:09 ` H. Peter Anvin @ 2013-05-23 0:19 ` Dave Jones 2013-05-23 3:39 ` Dave Jones 1 sibling, 0 replies; 25+ messages in thread From: Dave Jones @ 2013-05-23 0:19 UTC (permalink / raw) To: H. Peter Anvin; +Cc: Linux Kernel, x86, fenghua.yu On Wed, May 22, 2013 at 01:09:52PM -0700, H. Peter Anvin wrote: > Dave Jones <davej@redhat.com> wrote: > > >On Wed, May 22, 2013 at 12:03:04PM -0700, H. Peter Anvin wrote: > > > On 05/21/2013 04:03 PM, Dave Jones wrote: > > > > In 3.10-rc2 I see this happening.. > > > > > > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > > > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > > > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > >> > [ 252.702055] microcode: Microcode Update Driver: v2.00 > ><tigran@aivazian.fsnet.co.uk>, Peter Oruba > > > > > >> > For some reason the events for udev seem to be getting delayed 60s > >for each core. > > > > > > > > Dave > > > > > > > > > > Is this a regression from -rc1? > > > >No. Looks like it was introduced between 3.9 and 3.10-rc1 > > > >I see bisecting in my future. > > > A bisect would be appreciated. Cc Fenghua. Turns out that I can reproduce this on 3.9 too (and likely earlier) with the right config file. I'm not sure what option causes the delay yet. Isolating differences between configs now. Dave ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-22 20:09 ` H. Peter Anvin 2013-05-23 0:19 ` Dave Jones @ 2013-05-23 3:39 ` Dave Jones 2013-05-23 6:56 ` Takashi Iwai 2013-05-23 7:45 ` Ming Lei 1 sibling, 2 replies; 25+ messages in thread From: Dave Jones @ 2013-05-23 3:39 UTC (permalink / raw) To: H. Peter Anvin; +Cc: Linux Kernel, x86, fenghua.yu, Takashi Iwai > On 05/21/2013 04:03 PM, Dave Jones wrote: > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba > > For some reason the events for udev seem to be getting delayed 60s > for each core. Screwed up my .config, and had CONFIG_FW_LOADER_USER_HELPER inadvertantly set Odd though that it causes that 60 second delay, given that it's supposedly a 'fallback' when the direct loading fails. It seems I don't actually need to set that option, so I'm not bothered if there's an actual bug here or not, but the behaviour seems odd. Dave ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 3:39 ` Dave Jones @ 2013-05-23 6:56 ` Takashi Iwai 2013-05-23 7:45 ` Ming Lei 1 sibling, 0 replies; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 6:56 UTC (permalink / raw) To: Dave Jones; +Cc: H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Wed, 22 May 2013 23:39:11 -0400, Dave Jones wrote: > > > On 05/21/2013 04:03 PM, Dave Jones wrote: > > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba > > > > For some reason the events for udev seem to be getting delayed 60s > > for each core. > > Screwed up my .config, and had CONFIG_FW_LOADER_USER_HELPER inadvertantly set > Odd though that it causes that 60 second delay, given that it's supposedly a > 'fallback' when the direct loading fails. The 60 seconds delay is just because it falls back to usermode helper, ended in the default timeout. By some reason, either udev didn't return properly or the firmware loader didn't handle the response properly. Might be a regression in the firmware loader part, or might be user-space side. I'll take a look at this later. thanks, Takashi > It seems I don't actually need to set that option, so I'm not bothered > if there's an actual bug here or not, but the behaviour seems odd. > > Dave > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 3:39 ` Dave Jones 2013-05-23 6:56 ` Takashi Iwai @ 2013-05-23 7:45 ` Ming Lei 2013-05-23 8:06 ` Takashi Iwai 1 sibling, 1 reply; 25+ messages in thread From: Ming Lei @ 2013-05-23 7:45 UTC (permalink / raw) To: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu, Takashi Iwai On Thu, May 23, 2013 at 11:39 AM, Dave Jones <davej@redhat.com> wrote: > > On 05/21/2013 04:03 PM, Dave Jones wrote: > > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba > > > > For some reason the events for udev seem to be getting delayed 60s > > for each core. > > Screwed up my .config, and had CONFIG_FW_LOADER_USER_HELPER inadvertantly set > Odd though that it causes that 60 second delay, given that it's supposedly a > 'fallback' when the direct loading fails. udevd has the ugly problem previously at some situations(for example, request_firmware called in probe(), and that is why direct loading is introduced), but not sure why the direct loading is failed first. Could you enable 'CONFIG_DEBUG_DRIVER' and post 'dmesg' out? And better to check where the affected firmware(microcode) is in your distribution. > > It seems I don't actually need to set that option, so I'm not bothered > if there's an actual bug here or not, but the behaviour seems odd. If the option isn't set, the firmware will be lost for the requested CPU, :-) Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 7:45 ` Ming Lei @ 2013-05-23 8:06 ` Takashi Iwai 2013-05-23 10:16 ` Takashi Iwai 0 siblings, 1 reply; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 8:06 UTC (permalink / raw) To: Ming Lei; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Thu, 23 May 2013 15:45:32 +0800, Ming Lei wrote: > > On Thu, May 23, 2013 at 11:39 AM, Dave Jones <davej@redhat.com> wrote: > > > On 05/21/2013 04:03 PM, Dave Jones wrote: > > > > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > > > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba > > > > > > For some reason the events for udev seem to be getting delayed 60s > > > for each core. > > > > Screwed up my .config, and had CONFIG_FW_LOADER_USER_HELPER inadvertantly set > > Odd though that it causes that 60 second delay, given that it's supposedly a > > 'fallback' when the direct loading fails. > > udevd has the ugly problem previously at some situations(for example, > request_firmware called in probe(), and that is why direct loading is > introduced), > but not sure why the direct loading is failed first. The microcode update is optional, so it's no error even if the microcode firmwares are not found. But yes, this seems happening during the module probing. The lines "microcde: CPU..." show before "microcode: Microcode Update Driver...", which means the f/w loading has been done before finishing the module load. I thought (or hoped) this mess (60s stalls) was fixed in the recent udev, but apparently not...? > Could you enable 'CONFIG_DEBUG_DRIVER' and post 'dmesg' out? > And better to check where the affected firmware(microcode) is in your > distribution. > > > > > It seems I don't actually need to set that option, so I'm not bothered > > if there's an actual bug here or not, but the behaviour seems odd. > > If the option isn't set, the firmware will be lost for the requested CPU, :-) I don't think it's relevant. If firmware files exist, the microcode update should have been successful even without usermode helper kconfig. If not exist, no update -- so should it be. thanks, Takashi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 8:06 ` Takashi Iwai @ 2013-05-23 10:16 ` Takashi Iwai 2013-05-23 10:18 ` anish singh 2013-05-23 10:27 ` Ming Lei 0 siblings, 2 replies; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 10:16 UTC (permalink / raw) To: Takashi Iwai Cc: Ming Lei, Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Thu, 23 May 2013 10:06:56 +0200, Takashi Iwai wrote: > > At Thu, 23 May 2013 15:45:32 +0800, > Ming Lei wrote: > > > > On Thu, May 23, 2013 at 11:39 AM, Dave Jones <davej@redhat.com> wrote: > > > > On 05/21/2013 04:03 PM, Dave Jones wrote: > > > > > > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > > > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > > > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > > > > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba > > > > > > > > For some reason the events for udev seem to be getting delayed 60s > > > > for each core. > > > > > > Screwed up my .config, and had CONFIG_FW_LOADER_USER_HELPER inadvertantly set > > > Odd though that it causes that 60 second delay, given that it's supposedly a > > > 'fallback' when the direct loading fails. > > > > udevd has the ugly problem previously at some situations(for example, > > request_firmware called in probe(), and that is why direct loading is > > introduced), > > but not sure why the direct loading is failed first. > > The microcode update is optional, so it's no error even if the > microcode firmwares are not found. > > But yes, this seems happening during the module probing. The lines > "microcde: CPU..." show before "microcode: Microcode Update > Driver...", which means the f/w loading has been done before finishing > the module load. > > I thought (or hoped) this mess (60s stalls) was fixed in the recent > udev, but apparently not...? Thinking on this again, if the user-space continues to be broken in that point, we should provide request_firmware() variant without udev, e.g. request_firmware_direct(), and use it in known places like this? Takashi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 10:16 ` Takashi Iwai @ 2013-05-23 10:18 ` anish singh 2013-05-23 10:28 ` Takashi Iwai 2013-05-23 10:27 ` Ming Lei 1 sibling, 1 reply; 25+ messages in thread From: anish singh @ 2013-05-23 10:18 UTC (permalink / raw) To: Takashi Iwai Cc: Ming Lei, Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 3:46 PM, Takashi Iwai <tiwai@suse.de> wrote: > At Thu, 23 May 2013 10:06:56 +0200, > Takashi Iwai wrote: >> >> At Thu, 23 May 2013 15:45:32 +0800, >> Ming Lei wrote: >> > >> > On Thu, May 23, 2013 at 11:39 AM, Dave Jones <davej@redhat.com> wrote: >> > > > On 05/21/2013 04:03 PM, Dave Jones wrote: >> > > > >> > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 >> > > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 >> > > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 >> > > > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba >> > > > >> > > > For some reason the events for udev seem to be getting delayed 60s >> > > > for each core. >> > > >> > > Screwed up my .config, and had CONFIG_FW_LOADER_USER_HELPER inadvertantly set >> > > Odd though that it causes that 60 second delay, given that it's supposedly a >> > > 'fallback' when the direct loading fails. >> > >> > udevd has the ugly problem previously at some situations(for example, >> > request_firmware called in probe(), and that is why direct loading is >> > introduced), >> > but not sure why the direct loading is failed first. >> >> The microcode update is optional, so it's no error even if the >> microcode firmwares are not found. >> >> But yes, this seems happening during the module probing. The lines >> "microcde: CPU..." show before "microcode: Microcode Update >> Driver...", which means the f/w loading has been done before finishing >> the module load. >> >> I thought (or hoped) this mess (60s stalls) was fixed in the recent >> udev, but apparently not...? > > Thinking on this again, if the user-space continues to be broken in > that point, we should provide request_firmware() variant without udev, > e.g. request_firmware_direct(), and use it in known places like this? Is it not already there?I was thinking that some time back this support was added. > > > Takashi > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 10:18 ` anish singh @ 2013-05-23 10:28 ` Takashi Iwai 0 siblings, 0 replies; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 10:28 UTC (permalink / raw) To: anish singh Cc: Ming Lei, Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Thu, 23 May 2013 15:48:52 +0530, anish singh wrote: > > On Thu, May 23, 2013 at 3:46 PM, Takashi Iwai <tiwai@suse.de> wrote: > > At Thu, 23 May 2013 10:06:56 +0200, > > Takashi Iwai wrote: > >> > >> At Thu, 23 May 2013 15:45:32 +0800, > >> Ming Lei wrote: > >> > > >> > On Thu, May 23, 2013 at 11:39 AM, Dave Jones <davej@redhat.com> wrote: > >> > > > On 05/21/2013 04:03 PM, Dave Jones wrote: > >> > > > > >> > > > [ 72.318133] microcode: CPU1 sig=0x306c3, pf=0x2, revision=0x6 > >> > > > [ 132.446449] microcode: CPU2 sig=0x306c3, pf=0x2, revision=0x6 > >> > > > [ 192.573101] microcode: CPU3 sig=0x306c3, pf=0x2, revision=0x6 > >> > > > [ 252.702055] microcode: Microcode Update Driver: v2.00 <tigran@aivazian.fsnet.co.uk>, Peter Oruba > >> > > > > >> > > > For some reason the events for udev seem to be getting delayed 60s > >> > > > for each core. > >> > > > >> > > Screwed up my .config, and had CONFIG_FW_LOADER_USER_HELPER inadvertantly set > >> > > Odd though that it causes that 60 second delay, given that it's supposedly a > >> > > 'fallback' when the direct loading fails. > >> > > >> > udevd has the ugly problem previously at some situations(for example, > >> > request_firmware called in probe(), and that is why direct loading is > >> > introduced), > >> > but not sure why the direct loading is failed first. > >> > >> The microcode update is optional, so it's no error even if the > >> microcode firmwares are not found. > >> > >> But yes, this seems happening during the module probing. The lines > >> "microcde: CPU..." show before "microcode: Microcode Update > >> Driver...", which means the f/w loading has been done before finishing > >> the module load. > >> > >> I thought (or hoped) this mess (60s stalls) was fixed in the recent > >> udev, but apparently not...? > > > > Thinking on this again, if the user-space continues to be broken in > > that point, we should provide request_firmware() variant without udev, > > e.g. request_firmware_direct(), and use it in known places like this? > Is it not already there?I was thinking that some time back this support > was added. My point is the function that *only* uses the direct loading without fallback to user mode helper. Takashi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 10:16 ` Takashi Iwai 2013-05-23 10:18 ` anish singh @ 2013-05-23 10:27 ` Ming Lei 2013-05-23 10:36 ` Takashi Iwai 1 sibling, 1 reply; 25+ messages in thread From: Ming Lei @ 2013-05-23 10:27 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 6:16 PM, Takashi Iwai <tiwai@suse.de> wrote: > > Thinking on this again, if the user-space continues to be broken in > that point, we should provide request_firmware() variant without udev, > e.g. request_firmware_direct(), and use it in known places like this? As anish said, it has been already there, wrt. this problem, I think we need to know why the direct loading is failed. Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 10:27 ` Ming Lei @ 2013-05-23 10:36 ` Takashi Iwai 2013-05-23 10:45 ` Ming Lei 0 siblings, 1 reply; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 10:36 UTC (permalink / raw) To: Ming Lei; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Thu, 23 May 2013 18:27:57 +0800, Ming Lei wrote: > > On Thu, May 23, 2013 at 6:16 PM, Takashi Iwai <tiwai@suse.de> wrote: > > > > Thinking on this again, if the user-space continues to be broken in > > that point, we should provide request_firmware() variant without udev, > > e.g. request_firmware_direct(), and use it in known places like this? > > As anish said, it has been already there, No, f/w loader always fall back to user mode helper, as long as its support is built in. And doing that for microcode driver in that code path isn't only superfluous but also broken due to request_firmware call in module init. > wrt. this problem, I think we > need to know why the direct loading is failed. The reason is obvious: the requested f/w file doesn't exist. And it's fine, because the microcode update is an optional operation. If no f/w file is found, it's not handled as an error. It just means that no need to update, continuing to work. Takashi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 10:36 ` Takashi Iwai @ 2013-05-23 10:45 ` Ming Lei 2013-05-23 12:05 ` Takashi Iwai 0 siblings, 1 reply; 25+ messages in thread From: Ming Lei @ 2013-05-23 10:45 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai <tiwai@suse.de> wrote: > > No, f/w loader always fall back to user mode helper, as long as its > support is built in. And doing that for microcode driver in that code > path isn't only superfluous but also broken due to request_firmware > call in module init. Firstly, it is not good to do this since some distributions doesn't support direct loading and doesn't have udevd(such as, android). Secondly, returning failure from request_firmware_direct() doesn't mean the firmware doesn't exist since distribution may put the firmware other where. Anyway, this example is very specific(no firmware can be accepted), and request_firmware_nowait() should be OK for the situation. > >> wrt. this problem, I think we >> need to know why the direct loading is failed. > > The reason is obvious: the requested f/w file doesn't exist. > And it's fine, because the microcode update is an optional operation. > If no f/w file is found, it's not handled as an error. It just means > that no need to update, continuing to work. OK, as said above, the example is very specific, and might be workarounded by request_firmware_nowait(). Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 10:45 ` Ming Lei @ 2013-05-23 12:05 ` Takashi Iwai 2013-05-23 13:04 ` Ming Lei 0 siblings, 1 reply; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 12:05 UTC (permalink / raw) To: Ming Lei; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Thu, 23 May 2013 18:45:29 +0800, Ming Lei wrote: > > On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai <tiwai@suse.de> wrote: > > > > No, f/w loader always fall back to user mode helper, as long as its > > support is built in. And doing that for microcode driver in that code > > path isn't only superfluous but also broken due to request_firmware > > call in module init. > > Firstly, it is not good to do this since some distributions doesn't support > direct loading and doesn't have udevd(such as, android). > > Secondly, returning failure from request_firmware_direct() doesn't mean > the firmware doesn't exist since distribution may put the firmware other where. Right, the non-standard path is the problem, and basically the only problem. The distribution that doesn't support the direct loading means nothing but that. > Anyway, this example is very specific(no firmware can be accepted), and > request_firmware_nowait() should be OK for the situation. Oh no, rewriting with request_firmware_nowait() should be really the last choice. It would change the code flow awfully bad in most cases. The new kernel driver has a better firmware mechanism. If it's only the question of paths, we should move on toward that direction and drop the too complex old way. I'd vote for a warning shown when a firmware file is loaded via user mode helper (except for explicit cases like FW_ACTION_NOHOTPLUG), for example. > >> wrt. this problem, I think we > >> need to know why the direct loading is failed. > > > > The reason is obvious: the requested f/w file doesn't exist. > > And it's fine, because the microcode update is an optional operation. > > If no f/w file is found, it's not handled as an error. It just means > > that no need to update, continuing to work. > > OK, as said above, the example is very specific, and might be > workarounded by request_firmware_nowait(). It's not that easy in this case. The microcode loader driver core module doesn't invoke request_firmware() directly but it's via cpu driver. And the same callback is called in different code paths, not only at init but also via sysfs write. Thus the request_firmware() call must be synchronous there. Takashi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 12:05 ` Takashi Iwai @ 2013-05-23 13:04 ` Ming Lei 2013-05-23 13:21 ` Takashi Iwai 0 siblings, 1 reply; 25+ messages in thread From: Ming Lei @ 2013-05-23 13:04 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 8:05 PM, Takashi Iwai <tiwai@suse.de> wrote: > At Thu, 23 May 2013 18:45:29 +0800, > Ming Lei wrote: >> >> On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai <tiwai@suse.de> wrote: >> > >> > No, f/w loader always fall back to user mode helper, as long as its >> > support is built in. And doing that for microcode driver in that code >> > path isn't only superfluous but also broken due to request_firmware >> > call in module init. >> >> Firstly, it is not good to do this since some distributions doesn't support >> direct loading and doesn't have udevd(such as, android). >> >> Secondly, returning failure from request_firmware_direct() doesn't mean >> the firmware doesn't exist since distribution may put the firmware other where. > > Right, the non-standard path is the problem, and basically the only > problem. The distribution that doesn't support the direct loading > means nothing but that. Suppose it is, it is the fact, and it isn't OK to break this distribution. Also udev supports user-defined rules to load firmware, which means some drivers may not put their firmware in the default path of distribution's firmware. > >> Anyway, this example is very specific(no firmware can be accepted), and >> request_firmware_nowait() should be OK for the situation. > > Oh no, rewriting with request_firmware_nowait() should be really the > last choice. It would change the code flow awfully bad in most > cases. > > The new kernel driver has a better firmware mechanism. If it's only > the question of paths, we should move on toward that direction and > drop the too complex old way. I'd vote for a warning shown when a Simply dropping the old way may cause user space regression. > firmware file is loaded via user mode helper (except for explicit > cases like FW_ACTION_NOHOTPLUG), for example. As it is a very driver specific problem, it is better to solve it inside driver. > > >> >> wrt. this problem, I think we >> >> need to know why the direct loading is failed. >> > >> > The reason is obvious: the requested f/w file doesn't exist. >> > And it's fine, because the microcode update is an optional operation. >> > If no f/w file is found, it's not handled as an error. It just means >> > that no need to update, continuing to work. >> >> OK, as said above, the example is very specific, and might be >> workarounded by request_firmware_nowait(). > > It's not that easy in this case. The microcode loader driver core > module doesn't invoke request_firmware() directly but it's via cpu > driver. And the same callback is called in different code paths, not > only at init but also via sysfs write. Thus the request_firmware() > call must be synchronous there. I don't think the way is too difficult to implement. In the path which requires synchronization, it can be waited on one completion after calling request_firmware_nowait(). Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 13:04 ` Ming Lei @ 2013-05-23 13:21 ` Takashi Iwai 2013-05-23 14:28 ` Ming Lei 0 siblings, 1 reply; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 13:21 UTC (permalink / raw) To: Ming Lei; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Thu, 23 May 2013 21:04:53 +0800, Ming Lei wrote: > > On Thu, May 23, 2013 at 8:05 PM, Takashi Iwai <tiwai@suse.de> wrote: > > At Thu, 23 May 2013 18:45:29 +0800, > > Ming Lei wrote: > >> > >> On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai <tiwai@suse.de> wrote: > >> > > >> > No, f/w loader always fall back to user mode helper, as long as its > >> > support is built in. And doing that for microcode driver in that code > >> > path isn't only superfluous but also broken due to request_firmware > >> > call in module init. > >> > >> Firstly, it is not good to do this since some distributions doesn't support > >> direct loading and doesn't have udevd(such as, android). > >> > >> Secondly, returning failure from request_firmware_direct() doesn't mean > >> the firmware doesn't exist since distribution may put the firmware other where. > > > > Right, the non-standard path is the problem, and basically the only > > problem. The distribution that doesn't support the direct loading > > means nothing but that. > > Suppose it is, it is the fact, and it isn't OK to break this distribution. > > Also udev supports user-defined rules to load firmware, which > means some drivers may not put their firmware in the default > path of distribution's firmware. It's why I suggested to put a warning in that path as the first step. So we can see whether there is any actual user. > >> Anyway, this example is very specific(no firmware can be accepted), and > >> request_firmware_nowait() should be OK for the situation. > > > > Oh no, rewriting with request_firmware_nowait() should be really the > > last choice. It would change the code flow awfully bad in most > > cases. > > > > The new kernel driver has a better firmware mechanism. If it's only > > the question of paths, we should move on toward that direction and > > drop the too complex old way. I'd vote for a warning shown when a > > Simply dropping the old way may cause user space regression. It's already broken :) > > firmware file is loaded via user mode helper (except for explicit > > cases like FW_ACTION_NOHOTPLUG), for example. > > As it is a very driver specific problem, it is better to solve it inside driver. Yes, this proposal is basically not meant as a fix for this particular issue but rather for future movement in general. > >> >> wrt. this problem, I think we > >> >> need to know why the direct loading is failed. > >> > > >> > The reason is obvious: the requested f/w file doesn't exist. > >> > And it's fine, because the microcode update is an optional operation. > >> > If no f/w file is found, it's not handled as an error. It just means > >> > that no need to update, continuing to work. > >> > >> OK, as said above, the example is very specific, and might be > >> workarounded by request_firmware_nowait(). > > > > It's not that easy in this case. The microcode loader driver core > > module doesn't invoke request_firmware() directly but it's via cpu > > driver. And the same callback is called in different code paths, not > > only at init but also via sysfs write. Thus the request_firmware() > > call must be synchronous there. > > I don't think the way is too difficult to implement. In the path which > requires synchronization, it can be waited on one completion after > calling request_firmware_nowait(). This sounds already like unnecessary complexity. Also, what if concurrent accesses? Also, I wonder why the kernel needs to be "fixed" for this, if the problem is really the stuck in udev. In this regard, we didn't change anything from the beginning. There was an implicit "wish", that the f/w loading shouldn't be done in the module init, but this has been never treated as a golden rule. Takashi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 13:21 ` Takashi Iwai @ 2013-05-23 14:28 ` Ming Lei 2013-05-23 14:36 ` Takashi Iwai 0 siblings, 1 reply; 25+ messages in thread From: Ming Lei @ 2013-05-23 14:28 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 9:21 PM, Takashi Iwai <tiwai@suse.de> wrote: > At Thu, 23 May 2013 21:04:53 +0800, > Ming Lei wrote: >> >> On Thu, May 23, 2013 at 8:05 PM, Takashi Iwai <tiwai@suse.de> wrote: >> > At Thu, 23 May 2013 18:45:29 +0800, >> > Ming Lei wrote: >> >> >> >> On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai <tiwai@suse.de> wrote: >> >> > >> >> > No, f/w loader always fall back to user mode helper, as long as its >> >> > support is built in. And doing that for microcode driver in that code >> >> > path isn't only superfluous but also broken due to request_firmware >> >> > call in module init. >> >> >> >> Firstly, it is not good to do this since some distributions doesn't support >> >> direct loading and doesn't have udevd(such as, android). >> >> >> >> Secondly, returning failure from request_firmware_direct() doesn't mean >> >> the firmware doesn't exist since distribution may put the firmware other where. >> > >> > Right, the non-standard path is the problem, and basically the only >> > problem. The distribution that doesn't support the direct loading >> > means nothing but that. >> >> Suppose it is, it is the fact, and it isn't OK to break this distribution. >> >> Also udev supports user-defined rules to load firmware, which >> means some drivers may not put their firmware in the default >> path of distribution's firmware. > > It's why I suggested to put a warning in that path as the first step. > So we can see whether there is any actual user. If you plan to do it, it'd better to add default firmware path of some distributions into firmware_class.c first, otherwise it may cause unnecessary noise for this distribution. But if more default search paths are added, it might cause mistaken firmwares found under incorrect path, for example, android's default path is "/etc/firmware" and "/vendor/firmware"(maybe different for different versions). Also, putting default search paths into kernel isn't good, which was introduced unwillingly for well-known reason. > >> >> Anyway, this example is very specific(no firmware can be accepted), and >> >> request_firmware_nowait() should be OK for the situation. >> > >> > Oh no, rewriting with request_firmware_nowait() should be really the >> > last choice. It would change the code flow awfully bad in most >> > cases. >> > >> > The new kernel driver has a better firmware mechanism. If it's only >> > the question of paths, we should move on toward that direction and >> > drop the too complex old way. I'd vote for a warning shown when a >> >> Simply dropping the old way may cause user space regression. > > It's already broken :) It is different, the current issue is caused by udev, not by kernel, :-) > >> > firmware file is loaded via user mode helper (except for explicit >> > cases like FW_ACTION_NOHOTPLUG), for example. >> >> As it is a very driver specific problem, it is better to solve it inside driver. > > Yes, this proposal is basically not meant as a fix for this particular > issue but rather for future movement in general. > >> >> >> wrt. this problem, I think we >> >> >> need to know why the direct loading is failed. >> >> > >> >> > The reason is obvious: the requested f/w file doesn't exist. >> >> > And it's fine, because the microcode update is an optional operation. >> >> > If no f/w file is found, it's not handled as an error. It just means >> >> > that no need to update, continuing to work. >> >> >> >> OK, as said above, the example is very specific, and might be >> >> workarounded by request_firmware_nowait(). >> > >> > It's not that easy in this case. The microcode loader driver core >> > module doesn't invoke request_firmware() directly but it's via cpu >> > driver. And the same callback is called in different code paths, not >> > only at init but also via sysfs write. Thus the request_firmware() >> > call must be synchronous there. >> >> I don't think the way is too difficult to implement. In the path which >> requires synchronization, it can be waited on one completion after >> calling request_firmware_nowait(). > > This sounds already like unnecessary complexity. Also, what if > concurrent accesses? The request_firmware_no_wait() supports concurrent accesses on either same firmware or not. > > Also, I wonder why the kernel needs to be "fixed" for this, if the > problem is really the stuck in udev. In this regard, we didn't change > anything from the beginning. There was an implicit "wish", that the > f/w loading shouldn't be done in the module init, but this has been > never treated as a golden rule. No, there isn't the golden rule, and it is reasonable or inevitable sometimes to load firmware in module init, for example, I remember some wireless dongles in which people can't read its mac address without downloading firmware, that means some devices may not be initialized successfully without firmware. Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 14:28 ` Ming Lei @ 2013-05-23 14:36 ` Takashi Iwai 2013-05-23 14:48 ` Dave Jones 2013-05-23 15:21 ` Ming Lei 0 siblings, 2 replies; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 14:36 UTC (permalink / raw) To: Ming Lei; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Thu, 23 May 2013 22:28:51 +0800, Ming Lei wrote: > > On Thu, May 23, 2013 at 9:21 PM, Takashi Iwai <tiwai@suse.de> wrote: > > At Thu, 23 May 2013 21:04:53 +0800, > > Ming Lei wrote: > >> > >> On Thu, May 23, 2013 at 8:05 PM, Takashi Iwai <tiwai@suse.de> wrote: > >> > At Thu, 23 May 2013 18:45:29 +0800, > >> > Ming Lei wrote: > >> >> > >> >> On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai <tiwai@suse.de> wrote: > >> >> > > >> >> > No, f/w loader always fall back to user mode helper, as long as its > >> >> > support is built in. And doing that for microcode driver in that code > >> >> > path isn't only superfluous but also broken due to request_firmware > >> >> > call in module init. > >> >> > >> >> Firstly, it is not good to do this since some distributions doesn't support > >> >> direct loading and doesn't have udevd(such as, android). > >> >> > >> >> Secondly, returning failure from request_firmware_direct() doesn't mean > >> >> the firmware doesn't exist since distribution may put the firmware other where. > >> > > >> > Right, the non-standard path is the problem, and basically the only > >> > problem. The distribution that doesn't support the direct loading > >> > means nothing but that. > >> > >> Suppose it is, it is the fact, and it isn't OK to break this distribution. > >> > >> Also udev supports user-defined rules to load firmware, which > >> means some drivers may not put their firmware in the default > >> path of distribution's firmware. > > > > It's why I suggested to put a warning in that path as the first step. > > So we can see whether there is any actual user. > > If you plan to do it, it'd better to add default firmware path of some > distributions into firmware_class.c first, otherwise it may cause > unnecessary noise for this distribution. > > But if more default search paths are added, it might cause mistaken > firmwares found under incorrect path, for example, android's > default path is "/etc/firmware" and "/vendor/firmware"(maybe different > for different versions). > > Also, putting default search paths into kernel isn't good, which was > introduced unwillingly for well-known reason. Maybe we can create a new Kconfig to specify non-standard firmware path? > >> >> Anyway, this example is very specific(no firmware can be accepted), and > >> >> request_firmware_nowait() should be OK for the situation. > >> > > >> > Oh no, rewriting with request_firmware_nowait() should be really the > >> > last choice. It would change the code flow awfully bad in most > >> > cases. > >> > > >> > The new kernel driver has a better firmware mechanism. If it's only > >> > the question of paths, we should move on toward that direction and > >> > drop the too complex old way. I'd vote for a warning shown when a > >> > >> Simply dropping the old way may cause user space regression. > > > > It's already broken :) > > It is different, the current issue is caused by udev, not by kernel, :-) Yeah :) > >> > firmware file is loaded via user mode helper (except for explicit > >> > cases like FW_ACTION_NOHOTPLUG), for example. > >> > >> As it is a very driver specific problem, it is better to solve it inside driver. > > > > Yes, this proposal is basically not meant as a fix for this particular > > issue but rather for future movement in general. > > > >> >> >> wrt. this problem, I think we > >> >> >> need to know why the direct loading is failed. > >> >> > > >> >> > The reason is obvious: the requested f/w file doesn't exist. > >> >> > And it's fine, because the microcode update is an optional operation. > >> >> > If no f/w file is found, it's not handled as an error. It just means > >> >> > that no need to update, continuing to work. > >> >> > >> >> OK, as said above, the example is very specific, and might be > >> >> workarounded by request_firmware_nowait(). > >> > > >> > It's not that easy in this case. The microcode loader driver core > >> > module doesn't invoke request_firmware() directly but it's via cpu > >> > driver. And the same callback is called in different code paths, not > >> > only at init but also via sysfs write. Thus the request_firmware() > >> > call must be synchronous there. > >> > >> I don't think the way is too difficult to implement. In the path which > >> requires synchronization, it can be waited on one completion after > >> calling request_firmware_nowait(). > > > > This sounds already like unnecessary complexity. Also, what if > > concurrent accesses? > > The request_firmware_no_wait() supports concurrent accesses on > either same firmware or not. Yes, but I meant about the synchronization part. Then you'll need multiple waiters. > > Also, I wonder why the kernel needs to be "fixed" for this, if the > > problem is really the stuck in udev. In this regard, we didn't change > > anything from the beginning. There was an implicit "wish", that the > > f/w loading shouldn't be done in the module init, but this has been > > never treated as a golden rule. > > No, there isn't the golden rule, and it is reasonable or inevitable > sometimes to load firmware in module init, for example, I remember some > wireless dongles in which people can't read its mac address without > downloading firmware, that means some devices may not be initialized > successfully without firmware. Right. Takashi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 14:36 ` Takashi Iwai @ 2013-05-23 14:48 ` Dave Jones 2013-05-23 15:06 ` Takashi Iwai 2013-05-23 15:21 ` Ming Lei 1 sibling, 1 reply; 25+ messages in thread From: Dave Jones @ 2013-05-23 14:48 UTC (permalink / raw) To: Takashi Iwai; +Cc: Ming Lei, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 04:36:20PM +0200, Takashi Iwai wrote: > > >> Also udev supports user-defined rules to load firmware, which > > >> means some drivers may not put their firmware in the default > > >> path of distribution's firmware. > > > > > > It's why I suggested to put a warning in that path as the first step. > > > So we can see whether there is any actual user. > > > > If you plan to do it, it'd better to add default firmware path of some > > distributions into firmware_class.c first, otherwise it may cause > > unnecessary noise for this distribution. > > > > But if more default search paths are added, it might cause mistaken > > firmwares found under incorrect path, for example, android's > > default path is "/etc/firmware" and "/vendor/firmware"(maybe different > > for different versions). > > > > Also, putting default search paths into kernel isn't good, which was > > introduced unwillingly for well-known reason. > > Maybe we can create a new Kconfig to specify non-standard firmware > path? You keep mentioning non-standard paths for firmware, but in this case, I don't think Fedora is doing anything unusual. We have microcode firmware in /lib/firmware/intel-ucode/ just like (afaik) everyone else. What *is* happening, I think is that the CPU is new enough that there's no newer firmware file available for it. thoughts? Dave ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 14:48 ` Dave Jones @ 2013-05-23 15:06 ` Takashi Iwai 2013-05-23 15:27 ` Ming Lei 0 siblings, 1 reply; 25+ messages in thread From: Takashi Iwai @ 2013-05-23 15:06 UTC (permalink / raw) To: Dave Jones; +Cc: Ming Lei, H. Peter Anvin, Linux Kernel, x86, fenghua.yu At Thu, 23 May 2013 10:48:00 -0400, Dave Jones wrote: > > On Thu, May 23, 2013 at 04:36:20PM +0200, Takashi Iwai wrote: > > > > >> Also udev supports user-defined rules to load firmware, which > > > >> means some drivers may not put their firmware in the default > > > >> path of distribution's firmware. > > > > > > > > It's why I suggested to put a warning in that path as the first step. > > > > So we can see whether there is any actual user. > > > > > > If you plan to do it, it'd better to add default firmware path of some > > > distributions into firmware_class.c first, otherwise it may cause > > > unnecessary noise for this distribution. > > > > > > But if more default search paths are added, it might cause mistaken > > > firmwares found under incorrect path, for example, android's > > > default path is "/etc/firmware" and "/vendor/firmware"(maybe different > > > for different versions). > > > > > > Also, putting default search paths into kernel isn't good, which was > > > introduced unwillingly for well-known reason. > > > > Maybe we can create a new Kconfig to specify non-standard firmware > > path? > > You keep mentioning non-standard paths for firmware, but in this case, > I don't think Fedora is doing anything unusual. We have microcode firmware > in /lib/firmware/intel-ucode/ just like (afaik) everyone else. > > What *is* happening, I think is that the CPU is new enough that there's no > newer firmware file available for it. > > thoughts? Yes, in your case, everything is fine in the kernel itself. And no microcode update is needed for new CPU, thus no firmware. The problem is that the f/w loader tries to call udev and udev gets stuck when invoked from module init. This doesn't hit most drivers because usually the firmware is mandatory and it must exist. Thus the direct f/w loading always works for them. If it hits, it's only the error case. But, for the microcode loader, it's normal that the firmware doesn't exist, like your case. Unfortunately, this falls back to user helper mode, and now you're seeing the problem. So, the option would be to fix udev, let it behaving like before. The second option would be to change ("fix") the kernel behavior, but the question is which way. Takashi > Dave > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 15:06 ` Takashi Iwai @ 2013-05-23 15:27 ` Ming Lei 2013-05-23 15:55 ` Dave Jones 0 siblings, 1 reply; 25+ messages in thread From: Ming Lei @ 2013-05-23 15:27 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 11:06 PM, Takashi Iwai <tiwai@suse.de> wrote: > At Thu, 23 May 2013 10:48:00 -0400, > Dave Jones wrote: >> >> On Thu, May 23, 2013 at 04:36:20PM +0200, Takashi Iwai wrote: >> >> > > >> Also udev supports user-defined rules to load firmware, which >> > > >> means some drivers may not put their firmware in the default >> > > >> path of distribution's firmware. >> > > > >> > > > It's why I suggested to put a warning in that path as the first step. >> > > > So we can see whether there is any actual user. >> > > >> > > If you plan to do it, it'd better to add default firmware path of some >> > > distributions into firmware_class.c first, otherwise it may cause >> > > unnecessary noise for this distribution. >> > > >> > > But if more default search paths are added, it might cause mistaken >> > > firmwares found under incorrect path, for example, android's >> > > default path is "/etc/firmware" and "/vendor/firmware"(maybe different >> > > for different versions). >> > > >> > > Also, putting default search paths into kernel isn't good, which was >> > > introduced unwillingly for well-known reason. >> > >> > Maybe we can create a new Kconfig to specify non-standard firmware >> > path? >> >> You keep mentioning non-standard paths for firmware, but in this case, >> I don't think Fedora is doing anything unusual. We have microcode firmware >> in /lib/firmware/intel-ucode/ just like (afaik) everyone else. >> >> What *is* happening, I think is that the CPU is new enough that there's no >> newer firmware file available for it. >> >> thoughts? > > Yes, in your case, everything is fine in the kernel itself. And no > microcode update is needed for new CPU, thus no firmware. Can the driver decide if the CPU need microcode? Or there will be the microcode for the CPU in future? > > The problem is that the f/w loader tries to call udev and udev gets > stuck when invoked from module init. This doesn't hit most drivers > because usually the firmware is mandatory and it must exist. Thus the > direct f/w loading always works for them. If it hits, it's only the > error case. > > But, for the microcode loader, it's normal that the firmware doesn't > exist, like your case. Unfortunately, this falls back to user helper > mode, and now you're seeing the problem. The problem is that if driver call request_firmware(), it means it need the firmware and it know there should be one. So maybe the driver shouldn't call request_firmware() if it can figure out that case. > So, the option would be to fix udev, let it behaving like before. > The second option would be to change ("fix") the kernel behavior, but > the question is which way. Sounds like the microcode driver depends on userspace or filesystem to decide if there is one microcode available for me, is the way correct? Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 15:27 ` Ming Lei @ 2013-05-23 15:55 ` Dave Jones 0 siblings, 0 replies; 25+ messages in thread From: Dave Jones @ 2013-05-23 15:55 UTC (permalink / raw) To: Ming Lei; +Cc: Takashi Iwai, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 11:27:08PM +0800, Ming Lei wrote: > > Yes, in your case, everything is fine in the kernel itself. And no > > microcode update is needed for new CPU, thus no firmware. > > Can the driver decide if the CPU need microcode? Or there will > be the microcode for the CPU in future? The kernel can't know. The microcode updates are asynchonously released from the kernel. I might be running a months old kernel, and get a new microcode tomorrow. The kernel handles this by looking for a file with a name matching the cpu family/model/stepping. (See how they are formed in /lib/firmware/intel-ucode/*) DAve ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: microcode loading got really slow. 2013-05-23 14:36 ` Takashi Iwai 2013-05-23 14:48 ` Dave Jones @ 2013-05-23 15:21 ` Ming Lei 1 sibling, 0 replies; 25+ messages in thread From: Ming Lei @ 2013-05-23 15:21 UTC (permalink / raw) To: Takashi Iwai; +Cc: Dave Jones, H. Peter Anvin, Linux Kernel, x86, fenghua.yu On Thu, May 23, 2013 at 10:36 PM, Takashi Iwai <tiwai@suse.de> wrote: > At Thu, 23 May 2013 22:28:51 +0800, > Ming Lei wrote: >> >> On Thu, May 23, 2013 at 9:21 PM, Takashi Iwai <tiwai@suse.de> wrote: >> > At Thu, 23 May 2013 21:04:53 +0800, >> > Ming Lei wrote: >> >> >> >> On Thu, May 23, 2013 at 8:05 PM, Takashi Iwai <tiwai@suse.de> wrote: >> >> > At Thu, 23 May 2013 18:45:29 +0800, >> >> > Ming Lei wrote: >> >> >> >> >> >> On Thu, May 23, 2013 at 6:36 PM, Takashi Iwai <tiwai@suse.de> wrote: >> >> >> > >> >> >> > No, f/w loader always fall back to user mode helper, as long as its >> >> >> > support is built in. And doing that for microcode driver in that code >> >> >> > path isn't only superfluous but also broken due to request_firmware >> >> >> > call in module init. >> >> >> >> >> >> Firstly, it is not good to do this since some distributions doesn't support >> >> >> direct loading and doesn't have udevd(such as, android). >> >> >> >> >> >> Secondly, returning failure from request_firmware_direct() doesn't mean >> >> >> the firmware doesn't exist since distribution may put the firmware other where. >> >> > >> >> > Right, the non-standard path is the problem, and basically the only >> >> > problem. The distribution that doesn't support the direct loading >> >> > means nothing but that. >> >> >> >> Suppose it is, it is the fact, and it isn't OK to break this distribution. >> >> >> >> Also udev supports user-defined rules to load firmware, which >> >> means some drivers may not put their firmware in the default >> >> path of distribution's firmware. >> > >> > It's why I suggested to put a warning in that path as the first step. >> > So we can see whether there is any actual user. >> >> If you plan to do it, it'd better to add default firmware path of some >> distributions into firmware_class.c first, otherwise it may cause >> unnecessary noise for this distribution. >> >> But if more default search paths are added, it might cause mistaken >> firmwares found under incorrect path, for example, android's >> default path is "/etc/firmware" and "/vendor/firmware"(maybe different >> for different versions). >> >> Also, putting default search paths into kernel isn't good, which was >> introduced unwillingly for well-known reason. > > Maybe we can create a new Kconfig to specify non-standard firmware > path? Yes. > > >> >> >> Anyway, this example is very specific(no firmware can be accepted), and >> >> >> request_firmware_nowait() should be OK for the situation. >> >> > >> >> > Oh no, rewriting with request_firmware_nowait() should be really the >> >> > last choice. It would change the code flow awfully bad in most >> >> > cases. >> >> > >> >> > The new kernel driver has a better firmware mechanism. If it's only >> >> > the question of paths, we should move on toward that direction and >> >> > drop the too complex old way. I'd vote for a warning shown when a >> >> >> >> Simply dropping the old way may cause user space regression. >> > >> > It's already broken :) >> >> It is different, the current issue is caused by udev, not by kernel, :-) > > Yeah :) > > >> >> > firmware file is loaded via user mode helper (except for explicit >> >> > cases like FW_ACTION_NOHOTPLUG), for example. >> >> >> >> As it is a very driver specific problem, it is better to solve it inside driver. >> > >> > Yes, this proposal is basically not meant as a fix for this particular >> > issue but rather for future movement in general. >> > >> >> >> >> wrt. this problem, I think we >> >> >> >> need to know why the direct loading is failed. >> >> >> > >> >> >> > The reason is obvious: the requested f/w file doesn't exist. >> >> >> > And it's fine, because the microcode update is an optional operation. >> >> >> > If no f/w file is found, it's not handled as an error. It just means >> >> >> > that no need to update, continuing to work. >> >> >> >> >> >> OK, as said above, the example is very specific, and might be >> >> >> workarounded by request_firmware_nowait(). >> >> > >> >> > It's not that easy in this case. The microcode loader driver core >> >> > module doesn't invoke request_firmware() directly but it's via cpu >> >> > driver. And the same callback is called in different code paths, not >> >> > only at init but also via sysfs write. Thus the request_firmware() >> >> > call must be synchronous there. >> >> >> >> I don't think the way is too difficult to implement. In the path which >> >> requires synchronization, it can be waited on one completion after >> >> calling request_firmware_nowait(). >> > >> > This sounds already like unnecessary complexity. Also, what if >> > concurrent accesses? >> >> The request_firmware_no_wait() supports concurrent accesses on >> either same firmware or not. > > Yes, but I meant about the synchronization part. Then you'll need > multiple waiters. If request_firmware_no_wait() is called on one same firmware image, one waiter should be OK since the returned data is same for all requests. > >> > Also, I wonder why the kernel needs to be "fixed" for this, if the >> > problem is really the stuck in udev. In this regard, we didn't change >> > anything from the beginning. There was an implicit "wish", that the >> > f/w loading shouldn't be done in the module init, but this has been >> > never treated as a golden rule. >> >> No, there isn't the golden rule, and it is reasonable or inevitable >> sometimes to load firmware in module init, for example, I remember some >> wireless dongles in which people can't read its mac address without >> downloading firmware, that means some devices may not be initialized >> successfully without firmware. > > Right. > > > Takashi Thanks, -- Ming Lei ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2013-05-23 15:55 UTC | newest] Thread overview: 25+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-05-21 23:03 microcode loading got really slow Dave Jones 2013-05-22 19:03 ` H. Peter Anvin 2013-05-22 20:00 ` Dave Jones 2013-05-22 20:09 ` H. Peter Anvin 2013-05-23 0:19 ` Dave Jones 2013-05-23 3:39 ` Dave Jones 2013-05-23 6:56 ` Takashi Iwai 2013-05-23 7:45 ` Ming Lei 2013-05-23 8:06 ` Takashi Iwai 2013-05-23 10:16 ` Takashi Iwai 2013-05-23 10:18 ` anish singh 2013-05-23 10:28 ` Takashi Iwai 2013-05-23 10:27 ` Ming Lei 2013-05-23 10:36 ` Takashi Iwai 2013-05-23 10:45 ` Ming Lei 2013-05-23 12:05 ` Takashi Iwai 2013-05-23 13:04 ` Ming Lei 2013-05-23 13:21 ` Takashi Iwai 2013-05-23 14:28 ` Ming Lei 2013-05-23 14:36 ` Takashi Iwai 2013-05-23 14:48 ` Dave Jones 2013-05-23 15:06 ` Takashi Iwai 2013-05-23 15:27 ` Ming Lei 2013-05-23 15:55 ` Dave Jones 2013-05-23 15:21 ` Ming Lei
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox