On Saturday 29 April 2006 03:04, Dave Airlie wrote:
> > This patch adds an "enable" sysfs attribute to each PCI device. When read it
> > shows the "enabled-ness" of the device, but you can write a "0" into it to
> > disable a device, and a "1" to enable it.
> >
> > This later is needed for X and other cases where userspace wants to enable
> > the BARs on a device (typical example: to run the video bios on a secundary
> > head). Right now X does all this "by hand" via bitbanging, that's just evil.
> > This allows X to no longer do that but to just let the kernel do this.
I'm all in favor of cleaning up X. But making the X code prettier without
changing the underlying issues of claiming and sharing resources doesn't
help much. In fact, I suspect the ultimate plan for X does not involve
an "enable" attribute in sysfs, so this may just introduce ABI cruft that
will be difficult to remove later.
> This would allow me to remove the issue in X where loading the DRM at X
> startup acts differently than loading the DRM before X runs, due to Xs PCI
> probe running in-between... with this I can just enable all VGA devices
> and no worry whether they have a DRM or not..
This seems to be the main justification for the patch. But I don't know
enough about X and DRM to understand it, or why this patch is the best
way to solve it.
I think Jon has a pretty convincing argument, which I *do* understand.
Can you expand on this justification? Do you envision long-term usage
of the sysfs "enable" attribute?
Bjorn
Bjorn Helgaas wrote:
> On Saturday 29 April 2006 03:04, Dave Airlie wrote:
>>> This patch adds an "enable" sysfs attribute to each PCI device. When read it
>>> shows the "enabled-ness" of the device, but you can write a "0" into it to
>>> disable a device, and a "1" to enable it.
>>>
>>> This later is needed for X and other cases where userspace wants to enable
>>> the BARs on a device (typical example: to run the video bios on a secundary
>>> head). Right now X does all this "by hand" via bitbanging, that's just evil.
>>> This allows X to no longer do that but to just let the kernel do this.
>
> I'm all in favor of cleaning up X. But making the X code prettier without
> changing the underlying issues of claiming and sharing resources doesn't
> help much. In fact, I suspect the ultimate plan for X does not involve
> an "enable" attribute in sysfs, so this may just introduce ABI cruft that
> will be difficult to remove later.
it goes well beyond X. Things like vbetool need this too to get to the content
of the rom for example. There are several other such cases...
On Thursday 04 May 2006 13:11, Arjan van de Ven wrote:
> Bjorn Helgaas wrote:
> > On Saturday 29 April 2006 03:04, Dave Airlie wrote:
> >>> This patch adds an "enable" sysfs attribute to each PCI device. When read it
> >>> shows the "enabled-ness" of the device, but you can write a "0" into it to
> >>> disable a device, and a "1" to enable it.
> >>>
> >>> This later is needed for X and other cases where userspace wants to enable
> >>> the BARs on a device (typical example: to run the video bios on a secundary
> >>> head). Right now X does all this "by hand" via bitbanging, that's just evil.
> >>> This allows X to no longer do that but to just let the kernel do this.
> >
> > I'm all in favor of cleaning up X. But making the X code prettier without
> > changing the underlying issues of claiming and sharing resources doesn't
> > help much. In fact, I suspect the ultimate plan for X does not involve
> > an "enable" attribute in sysfs, so this may just introduce ABI cruft that
> > will be difficult to remove later.
>
> it goes well beyond X. Things like vbetool need this too to get to the content
> of the rom for example. There are several other such cases...
There's already a "rom" file in sysfs. Could vbetool and friends
use that?
How do vbetool and X coordinate their usage of "enable"? What if we
throw an in-kernel VGA driver into the mix? But I guess Jon has asked
all these questions before; I just didn't get warm fuzzies that there
were safe, maintainable answers.
Bjorn Helgaas <[email protected]> wrote:
> There's already a "rom" file in sysfs. Could vbetool and friends
> use that?
Not if you have multiple graphics cards.
> How do vbetool and X coordinate their usage of "enable"?
vbetool won't run at anything other than a text console, and X won't
mess with the graphics card if it's not on the current VT. You can mess
this up if you try hard enough (multihead, for instance) but by and
large it's a situation that you can avoid.
> What if we throw an in-kernel VGA driver into the mix? But I guess
> Jon has asked all these questions before; I just didn't get warm
> fuzzies that there were safe, maintainable answers.
This probably isn't the right long-term answer, but the right long-term
answer is going to be a very long time away. It's an improvement over
what we have now. I certainly don't intend to leave vbetool relying on
it - of course, the "right" answer is for graphics drivers to know how
to program cards from scratch so we can get rid of vbetool altogether,
but I'll probably be more concerned about getting my flying car to meet
new emission standards than I will be by graphics cards at that stage.
--
Matthew Garrett | [email protected]
Bjorn Helgaas wrote:
> On Thursday 04 May 2006 13:11, Arjan van de Ven wrote:
>> Bjorn Helgaas wrote:
>>> On Saturday 29 April 2006 03:04, Dave Airlie wrote:
>>>>> This patch adds an "enable" sysfs attribute to each PCI device. When read it
>>>>> shows the "enabled-ness" of the device, but you can write a "0" into it to
>>>>> disable a device, and a "1" to enable it.
>>>>>
>>>>> This later is needed for X and other cases where userspace wants to enable
>>>>> the BARs on a device (typical example: to run the video bios on a secundary
>>>>> head). Right now X does all this "by hand" via bitbanging, that's just evil.
>>>>> This allows X to no longer do that but to just let the kernel do this.
>>> I'm all in favor of cleaning up X. But making the X code prettier without
>>> changing the underlying issues of claiming and sharing resources doesn't
>>> help much. In fact, I suspect the ultimate plan for X does not involve
>>> an "enable" attribute in sysfs, so this may just introduce ABI cruft that
>>> will be difficult to remove later.
>> it goes well beyond X. Things like vbetool need this too to get to the content
>> of the rom for example. There are several other such cases...
>
> There's already a "rom" file in sysfs. Could vbetool and friends
> use that?
not unless the device is enabled.
> How do vbetool and X coordinate their usage of "enable"?
Just never disable :)
On 5/4/06, Matthew Garrett <[email protected]> wrote:
> Bjorn Helgaas <[email protected]> wrote:
>
> > There's already a "rom" file in sysfs. Could vbetool and friends
> > use that?
>
> Not if you have multiple graphics cards.
Not true, the rom attribute maps the ROM into PCI space where ever the
kernel tells it to and reads it from there. It is the PCI VGA
emulation feature that forces the ROM to appear at C000:0. You can
have the ROM mapped and VGA emulation turned off.
This brings up another major point. X changes the PCI VGA emulation
routing from user space, another thing that it should not be doing. I
have posted patches before providing a sysfs VGA attribute on class
VGA devices. By setting the attribute to 1 you can control the active
VGA emulation device.
This is yet another way that user space can mess up the kernel. If VGA
routing is changes under fbdev (my attribute notifies fbdev, the fbdev
code for processing the notification did get checked in) then the
console will screw up. The usual screw up is that the console goes
blank because hardware fonts are not setup correctly on the new
console.
I also remember posting a patch for initializing all class VGA devices
at boot. This is a complication process since running the ROM will
leave that card as the active VGA device. The initialization code
made sure to set the console back to the original VGA card after the
secondary one was initialized. The initialization process needs to
run a user space app which provides real mode access or an x86
emulator. BenH has the x86 emulator code (people sticking PC hardware
into PowerPCs need the emulator). This code for running the ROM should
go into the kernel tree and work with klibc.
I would really like to see a well designed, comprehensive plan for
handling these issues instead of just providing convenience APIs (that
may be hard to get rid of) for the old X code. I'm willing to help if
people really are serious about fixing the problems.
I wrote down a lot of my thoughts on this area in the State of Linux
Graphics article last summer. There is a large section about kernel
issues.
http://people.freedesktop.org/~jonsmirl/graphics.html
>
> > How do vbetool and X coordinate their usage of "enable"?
>
> vbetool won't run at anything other than a text console, and X won't
> mess with the graphics card if it's not on the current VT. You can mess
> this up if you try hard enough (multihead, for instance) but by and
> large it's a situation that you can avoid.
>
> > What if we throw an in-kernel VGA driver into the mix? But I guess
> > Jon has asked all these questions before; I just didn't get warm
> > fuzzies that there were safe, maintainable answers.
>
> This probably isn't the right long-term answer, but the right long-term
> answer is going to be a very long time away. It's an improvement over
> what we have now. I certainly don't intend to leave vbetool relying on
> it - of course, the "right" answer is for graphics drivers to know how
> to program cards from scratch so we can get rid of vbetool altogether,
> but I'll probably be more concerned about getting my flying car to meet
> new emission standards than I will be by graphics cards at that stage.
>
> --
> Matthew Garrett | [email protected]
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
--
Jon Smirl
[email protected]
On Thu, 2006-05-04 at 16:40 -0400, Jon Smirl wrote:
> On 5/4/06, Matthew Garrett <[email protected]> wrote:
> > Bjorn Helgaas <[email protected]> wrote:
> >
> > > There's already a "rom" file in sysfs. Could vbetool and friends
> > > use that?
> >
> > Not if you have multiple graphics cards.
>
> Not true, the rom attribute maps the ROM into PCI space where ever the
> kernel tells it to and reads it from there. It is the PCI VGA
> emulation feature that forces the ROM to appear at C000:0. You can
> have the ROM mapped and VGA emulation turned off.
It doesn't matter -- you can accomplish the same thing with e.g.
libx86emu and simply mapping the option rom to 0xc0000. But you want to
do that in userland, not in the kernel.
> This brings up another major point. X changes the PCI VGA emulation
> routing from user space, another thing that it should not be doing. I
> have posted patches before providing a sysfs VGA attribute on class
> VGA devices. By setting the attribute to 1 you can control the active
> VGA emulation device.
>
> This is yet another way that user space can mess up the kernel. If VGA
> routing is changes under fbdev (my attribute notifies fbdev, the fbdev
> code for processing the notification did get checked in) then the
> console will screw up.
And this change allows userland to avoid doing that.
> The usual screw up is that the console goes
> blank because hardware fonts are not setup correctly on the new
> console.
And that's completely unrelated to this problem.
--
Peter
Hello!
> > This is yet another way that user space can mess up the kernel. If VGA
> > routing is changes under fbdev (my attribute notifies fbdev, the fbdev
> > code for processing the notification did get checked in) then the
> > console will screw up.
>
> And this change allows userland to avoid doing that.
Could you explain how? I don't see how adding a "enable" attribute
to sysfs can solve these problems.
I agree with Arjan that the attribute could be useful for some special
tools, but using it in X is likely to be utterly wrong.
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://atrey.karlin.mff.cuni.cz/~mj/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
A mathematician is a machine for converting coffee into theorems.
On 5/4/06, Peter Jones <[email protected]> wrote:
> On Thu, 2006-05-04 at 16:40 -0400, Jon Smirl wrote:
> > On 5/4/06, Matthew Garrett <[email protected]> wrote:
> > > Bjorn Helgaas <[email protected]> wrote:
> > >
> > > > There's already a "rom" file in sysfs. Could vbetool and friends
> > > > use that?
> > >
> > > Not if you have multiple graphics cards.
> >
> > Not true, the rom attribute maps the ROM into PCI space where ever the
> > kernel tells it to and reads it from there. It is the PCI VGA
> > emulation feature that forces the ROM to appear at C000:0. You can
> > have the ROM mapped and VGA emulation turned off.
>
> It doesn't matter -- you can accomplish the same thing with e.g.
> libx86emu and simply mapping the option rom to 0xc0000. But you want to
> do that in userland, not in the kernel.
It is much more complicated than than you describe. Go look at the ROM
code already checked in. Laptop video ROMs are not simple PCI devices
that can be mapped around. They are stored in compressed form inside
the system ROM and expanded at boot. If you lose the shadow copy in
RAM there is no API for getting it back. These compressed ROMs are the
source of a lot of laptop user's problems with suspend/resume on
Linux.
VGA support for multiple cards is a very complicated problem. It
really needs to be comprehenisvely designed (but not necessarily
implemented all at once). What we have now is twenty years worth of
hacks that work 95% of the time. But there are many, many holes in the
current scheme.
>
> > This brings up another major point. X changes the PCI VGA emulation
> > routing from user space, another thing that it should not be doing. I
> > have posted patches before providing a sysfs VGA attribute on class
> > VGA devices. By setting the attribute to 1 you can control the active
> > VGA emulation device.
> >
> > This is yet another way that user space can mess up the kernel. If VGA
> > routing is changes under fbdev (my attribute notifies fbdev, the fbdev
> > code for processing the notification did get checked in) then the
> > console will screw up.
>
> And this change allows userland to avoid doing that.
>
> > The usual screw up is that the console goes
> > blank because hardware fonts are not setup correctly on the new
> > console.
>
> And that's completely unrelated to this problem.
>
> --
> Peter
>
>
--
Jon Smirl
[email protected]
On Thu, 2006-05-04 at 23:17 +0200, Martin Mares wrote:
> Hello!
>
> > > This is yet another way that user space can mess up the kernel. If VGA
> > > routing is changes under fbdev (my attribute notifies fbdev, the fbdev
> > > code for processing the notification did get checked in) then the
> > > console will screw up.
> >
> > And this change allows userland to avoid doing that.
>
> Could you explain how? I don't see how adding a "enable" attribute
> to sysfs can solve these problems.
By allowing tools to read the rom from the pci device itself, instead of
whichever device's rom happens to be at 0xC0000. libx86emul allows you
to define routines that it uses for memory access, so you can copy the
ROM out to normal memory, and map that memory to the appropriate address
that way. X and vbetool both already have to do this, but without
copying the firmware image, to do DDC probing on x86_64.
> I agree with Arjan that the attribute could be useful for some special
> tools, but using it in X is likely to be utterly wrong.
I'm actually talking about vbetool here, though X could use these
methods. Indeed, libx86emul comes from X itself.
--
Peter
On 5/4/06, Peter Jones <[email protected]> wrote:
> On Thu, 2006-05-04 at 23:17 +0200, Martin Mares wrote:
> > Hello!
> >
> > > > This is yet another way that user space can mess up the kernel. If VGA
> > > > routing is changes under fbdev (my attribute notifies fbdev, the fbdev
> > > > code for processing the notification did get checked in) then the
> > > > console will screw up.
> > >
> > > And this change allows userland to avoid doing that.
> >
> > Could you explain how? I don't see how adding a "enable" attribute
> > to sysfs can solve these problems.
>
> By allowing tools to read the rom from the pci device itself, instead of
> whichever device's rom happens to be at 0xC0000. libx86emul allows you
> to define routines that it uses for memory access, so you can copy the
> ROM out to normal memory, and map that memory to the appropriate address
> that way. X and vbetool both already have to do this, but without
> copying the firmware image, to do DDC probing on x86_64.
# cd /sys/bus/pci/devices/0000:01:00.0
# echo 1 >rom
# hexdump -C rom
As far as I know this works on every platform, not just the PC one.
Don't mess around with the hardware trying to get to the ROM. Use the
API provided by the kernel. Messing with the hardware will get it into
a state that the kernel doesn't know about and can ultimately crash
your system.
>
> > I agree with Arjan that the attribute could be useful for some special
> > tools, but using it in X is likely to be utterly wrong.
>
> I'm actually talking about vbetool here, though X could use these
> methods. Indeed, libx86emul comes from X itself.
>
> --
> Peter
>
>
--
Jon Smirl
[email protected]
Hello!
> By allowing tools to read the rom from the pci device itself, instead of
> whichever device's rom happens to be at 0xC0000.
The ROMs are already accessible through sysfs, aren't they?
Have a nice fortnight
--
Martin `MJ' Mares <[email protected]> http://atrey.karlin.mff.cuni.cz/~mj/
Faculty of Math and Physics, Charles University, Prague, Czech Rep., Earth
Anyone who says sunshine brings happiness has never danced in the rain.
On Thu, 2006-05-04 at 17:18 -0400, Jon Smirl wrote:
> On 5/4/06, Peter Jones <[email protected]> wrote:
> > It doesn't matter -- you can accomplish the same thing with e.g.
> > libx86emu and simply mapping the option rom to 0xc0000. But you want to
> > do that in userland, not in the kernel.
>
> It is much more complicated than than you describe.
I didn't really feel like explaining the parts we both already know.
I'll try to remember to do so in the future.
> Go look at the ROM code already checked in. Laptop video ROMs are not
> simple PCI devices that can be mapped around. They are stored in
> compressed form inside the system ROM and expanded at boot.
Yes, and this format is documented, too. But right now there's no way
to get access to it with tools to actually do anything.
> If you lose the shadow copy in RAM there is no API for getting it back.
Except to enable the BAR and read it from the assigned address...
> These compressed ROMs are the source of a lot of laptop user's
> problems with suspend/resume on Linux.
Absolutely. That's why I want a method to access them, which this
"enable" file provides.
> VGA support for multiple cards is a very complicated problem.
Please quit jumping up and down in the bicycle path telling everybody
how hard it is to ride a bike.
--
Peter
On 5/4/06, Peter Jones <[email protected]> wrote:
> On Thu, 2006-05-04 at 17:18 -0400, Jon Smirl wrote:
> > On 5/4/06, Peter Jones <[email protected]> wrote:
>
> > > It doesn't matter -- you can accomplish the same thing with e.g.
> > > libx86emu and simply mapping the option rom to 0xc0000. But you want to
> > > do that in userland, not in the kernel.
> >
> > It is much more complicated than than you describe.
>
> I didn't really feel like explaining the parts we both already know.
> I'll try to remember to do so in the future.
>
> > Go look at the ROM code already checked in. Laptop video ROMs are not
> > simple PCI devices that can be mapped around. They are stored in
> > compressed form inside the system ROM and expanded at boot.
>
> Yes, and this format is documented, too. But right now there's no way
> to get access to it with tools to actually do anything.
>
> > If you lose the shadow copy in RAM there is no API for getting it back.
>
> Except to enable the BAR and read it from the assigned address...
Let me be clear here. A lot of laptop video hardware does not have a
video ROM. Therefore you can not enable the BAR and read it from an
assigned address since there is no ROM to be read. Instead the video
ROM images are compressed and stored inside the system BIOS ROM. The
location of the image is not a public thing. At boot time the
compressed video ROM is expanded out of the system ROM into shadow RAM
at C000:0. There is no real ROM there, it is only a copy in RAM. If
you lose that copy the only way to get it back is to reset the
machine. The ROM code already in the kernel knows about these shadow
copies and won't mess them up.
> > These compressed ROMs are the source of a lot of laptop user's
> > problems with suspend/resume on Linux.
>
> Absolutely. That's why I want a method to access them, which this
> "enable" file provides.
>
> > VGA support for multiple cards is a very complicated problem.
>
> Please quit jumping up and down in the bicycle path telling everybody
> how hard it is to ride a bike.
>
> --
> Peter
>
>
--
Jon Smirl
[email protected]
On Thu, 2006-05-04 at 17:48 -0400, Jon Smirl wrote:
> Let me be clear here. A lot of laptop video hardware does not have a
> video ROM.
Why do you assume we're only talking about laptops?
--
Peter
On 5/4/06, Peter Jones <[email protected]> wrote:
> On Thu, 2006-05-04 at 17:48 -0400, Jon Smirl wrote:
>
> > Let me be clear here. A lot of laptop video hardware does not have a
> > video ROM.
>
> Why do you assume we're only talking about laptops?
Because when you make statements like...
> Except to enable the BAR and read it from the assigned address...
That statement doesn't hold true for laptops. It indicates that you
may not be aware of all of the issues around VGA bus routing and video
ROMs. For example there are a lot of special issues with VGA support
in large large machines like the SGI Altix that have multiple
independent PCI domains. Owners of all of these other architectures
(PPC especially) having been beating me up for two years making me
aware of all of their weird cases.
>
> --
> Peter
>
>
--
Jon Smirl
[email protected]
On Thu, 2006-05-04 at 17:38 -0400, Jon Smirl wrote:
> # cd /sys/bus/pci/devices/0000:01:00.0
> # echo 1 >rom
> # hexdump -C rom
>
> As far as I know this works on every platform, not just the PC one.
Yep, you're right, this works. So we don't necessarily need it for the
vbetool case. X still could use it though, instead of their scary
poke-at-memory way.
> Don't mess around with the hardware trying to get to the ROM. Use the
> API provided by the kernel. Messing with the hardware will get it into
> a state that the kernel doesn't know about and can ultimately crash
> your system.
Exactly who do you see here messing with the hardware directly instead
of using kernel APIs?
--
Peter
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Peter Jones wrote:
> On Thu, 2006-05-04 at 17:38 -0400, Jon Smirl wrote:
>
>># cd /sys/bus/pci/devices/0000:01:00.0
>># echo 1 >rom
>># hexdump -C rom
>>
>>As far as I know this works on every platform, not just the PC one.
>
> Yep, you're right, this works. So we don't necessarily need it for the
> vbetool case. X still could use it though, instead of their scary
> poke-at-memory way.
Dave Airlie recently changed X to use sysfs for reading ROMs. I'm also
working on some changes to eliminate nearly all of the PCI bus poking
that X does. Search for "PCI rework" or "libpciaccess" in the xorg list
archives.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2.2 (GNU/Linux)
iD8DBQFEW6WDX1gOwKyEAw8RAt/aAJ0VDgqBNLbvyKExD0oS3nM5UsAFugCfaJpf
MLSBYFA6e96gyiAr4/j4T14=
=pJQD
-----END PGP SIGNATURE-----
On 5/5/06, Ian Romanick <[email protected]> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> Peter Jones wrote:
> > On Thu, 2006-05-04 at 17:38 -0400, Jon Smirl wrote:
> >
> >># cd /sys/bus/pci/devices/0000:01:00.0
> >># echo 1 >rom
> >># hexdump -C rom
> >>
> >>As far as I know this works on every platform, not just the PC one.
> >
> > Yep, you're right, this works. So we don't necessarily need it for the
> > vbetool case. X still could use it though, instead of their scary
> > poke-at-memory way.
>
> Dave Airlie recently changed X to use sysfs for reading ROMs. I'm also
> working on some changes to eliminate nearly all of the PCI bus poking
> that X does. Search for "PCI rework" or "libpciaccess" in the xorg list
> archives.
What's the story with PCI VGA routing?
DaveA I were chatting about an alternative to 'enable' attribute. You
would make a VGA driver that isn't bound to any PCI IDs. After the
kernel boots a user space program uses sysfs/vga/new_id to load it
with PCI IDs for all PCI class = VGA hardware. Anything that doesn't
have a driver loaded will then get bound to this VGA driver. In
conjunction with the driver udev would make a device node appear.
Opening the new device node would enable the device and control
ownership. If you want to load a device specific driver later, set
sysfs/vga/unbind=1, load the new device specific driver, set
sysfs/vga/bind=1.
This scheme all works within existing kernel mechanisms and does not
require adding an 'enable' attribute. It also addresses the problem of
who owns the state in the hardware; the state is owned by whoever has
the device node open. If another app wants to mess with the hardware
it won't be able to open the device node.
I would like to see other design alternatives considered on this
issue. The 'enable' attribute has a clear problem in that you can't
tell which user space program is trying to control the device.
Multiple programs accessing the video hardware with poor coordination
is already the source of many problems.
--
Jon Smirl
[email protected]
On Fri, May 05, 2006 at 04:14:00PM -0400, Jon Smirl wrote:
> I would like to see other design alternatives considered on this
> issue. The 'enable' attribute has a clear problem in that you can't
> tell which user space program is trying to control the device.
> Multiple programs accessing the video hardware with poor coordination
> is already the source of many problems.
Who cares who "enabled" the device. Remember, the majority of PCI
devices in the system are not video ones. Lots of other types of
devices want this ability to enable PCI devices from userspace. I've
been talking with some people about how to properly write PCI drivers in
userspace, and this attribute is a needed part of it.
And if X gets enabling the device wrong, again, who cares, it's not a
kernel issue. :)
thanks,
greg k-h
On 5/5/06, Greg KH <[email protected]> wrote:
> On Fri, May 05, 2006 at 04:14:00PM -0400, Jon Smirl wrote:
> > I would like to see other design alternatives considered on this
> > issue. The 'enable' attribute has a clear problem in that you can't
> > tell which user space program is trying to control the device.
> > Multiple programs accessing the video hardware with poor coordination
> > is already the source of many problems.
>
> Who cares who "enabled" the device. Remember, the majority of PCI
> devices in the system are not video ones. Lots of other types of
> devices want this ability to enable PCI devices from userspace. I've
> been talking with some people about how to properly write PCI drivers in
> userspace, and this attribute is a needed part of it.
User space program enables the device.
Next I load a device driver
next I rmmod the device driver and it disables the device
user space program trys to use the device
No coordination and user space program faults
Don't say this can't happen, it is a current source of conflict
between X and fbdev.
Should we just remove the ability to disable hardware?
How would that interact with hotplug?
> And if X gets enabling the device wrong, again, who cares, it's not a
> kernel issue. :)
>
> thanks,
>
> greg k-h
>
--
Jon Smirl
[email protected]
> On 5/5/06, Greg KH <[email protected]> wrote:
> > Who cares who "enabled" the device. Remember, the majority of PCI
> > devices in the system are not video ones. Lots of other types of
> > devices want this ability to enable PCI devices from userspace. I've
> > been talking with some people about how to properly write PCI drivers in
> > userspace, and this attribute is a needed part of it.
The problem is not who 'enabled' the device. The problem is who owns
the state that has been loaded into the device. Without a mechanism
like open there is no way to serialize the programs trying to set
state into the device.
fbdev and X have this fight currently. On every VT swap they each
reload their state into the video hardware. There is no coordination
so both systems have to assume worst case and rebuild everything. This
is not a good environment to program in. Every time one of the systems
starts using some new feature of hardware (like acceleration
functions) new state recovery code needs to be written.
--
Jon Smirl
[email protected]
On Fri, May 05, 2006 at 04:35:17PM -0400, Jon Smirl wrote:
> On 5/5/06, Greg KH <[email protected]> wrote:
> >On Fri, May 05, 2006 at 04:14:00PM -0400, Jon Smirl wrote:
> >> I would like to see other design alternatives considered on this
> >> issue. The 'enable' attribute has a clear problem in that you can't
> >> tell which user space program is trying to control the device.
> >> Multiple programs accessing the video hardware with poor coordination
> >> is already the source of many problems.
> >
> >Who cares who "enabled" the device. Remember, the majority of PCI
> >devices in the system are not video ones. Lots of other types of
> >devices want this ability to enable PCI devices from userspace. I've
> >been talking with some people about how to properly write PCI drivers in
> >userspace, and this attribute is a needed part of it.
>
> User space program enables the device.
> Next I load a device driver
> next I rmmod the device driver and it disables the device
> user space program trys to use the device
> No coordination and user space program faults
Gun. Foot. Shoot.
> Don't say this can't happen, it is a current source of conflict
> between X and fbdev.
Not a PCI kernel issue :)
> Should we just remove the ability to disable hardware?
Huh? Why?
> How would that interact with hotplug?
What does hotplug have to do at all with this?
thanks,
greg "not all the world is video cards" k-h
On Fri, May 05, 2006 at 04:43:15PM -0400, Jon Smirl wrote:
> >On 5/5/06, Greg KH <[email protected]> wrote:
> >> Who cares who "enabled" the device. Remember, the majority of PCI
> >> devices in the system are not video ones. Lots of other types of
> >> devices want this ability to enable PCI devices from userspace. I've
> >> been talking with some people about how to properly write PCI drivers in
> >> userspace, and this attribute is a needed part of it.
>
> The problem is not who 'enabled' the device. The problem is who owns
> the state that has been loaded into the device. Without a mechanism
> like open there is no way to serialize the programs trying to set
> state into the device.
>
> fbdev and X have this fight currently. On every VT swap they each
> reload their state into the video hardware. There is no coordination
> so both systems have to assume worst case and rebuild everything. This
> is not a good environment to program in. Every time one of the systems
> starts using some new feature of hardware (like acceleration
> functions) new state recovery code needs to be written.
Yes, I agree that this is a big issue and one that needs to be worked
on. However, this simple "enable" file has nothing to do with that
issue at all...
thanks,
greg k-h
On 5/5/06, Greg KH <[email protected]> wrote:
> On Fri, May 05, 2006 at 04:35:17PM -0400, Jon Smirl wrote:
> > On 5/5/06, Greg KH <[email protected]> wrote:
> > >On Fri, May 05, 2006 at 04:14:00PM -0400, Jon Smirl wrote:
> > >> I would like to see other design alternatives considered on this
> > >> issue. The 'enable' attribute has a clear problem in that you can't
> > >> tell which user space program is trying to control the device.
> > >> Multiple programs accessing the video hardware with poor coordination
> > >> is already the source of many problems.
> > >
> > >Who cares who "enabled" the device. Remember, the majority of PCI
> > >devices in the system are not video ones. Lots of other types of
> > >devices want this ability to enable PCI devices from userspace. I've
> > >been talking with some people about how to properly write PCI drivers in
> > >userspace, and this attribute is a needed part of it.
> >
> > User space program enables the device.
> > Next I load a device driver
> > next I rmmod the device driver and it disables the device
> > user space program trys to use the device
> > No coordination and user space program faults
>
> Gun. Foot. Shoot.
Why do we want to create problem like this when there is a simple
solution to preventing them. All it takes is a couple of rules:
1) To use a device it must have a device driver. It may be as simple
as a couple of lines of code. This driver will cause a device node to
be created.
2) If a user app want to use the device it opens the device node.
This builds a system where everybody knows what is going on. The
driver knows that user space is using the device. Multiple user space
users are blocked from conflicting because of the open. There is no
way to shoot yourself in the foot.
--
Jon Smirl
[email protected]
On Fri, May 05, 2006 at 05:15:02PM -0400, Jon Smirl wrote:
> On 5/5/06, Greg KH <[email protected]> wrote:
> >On Fri, May 05, 2006 at 04:35:17PM -0400, Jon Smirl wrote:
> >> On 5/5/06, Greg KH <[email protected]> wrote:
> >> >On Fri, May 05, 2006 at 04:14:00PM -0400, Jon Smirl wrote:
> >> >> I would like to see other design alternatives considered on this
> >> >> issue. The 'enable' attribute has a clear problem in that you can't
> >> >> tell which user space program is trying to control the device.
> >> >> Multiple programs accessing the video hardware with poor coordination
> >> >> is already the source of many problems.
> >> >
> >> >Who cares who "enabled" the device. Remember, the majority of PCI
> >> >devices in the system are not video ones. Lots of other types of
> >> >devices want this ability to enable PCI devices from userspace. I've
> >> >been talking with some people about how to properly write PCI drivers in
> >> >userspace, and this attribute is a needed part of it.
> >>
> >> User space program enables the device.
> >> Next I load a device driver
> >> next I rmmod the device driver and it disables the device
> >> user space program trys to use the device
> >> No coordination and user space program faults
> >
> >Gun. Foot. Shoot.
>
> Why do we want to create problem like this when there is a simple
> solution to preventing them. All it takes is a couple of rules:
>
> 1) To use a device it must have a device driver. It may be as simple
> as a couple of lines of code. This driver will cause a device node to
> be created.
>
> 2) If a user app want to use the device it opens the device node.
>
> This builds a system where everybody knows what is going on. The
> driver knows that user space is using the device. Multiple user space
> users are blocked from conflicting because of the open. There is no
> way to shoot yourself in the foot.
That sounds like a nice way to mediate userspace usages of PCI devices,
yes. Much like a dot lockfile is done for tty devices today, all in
userspace (which hints that this too can be done in userspace with no
kernel involvement...)
But it still has nothing to do with this enable sysfs file :)
thanks,
greg k-h
On 5/5/06, Greg KH <[email protected]> wrote:
> On Fri, May 05, 2006 at 05:15:02PM -0400, Jon Smirl wrote:
> > On 5/5/06, Greg KH <[email protected]> wrote:
> > >On Fri, May 05, 2006 at 04:35:17PM -0400, Jon Smirl wrote:
> > >> On 5/5/06, Greg KH <[email protected]> wrote:
> > >> >On Fri, May 05, 2006 at 04:14:00PM -0400, Jon Smirl wrote:
> > >> >> I would like to see other design alternatives considered on this
> > >> >> issue. The 'enable' attribute has a clear problem in that you can't
> > >> >> tell which user space program is trying to control the device.
> > >> >> Multiple programs accessing the video hardware with poor coordination
> > >> >> is already the source of many problems.
> > >> >
> > >> >Who cares who "enabled" the device. Remember, the majority of PCI
> > >> >devices in the system are not video ones. Lots of other types of
> > >> >devices want this ability to enable PCI devices from userspace. I've
> > >> >been talking with some people about how to properly write PCI drivers in
> > >> >userspace, and this attribute is a needed part of it.
> > >>
> > >> User space program enables the device.
> > >> Next I load a device driver
> > >> next I rmmod the device driver and it disables the device
> > >> user space program trys to use the device
> > >> No coordination and user space program faults
> > >
> > >Gun. Foot. Shoot.
> >
> > Why do we want to create problem like this when there is a simple
> > solution to preventing them. All it takes is a couple of rules:
> >
> > 1) To use a device it must have a device driver. It may be as simple
> > as a couple of lines of code. This driver will cause a device node to
> > be created.
> >
> > 2) If a user app want to use the device it opens the device node.
> >
> > This builds a system where everybody knows what is going on. The
> > driver knows that user space is using the device. Multiple user space
> > users are blocked from conflicting because of the open. There is no
> > way to shoot yourself in the foot.
>
> That sounds like a nice way to mediate userspace usages of PCI devices,
> yes. Much like a dot lockfile is done for tty devices today, all in
> userspace (which hints that this too can be done in userspace with no
> kernel involvement...)
>
> But it still has nothing to do with this enable sysfs file :)
It has everything to do with the 'enable' file. The 'enable' file lets
you change the state of the hardware without an ownership mechanism.
Other device users will not be notified of the state change. Since the
other users can't be sure of the state of the hardware when they are
activated, they will have to reload their state into the hardware on
every activation.
So as a result of this every interrupt service routine should now
include pci_enable(). If you don't include this and someone from user
space disables the hardware you're going to GPF. fbdev is already
forced to take defensive measures like this since X will randomly
disable it's hardware while it has an ISR active.
On the other hand, if you say you're only going to enable hardware and
not disable it, then we should simply get rid of the PCI
enable/disable functions and permanently enable everything at boot.
--
Jon Smirl
[email protected]
> It has everything to do with the 'enable' file. The 'enable' file lets
> you change the state of the hardware without an ownership mechanism.
> Other device users will not be notified of the state change. Since the
> other users can't be sure of the state of the hardware when they are
> activated, they will have to reload their state into the hardware on
> every activation.
Jon
you seem to miss the fact that this can be done now without the enable
flag, setpci can be used to disable the BARs, again the enable flag
doesn't change that....
Dave.
On 5/5/06, Dave Airlie <[email protected]> wrote:
> > It has everything to do with the 'enable' file. The 'enable' file lets
> > you change the state of the hardware without an ownership mechanism.
> > Other device users will not be notified of the state change. Since the
> > other users can't be sure of the state of the hardware when they are
> > activated, they will have to reload their state into the hardware on
> > every activation.
>
> Jon
>
> you seem to miss the fact that this can be done now without the enable
> flag, setpci can be used to disable the BARs, again the enable flag
> doesn't change that....
Existence of a problem API doesn't justify creating more of them.
The minimal vga driver combined with new_id scheme is very simple, it
works on older kernels, it does not create a new API and it tracks
ownership of the hardware state. But instead everyone seems to want
the 'enable' file despite it's obvious problems.
--
Jon Smirl
[email protected]
"Jon Smirl" <[email protected]> writes:
> The minimal vga driver combined with new_id scheme is very simple, it
> works on older kernels, it does not create a new API and it tracks
> ownership of the hardware state.
But it works only with VGAs (I, for example, use setpci-alike things
with non-VGA cards but it's dangerous - who knows if the BARs are set
correctly if the device wasn't enabled by the kernel).
--
Krzysztof Halasa
On 5/6/06, Krzysztof Halasa <[email protected]> wrote:
> "Jon Smirl" <[email protected]> writes:
>
> > The minimal vga driver combined with new_id scheme is very simple, it
> > works on older kernels, it does not create a new API and it tracks
> > ownership of the hardware state.
>
> But it works only with VGAs (I, for example, use setpci-alike things
> with non-VGA cards but it's dangerous - who knows if the BARs are set
> correctly if the device wasn't enabled by the kernel).
Substitute vga with the name of whatever class of device you are
working on and build it a minimal driver for it. The technique is
generic.
> --
> Krzysztof Halasa
>
--
Jon Smirl
[email protected]
"Jon Smirl" <[email protected]> writes:
> Substitute vga with the name of whatever class of device you are
> working on and build it a minimal driver for it. The technique is
> generic.
The card in question _has_ a driver. I, for example, just need a way
to write EEPROM data to it (vendor/device ID etc). The card has to be
selected by PCI bus and slot (device) number, not by device class
and/or IDs, because it can contain garbage and/or some generic IDs
with generic device class.
I'm not against the additional driver but it has to be able to work
with any specified card (as setpci does). But if it's that simple
then why not do that in the PCI code instead (holding some device
file open isn't a problem)?
--
Krzysztof Halasa
On 5/6/06, Krzysztof Halasa <[email protected]> wrote:
> "Jon Smirl" <[email protected]> writes:
>
> > Substitute vga with the name of whatever class of device you are
> > working on and build it a minimal driver for it. The technique is
> > generic.
>
> The card in question _has_ a driver. I, for example, just need a way
> to write EEPROM data to it (vendor/device ID etc). The card has to be
> selected by PCI bus and slot (device) number, not by device class
> and/or IDs, because it can contain garbage and/or some generic IDs
> with generic device class.
Hardware like you describe violates the PCI spec and it should not be
expected that Linux will support it in the general case. It sounds
like this is some kind of prototype hardware.
I would probably handle this by writing an unbound device driver that
exposes a sysfs file for bus:slot. When you write the bus:slot to the
file it would then bind to the appropriate hardware and enable it. The
newly bound driver would support the driver firmware loader interface
as a means of getting your data in.
>
> I'm not against the additional driver but it has to be able to work
> with any specified card (as setpci does). But if it's that simple
> then why not do that in the PCI code instead (holding some device
> file open isn't a problem)?
> --
> Krzysztof Halasa
>
--
Jon Smirl
[email protected]
"Jon Smirl" <[email protected]> writes:
>> The card in question _has_ a driver. I, for example, just need a way
>> to write EEPROM data to it (vendor/device ID etc). The card has to be
>> selected by PCI bus and slot (device) number, not by device class
>> and/or IDs, because it can contain garbage and/or some generic IDs
>> with generic device class.
>
> Hardware like you describe violates the PCI spec
What exactly does it violate?
> and it should not be
> expected that Linux will support it in the general case. It sounds
> like this is some kind of prototype hardware.
No. Just one of the final steps of production, or in-system update.
> I would probably handle this by writing an unbound device driver that
> exposes a sysfs file for bus:slot. When you write the bus:slot to the
> file it would then bind to the appropriate hardware and enable it. The
> newly bound driver would support the driver firmware loader interface
> as a means of getting your data in.
What is also needed is that end users perform this task from time to
time. They generally don't want to have to compile the kernel :-)
You know, even now it can be done (entirely in userspace), and only
enabling the device seems a bit dangerous.
--
Krzysztof Halasa
On May 6, 2006, at 19:16:25, Krzysztof Halasa wrote:
> "Jon Smirl" <[email protected]> writes:
>>> The card in question _has_ a driver. I, for example, just need a
>>> way to write EEPROM data to it (vendor/device ID etc). The card
>>> has to be selected by PCI bus and slot (device) number, not by
>>> device class and/or IDs, because it can contain garbage and/or
>>> some generic IDs with generic device class.
>>
>> Hardware like you describe violates the PCI spec
>
> What exactly does it violate?
"device class and/or IDs ... can contain garbage". That seems to
violate PCI spec to me.
>> I would probably handle this by writing an unbound device driver
>> that exposes a sysfs file for bus:slot. When you write the
>> bus:slot to the file it would then bind to the appropriate
>> hardware and enable it. The newly bound driver would support the
>> driver firmware loader interface as a means of getting your data in.
>
> What is also needed is that end users perform this task from time
> to time. They generally don't want to have to compile the kernel :-)
>
> You know, even now it can be done (entirely in userspace), and only
> enabling the device seems a bit dangerous.
Jon Smirl gave a great description of exactly how to write such a
"driver". I seem to recall that we already have the ability to
trigger manual PCI binding by bus:slot number; in combination with an
appropriate "write EEPROM with firmware file" driver that has no
default list of PCI devices; you could easily manually trigger a bind
of the device. On the other hand I would personally never put such a
device in my system, what if the garbage device IDs happened to match
a device with poorly-written PCI IDE drivers that panic when they
bind to a device which does not match their expectations? In any
case what you really need for all those cases is a simplistic stub
driver that provides some sort of in-kernel mutual exclusion.
Cheers,
Kyle Moffett
On Fri, May 05, 2006 at 01:26:03PM -0700, Greg KH wrote:
> On Fri, May 05, 2006 at 04:14:00PM -0400, Jon Smirl wrote:
> > I would like to see other design alternatives considered on this
> > issue. The 'enable' attribute has a clear problem in that you can't
> > tell which user space program is trying to control the device.
> > Multiple programs accessing the video hardware with poor coordination
> > is already the source of many problems.
>
> Who cares who "enabled" the device. Remember, the majority of PCI
> devices in the system are not video ones. Lots of other types of
> devices want this ability to enable PCI devices from userspace. I've
> been talking with some people about how to properly write PCI drivers in
> userspace, and this attribute is a needed part of it.
>
> And if X gets enabling the device wrong, again, who cares, it's not a
> kernel issue. :)
>
> thanks,
>
> greg k-h
> -
The main issue I see with the "enable" sysfs attribute is that it doesn't
mediate who has ownership of the device's state and context, as Jon has
mentioned. As a result, userspace might, not knowing any better, disable
the device while its kernel driver is trying to access it, or a number of
other fatal senarios.
However, I understand that video is not the only issue at hand, and there is
some interest in userspace PCI drivers. At the very least, we should prevent
any writing to the "enable" attribute if a kernelspace driver is bound to
the device. This patch completely ignores such issues.
We might want to take this a step further and create a generic PCI userspace
kernel driver that exports knobs such as "enable" in sysfs and also handles
some baseline suspend/resume behavior and whatever else is needed for
userspace drivers. This will ensure that the kernel is aware that the
hardware is owned by a userspace driver, and it will make it clear to a user
app that it is allowed to control the device.
Thanks,
Adam
Kyle Moffett <[email protected]> writes:
> "device class and/or IDs ... can contain garbage". That seems to
> violate PCI spec to me.
Well, maybe not exactly garbage but just no useful IDs.
> Jon Smirl gave a great description of exactly how to write such a
> "driver". I seem to recall that we already have the ability to
> trigger manual PCI binding by bus:slot number; in combination with an
> appropriate "write EEPROM with firmware file" driver that has no
> default list of PCI devices; you could easily manually trigger a bind
> of the device.
Writing EEPROM is not a problem and can be done safely from user space
(mmap /dev/mem). Doing it in the kernel seems like an overkill,
especially if you do the operation once in few years it's much easier
to just download a (statically linked?) binary than messing with
the kernel.
It doesn't even interfere with the "main" driver and can be done while
the device is operating (given that previous EEPROM made sense,
otherwise the driver would not load).
> On the other hand I would personally never put such a
> device in my system, what if the garbage device IDs happened to match
> a device with poorly-written PCI IDE drivers that panic when they
> bind to a device which does not match their expectations?
That isn't possible in this case.
The EEPROM has to be written somehow anyway.
> In any
> case what you really need for all those cases is a simplistic stub
> driver that provides some sort of in-kernel mutual exclusion.
Right. I.e., "enable" interface with, possibly, locking mechanism,
instead of some per-class "driver".
--
Krzysztof Halasa
On May 7, 2006, at 08:05:59, Krzysztof Halasa wrote:
> Kyle Moffett <[email protected]> writes:
>> Jon Smirl gave a great description of exactly how to write such a
>> "driver". I seem to recall that we already have the ability to
>> trigger manual PCI binding by bus:slot number; in combination with
>> an appropriate "write EEPROM with firmware file" driver that has
>> no default list of PCI devices; you could easily manually trigger
>> a bind of the device.
>
> Writing EEPROM is not a problem and can be done safely from user
> space (mmap /dev/mem).
This is *exactly* what we don't want to do! The whole point of this
thread is to prevent the need to use /dev/mem and /dev/kmem for
anything except debugging.
> Doing it in the kernel seems like an overkill, especially if you do
> the operation once in few years it's much easier to just download a
> (statically linked?) binary than messing with the kernel.
Ewww, I certainly wouldn't trust a binary statically-linked binary
program that mmaps /dev/mem or /dev/kmem, and I certainly bet that
90% of the people on this list would feel likewise. We'd much prefer
a program which does this:
#! /bin/sh
cp firmware.bin /lib/firmware/some_firmware_file.bin
echo -n eeprom_load_driver >/sys/device/$PCI_ID/bind
echo -n 1 >/sys/device/$PCI_ID/unbind
Simple, obviously correct, and uses a nice reuseable driver too!
> It doesn't even interfere with the "main" driver and can be done
> while the device is operating (given that previous EEPROM made
> sense, otherwise the driver would not load).
No! That would be even worse! You're then having userspace poke at
the driver while a kernel driver is loaded, which is *exactly* what X
is getting into trouble for doing. If you want to add firmware
update capability, add it to the preexisting primary driver.
>> In any case what you really need for all those cases is a
>> simplistic stub driver that provides some sort of in-kernel mutual
>> exclusion.
>
> Right. I.e., "enable" interface with, possibly, locking mechanism,
> instead of some per-class "driver".
No, not an "enable" interface. In this case the kernel should do
basically all of the poking at PCI resources for you. If you
_really_ want to do that kind of update in userspace, write a stub
driver which just enables the device on bind, disables it on unbind,
and mmap and write to the sysfs "rom" file.
Cheers,
Kyle Moffett
Kyle Moffett <[email protected]> writes:
> This is *exactly* what we don't want to do! The whole point of this
> thread is to prevent the need to use /dev/mem and /dev/kmem for
> anything except debugging.
Look, it's me who's using that and I tell you I want just that :-)
> Ewww, I certainly wouldn't trust a binary statically-linked binary
> program that mmaps /dev/mem or /dev/kmem
And would you trust a binary which doesn't have "/dev/mem" string
in it?
Anyway you can compile it yourself if you want. It's not about trust,
it's about simplicity and robustness.
> #! /bin/sh
> cp firmware.bin /lib/firmware/some_firmware_file.bin
> echo -n eeprom_load_driver >/sys/device/$PCI_ID/bind
> echo -n 1 >/sys/device/$PCI_ID/unbind
>
> Simple, obviously correct, and uses a nice reuseable driver too!
Sure. If the driver is loaded/available. What if, say, the
distribution you use doesn't have it?
> No! That would be even worse! You're then having userspace poke at
> the driver while a kernel driver is loaded, which is *exactly* what X
> is getting into trouble for doing.
So what? The driver and EEPROM updater don't conflict.
> If you want to add firmware
> update capability, add it to the preexisting primary driver.
It will not load with blank or invalid EEPROM :-)
> No, not an "enable" interface. In this case the kernel should do
> basically all of the poking at PCI resources for you.
Because?
> If you
> _really_ want to do that kind of update in userspace, write a stub
> driver which just enables the device on bind, disables it on unbind,
> and mmap and write to the sysfs "rom" file.
It has nothing to do with any "ROM".
--
Krzysztof Halasa
>
> So as a result of this every interrupt service routine should now
> include pci_enable(). If you don't include this and someone from user
> space disables the hardware you're going to GPF. fbdev is already
> forced to take defensive measures like this since X will randomly
> disable it's hardware while it has an ISR active.
Jon please stop spouting crap at least, every ISR doesn't need pci_enable,
as it doesn't need it now, you are NOT listening, adding the enable bit
DOES NOT change things, if someone runs setpci from userspace now they can
do this, ROOT CAN CRASH YOUR MACHINE, FILM AT 11.
If you enable the rom at the moment you can crash things, some devices
don't have enough address decoders to decode a bar and the ROM, so me
enabling ROM decoding as your original patch did, can cause system
lockups,
why is this different, why didn't you write the ROM code patch properly
then and we woulnd't have to to hear about it now...
Dave.
--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
Linux kernel - DRI, VAX / pam_smb / ILUG
On 5/8/06, Dave Airlie <[email protected]> wrote:
> >
> > So as a result of this every interrupt service routine should now
> > include pci_enable(). If you don't include this and someone from user
> > space disables the hardware you're going to GPF. fbdev is already
> > forced to take defensive measures like this since X will randomly
> > disable it's hardware while it has an ISR active.
>
> Jon please stop spouting crap at least, every ISR doesn't need pci_enable,
> as it doesn't need it now, you are NOT listening, adding the enable bit
> DOES NOT change things, if someone runs setpci from userspace now they can
> do this, ROOT CAN CRASH YOUR MACHINE, FILM AT 11.
Do what you want, this has gone on too long. There are simple schemes
that can fix these holes but nobody seems to want to use them. Just
because something was broken in the past doesn't justify a new API to
continue supporting the breakage in the future. I am amazed at the
amount energy being expended to support this flawed API when
alternatives are available.
> If you enable the rom at the moment you can crash things, some devices
> don't have enough address decoders to decode a bar and the ROM, so me
> enabling ROM decoding as your original patch did, can cause system
> lockups,
>
> why is this different, why didn't you write the ROM code patch properly
> then and we woulnd't have to to hear about it now...
This is a known part of the PCI spec, it is not a bug. There is a ROM
API that the driver for the offending hardware should call to disable
(or copy) the ROM attribute. If the driver is missing the ROM disable
call that is a bug. I don't know what hardware has the address decoder
problem so I can't fix the drivers. This is documented in the header
file and has been posted to LKML. This hardware is uncommon, but If
you do identify hardware that is missing the address decoder please
inform me and the driver maintainer and we will add a call to disable
the ROM attribute.
As far as I know only a single person has hit this and they didn't
tell me the PCI ID of the problem hardware. I suspect the problem
hardware is an older Adaptec RAID controller.
--
Jon Smirl
[email protected]
Hi!
> >It has everything to do with the 'enable' file. The
> >'enable' file lets
> >you change the state of the hardware without an
> >ownership mechanism.
> >Other device users will not be notified of the state
> >change. Since the
> >other users can't be sure of the state of the hardware
> >when they are
> >activated, they will have to reload their state into
> >the hardware on
> >every activation.
>
> you seem to miss the fact that this can be done now
> without the enable
> flag, setpci can be used to disable the BARs, again the
> enable flag
> doesn't change that....
...well, when you launch setpci, you are firmly in 'unsupported' land.
While 'enable' sounds like something where users expect it to be
supported.
--
Thanks for all the (sleeping) penguins.
On May 7, 2006, at 09:12:01, Pavel Machek wrote:
>>> It has everything to do with the 'enable' file. The 'enable' file
>>> lets you change the state of the hardware without an ownership
>>> mechanism. Other device users will not be notified of the state
>>> change. Since the other users can't be sure of the state of the
>>> hardware when they are activated, they will have to reload their
>>> state into the hardware on every activation.
>>
>> you seem to miss the fact that this can be done now without the
>> enable flag, setpci can be used to disable the BARs, again the
>> enable flag doesn't change that....
>
> ...well, when you launch setpci, you are firmly in 'unsupported'
> land. While 'enable' sounds like something where users expect it
> to be supported.
*Especially* since there are a number of users (including myself) who
have tendencies to go wandering around sysfs tinkering with the
available values. Not having seen this thread, I would have had no
problem doing "echo -n 0 >enable" on some device thinking that it was
a fairly standard way to turn off the power to my soundcard when I'm
not using it, and likely result in a kernel panic because I suddenly
disabled the BARs on the device out from under the driver.
Cheers,
Kyle Moffett
> *Especially* since there are a number of users (including myself) who
> have tendencies to go wandering around sysfs tinkering with the
> available values. Not having seen this thread, I would have had no
> problem doing "echo -n 0 >enable" on some device thinking that it was
> a fairly standard way to turn off the power to my soundcard
it is
> when I'm
> not using it, and likely result in a kernel panic because I suddenly
> disabled the BARs on the device out from under the driver.
well it is in the pci dir, not in the sound dir. You know you're not
using the driver layer at that point, but are directly poking the
hardware. Just like the resources (eg the contents of the bars) are in
that same directory... you're not echo;ing random crap into those as
well, are you?
> By allowing tools to read the rom from the pci device itself, instead of
> whichever device's rom happens to be at 0xC0000. libx86emul allows you
> to define routines that it uses for memory access, so you can copy the
> ROM out to normal memory, and map that memory to the appropriate address
> that way. X and vbetool both already have to do this, but without
> copying the firmware image, to do DDC probing on x86_64.
>
> > I agree with Arjan that the attribute could be useful for some special
> > tools, but using it in X is likely to be utterly wrong.
>
> I'm actually talking about vbetool here, though X could use these
> methods. Indeed, libx86emul comes from X itself.
There are reasons why you may have to read the image at c0000... There's
a bunch of laptops where it's in fact the only way to get to the video
BIOS as it doesn't have a ROM attached to the video chip (it's burried
in the main BIOS which thankfully copied it to c0000 when running it).
In some cases, the BISO ROM self-modifies it's c0000 and it's that
modified copy that the X (or fbdev) driver should get. Remeber that
drivers needs access to the ROM for more than just POSTing the chip...
In addition, we need to centralize VGA routing mecanism in the kernel
for other reasons. For example, because it might imply disabling MMIO or
IO access to other cards (currently, we have to disable _all_ other
cards on the same bus segment that claim to be VGA since we have no way
to know which ones might be able to simply be configured not to decode
legacy VGA accesses, though an appropriate arbiter ABI would allow to
handle that too). Thus am fbdev or X server currently running on another
card would explode in flames while something like vbetool is re-routing
VGA access to a card for POST'ing or other things vbetool can do.
It's a shitty problem, it's complicated, but there is no easy way out
thanks to PC manufacturers and Intel keeping absolutely disgusting VGA
"legacy" crap part of the PCI standard for so long.
Note that I posted various prototypes of what a VGA abiter could look
like a while ago. I have no time to resume work on that and my
prototypes weren't perfect, but that's something one interested party
could probably use as a starting point to a more complete solution.
Ben.
On Saturday 13 May 2006 20:29, Benjamin Herrenschmidt wrote:
> a long post.
So, why do we insist on keeping legacy hardware around? I mean, serial and
parallel ports are basically dead, as are ps/2 ports (USB killed them all).
VGA basically died out when DVI came around. Traditional IA32 is now dying
out thanks to x86-64. The basic internals have been surplanted by APIC. We
have a power management API, ACPI, which was unheard of on x86 15 years ago.
IDE is dying out, being replaced with SATA. Old SCSI methods are being
replaced with Serial SCSI. I mean, I could basically go on for hours saying
that pretty much all legacy hardware and methods are being replaced or have
already been replaced.
Even the traditional IBM PC BIOS is being replaced, by various new firmware
logic methods (such as openfirmware on x86, or intel's one who's name I
forget.)
So, why do we continue keeping VGA around? Why isn't there a better method?
Benjamin's post basically describes a technology that has been hacked to hell
and back and has been around entirely too long, so why hasn't someone created
something new?
--
Patrick McFarland || http://www.AdAstraPerAspera.com
"Computer games don't affect kids; I mean if Pac-Man affected us as kids,
we'd all be running around in darkened rooms, munching magic pills and
listening to repetitive electronic music." -- Kristian Wilson, Nintendo,
Inc, 1989
On 5/13/06, Benjamin Herrenschmidt <[email protected]> wrote:
> There are reasons why you may have to read the image at c0000... There's
> a bunch of laptops where it's in fact the only way to get to the video
> BIOS as it doesn't have a ROM attached to the video chip (it's burried
> in the main BIOS which thankfully copied it to c0000 when running it).
> In some cases, the BISO ROM self-modifies it's c0000 and it's that
> modified copy that the X (or fbdev) driver should get. Remeber that
> drivers needs access to the ROM for more than just POSTing the chip...
Whenever klibc gets merged it would probably be good to add a
libemu86. Did you get one put together that you're happy with?
Between the ROM attribute, klibc and libemu86 there will then be
enough support to write a tiny POST program that POSTs secondary and
non-x86 primary cards at boot. It will still need a little support in
sysfs for PCI bus VGA routing but we're almost there.
--
Jon Smirl
[email protected]
On 5/13/06, Patrick McFarland <[email protected]> wrote:
> On Saturday 13 May 2006 20:29, Benjamin Herrenschmidt wrote:
> > a long post.
>
> So, why do we insist on keeping legacy hardware around? I mean, serial and
> parallel ports are basically dead, as are ps/2 ports (USB killed them all).
> VGA basically died out when DVI came around. Traditional IA32 is now dying
> out thanks to x86-64. The basic internals have been surplanted by APIC. We
> have a power management API, ACPI, which was unheard of on x86 15 years ago.
Because it is the only video interface we have documentation for.
Almost all of the video hardware can run in non-VGA mode but we don't
have the docs to do this on NVidia/ATI.
It is also a universal interface supported by all video cards. You can
get things like GRUB and the BIOS up on it with minimal code that will
work on all video cards.
To get rid of it the video hardware manufacturers would have to come
together and define a new, open standard. That doesn't look likely to
happen so we are stuck with it. I do agree that it is extremely messy
to work with.
--
Jon Smirl
[email protected]
On Sat, 2006-05-13 at 20:56 -0400, Jon Smirl wrote:
> On 5/13/06, Benjamin Herrenschmidt <[email protected]> wrote:
> > There are reasons why you may have to read the image at c0000... There's
> > a bunch of laptops where it's in fact the only way to get to the video
> > BIOS as it doesn't have a ROM attached to the video chip (it's burried
> > in the main BIOS which thankfully copied it to c0000 when running it).
> > In some cases, the BISO ROM self-modifies it's c0000 and it's that
> > modified copy that the X (or fbdev) driver should get. Remeber that
> > drivers needs access to the ROM for more than just POSTing the chip...
>
> Whenever klibc gets merged it would probably be good to add a
> libemu86. Did you get one put together that you're happy with?
No, not so far yet. Haven't had much time though. Did some expriments
based on what you sent me back then, got it to work with loads of hacks,
but I never properly cleaned it up.
> Between the ROM attribute, klibc and libemu86 there will then be
> enough support to write a tiny POST program that POSTs secondary and
> non-x86 primary cards at boot. It will still need a little support in
> sysfs for PCI bus VGA routing but we're almost there.
And we need to "capture" the resulting BIOS image after POST and have a
away to give that to the drivers as it will contain useful tables that
the driver will need as well...
Ben.
On 5/14/06, Benjamin Herrenschmidt <[email protected]> wrote:
> On Sat, 2006-05-13 at 20:56 -0400, Jon Smirl wrote:
> > On 5/13/06, Benjamin Herrenschmidt <[email protected]> wrote:
> > > There are reasons why you may have to read the image at c0000... There's
> > > a bunch of laptops where it's in fact the only way to get to the video
> > > BIOS as it doesn't have a ROM attached to the video chip (it's burried
> > > in the main BIOS which thankfully copied it to c0000 when running it).
> > > In some cases, the BISO ROM self-modifies it's c0000 and it's that
> > > modified copy that the X (or fbdev) driver should get. Remeber that
> > > drivers needs access to the ROM for more than just POSTing the chip...
> >
> > Whenever klibc gets merged it would probably be good to add a
> > libemu86. Did you get one put together that you're happy with?
>
> No, not so far yet. Haven't had much time though. Did some expriments
> based on what you sent me back then, got it to work with loads of hacks,
> but I never properly cleaned it up.
>
> > Between the ROM attribute, klibc and libemu86 there will then be
> > enough support to write a tiny POST program that POSTs secondary and
> > non-x86 primary cards at boot. It will still need a little support in
> > sysfs for PCI bus VGA routing but we're almost there.
>
> And we need to "capture" the resulting BIOS image after POST and have a
> away to give that to the drivers as it will contain useful tables that
> the driver will need as well...
The ROM API already has support it in for storing a RAM based copy. I
believe it works but no one is using it so it is not well tested.
Something will need to be hooked up to get the copy from user space
back into the kernel. At one point I was working on generalizing
request_firmware to make it handle more that firmware downloads.
>
> Ben.
>
>
>
--
Jon Smirl
[email protected]