Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Borislav Petkov]

On Mon, Aug 29, 2011 at 11:47:24AM -0400, David Airlie wrote:
> > On Mon, Aug 29, 2011 at 09:48:22AM -0400, Alex Deucher wrote:
> > > >> Should we make Kconfig pop up a dialog and ask for the
> > > >> whereabouts of
> > > >> these firmware thingies when you mark the driver =y?
> > > >>
> > > >> This all sounds like magic to me, having to know you need to add
> > > >> to
> > > >> EXTRA_FIRMWARE, having to know what file it needs etc.. For all
> > > >> intents
> > > >> and purposes =y just doesn't work and that's broken.
> > > >
> > > > Yep, you make a lot of sense. I had to fumble the build/reboot
> > > > cycle a
> > > > couple of times and do some code staring even to figure this out.
> > > > In
> > > > the end, I copied the whole radeon/ folder from David's firmware
> > > > git
> > > > repo into /lib/firmware and made radeon.ko =m again so that I
> > > > don't have
> > > > to add *.bin entries to CONFIG_EXTRA_FIRMWARE each time I'm
> > > > building a
> > > > kernel on a different machine.
> > > >
> > > > Besides, there was this other issue on lkml today where
> > > > CONFIG_EXTRA_FIRMWARE can cause nconf to segfault when you
> > > > overflow
> > > > its length of 256 by trying to include a bunch of firmware *bin
> > > > files:
> > > > http://lkml.org/lkml/2011/8/29/86
> > >
> > > If you are going to build the ucode into your kernel you'll need to
> > > pick the ones you want to include or increase the limit regardless
> > > of
> > > whether it's radeon ucode or ucode for some other chip. For a
> > > particular card you only need the ones for that card (e.g.,
> > > CEDAR_*.bin or REDWOOD_*.bin, etc.)
> > 
> > Alex, Dave, yeah, that's all fine.
> > 
> > The question Peter asked is, how to make this much more understandable
> > to the user so that she/he doesn't have to figure it out on their own.
> > IOW, if one sets RADEON to =y in Kconfig, it should automatically
> > generate a selection menu with all the firmware required so that
> > the user can select from it either the CEDAR* or the REDWOOD* (or
> > the DOUGHNUT* :-)) ones for her/his card and when the user selects
> > one entry, the required strings are added to CONFIG_EXTRA_FIRMWARE
> > _automatically_.
> > 
> > Maybe even Kbuild should try to find them on the system, and, if
> > unable
> > to, remind the user to install the needed firmware package.
> > 
> > Anyway, something to that effect, the above is just to illustrate the
> > intention, I don't know whether it would work. In any case, we're
> > lacking user help there and we don't want to put every user through
> > the process of finding which firmware files she/he needs when setting
> > RADEON=y.
> > 
> > Does that make more sense?
> 
> Oh it makes sense, just neither of us are Kbuild hackers and I'm not sure that'll change at any point :)
> 
> It also sounds like something that could apply to any driver with external firmware.

So it seems like all the request_firmware() drivers could use a Kbuild
functionality of some sort which presents the user with an option to
select the firmware blobs for his hw when those drivers are =y. Hairy.

Adding kbuild ML.

-- 
Regards/Gruss,
Boris.

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Arnaud Lacombe]

Hi,

