Re: [PATCH] acpi: Support CONFIG_ACPI without CONFIG_PCI

["Jitindar Singh, Suraj" <surajjs@amazon.com>]

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@amazon.com> 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.

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.

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.

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.

Thanks

> 
> > Fixes: bd23fac3eaaa ("ACPICA: Remove PCI bits from ACPICA when
> > CONFIG_PCI is unset")
> > CC: stable@vger.kernel.org # 5.0.x-
> > Signed-off-by: Suraj Jitindar Singh <surajjs@amazon.com>
> > ---
> >  drivers/acpi/acpica/evhandler.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/acpi/acpica/evhandler.c
> > b/drivers/acpi/acpica/evhandler.c
> > index 1c8cb6d924df..371093acb362 100644
> > --- a/drivers/acpi/acpica/evhandler.c
> > +++ b/drivers/acpi/acpica/evhandler.c
> > @@ -26,7 +26,9 @@ acpi_ev_install_handler(acpi_handle obj_handle,
> >  u8 acpi_gbl_default_address_spaces[ACPI_NUM_DEFAULT_SPACES] = {
> >         ACPI_ADR_SPACE_SYSTEM_MEMORY,
> >         ACPI_ADR_SPACE_SYSTEM_IO,
> > +#ifdef ACPI_PCI_CONFIGURED
> >         ACPI_ADR_SPACE_PCI_CONFIG,
> > +#endif
> >         ACPI_ADR_SPACE_DATA_TABLE
> >  };
> > 
> > --
> > 2.34.1
> > 
> >