Skip to content

Commit

Permalink
Clarify why Buffer is used instead of Package
Browse files Browse the repository at this point in the history
  • Loading branch information
Lekensteyn committed Jul 31, 2013
1 parent c166b27 commit ee0591b
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions bbswitch.c
Expand Up @@ -120,6 +120,9 @@ static int acpi_call_dsm(acpi_handle handle, const char muid[16], int revid,
params[1].integer.value = revid;
params[2].type = ACPI_TYPE_INTEGER;
params[2].integer.value = func;
/* Although the ACPI spec defines Arg3 as a Package, in practise

This comment has been minimized.

Copy link
@amonakov

amonakov Aug 6, 2013

Minor nit: "practice" (practise is a verb). Thanks for adding this!

This comment has been minimized.

Copy link
@Lekensteyn

Lekensteyn Aug 6, 2013

Author Member

Thanks for mentioning, I'll remember it for the future. I think I also made the same mistake for a nouveau patch, ah well.

* implementations expect a Buffer (CreateWordField and Index functions are
* applied to it). */
params[3].type = ACPI_TYPE_BUFFER;
params[3].buffer.length = 4;
if (args) {
Expand Down

31 comments on commit ee0591b

@acpibob
Copy link

@acpibob acpibob commented on ee0591b Aug 9, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Index on a package is OK. Can you send or post a DSDT with an example of _DSM requiring a buffer?

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Index on a Package is indeed OK, I thought it does not apply to Integer types, but it looks like there is also an implicit conversion rule for Integer -> Buffer? Example of _DSM that requires a Buffer:
https://github.com/Lekensteyn/acpi-stuff/blob/master/dsl/Clevo_B7130/SSDT1.dsl#L601

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this bug be fixed in the BIOS?

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are too many affected BIOSes which makes blacklisting a pain. I think that NVIDIA made a mistake in its reference implementation.

Some affected machines that use CreateField on Arg3:

  • Lenovo ThinkPad T420 (4236NVG, 4177CTO)
  • Clevo B7130
  • Clevo W150HNM-W170HN.0.1
  • Lenovo "4242RF1" (?)
  • Lenovo IdeaPad Y470
  • Acer Aspire 3830TG.V1.05
  • Dell Alienwaer M11xR3
  • Lenovo B560
  • Dell L502x
  • Packard Bell EasyNote TS11HR.V1.07
  • Acer Aspire 5755G.V1.15
  • (many others, simply grep for If (LEqual (_T_0, 0x1A)) with some context)

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the same device driver used on all of these machines (Same driver to execute the particular bad _DSM method?)

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nouveau, nvidia, bbswitch are the known modules using this method. The device is always from vendor nvidia (10de) and is either of class "VGA Compatible Controller" or "3D controller". Are you going to add a blacklist in the acpi core for ignoring debug messages from this source?

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACPICA core has no mechanism to "blacklist" individual machines. However, we may relax the (internal) definition of _DSM to allow either a buffer or a package. Not sure about this yet, however. I don't think the ACPI promoters are willing to change the spec for this at this time.

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next question has to be: What do these machines do on windows? Does it appear that _DSM is invoked correctly?

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The purpose of the function is to prepare for disabling the graphics card, that works.

@ArchangeGabriel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose that those lines in dmesg mean I'm affected too (Asus U43Jc) with a bug of this kind:

[   10.842446] ACPI Warning: \_SB_.PCI0.VGA_._DSM: Argument #4 type mismatch - Found [Integer], ACPI requires [Package] (20130517/nsarguments-95)
[   10.842488] ACPI Warning: \_SB_.PCI0.VGA_._DSM: Argument #4 type mismatch - Found [Integer], ACPI requires [Package] (20130517/nsarguments-95)
[   10.842509] ACPI Warning: \_SB_.PCI0.RP00.VGA_._DSM: Argument #4 type mismatch - Found [Integer], ACPI requires [Package] (20130517/nsarguments-95)
[   10.842523] ACPI Warning: \_SB_.PCI0.RP00.VGA_._DSM: Argument #4 type mismatch - Found [Integer], ACPI requires [Package] (20130517/nsarguments-95)

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acpibob
Copy link

@acpibob acpibob commented on ee0591b Aug 14, 2013

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Argument #4 type mismatch - Found [Integer], ACPI requires [Package]

This looks like a case where the warning was useful -- the driver was passing an incorrect argument.

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is Nvidia aware of this bug in their code?

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know and I think they would not care/fix this because of backwards compatibility. I just peeked in the GPL wrapper of the nvidia proprietary driver and found this in nv-acpi.c:1118 (version 313.26):

     // 3rd dsm argument always a dword. inParamSize checked on entry to this routine.
     dsm_params[3].buffer.type    = ACPI_TYPE_BUFFER;
     dsm_params[3].buffer.length  = inParamSize;
     memcpy(argument3, pInParams, dsm_params[3].buffer.length);
     dsm_params[3].buffer.pointer = argument3;

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think Nvidia should be informed of this problem. Their code (both driver and ASL) is in clear violation of the ACPI specification.

@ArchangeGabriel
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for your information, the ACPI warning I had before disappeared, however I have those ones now, that are, I supposed, related to this commit this time:

[   32.246370] bbswitch: module verification failed: signature and/or required key missing - tainting kernel
[   32.246675] bbswitch: version 0.7
[   32.246682] bbswitch: Found integrated VGA device 0000:00:02.0: \_SB_.PCI0.VGA_
[   32.246689] bbswitch: Found discrete VGA device 0000:01:00.0: \_SB_.PCI0.RP00.VGA_
[   32.246704] ACPI Warning: \_SB_.PCI0.RP00.VGA_._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20130725/nsarguments-95)
[   32.246783] bbswitch: detected an Optimus _DSM function
[   32.246796] pci 0000:01:00.0: enabling device (0000 -> 0003)
[   32.246850] bbswitch: Succesfully loaded. Discrete card 0000:01:00.0 is on
[   32.249734] bbswitch: disabling discrete graphics
[   32.249747] ACPI Warning: \_SB_.PCI0.RP00.VGA_._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20130725/nsarguments-95)

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ArchangeGabriel The ACPI Warning here is harmless (though it can be annoying because it gets repeatedly printed during toggles). The module verification warning is unrelated, it means that the kernel you use (Ubuntu?) has module verification enabled and that the key was not available when building bbswitch (obviously, the keys must be kept secret or it would be rather useless to have module signing).

