linux-kernel - Re: [patch 4/4] acpi: ioapic: Respect the resource flags

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]

Date:	Thu, 11 Dec 2014 18:44:58 -0800
From:	Yinghai Lu <yinghai@...nel.org>
To:	Thomas Gleixner <tglx@...utronix.de>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Jiang Liu <jiang.liu@...ux.intel.com>,
	"the arch/x86 maintainers" <x86@...nel.org>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Borislav Petkov <bp@...en8.de>
Subject: Re: [patch 4/4] acpi: ioapic: Respect the resource flags

On Thu, Dec 11, 2014 at 11:48 AM, Thomas Gleixner <tglx@...utronix.de> wrote:
> The acpi parser can set the DISABLED flag on a resource. In
> setup_res() we clear all flags except MEM, so we ignore the DISABLED
> flag. Add proper checks and preserve the flags.
>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
>  drivers/acpi/ioapic.c |   38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
>
> Index: tip/drivers/acpi/ioapic.c
> ===================================================================
> --- tip.orig/drivers/acpi/ioapic.c
> +++ tip/drivers/acpi/ioapic.c
> @@ -40,29 +40,33 @@ struct acpi_pci_ioapic {
>  static LIST_HEAD(ioapic_list);
>  static DEFINE_MUTEX(ioapic_list_lock);
>
> +static inline bool is_valid_mem_resource(struct resource *res)
> +{
> +       return !(res->flags & IORESOURCE_DISABLED) &&
> +               (res->flags & IORESOURCE_MEM);
> +}
> +
>  static acpi_status setup_res(struct acpi_resource *acpi_res, void *data)
>  {
> +       struct acpi_resource_address64 addr;
>         struct resource *res = data;
>
>         memset(res, 0, sizeof(*res));
> -       if (acpi_dev_resource_memory(acpi_res, res)) {
> -               res->flags &= IORESOURCE_MEM;
> -               if (res->flags)
> -                       return AE_OK;
> -       } else if (acpi_dev_resource_address_space(acpi_res, res)) {
> -               struct acpi_resource_address64 addr;
> +       if (acpi_dev_resource_memory(acpi_res, res))
> +               return AE_OK;
>
> -               res->flags &= IORESOURCE_MEM;
> -               if (res->flags &&
> -                   ACPI_SUCCESS(acpi_resource_to_address64(acpi_res, &addr)) &&
> -                   addr.info.mem.caching != ACPI_PREFETCHABLE_MEMORY) {
> -                       res->start += addr.translation_offset;
> -                       res->end += addr.translation_offset;
> -                       return AE_OK;
> -               }
> +       if (!acpi_dev_resource_address_space(acpi_res, res) ||
> +           !is_valid_mem_resource(res))
> +               return AE_OK;
> +       /*
> +        * FIXME: This lacks a proper comment, why the resource
> +        * address needs to be translated.
> +        */
> +       if (ACPI_SUCCESS(acpi_resource_to_address64(acpi_res, &addr)) &&
> +           addr.info.mem.caching != ACPI_PREFETCHABLE_MEMORY) {
> +               res->start += addr.translation_offset;
> +               res->end += addr.translation_offset;
>         }
> -       res->flags = 0;
> -
>         return AE_OK;
>  }
>
> @@ -150,7 +154,7 @@ static acpi_status handle_ioapic_add(acp
>
>                 res = &ioapic->res;
>                 acpi_walk_resources(handle, METHOD_NAME__CRS, setup_res, res);
> -               if (res->flags == IORESOURCE_UNSET) {
> +               if (!is_valid_mem_resource(res)) {
>                         acpi_handle_warn(handle, "failed to get resource\n");
>                         goto exit_free;
>                 } else if (request_resource(&iomem_resource, res)) {
>
>

There is minor problem about mem pref handling, original code will ignore them.

with this patch will let it follow through.

should change is_valid_mem_resource to is_valid_mem_nonpref_resource()...

+static inline bool is_valid_mem_nonpref_resource(struct resource *res)
 {
        return !(res->flags & IORESOURCE_DISABLED) &&
-               (res->flags & IORESOURCE_MEM);
+               (res->flags & IORESOURCE_MEM) &&
+               !(res->flags & IORESOURCE_PREFETCH);
 }


other than that for whole patch set:
Acked-by: Yinghai Lu <yinghai@...nel.org>

Also please check attached following patch that will move code to acpi core.

Thanks

Yinghai

View attachment "x86_pci_acpi_5.patch" of type "text/x-patch" (7448 bytes)