Re: [Qemu-devel] [RESEND PATCH v4 6/6] acpi: Add hardware implementation for memory hot unplug

[Zhu Guihua <zhugh.fnst@cn.fujitsu.com>]

On 03/23/2015 08:47 PM, Igor Mammedov wrote:
> On Mon, 23 Mar 2015 18:59:28 +0800
> Zhu Guihua <zhugh.fnst@cn.fujitsu.com> wrote:
>
>> On 03/16/2015 10:59 PM, Igor Mammedov wrote:
>> [...]
>>>    
>>> diff --git a/hw/i386/acpi-dsdt-mem-hotplug.dsl b/hw/i386/acpi-dsdt-mem-hotplug.dsl
>>> index 1e9ec39..ef847e2 100644
>>> --- a/hw/i386/acpi-dsdt-mem-hotplug.dsl
>>> +++ b/hw/i386/acpi-dsdt-mem-hotplug.dsl
>>> @@ -29,6 +29,7 @@
>>>                External(MEMORY_SLOT_PROXIMITY, FieldUnitObj) // read only
>>>                External(MEMORY_SLOT_ENABLED, FieldUnitObj) // 1 if enabled, read only
>>>                External(MEMORY_SLOT_INSERT_EVENT, FieldUnitObj) // (read) 1 if has a insert event. (write) 1 to clear event
>>> +            External(MEMORY_SLOT_REMOVE_EVENT, FieldUnitObj) // (read) 1 if has a remove event. (write) 1 to clear event
>>>                External(MEMORY_SLOT_SLECTOR, FieldUnitObj) // DIMM selector, write only
>>>                External(MEMORY_SLOT_OST_EVENT, FieldUnitObj) // _OST event code, write only
>>>                External(MEMORY_SLOT_OST_STATUS, FieldUnitObj) // _OST status code, write only
>>> @@ -55,6 +56,8 @@
>>>                        If (LEqual(MEMORY_SLOT_INSERT_EVENT, One)) { // Memory device needs check
>>>                            MEMORY_SLOT_NOTIFY_METHOD(Local0, 1)
>>>                            Store(1, MEMORY_SLOT_INSERT_EVENT)
>>> +                    } Elseif (LEqual(MEMORY_SLOT_REMOVE_EVENT, One)) { // Ejection request
>>> +                        MEMORY_SLOT_NOTIFY_METHOD(Local0, 3)
>>> clear removing field here.
>> You mean clear remove event here?
> yes

I tested this method, clear remove event here will lead to guest
kernel panic.

>>>>                        }
>>>>                        // TODO: handle memory eject request
>>>>                        Add(Local0, One, Local0) // goto next DIMM
>>>> @@ -156,5 +159,12 @@
>>>>                    Store(Arg2, MEMORY_SLOT_OST_STATUS)
>>>>                    Release(MEMORY_SLOT_LOCK)
>>>>                }
>>>> +
>>>> +            Method(MEMORY_SLOT_EJECT_METHOD, 2) {
>>>> +                Acquire(MEMORY_SLOT_LOCK, 0xFFFF)
>>>> +                Store(ToInteger(Arg0), MEMORY_SLOT_SLECTOR) // select DIMM
>>>> +                Store(One, MEMORY_SLOT_REMOVE_EVENT)
>>> redo it using enabled field. Otherwise it could cause guest/QEMU crash:
>>>
>>> - if 1st memory was asked to be removed
>>> - then OSPM processes it but has not called _EJ0 yet leaving is_removed set
>>> - then QEMU adds/removes another DIMM triggering slots scan
>>>      which would issue second Notify(remove) since is_removed is still set
>>> - as result it will cause failure in OSPM or in QEMU if OSPM issues double EJ0()
>>>
>> If OSPM processed the ejection request but not called _EJ0, the device
>> will still be present in qemu,
>> we must handle this.
> There is nothing to handle in this case, if OSPM hasn't called _EJ0 then
> nothing happens and device stays in QEMU.
>
>> So I think OSPM issues double EJ0 maybe reasonable
>> in this situation.
>> What's your opinion?
> the first _EJ0 must do ejection, as for the second I think it should be NOP.

So we should judge the enabled field to check whether the device is 
present before
issuing Notify(remove)?

Thanks,
Zhu

>> Thanks,
>> Zhu
>>
> [...]
> .
>