Re: [Qemu-devel] [PATCH] hw/isa/lpc_ich9: inject the SMI on the VCPU that is writing to APM_CNT

[Laszlo Ersek]

On 10/23/15 20:20, Jordan Justen wrote:
> On 2015-10-23 05:53:17, Laszlo Ersek wrote:
>> On 10/23/15 09:26, Paolo Bonzini wrote:
>>>
>>> On 23/10/2015 06:41, Jordan Justen wrote:
>>>> On 2015-10-22 12:46:56, Paolo Bonzini wrote:
>>>>>
>>>>> On 22/10/2015 20:04, Kevin O'Connor wrote:
>>>>>> On Thu, Oct 22, 2015 at 10:40:08AM +0200, Paolo Bonzini wrote:
>>>>>>> On 21/10/2015 20:36, Jordan Justen wrote:
>>>>>>>> On 2015-10-20 11:14:00, Laszlo Ersek wrote:
>>>>>>>>> Commit 4d00636e97b7 ("ich9: Add the lpc chip", Nov 14 2012) added the
>>>>>>>>> ich9_apm_ctrl_changed() ioport write callback function such that it 
>>>>>>>>> would
>>>>>>>>> inject the SMI, in response to a write to the APM_CNT register, on the
>>>>>>>>> first CPU, invariably.
>>>>>>>>>
>>>>>>>>> Since this register is used by guest code to trigger an SMI 
>>>>>>>>> synchronously,
>>>>>>>>> the interrupt should be injected on the VCPU that is performing the 
>>>>>>>>> write.
>>>>>>>>
>>>>>>>> Why not send an SMI to *all* processors, like the real chipsets do?
>>>>>>>
>>>>>>> That's much less scalable, and more important I would have to check that
>>>>>>> SeaBIOS can handle that correctly.  It probably doesn't, as it doesn't
>>>>>>> relocate SMBASEs.
>>>>>>
>>>>>> SeaBIOS is only expecting its SMI handler to be called once in
>>>>>> response to a synchronous SMI.  We can change SeaBIOS to fix that.
>>>>>>
>>>>>> SeaBIOS does relocate the smbase from 0x30000 to 0xa0000 during its
>>>>>> init phase (by creating a synchronous SMI on the BSP and then setting
>>>>>> the smbase register to 0xa0000 in the smi handler).
>>>>>
>>>>> Right; however it would also have to relocate the SMBASE on the APs (in
>>>>> case they were halted with cli;hlt and not INITed).  It's really not
>>>>> worth the hassle,
>>>>
>>>> It's not worth the hassle to relocate the SMBASE of the APs?
>>>> So, basically, write to 0x30000-0x38000, then send an SMI IPI to the
>>>> AP and now you have the AP running in SMI and it has extra privileges?
>>>
>>> Extra privileges compared to what?  Legacy BIOS does not really put
>>> anything privileged in SMRAM,
> 
> Why does seabios even bother relocating the BSP's SMBASE if it doesn't
> relocate the SMBASE for the APs?
> 
>>> while OVMF does and _hence_ relocates the
>>> SMBASE of the AP.  It would have been nice to get it right from the
>>> beginning, but right now it's not worth forcing a lockstep QEMU-SeaBIOS
>>> update.
>>
>> So what are we thinking about a magic APM_STS value to trigger an SMI
>> for all VCPUs? 0x51 ('Q') would be cool. :)
> 
> This seems like a further deviation from the actual hardware. I
> understand that QEMU draws a line about strict hardware emulation, but
> I just wanted to point out the discrepancy.

I'm completely okay if we control this behavior with another method, for
example another -machine option, like "-machine smi=current" vs.
"-machine smi=all".

I needed something to work with, and the question should be discussed on
the list, so I think the patch was good for that at least.

BTW I have some incredible news to report, but that will take a separate
email. :)

Thanks
Laszlo

> 
> So, the trouble with changing QEMU to better emulate the hardware is
> that seabios can't handle multiple processors entering SMM?
> 
> I think if SMM does anything interesting, then it is basically a
> requirement for all processors to enter SMM together. If not, you have
> an OS running that just lost one its processors. Maybe the OS will
> decide it try to restart the processor to regain control. Maybe the OS
> will try to talk to some chipset hardware in a way that will conflict
> with whatever the processor in SMM is doing (or vice-versa).
> 
> -Jordan
>