On Mon, Aug 29, 2011 at 11:55 AM, Borislav Petkov <bp@alien8.de> wrote:
> On Mon, Aug 29, 2011 at 11:47:24AM -0400, David Airlie wrote:
>> > On Mon, Aug 29, 2011 at 09:48:22AM -0400, Alex Deucher wrote:
>> > > >> Should we make Kconfig pop up a dialog and ask for the
>> > > >> whereabouts of
>> > > >> these firmware thingies when you mark the driver =y?
>> > > >>
>> > > >> This all sounds like magic to me, having to know you need to add
>> > > >> to
>> > > >> EXTRA_FIRMWARE, having to know what file it needs etc.. For all
>> > > >> intents
>> > > >> and purposes =y just doesn't work and that's broken.
>> > > >
>> > > > Yep, you make a lot of sense. I had to fumble the build/reboot
>> > > > cycle a
>> > > > couple of times and do some code staring even to figure this out.
>> > > > In
>> > > > the end, I copied the whole radeon/ folder from David's firmware
>> > > > git
>> > > > repo into /lib/firmware and made radeon.ko =m again so that I
>> > > > don't have
>> > > > to add *.bin entries to CONFIG_EXTRA_FIRMWARE each time I'm
>> > > > building a
>> > > > kernel on a different machine.
>> > > >
>> > > > Besides, there was this other issue on lkml today where
>> > > > CONFIG_EXTRA_FIRMWARE can cause nconf to segfault when you
>> > > > overflow
>> > > > its length of 256 by trying to include a bunch of firmware *bin
>> > > > files:
>> > > > http://lkml.org/lkml/2011/8/29/86
>> > >
>> > > If you are going to build the ucode into your kernel you'll need to
>> > > pick the ones you want to include or increase the limit regardless
>> > > of
>> > > whether it's radeon ucode or ucode for some other chip. For a
>> > > particular card you only need the ones for that card (e.g.,
>> > > CEDAR_*.bin or REDWOOD_*.bin, etc.)
>> >
>> > Alex, Dave, yeah, that's all fine.
>> >
>> > The question Peter asked is, how to make this much more understandable
>> > to the user so that she/he doesn't have to figure it out on their own.
>> > IOW, if one sets RADEON to =y in Kconfig, it should automatically
>> > generate a selection menu with all the firmware required so that
>> > the user can select from it either the CEDAR* or the REDWOOD* (or
>> > the DOUGHNUT* :-)) ones for her/his card and when the user selects
>> > one entry, the required strings are added to CONFIG_EXTRA_FIRMWARE
>> > _automatically_.
>> >
>> > Maybe even Kbuild should try to find them on the system, and, if
>> > unable
>> > to, remind the user to install the needed firmware package.
>> >
>> > Anyway, something to that effect, the above is just to illustrate the
>> > intention, I don't know whether it would work. In any case, we're
>> > lacking user help there and we don't want to put every user through
>> > the process of finding which firmware files she/he needs when setting
>> > RADEON=y.
>> >
>> > Does that make more sense?
>>
>> Oh it makes sense, just neither of us are Kbuild hackers and I'm not sure that'll change at any point :)
>>
>> It also sounds like something that could apply to any driver with external firmware.
>
> So it seems like all the request_firmware() drivers could use a Kbuild
> functionality of some sort which presents the user with an option to
> select the firmware blobs for his hw when those drivers are =y. Hairy.
>
> Adding kbuild ML.
>
do you want something ala:

config EXTRA_FIRMWARE
    string
    default ""
    append "FOO" if BAR
    append "FOZ" if BAZ

or maybe a new type "list" which would behave as a comma/space separated value.

config EXTRA_FIRMWARE
    list "bla"
    append "FOO" if BAR
    append "FOZ" if BAZ

?

Thanks,
 - Arnaud

> --
> Regards/Gruss,
> Boris.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Borislav Petkov]

On Mon, Aug 29, 2011 at 12:10:45PM -0400, Arnaud Lacombe wrote:
> do you want something ala:
> 
> config EXTRA_FIRMWARE
>     string
>     default ""
>     append "FOO" if BAR
>     append "FOZ" if BAZ
> 
> or maybe a new type "list" which would behave as a comma/space separated value.
> 
> config EXTRA_FIRMWARE
>     list "bla"
>     append "FOO" if BAR
>     append "FOZ" if BAZ

Yeah, actually Kyle's note (http://lkml.org/lkml/2011/8/29/289) makes
much more sense for linux and the whole firmware in the kernel deal.
So it looks like the drivers should be taught to wait for userspace to
appear instead of Kconfig helping them build-in firmware. And then we
should go ahead and remove CONFIG_EXTRA_FIRMWARE completely...

Thanks.

-- 
Regards/Gruss,
Boris.

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Michel Dänzer]

