Page MenuHomeFreeBSD

acpi_ged: New driver to ACPI generic event device
ClosedPublic

Authored by takawata on Oct 18 2022, 6:41 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, May 17, 11:10 PM
Unknown Object (File)
Fri, May 17, 5:11 PM
Unknown Object (File)
Sun, May 12, 7:12 PM
Unknown Object (File)
Sun, May 12, 7:12 PM
Unknown Object (File)
Sun, May 12, 7:12 PM
Unknown Object (File)
Sun, May 12, 7:12 PM
Unknown Object (File)
Sun, May 12, 7:12 PM
Unknown Object (File)
Sun, May 12, 7:12 PM

Details

Summary

New driver to ACPI generic event device, defined in ACPI spec.
Some ACPI power button may not work without this.

Test Plan

In qemu arm64 with "virt" machine, with ACPI firmware,

# hexdump /dev/input/event2

and in qemu monitor

(qemu) system_poweroff

and make sure some power button input event appear.
( setting sysctl hw.acpi.power_button_state=S5 is not work,
because ACPI tree does not have \_S5 object.)

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

takawata held this revision as a draft.
takawata edited the test plan for this revision. (Show Details)
takawata edited the test plan for this revision. (Show Details)

I've tested on Parallels on a m1 Mac & do see an event. I guess it still needs something to turn that into a shutdown command?

acpi_ged.c looks like it needs some style cleanups, e.g. use tabs for indentation, space after if/for, and brace shouldn't be on a new line for if/for.

sys/arm64/conf/std.virt
27

I think we would need this in all std.* files with device acpi

sys/dev/acpica/acpi_ged.c
31

Is opt_evdev.h needed?

132

No need to zero this, the softc will be zero when allocated.

I've tested on Parallels on a m1 Mac & do see an event. I guess it still needs something to turn that into a shutdown command?

devd(8) can catch power button notification.
For example,

notify 10 {
        match "system"          "ACPI";
        match "subsystem"       "Button";
        match "notify"              "0x00"
        action "shutdown -p now";
};

note that notify value is not actual notify value but presents kind of button.Here, notify value "0x00" means power button event.

acpi_ged.c looks like it needs some style cleanups, e.g. use tabs for indentation, space after if/for, and brace shouldn't be on a new line for if/for.

I 'll clang-format to update this.

Update.
Use clang-formatter.

takawata added inline comments.
sys/arm64/conf/std.virt
27

Do we need std.acpi to describe acpi-aware setting?
Anyway, I think it is beyond the matter. Use kld module for now.

takawata marked an inline comment as not done.Oct 19 2022, 4:58 AM

I aware the value from rman_get_start and the value written in ACPI resource(_CRS) is not same. It seems that INTRNG code translate this.
How can I get raw _CRS value from irq resource......

I managed to get raw value of _CRS resource.

Move raw irq resolve in attach routine and add sanity check.

hrs added inline comments.
share/man/man4/acpi_ged.4
32

s/Devcice/Device/

50

Insert a newline after the first period just after "interface".

51

s/interrupt/interrupts/
s/evaluate/evaluates/
s/specific/the specific/
s/may generate/may optionally generate/

And insert a newline after the end of the sentence. You can double-check it by using "mandoc -Tlint acpi_ged.4".

52

s/other device/another device/

sys/dev/acpica/acpi_ged.c
49

I suggest s/events/event/ because this structure is for a single event.

72

DEVMETHOD_END should be on a separate line.

97

I am not sure if this matters, but according to the specification[1], a _CRS can have _Exx or _Lxx method, instead of _EVT, for the interrupt-signaled event with an event number <= 255.

Also, in the case that there is no _Exx/_Lxx/_EVT method, acpi_ged_evt() is called, and it fails. Registering such event handlers does not make sense.

Considering them, it looks better for me to get the ACPI handle for the method, put it into "struct acpi_ged_event" during acpi_ged_attach(), and call it upon an interrupt. If we cannot locate the ACPI handle, we should ignore it with displaying a warning.

[1] https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#crs-object-for-interrupt-signaled-events

142

I wonder why we need to allocate an IRQ with no RF_SHAREABLE (or without checking whether it can be RF_SHAREABLE). Is there any reason?

Support _Exx _Lxx calling case, though untested.
Fix man page.

formatting should be X not x.

modify DSDT to replace _EVT with _E29 and test it.
Fix the beheivior.

This revision was not accepted when it landed; it landed in state Needs Review.Oct 24 2022, 10:02 AM
This revision was automatically updated to reflect the committed changes.
cy added inline comments.
sys/dev/acpica/acpi_ged.c
201

Should evt be evts here?