|
From: | Tang Chen |
Subject: | Re: [Qemu-devel] [RESEND PATCH v3 6/8] acpi: Add hardware implementation for memory hot unplug. |
Date: | Tue, 16 Sep 2014 18:12:02 +0800 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.1.0 |
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;.
[Prev in Thread] | Current Thread | [Next in Thread] |