@petamem
Copy link

@petamem petamem commented on ee0591b Nov 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Harmless it may be, but +1 for the annoyance. 3y later - make at least some option to turn it off.

[  185.121418] NVRM: loading NVIDIA UNIX x86_64 Kernel Module  375.10  Fri Oct 14 10:30:06 PDT 2016 (using threaded interrupts)
[  185.124792] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  185.124886] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  185.124939] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  185.124988] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  185.125024] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  185.125073] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  185.125107] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  185.224526] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  185.521644] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  204.924800] bbswitch: device 0000:01:00.0 is in use by driver 'nvidia', refusing OFF
[  207.374261] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  207.374326] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  207.374373] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  207.374421] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  207.374469] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  207.374533] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  207.374579] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  207.378308] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  207.517085] ACPI Warning: \_SB.PCI0.PEG0.PEGP._DSM: Argument #4 type mismatch - Found [Buffer], ACPI requires [Package] (20160422/nsarguments-95)
[  214.540205] bbswitch: device 0000:01:00.0 is in use by driver 'nvidia', refusing OFF

@Lekensteyn
Copy link
Member Author

@Lekensteyn Lekensteyn commented on ee0591b Nov 5, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@acpibob Btw, NVIDIA's Windows driver also passes Buffer instead of a Package, see this ACPI trace:
#115 (comment)

@petamem You could try to fill a bug with the ACPI subsystem in kernel bugzilla (https://bugzilla.kernel.org/), but it seems unlikely to get fixed since it is a violation of the ACPI spec.

@RyanNerd
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so this is a feature and not a bug. Why do I have deja vu of Microsoft errors (error 5 occurred reporting error 5)?

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about a mechanism to report the warning once only, then go silent?

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the mechanism already exists, but this particular warning is currently always enabled. Probably because it is rather serious (and should probably be an error, not just a warning).

@mirh
Copy link

@mirh mirh commented on ee0591b Mar 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wonders why robert and zeta seems to work on separate tracks Ok, being divided by the ocean seems a valid reason.

Nevertheless, guess this should be fixed when acpica/acpica#189 lands?
It will be a good race to see if that pull maturation will come earlier or later than fglrx (which I'm pretty sure, together with this, are really suffering the otherwise wise choice of sharing Windows code) total degradation [:

@zetalog
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevertheless, guess this should be fixed when acpica/acpica#189 lands?

Not related, this bug is talking about implicit source conversion.
While #189 is talking about NameString in packages.

@masli5
Copy link

@masli5 masli5 commented on ee0591b Nov 23, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The bug is really affecting my boot time, what exactly should I do to fix it?

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@masli5 The message is not a bug. It is triggered when nouveau tries to do runtime PM on "older" laptops (changing power state introduces some delay). It is also triggered by the nouveau and proprietary nvidia driver which needs to lookup some information (like ROM).

@masli5
Copy link

@masli5 masli5 commented on ee0591b Nov 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Lekensteyn Ok, it's a messagge. However, during boot it is displayed about 15 times, in 6-7 times it follows a 5 second delay (so I'm assuming that nouveau is waiting for 5 seconds for whatever is sending the messagge to do its thing, but that doesn't happen). Since my laptop is a year old, with Core i7, nvidia gtx950m and 8GB RAM, I find 2 minutes boots a bit too much (am I wrong?) and the messagge seems to follow the main problem. Is there a fix?

@mirh
Copy link

@mirh mirh commented on ee0591b Nov 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACPICA guys are like constantly working on it.

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why a few messages would increase boot time significantly.

@Lekensteyn
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The messages are a red herring, they occur also in normal situations. Some hybrid graphics laptops (Intel iGPU + Nvidia GTX >= 9xx) however have an issue where the ACPI code from the vendor is stuck in an infinite loop, polling for device power on. If the device never regains power, you can observe this delay. See also https://bugzilla.kernel.org/show_bug.cgi?id=156341

Most likely system sleep (suspend) is also broken for you and shutdown takes a while. @masli5

@acpibob
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The infinite loop timeout in ACPICA is configurable, you might try changing that. Other than that, there is not much that ACPICA can do about such things.

Please sign in to comment.