On Mon, 2011-08-29 at 19:17 +0200, Borislav Petkov wrote: 
> On Mon, Aug 29, 2011 at 12:10:45PM -0400, Arnaud Lacombe wrote:
> > do you want something ala:
> > 
> > config EXTRA_FIRMWARE
> >     string
> >     default ""
> >     append "FOO" if BAR
> >     append "FOZ" if BAZ
> > 
> > or maybe a new type "list" which would behave as a comma/space separated value.
> > 
> > config EXTRA_FIRMWARE
> >     list "bla"
> >     append "FOO" if BAR
> >     append "FOZ" if BAZ
> 
> Yeah, actually Kyle's note (http://lkml.org/lkml/2011/8/29/289) makes
> much more sense for linux and the whole firmware in the kernel deal.
> So it looks like the drivers should be taught to wait for userspace to
> appear [...]

What would be the point of building radeon into the kernel then? It's
not gonna be active before the module could be loaded...


-- 
Earthling Michel Dänzer           |                   http://www.amd.com
Libre software enthusiast         |          Debian, X and DRI developer

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Peter Zijlstra]

On Mon, 2011-08-29 at 19:17 +0200, Borislav Petkov wrote:
> On Mon, Aug 29, 2011 at 12:10:45PM -0400, Arnaud Lacombe wrote:
> > do you want something ala:
> > 
> > config EXTRA_FIRMWARE
> >     string
> >     default ""
> >     append "FOO" if BAR
> >     append "FOZ" if BAZ
> > 
> > or maybe a new type "list" which would behave as a comma/space separated value.
> > 
> > config EXTRA_FIRMWARE
> >     list "bla"
> >     append "FOO" if BAR
> >     append "FOZ" if BAZ
> 
> Yeah, actually Kyle's note (http://lkml.org/lkml/2011/8/29/289) makes
> much more sense for linux and the whole firmware in the kernel deal.
> So it looks like the drivers should be taught to wait for userspace to
> appear instead of Kconfig helping them build-in firmware. And then we
> should go ahead and remove CONFIG_EXTRA_FIRMWARE completely...

That would suck, suppose this radeon thing is the only console you've
got (ppc64/sparc64 don't have text mode iirc) and userspace doesn't come
up?

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Peter Zijlstra]

On Mon, 2011-08-29 at 19:50 +0200, Peter Zijlstra wrote:
> On Mon, 2011-08-29 at 19:17 +0200, Borislav Petkov wrote:
> > On Mon, Aug 29, 2011 at 12:10:45PM -0400, Arnaud Lacombe wrote:
> > > do you want something ala:
> > > 
> > > config EXTRA_FIRMWARE
> > >     string
> > >     default ""
> > >     append "FOO" if BAR
> > >     append "FOZ" if BAZ
> > > 
> > > or maybe a new type "list" which would behave as a comma/space separated value.
> > > 
> > > config EXTRA_FIRMWARE
> > >     list "bla"
> > >     append "FOO" if BAR
> > >     append "FOZ" if BAZ
> > 
> > Yeah, actually Kyle's note (http://lkml.org/lkml/2011/8/29/289) makes
> > much more sense for linux and the whole firmware in the kernel deal.
> > So it looks like the drivers should be taught to wait for userspace to
> > appear instead of Kconfig helping them build-in firmware. And then we
> > should go ahead and remove CONFIG_EXTRA_FIRMWARE completely...
> 
> That would suck, suppose this radeon thing is the only console you've
> got (ppc64/sparc64 don't have text mode iirc) and userspace doesn't come
> up?

Same is true for NICs and netconsole of course. Not being able to stick
blobs into the kernel image would so suck.

Silently building a kernel without one and then getting stuck on boot,
only to figure out after much pain that you need EXTRA_FIRMWARE and the
more pain to figure out what blob to stick in is beyond crazy.

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Peter Zijlstra]

On Mon, 2011-08-29 at 20:09 +0200, Peter Zijlstra wrote:
> > That would suck, suppose this radeon thing is the only console you've
> > got (ppc64/sparc64 don't have text mode iirc) and userspace doesn't come
> > up?
> 
> Same is true for NICs and netconsole of course. Not being able to stick
> blobs into the kernel image would so suck. 

NICs and nfsroot, I suppose can you use an initrd over tftp as well, but
really, what's the point of an in-kernel nfsroot if you need initrd
crap.

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Borislav Petkov]

