Re: [PATCH] x86/apic: Don't disable x2APIC if locked

[Daniel Sneddon]

On 8/10/22 17:38, Thomas Gleixner wrote:
> On Wed, Aug 10 2022 at 17:01, Daniel Sneddon wrote:
>> On 8/10/22 16:44, Dave Hansen wrote:
>>> On 8/10/22 16:38, Daniel Sneddon wrote:
>>>>>
>>>>> config INTEL_TDX_GUEST
>>>>>         bool "Intel TDX (Trust Domain Extensions) - Guest Support"
>>>>>         depends on X86_64 && CPU_SUP_INTEL
>>>>>         depends on X86_X2APIC
>>>> So I got some more input.  SPR and newer will lock the APIC.  
>>>
>>> Could you get a _little_ more clarity on this, please?  Exactly how and
>>> when will it be locked?  What does the BIOS writer's guide say?  Will
>>> there be an explicit x2APIC lock option?  Or, will it be implicitly
>>> locked when SGX or TDX is enabled?
>>
>> The BIOS doesn't explicitly lock the APIC.  The APIC will be locked if X2APIC
>> mode is enabled when the BIOS does an MCHECK.  X2APIC mode will be enabled if
>> SGX or TDX are enabled.  So when exactly does the BIOS do an MCHECK?  That I'll
>> have to get clarification on.
> 
> Sorry, this is uncomprehensible word salad and none of this makes any
> sense at all to me.
> 
> Can you please explain this in comprehensible sentences understandable
> by mere mortals?

Basically if the BIOS is configured to enable SGX or TDX, it'll program the APIC
to use x2APIC mode.  At some point it'll have to run MCHECK (which is just an
MSR write).  When exactly the BIOS does this, I'm not sure.  I've asked for
clarification on that.  At the point the BIOS does the MCHECK, if X2APIC mode is
enabled, the ucode will set the LOCK bit, and any attempt after that to disable
the APIC will result in the fault.  Now, this assumes the BIOS will DTRT, and
always enable x2APIC when SGX or TDX are enabled AND always perform the MCHECK,
AND do those things in the right order.  I'm not a BIOS guy so I have no idea
where to even look to see if/where that is documented.  I can certainly try to
track that down if needed.

I'm not sure if that's any clearer?  I guess I could try some code:

if (SGX_enabled() || TDX_enabled()
	set_x2apic_mode();

.....

MCHECK <-----At this point if x2APIC mode is on then the ucode set's the lock bit.

.....


Hope that helps.

> 
> Thanks,
> 
>         tglx