diff mbox series

[1/3] x86/smpboot: Re-position the call to tboot_wake_ap()

Message ID 20210115231046.31785-2-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86: Initial Trenchboot/SKINIT support | expand

Commit Message

Andrew Cooper Jan. 15, 2021, 11:10 p.m. UTC
So all the moving parts are in one function.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Marek Kasiewicz <marek.kasiewicz@3mdeb.com>
CC: Norbert Kamiński <norbert.kaminski@3mdeb.com>
CC: Michal Zygowski <michal.zygowski@3mdeb.com>
CC: Piotr Krol <piotr.krol@3mdeb.co>
CC: Krystian Hebel <krystian.hebel@3mdeb.com>
CC: Daniel P. Smith <dpsmith@apertussolutions.com>
CC: Rich Persaud <persaur@gmail.com>
CC: Christopher Clark <christopher.w.clark@gmail.com>
---
 xen/arch/x86/smpboot.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Comments

Roger Pau Monné Jan. 19, 2021, 2:38 p.m. UTC | #1
On Fri, Jan 15, 2021 at 11:10:44PM +0000, Andrew Cooper wrote:
> So all the moving parts are in one function.
> 
> No functional change.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Marek Kasiewicz <marek.kasiewicz@3mdeb.com>
> CC: Norbert Kamiński <norbert.kaminski@3mdeb.com>
> CC: Michal Zygowski <michal.zygowski@3mdeb.com>
> CC: Piotr Krol <piotr.krol@3mdeb.co>
> CC: Krystian Hebel <krystian.hebel@3mdeb.com>
> CC: Daniel P. Smith <dpsmith@apertussolutions.com>
> CC: Rich Persaud <persaur@gmail.com>
> CC: Christopher Clark <christopher.w.clark@gmail.com>
> ---
>  xen/arch/x86/smpboot.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index 67e727cebd..9eca452ce1 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -426,6 +426,13 @@ static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
>      int maxlvt, timeout, i;
>  
>      /*
> +     * Some versions of tboot might be able to handle the entire wake sequence
> +     * on our behalf.
> +     */
> +    if ( tboot_in_measured_env() && tboot_wake_ap(phys_apicid, start_eip) )

I think you are missing a ! in front of tboot_wake_ap?

Thanks, Roger.
Andrew Cooper Jan. 19, 2021, 2:55 p.m. UTC | #2
On 19/01/2021 14:38, Roger Pau Monné wrote:
> On Fri, Jan 15, 2021 at 11:10:44PM +0000, Andrew Cooper wrote:
>> So all the moving parts are in one function.
>>
>> No functional change.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Wei Liu <wl@xen.org>
>> CC: Marek Kasiewicz <marek.kasiewicz@3mdeb.com>
>> CC: Norbert Kamiński <norbert.kaminski@3mdeb.com>
>> CC: Michal Zygowski <michal.zygowski@3mdeb.com>
>> CC: Piotr Krol <piotr.krol@3mdeb.co>
>> CC: Krystian Hebel <krystian.hebel@3mdeb.com>
>> CC: Daniel P. Smith <dpsmith@apertussolutions.com>
>> CC: Rich Persaud <persaur@gmail.com>
>> CC: Christopher Clark <christopher.w.clark@gmail.com>
>> ---
>>  xen/arch/x86/smpboot.c | 10 ++++++++--
>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>
>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>> index 67e727cebd..9eca452ce1 100644
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -426,6 +426,13 @@ static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
>>      int maxlvt, timeout, i;
>>  
>>      /*
>> +     * Some versions of tboot might be able to handle the entire wake sequence
>> +     * on our behalf.
>> +     */
>> +    if ( tboot_in_measured_env() && tboot_wake_ap(phys_apicid, start_eip) )
> I think you are missing a ! in front of tboot_wake_ap?

Oh - so I am.  That function is totally backwards.

Fixed.

~Andrew
Jan Beulich Jan. 20, 2021, 9:11 a.m. UTC | #3
On 19.01.2021 15:55, Andrew Cooper wrote:
> On 19/01/2021 14:38, Roger Pau Monné wrote:
>> On Fri, Jan 15, 2021 at 11:10:44PM +0000, Andrew Cooper wrote:
>>> So all the moving parts are in one function.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>>> ---
>>> CC: Jan Beulich <JBeulich@suse.com>
>>> CC: Roger Pau Monné <roger.pau@citrix.com>
>>> CC: Wei Liu <wl@xen.org>
>>> CC: Marek Kasiewicz <marek.kasiewicz@3mdeb.com>
>>> CC: Norbert Kamiński <norbert.kaminski@3mdeb.com>
>>> CC: Michal Zygowski <michal.zygowski@3mdeb.com>
>>> CC: Piotr Krol <piotr.krol@3mdeb.co>
>>> CC: Krystian Hebel <krystian.hebel@3mdeb.com>
>>> CC: Daniel P. Smith <dpsmith@apertussolutions.com>
>>> CC: Rich Persaud <persaur@gmail.com>
>>> CC: Christopher Clark <christopher.w.clark@gmail.com>
>>> ---
>>>  xen/arch/x86/smpboot.c | 10 ++++++++--
>>>  1 file changed, 8 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>>> index 67e727cebd..9eca452ce1 100644
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -426,6 +426,13 @@ static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
>>>      int maxlvt, timeout, i;
>>>  
>>>      /*
>>> +     * Some versions of tboot might be able to handle the entire wake sequence
>>> +     * on our behalf.
>>> +     */
>>> +    if ( tboot_in_measured_env() && tboot_wake_ap(phys_apicid, start_eip) )
>> I think you are missing a ! in front of tboot_wake_ap?
> 
> Oh - so I am.  That function is totally backwards.
> 
> Fixed.

And then

Acked-by: Jan Beulich <jbeulich@suse.com>

Jan
diff mbox series

Patch

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 67e727cebd..9eca452ce1 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -426,6 +426,13 @@  static int wakeup_secondary_cpu(int phys_apicid, unsigned long start_eip)
     int maxlvt, timeout, i;
 
     /*
+     * Some versions of tboot might be able to handle the entire wake sequence
+     * on our behalf.
+     */
+    if ( tboot_in_measured_env() && tboot_wake_ap(phys_apicid, start_eip) )
+        return 0;
+
+    /*
      * Be paranoid about clearing APIC errors.
      */
     apic_write(APIC_ESR, 0);
@@ -570,8 +577,7 @@  static int do_boot_cpu(int apicid, int cpu)
     set_cpu_state(CPU_STATE_INIT);
 
     /* Starting actual IPI sequence... */
-    if ( !tboot_in_measured_env() || tboot_wake_ap(apicid, start_eip) )
-        boot_error = wakeup_secondary_cpu(apicid, start_eip);
+    boot_error = wakeup_secondary_cpu(apicid, start_eip);
 
     if ( !boot_error )
     {