On Mon, Aug 29, 2011 at 08:16:53PM +0200, Peter Zijlstra wrote:
> On Mon, 2011-08-29 at 20:09 +0200, Peter Zijlstra wrote:
> > > That would suck, suppose this radeon thing is the only console you've
> > > got (ppc64/sparc64 don't have text mode iirc) and userspace doesn't come
> > > up?
> > 
> > Same is true for NICs and netconsole of course. Not being able to stick
> > blobs into the kernel image would so suck. 
> 
> NICs and nfsroot, I suppose can you use an initrd over tftp as well, but
> really, what's the point of an in-kernel nfsroot if you need initrd
> crap.

Damn,

just when I thought that the lofty goal of Linux distancing itself from
firmware crap is within reach, you came up with all those real-life,
nagging examples.

So here's the whole deal IMHO:

* driver =m and request_firmware() works fine - you only need the blobs
in the right place.

* driver =y and request_firmware() should be fixed in all cases where
driver can wait.

Your use cases could probably be addressed by supplying firmware blobs
from the bootloader. We had this talk already concerning CPU microcode
updates and how the current method is to load CPU ucode when the module
gets loaded, which might be too late for some obscure (and not so
obscure) cases.

So, hypothetically speaking, hpa suggested then that we could pass
firmware blobs over the linked list setup_data thing in the real-mode
kernel header and parse_setup_data() can look at them and map them
somewhere later for the driver to find. This should be doable because
you're only gonna need a handful of blobs for CPU ucode, network and GPU
if the last is compiled in.

I wanted to take a serious look at that for the ucode loading, maybe I
should try to shuffle some time for it...

-- 
Regards/Gruss,
    Boris.

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Henrique de Moraes Holschuh]

On Mon, 29 Aug 2011, Borislav Petkov wrote:
> So, hypothetically speaking, hpa suggested then that we could pass
> firmware blobs over the linked list setup_data thing in the real-mode
> kernel header and parse_setup_data() can look at them and map them
> somewhere later for the driver to find. This should be doable because
> you're only gonna need a handful of blobs for CPU ucode, network and GPU
> if the last is compiled in.
> 
> I wanted to take a serious look at that for the ucode loading, maybe I
> should try to shuffle some time for it...

It would be very useful, yes.

Alternatively, you could extend the initrd format to have a firmware
directory appended after the filesystem image.  ACPI is going to abuse
the initrd in just that way to override ACPI tables very soon (patches
have been already submitted to linux-acpi), so if a more structured and
extensible way to piggy-back early-init data in the initrd is needed, it
would be good to bring that to the table NOW.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Borislav Petkov]

On Mon, Aug 29, 2011 at 11:08:28PM -0300, Henrique de Moraes Holschuh wrote:
> On Mon, 29 Aug 2011, Borislav Petkov wrote:
> > So, hypothetically speaking, hpa suggested then that we could pass
> > firmware blobs over the linked list setup_data thing in the real-mode
> > kernel header and parse_setup_data() can look at them and map them
> > somewhere later for the driver to find. This should be doable because
> > you're only gonna need a handful of blobs for CPU ucode, network and GPU
> > if the last is compiled in.
> > 
> > I wanted to take a serious look at that for the ucode loading, maybe I
> > should try to shuffle some time for it...
> 
> It would be very useful, yes.
> 
> Alternatively, you could extend the initrd format to have a firmware
> directory appended after the filesystem image.  ACPI is going to abuse
> the initrd in just that way to override ACPI tables very soon (patches
> have been already submitted to linux-acpi),

.. lemme guess: they didn't put the correct tables in the BIOS in the
first place and now it is too late for a BIOS fix and want to shuffle in
all those "fixes" through initrd ontop of ACPI. Oh boy, do I love all
the ACPI crap stories :).

> so if a more structured and extensible way to piggy-back early-init
> data in the initrd is needed, it would be good to bring that to the
> table NOW.

Yeah, that's not more than an idea right now, I haven't even started
playing with it. If it turns out to be more superior, it could be reused
for other things too but right now it's too early to tell.

-- 
Regards/Gruss,
    Boris.

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Henrique de Moraes Holschuh]

