On 10/20/15 18:27, Laszlo Ersek wrote:
> On 10/20/15 18:24, Laszlo Ersek wrote:
>> On 10/20/15 16:37, Laszlo Ersek wrote:
>>
>>> The variable access fails *iff* the access is made by a VCPU that is
>>> *not* VCPU#0.
>>>
>>> I added a bunch of debug messages to the following functions:
>>>
>>> - InitCommunicateBuffer(), SendCommunicateBuffer()
>>> [MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.c]
>>>
>>> - BSPHandler(), APHandler()
>>> [UefiCpuPkg/PiSmmCpuDxeSmm/MpService.c]
>>>
>>> Now, when the "efibootmgr" utility is run on CPU#0, everything works:
>>>
>>>> [root@ovmf-fedlet ~]# taskset -c 0 efibootmgr
>>>> BootCurrent: 0004
>>>> Timeout: 0 seconds
>>>> BootOrder: 0004,0003
>>>> Boot0003* EFI Internal Shell
>>>> Boot0004* fedlet grub
>>>
>>> and the OVMF debug log contains a sequence of the following snippets --
>>> the snippet is repeated once per variable:
>>>
>>>> InitCommunicateBuffer: Function=1
>>>> InitCommunicateBuffer: Status=Success
>>>> BSPHandler: 318: CpuIndex=0
>>>> BSPHandler: 544: CpuIndex=0
>>>> SendCommunicateBuffer: ReturnStatus=Buffer Too Small
>>>>
>>>> InitCommunicateBuffer: Function=1
>>>> InitCommunicateBuffer: Status=Success
>>>> BSPHandler: 318: CpuIndex=0
>>>> BSPHandler: 544: CpuIndex=0
>>>> SendCommunicateBuffer: ReturnStatus=Success
>>>
>>> Here Function=1 is
>>>
>>> SMM_VARIABLE_FUNCTION_GET_VARIABLE
>>> [MdeModulePkg/Include/Guid/SmmVariableCommon.h]
>>>
>>> It is one of the message types that the "unprivileged" runtime variable
>>> driver composes for the "privileged" SMM variable driver. In the above
>>> log, you can see
>>> (1) the message buffer being initialized for this message type -- in the
>>> unprivileged driver --,
>>> (2) then the BSPHandler() function starting and completing (in SMM),
>>> (3) finally the unprivileged driver looking at the status code coming
>>> back, in the message buffer, from the privileged driver.
>>>
>>> The first triplet ends with "Buffer Too Small", because efibootmgr --
>>> apparently -- only queries the buffer size it needs to allocate for
>>> fetching the variable's contents. The second triplet succeeds.
>>>
>>> Side note: when testing variable access from a UEFI application or the
>>> UEFI shell directly, the code *always* runs on VCPU#0! That's why such
>>> testing always succeeds.
>>>
>>> Now, here's what happens when the same is attempted from VCPU#1:
>>>
>>>> [root@ovmf-fedlet ~]# taskset -c 1 efibootmgr
>>>> efibootmgr: efibootmgr: Invalid argument
>>>
>>> and the matching OVMF debug log is:
>>>
>>>> InitCommunicateBuffer: Function=1
>>>> InitCommunicateBuffer: Status=Success
>>>> SendCommunicateBuffer: ReturnStatus=Buffer Too Small
>>>>
>>>> InitCommunicateBuffer: Function=1
>>>> InitCommunicateBuffer: Status=Success
>>>> SendCommunicateBBSPHandler: 318: CpuIndex=0
>>>> BSPHandler: 544: CpuIndex=0
>>>> uffer: ReturnStatus=Buffer Too Small
>>>
>>> Observations:
>>> - the first triplet seems to complete similarly, however we don't see
>>> either BSPHandler or APHandler logs for it
>>>
>>> - the second triplet fails too (which is why efibootmgr reports EINVAL
>>> -- the second call uses the right variable buffer size, so it should
>>> succeed)
>>>
>>> - there are no APHandler() log entries at all, despite the efibootmgr
>>> process being bound to VCPU#1. Also note that BSPHandler logs
>>> CpuIndex=0.
>>>
>>> This *cannot* be right. CpuIndex should be 1 in SmiRendezvous(),
>>> APHandler() should be entered, and APHandler() should bring in the
>>> BSP. The BSP should finally dispatch the SMI to the SMM core and the
>>> variable driver.
>>
>> I think I have found the bug -- it's in QEMU.
>>
>> Namely, in ich9_apm_ctrl_changed(), the SMI is *always* injected on the
>> first CPU:
>>
>> /* SMI_EN = PMBASE + 30. SMI control and enable register */
>> if (lpc->pm.smi_en & ICH9_PMIO_SMI_EN_APMC_EN) {
>> cpu_interrupt(first_cpu, CPU_INTERRUPT_SMI);
>> }
>>
>> There's another similar function in this file, ich9_generate_smi(),
>> which similarly insists on the first CPU only, but that should be
>> alright. That function comes from:
>>
>> commit 920557971b60e53c2f3f22e5d6c620ab1ed411fd
>> Author: Paulo Alcantara <[email protected]>
>> Date: Sun Jun 28 14:58:56 2015 -0300
>>
>> ich9: add TCO interface emulation
>>
>> and is only called from "hw/acpi/tco.c".
>>
>> A watchdog device may plausibly be permitted to inject the SMI on the
>> first CPU only, but for a synchronously triggered SMI, the SMI should be
>> injected on the exact CPU that triggers it.
>>
>> The bug in ich9_apm_ctrl_changed() dates back to:
>>
>> commit 4d00636e97b7f55810ff7faccff594159175e24e
>> Author: Jason Baron <[email protected]>
>> Date: Wed Nov 14 15:54:05 2012 -0500
>>
>> ich9: Add the lpc chip
>>
>> Paolo, do you have an idea how the executing CPU's identity could be
>> preserved from the exec-stuff until the device emulation code? I think
>> it is trivial to know for someone who knows, but I don't know (yet). :)
>> With that info surviving, ich9_apm_ctrl_changed() should be modified to
>> inject the SMI on the subject CPU.
>
> Can I simply rely on the global variable "current_cpu"?
Said one-liner QEMU patch makes variable access work from VCPU#1 as
well; APHandler is correctly entered and the synchronization occurs.
> InitCommunicateBuffer: Function=1
> InitCommunicateBuffer: Status=Success
> APHandler: 567: CpuIndex=1
> BSPHandler: 318: CpuIndex=0 &CpuIndex=[7FFC6FA8]
> APHandler: 747: CpuIndex=1
> BSPHandler: 544: CpuIndex=0
> SendCommunicateBuffer: ReturnStatus=Buffer Too Small
>
> InitCommunicateBuffer: Function=1
> InitCommunicateBuffer: Status=Success
> APHandler: 567: CpuIndex=1
> BSPHandler: 318: CpuIndex=0 &CpuIndex=[7FFC6FA8]
> APHandler: 747: CpuIndex=1
> BSPHandler: 544: CpuIndex=0
> SendCommunicateBuffer: ReturnStatus=Success
One drawback is that APHandler() runs the familiar SyncTimer up to two
times, *regardless* of the SyncMode PCD. Each loop takes
PcdCpuSmmApSyncTimeout microseconds, which (using the UefiCpuPkg
defaults) means 1 full second.
That makes variable access very slow. "taskset -c 0 efibootmgr" takes
about half a second (wall clock time measured within the guest with the
"time" command); whereas "taskset -c 1 efibootmgr" takes 35 (!) seconds.
I don't know how this is supposed to work on physical hardware.
Also... S3 suspend / resume seems to work per se, but variable access
stops working after resume (even on VCPU#0) -- any such attempts hang.
But that's for another day.
For now I'll submit the QEMU patch.
Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
