linux-kernel - Re: [PATCH v4 2/5] ACPI: platform: Ignore SMB0001 only when it has resources

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJZ5v0iZzvdkxe3NjHbiZzDjea=CoK-pGw3hjSoa9u8f87TLtg@mail.gmail.com>
Date:   Tue, 4 Jul 2023 19:32:43 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     "Rafael J. Wysocki" <rafael.j.wysocki@...el.com>,
        Wolfram Sang <wsa@...nel.org>,
        Andi Shyti <andi.shyti@...nel.org>, linux-acpi@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-i2c@...r.kernel.org,
        acpica-devel@...ts.linuxfoundation.org,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Len Brown <lenb@...nel.org>,
        Robert Moore <robert.moore@...el.com>,
        Michael Walle <michael@...le.cc>
Subject: Re: [PATCH v4 2/5] ACPI: platform: Ignore SMB0001 only when it has resources

On Mon, Jul 3, 2023 at 2:14 PM Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
>
> After switching i2c-scmi driver to be a platform one, it stopped
> being enumerated on number of Kontron platforms, because it's
> listed in the forbidden_id_list.
>
> To resolve the situation, add a flag to driver data to allow devices
> with no resources in _CRS to be enumerated via platform bus.
>
> Fixes: 03d4287add6e ("i2c: scmi: Convert to be a platform driver")
> Closes: https://lore.kernel.org/r/60c1756765b9a3f1eab0dcbd84f59f00fe1caf48.camel@kontron.com
> Link: https://lore.kernel.org/r/20230621151652.79579-1-andriy.shevchenko@linux.intel.com
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Reviewed-by: Andi Shyti <andi.shyti@...nel.org>
> ---
>  drivers/acpi/acpi_platform.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/acpi_platform.c b/drivers/acpi/acpi_platform.c
> index fe00a5783f53..c2ce558bd032 100644
> --- a/drivers/acpi/acpi_platform.c
> +++ b/drivers/acpi/acpi_platform.c
> @@ -9,6 +9,7 @@
>   */
>
>  #include <linux/acpi.h>
> +#include <linux/bits.h>
>  #include <linux/device.h>
>  #include <linux/err.h>
>  #include <linux/kernel.h>
> @@ -19,13 +20,16 @@
>
>  #include "internal.h"
>
> +/* Exclude devices that have no _CRS resources provided */
> +#define ACPI_ALLOW_WO_RESOURCES                BIT(0)
> +
>  static const struct acpi_device_id forbidden_id_list[] = {
>         {"ACPI0009", 0},        /* IOxAPIC */
>         {"ACPI000A", 0},        /* IOAPIC */
>         {"PNP0000",  0},        /* PIC */
>         {"PNP0100",  0},        /* Timer */
>         {"PNP0200",  0},        /* AT DMA Controller */
> -       {"SMB0001",  0},        /* ACPI SMBUS virtual device */
> +       {"SMB0001",  ACPI_ALLOW_WO_RESOURCES},  /* ACPI SMBUS virtual device */
>         { }
>  };
>
> @@ -83,6 +87,15 @@ static void acpi_platform_fill_resource(struct acpi_device *adev,
>                 dest->parent = pci_find_resource(to_pci_dev(parent), dest);
>  }
>
> +static unsigned int acpi_platform_resource_count(struct acpi_resource *ares, void *data)
> +{
> +       bool *has_resources = data;
> +
> +       *has_resources = true;
> +
> +       return AE_CTRL_TERMINATE;
> +}
> +
>  /**
>   * acpi_create_platform_device - Create platform device for ACPI device node
>   * @adev: ACPI device node to create a platform device for.
> @@ -100,17 +113,28 @@ struct platform_device *acpi_create_platform_device(struct acpi_device *adev,
>         struct acpi_device *parent = acpi_dev_parent(adev);
>         struct platform_device *pdev = NULL;
>         struct platform_device_info pdevinfo;
> +       const struct acpi_device_id *match;
>         struct resource_entry *rentry;
>         struct list_head resource_list;
>         struct resource *resources = NULL;
> +       bool has_resources;
>         int count;
>
>         /* If the ACPI node already has a physical device attached, skip it. */
>         if (adev->physical_node_count)
>                 return NULL;
>
> -       if (!acpi_match_device_ids(adev, forbidden_id_list))
> -               return ERR_PTR(-EINVAL);
> +       match = acpi_match_acpi_device(forbidden_id_list, adev);
> +       if (match) {
> +               if (match->driver_data & ACPI_ALLOW_WO_RESOURCES) {
> +                       acpi_walk_resources(adev->handle, METHOD_NAME__CRS,
> +                                           acpi_platform_resource_count, &has_resources);
> +                       if (has_resources)
> +                               return ERR_PTR(-EINVAL);
> +               } else {
> +                       return ERR_PTR(-EINVAL);
> +               }
> +       }
>
>         INIT_LIST_HEAD(&resource_list);
>         count = acpi_dev_get_resources(adev, &resource_list, NULL, NULL);
> --

Applied as 6.5-rc material along with the rest of the series (except
for the last patch that has been superseded), but I moved the
has_resources definition to the block where it is used and added a
'false' initial value to it. as the code might not work as expected
without initializing it.

Please double check the bleeding-edge branch of linux-pm.

Thanks!