On Wed, Jul 3, 2024 at 1:01 AM Jitindar Singh, Suraj <surajjs@xxxxxxxxxx> wrote:
>
> On Fri, 2024-06-14 at 13:08 +0200, Rafael J. Wysocki wrote:
> > On Thu, Jun 13, 2024 at 11:10 PM Suraj Jitindar Singh
> > <surajjs@xxxxxxxxxx> wrote:
> > >
> > > Make is possible to use ACPI without having CONFIG_PCI set.
> > >
> > > When initialising ACPI the following call chain occurs:
> > >
> > > acpi_init() ->
> > > acpi_bus_init() ->
> > > acpi_load_tables() ->
> > > acpi_ev_install_region_handlers() ->
> > >
> > > acpi_ev_install_region_handlers() calls
> > > acpi_ev_install_space_handler() on
> > > each of the default address spaces defined as:
> > >
> > > u8 acpi_gbl_default_address_spaces[ACPI_NUM_DEFAULT_SPACES] = {
> > > ACPI_ADR_SPACE_SYSTEM_MEMORY,
> > > ACPI_ADR_SPACE_SYSTEM_IO,
> > > ACPI_ADR_SPACE_PCI_CONFIG,
> > > ACPI_ADR_SPACE_DATA_TABLE
> > > };
> > >
> > > However in acpi_ev_install_space_handler() the case statement for
> > > ACPI_ADR_SPACE_PCI_CONFIG is ifdef'd as:
> > >
> > > #ifdef ACPI_PCI_CONFIGURED
> > > case ACPI_ADR_SPACE_PCI_CONFIG:
> > >
> > > handler =
> > > acpi_ex_pci_config_space_handler;
> > > setup = acpi_ev_pci_config_region_setup;
> > > break;
> > > #endif
> > >
> > > ACPI_PCI_CONFIGURED is not defined if CONFIG_PCI is not enabled,
> > > thus the
> > > attempt to install the handler fails.
> > >
> > > Fix this by ifdef'ing ACPI_ADR_SPACE_PCI_CONFIG in the list of
> > > default
> > > address spaces.
> >
> > What if there are PCI operation regions in the AML on the platform?
> > How are they going to be handled?
>
> Hi,
>
> Appreciate the response.
>
> I think the short answer is that if there are PCI operation regions in
> the AML on the platform then the kernel will need to be built with PCI
> support (CONFIG_PCI) and if it isn't then there won't be a handler
> installed and the operation will error.
A problem with this approach is that AML has no good way of handling
such errors. It accesses a location in an address space of some sort
and expects the access to be successful.
The interpreter can catch them, but then the only thing it can do is
to abort the AML which then may lead to all sorts of unexpected
behavior of the platform.
> Correct me if I'm wrong but it seems the intention of the patch series:
>
> 36ad7d2b9e9b ACPI: Move PCI reset to a separate function
> 86689776878f ACPI: Allow CONFIG_PCI to be unset for reboot
> bd23fac3eaaa ACPICA: Remove PCI bits from ACPICA when CONFIG_PCI is
> unset
> 5d32a66541c4 PCI/ACPI: Allow ACPI to be built without CONFIG_PCI set
>
> was to decouple the dependency between CONFIG_PCI and CONFIG_ACPI.
Yes, and as per the above, it was a mistake.
> bd23fac3eaaa ("ACPICA: Remove PCI bits from ACPICA when CONFIG_PCI is
> unset") added an ifdef around the code to install the handler for the
> PCI CONFIG region making it dependent on ACPI_PCI_CONFIGURED (and thus
> CONFIG_PCI). Thus it is not possible to install the default handler for
> the PCI CONFIG region unless CONFIG_PCI is set meaning it makes no
> sense to have it in the list of default address spaces.
>
> I can gather that this leads to 2 possibilities:
>
> 1. If there are PCI operation regions in the AML on the platform then
> these will error on a kernel not compiled with CONFIG_PCI.
But as I said, there is no good way of handling such errors.
Basically, the kernel should panic() in those cases.
> or,
>
> 2. The code to install the handler for the PCI CONFIG region should not
> be ifdef'ed and should be executed irrespective of if the kernel is
> compiled with CONFIG_PCI to allow for PCI CONFIG regions in the AML.
That doesn't work either because the PCI config address space may not
be really accessible without CONFIG_PCI.
IOW, I don't see how this can be made work.
Can you please remind me what the use case for ACPI without PCI is?
