linux-kernel - Re: [PATCHv7 21/30] x86/acpi, x86/boot: Add multiprocessor wake-up support

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: <2847763c-6202-9e2a-54c5-44c761b59a63@intel.com>
Date:   Fri, 18 Mar 2022 11:23:59 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        luto@...nel.org, peterz@...radead.org
Cc:     sathyanarayanan.kuppuswamy@...ux.intel.com, aarcange@...hat.com,
        ak@...ux.intel.com, dan.j.williams@...el.com, david@...hat.com,
        hpa@...or.com, jgross@...e.com, jmattson@...gle.com,
        joro@...tes.org, jpoimboe@...hat.com, knsathya@...nel.org,
        pbonzini@...hat.com, sdeep@...are.com, seanjc@...gle.com,
        tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
        thomas.lendacky@....com, brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org,
        Sean Christopherson <sean.j.christopherson@...el.com>,
        "Rafael J . Wysocki" <rafael.j.wysocki@...el.com>
Subject: Re: [PATCHv7 21/30] x86/acpi, x86/boot: Add multiprocessor wake-up
 support

On 3/18/22 08:30, Kirill A. Shutemov wrote:
> +	/*
> +	 * After writing the wakeup command, wait for maximum timeout of 0xFF
> +	 * for firmware to reset the command address back zero to indicate
> +	 * the successful reception of command.
> +	 * NOTE: 0xFF as timeout value is decided based on our experiments.
> +	 *
> +	 * XXX: Change the timeout once ACPI specification comes up with
> +	 *      standard maximum timeout value.
> +	 */
> +	timeout = 0xFF;
> +	while (READ_ONCE(acpi_mp_wake_mailbox->command) && --timeout)
> +		cpu_relax();

I don't feel like this was ever actually resolved.  This timeout
basically boiled down to "this value seems to work for us".  There are
also *SURELY* timeouts that are going to happen here.

In the end, the host has to choose to run the guest.  If it decides for
some odd reason not to run the CPU that's being booted, we'll get timeouts.

The other thing that baffles me is the re-wakeup bits:

>         /*
>          * If the CPU wakeup process is successful, store the
>          * status in apic_id_wakemap to prevent re-wakeup
>          * requests.
>          */
>         physid_set(apicid, apic_id_wakemap);
> 
>         return 0;
> }

If this goes wrong, won't the new wakeup just timeout?  Do we really
need a dedicated mechanism to stop re-wakeups?  How much of a problem is
this going to be?

Anyway, see the totally untested hunk I've attached.  It fixes a bunch
of issues IMNHO.
View attachment "0001-x86-acpi-x86-boot-Add-multiprocessor-wake-up-support.patch" of type "text/x-patch" (8163 bytes)