Hi Igor,
On 09/04/2014 10:20 PM, Igor Mammedov wrote:
......
+
+ acpi_handle_ost_event(mdev);
_OST is optional and OSPM doesn't have to call it at all,
it was already discussed on list and using _OST for device removal
was evaluated as not usable.
We use _OST here as supplementary status information for management tools
and no more /i.e. as LEDs on real hardware/.
See "Figure 6-37 Device Ejection Flow Example Using _OST." in ACPI 5.1 spec
and note below it.
OK, I agree we should not make memory hotplug process depend on _OST.
But we do need to handle one problem: When guest OS failed to remove
device,
we should not clear QEmu data of that device.
As you and ACPI spec said, Platform (which is QEmu here) should reply _OST
rised by guest OS except things such as LEDs on real hardware. But _OST is
the only way I can find to get status from guest OS.
So, let's do it in the following way:
1. Make device hotplug process independent from _OST for the
compatibility reason.
2. For OSPMs that support _OST, QEmu use _OST to catch error from guest OS.
May be report an error message only, and stop QEmu from remove
related data.
(Please see/review patch-set:
https://www.mail-archive.com/qemu-devel%40nongnu.org/msg253025.html)
3. For OSPMs that do not support _OST, do not handle guest OS error for now.
How do you think ?
Thanks.
break;
- case 0x14:
+ case 0x14: /* set is_* fields */
mdev = &mem_st->devs[mem_st->selector];
+
if (data & 2) { /* clear insert event */
mdev->is_inserting = false;
trace_mhp_acpi_clear_insert_evt(mem_st->selector);
+ } else if (data & 4) { /* request removal of device */
+ mdev->is_enabled = false;
device tear-down should be initiated here, when OSPM calls _EJ0 method
and when we return from this function it should be destroyed.
}
+
+ break;
+ default:
break;
}
-
}
static const MemoryRegionOps acpi_memory_hotplug_ops = {
.read = acpi_memory_hotplug_read,
@@ -198,6 +250,7 @@ void acpi_memory_plug_cb(ACPIREGS *ar, qemu_irq irq,
MemHotplugState *mem_st,
void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState
*mem_st,
DeviceState *dev, Error **errp)
{
+ MemStatus *mdev;
Error *local_err = NULL;
int slot = object_property_get_int(OBJECT(dev), "slot", &local_err);
@@ -215,6 +268,9 @@ void acpi_memory_unplug_cb(ACPIREGS *ar, qemu_irq irq, MemHotplugState *mem_st,
return;
}
+ mdev = &mem_st->devs[slot];
+ mdev->is_removing = true;
+
/* do ACPI magic */
ar->gpe.sts[0] |= ACPI_MEMORY_HOTPLUG_STATUS;
acpi_update_sci(ar, irq);
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index 1f678b4..e105e45 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -91,6 +91,20 @@
/* PM2_CNT */
#define ACPI_BITMASK_ARB_DISABLE 0x0001
+/* OST_EVENT */
+#define ACPI_NOTIFY_EJECT_REQUEST 0x03
+#define ACPI_OSPM_EJECT 0x103
+
+/* OST_STATUS */
+#define ACPI_SUCCESS 0x0
+#define ACPI_FAILURE 0x1
+#define ACPI_UNRECOGNIZED_NOTIFY 0x2
+#define ACPI_EJECT_NOT_SUPPORTED 0x80
+#define ACPI_EJECT_DEVICE_IN_USE 0x81
+#define ACPI_EJECT_DEVICE_BUSY 0x82
+#define ACPI_EJECT_DEPENDENCY_BUSY 0x83
+#define ACPI_EJECT_IN_PROGRESS 0x84
+
/* structs */
typedef struct ACPIPMTimer ACPIPMTimer;
typedef struct ACPIPM1EVT ACPIPM1EVT;
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index fc6b868..fe41268 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -11,6 +11,7 @@ typedef struct MemStatus {
DeviceState *dimm;
bool is_enabled;
bool is_inserting;
+ bool is_removing;
uint32_t ost_event;
uint32_t ost_status;
} MemStatus;
.