On Tue, 30 Aug 2011, Borislav Petkov wrote:
> On Mon, Aug 29, 2011 at 11:08:28PM -0300, Henrique de Moraes Holschuh wrote:
> > On Mon, 29 Aug 2011, Borislav Petkov wrote:
> > > So, hypothetically speaking, hpa suggested then that we could pass
> > > firmware blobs over the linked list setup_data thing in the real-mode
> > > kernel header and parse_setup_data() can look at them and map them
> > > somewhere later for the driver to find. This should be doable because
> > > you're only gonna need a handful of blobs for CPU ucode, network and GPU
> > > if the last is compiled in.
> > > 
> > > I wanted to take a serious look at that for the ucode loading, maybe I
> > > should try to shuffle some time for it...
> > 
> > It would be very useful, yes.
> > 
> > Alternatively, you could extend the initrd format to have a firmware
> > directory appended after the filesystem image.  ACPI is going to abuse
> > the initrd in just that way to override ACPI tables very soon (patches
> > have been already submitted to linux-acpi),
> 
> .. lemme guess: they didn't put the correct tables in the BIOS in the
> first place and now it is too late for a BIOS fix and want to shuffle in
> all those "fixes" through initrd ontop of ACPI. Oh boy, do I love all
> the ACPI crap stories :).

Heh.

Actually, the patches were created by a distro for debug purposes.  No
vendor pushed for them.  But it is a safe bet that users will end up using
them to work around ACPI vendor crap that will never get fixed properly
through a BIOS update, etc.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Peter Zijlstra]

On Mon, 2011-08-29 at 23:08 -0300, Henrique de Moraes Holschuh wrote:
> On Mon, 29 Aug 2011, Borislav Petkov wrote:
> > So, hypothetically speaking, hpa suggested then that we could pass
> > firmware blobs over the linked list setup_data thing in the real-mode
> > kernel header and parse_setup_data() can look at them and map them
> > somewhere later for the driver to find. This should be doable because
> > you're only gonna need a handful of blobs for CPU ucode, network and GPU
> > if the last is compiled in.
> > 
> > I wanted to take a serious look at that for the ucode loading, maybe I
> > should try to shuffle some time for it...
> 
> It would be very useful, yes.
> 
> Alternatively, you could extend the initrd format to have a firmware
> directory appended after the filesystem image.  ACPI is going to abuse
> the initrd in just that way to override ACPI tables very soon (patches
> have been already submitted to linux-acpi), so if a more structured and
> extensible way to piggy-back early-init data in the initrd is needed, it
> would be good to bring that to the table NOW.

Uhm,.. does that mean that soon we can't boot kernels without initrd?
That too is a massive regression in my eyes.

Re: Kernel almost hangs when CONFIG_DRM_RADEON=y

[Henrique de Moraes Holschuh]

On Tue, 30 Aug 2011, Peter Zijlstra wrote:
> On Mon, 2011-08-29 at 23:08 -0300, Henrique de Moraes Holschuh wrote:
> > On Mon, 29 Aug 2011, Borislav Petkov wrote:
> > > So, hypothetically speaking, hpa suggested then that we could pass
> > > firmware blobs over the linked list setup_data thing in the real-mode
> > > kernel header and parse_setup_data() can look at them and map them
> > > somewhere later for the driver to find. This should be doable because
> > > you're only gonna need a handful of blobs for CPU ucode, network and GPU
> > > if the last is compiled in.
> > > 
> > > I wanted to take a serious look at that for the ucode loading, maybe I
> > > should try to shuffle some time for it...
> > 
> > It would be very useful, yes.
> > 
> > Alternatively, you could extend the initrd format to have a firmware
> > directory appended after the filesystem image.  ACPI is going to abuse
> > the initrd in just that way to override ACPI tables very soon (patches
> > have been already submitted to linux-acpi), so if a more structured and
> > extensible way to piggy-back early-init data in the initrd is needed, it
> > would be good to bring that to the table NOW.
> 
> Uhm,.. does that mean that soon we can't boot kernels without initrd?
> That too is a massive regression in my eyes.

Well, if work starts soon enough on a bootloader extension to avoid messing
with the initrd, we could refuse to set that initrd-based ACPI table
override as ABI on the grounds that it is a debug thing, and later move it
to the properly designed firmware bootloader extension.

Maybe the grub multiboot protocol[1] is worth looking at, assuming that
thing is salvagable and would actually work for both 32bit and 64bit BIOS
and UEFI bootstrapping?

[1]http://www.gnu.org/software/grub/manual/multiboot/multiboot.html

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh