Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle platforms where SCI is not available. #132

Closed
wants to merge 1 commit into from
Closed

Handle platforms where SCI is not available. #132

wants to merge 1 commit into from

Conversation

kiznit
Copy link

@kiznit kiznit commented Nov 2, 2022

I am using LAI on qemu-system-aarch64 (virt machine) with the OVMF firmware. On that platform, SCI is not available.

  • I/O ports don't exist, everything is MMIO on ARM
  • fadt->smi_command_port is zero
  • fadt->acpi_enable is zero
  • fadt->acpi_disable is zero
  • fadt->sci_irq is zero
  • fadt->pm1a_control_block is zero
  • fadt->pm1b_control_block is zero
  • fadt->pm1a_event_block is zero
  • fadt->pm1b_event_block is zero

I did consider simply not calling lai_enable_acpi() on such platforms, but there is still useful code in there aside from enabling SCI.

The fix is to:

  • Not call the I/O port functions when not needed.
  • Not check for the existence of the I/O port functions when they are not needed.

I have tested this change (QEMU, OVMF) for both x86_64 and aarch64. It appears to work correctly.

I created a thread on OsDev with more details: https://forum.osdev.org/viewtopic.php?f=1&t=56563

@kiznit
Copy link
Author

kiznit commented Nov 16, 2022

Is this project still active?

@no92
Copy link
Member

no92 commented Nov 16, 2022

It is still alive, but contributors might be busy.

Copy link
Member

@qookei qookei left a comment

Choose a reason for hiding this comment

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

Seems fine to me

@kiznit
Copy link
Author

kiznit commented Nov 29, 2022

@qookei - Any chance of taking a look at this PR?

@thomtl
Copy link
Member

thomtl commented Nov 29, 2022

Apologies for the late response,
but it seems to me that this is a sign of a HW-reduced ACPI machine. The qemu code that builds the ACPI table confirms this:
https://github.com/qemu/qemu/blob/master/hw/arm/virt-acpi-build.c#L817

Imho we should properly handle ACPI HW-Reduced machines instead of checking if the ports are 0.
HW-Reduced seems to effect Events (which lai doesn't implement yet), sleep, and initialization. So lai_enter_sleep in helpers/pm.c should also be updated. I'll open a PR soon to address these issues, until then please keep using these patches.

@thomtl thomtl closed this Nov 29, 2022
@kiznit
Copy link
Author

kiznit commented Nov 29, 2022

I wasn't aware that existed such things as "HW-reduced ACPI machines". Looking forward to your PR. Thanks!

@thomtl
Copy link
Member

thomtl commented Nov 30, 2022

@kiznit Could you test the patch in #134 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants