mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Ming Lei <tom.leiming@gmail.com>
Cc: Dave Jones <davej@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Linux Kernel <linux-kernel@vger.kernel.org>,
	x86@kernel.org, fenghua.yu@intel.com
Subject: Re: microcode loading got really slow.
Date: Thu, 23 May 2013 14:05:46 +0200	[thread overview]
Message-ID: <s5hmwrlx5np.wl%tiwai@suse.de> (raw)
In-Reply-To: <CACVXFVNO9FeCaJ_YFbvPMUHOBjneLqqW+ucYprZb7884Eth-DA@mail.gmail.com>

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

  reply	other threads:[~2013-05-23 12:05 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-05-21 23:03 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 [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5hmwrlx5np.wl%tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=davej@redhat.com \
    --cc=fenghua.yu@intel.com \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tom.leiming@gmail.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®