On Mon, 2 Jun 2014 16:29:32 +0200
Vasilis Liaskovitis <vasilis.liaskovitis@xxxxxxxxxxxxxxxx> wrote:
> On Thu, May 29, 2014 at 11:12:37AM +0200, Igor Mammedov wrote:
> > On Wed, 28 May 2014 18:38:13 +0200
> > Vasilis Liaskovitis <vasilis.liaskovitis@xxxxxxxxxxxxxxxx> wrote:
> >
> > > On Wed, May 28, 2014 at 03:26:42PM +0200, Igor Mammedov wrote:
> > > > On Wed, 28 May 2014 14:23:13 +0200
> > > > Vasilis Liaskovitis <vasilis.liaskovitis@xxxxxxxxxxxxxxxx> wrote:
> > > >
> > > > > On Wed, May 28, 2014 at 10:07:22AM +0200, Igor Mammedov wrote:
> > > > > > On Tue, 27 May 2014 17:57:31 +0200
> > > > > > Anshul Makkar <anshul.makkar@xxxxxxxxxxxxxxxx> wrote:
> > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > I tested the hot unplug patch and doesn't seem to work properly with Debian
> > > > > > > 6 and Ubuntu host.
> > > > > > >
> > > > > > > Scenario:
> > > > > > > I added 3 dimm devices of 1G each:
> > > > > > >
> > > > > > > object_add memory-ram,id=ram0,size=1G, device_add dimm,id=dimm1,memdev=ram0
> > > > > > >
> > > > > > > object_add memory-ram,id=ram1,size=1G, device_add dimm,id=dimm2,memdev=ram1
> > > > > > >
> > > > > > > object_add memory-ram,id=ram2,size=1G, device_add dimm,id=dimm3,memdev=ram2
> > > > > > >
> > > > > > > device_del dimm3: I get the OST EVENT EJECT 0x3 and OST STATUS as 0x84(IN
> > > > > > > PROGRESS) If I check on the guest, the device has been successfully
> > > > > > > removed. But no OST EJECT SUCCESS event was received.
> > > > > > I think there should be a SUCCESS event,
> > > > > > it should be investigated from the guest side first, OST support in kernel
> > > > > > is relatively new.
> > > > >
> > > > > When testing older guest kernel (3.11), _OST success events are not sent
> > > > > from the guest. I haven't tried newer versions yet.
> > > > >
> > > > > In terms of OSPM _OST behaviour, I am not sure if returning OST success status
> > > > > on succcesful removal is *required*. Figure 6-37, page 306 of ACPI spec5.0
> > > > > shows that on succcesfull OS ejection ejection, _EJ0 is evaluated. Evaluating
> > > > > _OST does not seem to be a requirement, is it? (cc'ing linux-acpi for input)
> > > > >
> > > > > In linux guests, on successful removal, _EJ0 is always evaluated. I believe we
> > > > > should be handling _EJ0 and doing the dimm removal (object_unparent) there.
> > > > > Currently OST successes are never received and dimm devices remain in QEMU even
> > > > > when successfully ejected from guest.
> > > > > E.g. a quick patch for _EJ0 handling, on top of Hu Tao's series:
> > > > >
> > The same register [0x14] is control register when writing there, so we can use-reuse
> > the same bit position as MRMV for signaling QEMU to perform eject on writing 1 there,
> > similarly like it's done for insert event.
>
> thanks, on a closer look that makes sense. MRMV is sufficient, example patch
> below.
>
> With the new "query-acpi-ospm-status" we can read OST status from QMP. However
> as I mentioned, on succesful removal, the guest kernel does not send the OST success
> status (0x0).
>
> This leads to the scenario where memory device is succesfully ejected, _EJ0 is
> evaluated in guest and device is deleted in qemu (with the patch below). The
> dimm will no longer show up in "query-memory-devices", however the ospm status
> of the dimm slot will still remain at 0x84 (ejection in progress). This can
> be confusing to management layers. Do you have any suggestion for how to
> report the succesful _EJ0? We could simply write a succesful status:
>
> mdev->ost_status = 0x0
>
> when MRMV is written to, but it is a bit hacky. On the other hand, having another
> command only for _EJ0 notification sounds like overkill.
I'd go with your suggestion, it provides management with necessary information
without complicating interface further needlessly.
>
> In terms of linux kernel ACPI conformity, Figure 6-37 in the spec implies that
> _OST evaluation is not required after succesfull _EJ0 evaluation. Also see
> relevant comment in drivers/acpi/scan.c: acpi_scan_hot_remove() line 378
> (3.15-rc8):
> /*
> * Verify if eject was indeed successful. If not, log an error
> * message. No need to call _OST since _EJ0 call was made
> * OK.
> */
>
>
>
> acpi, memory-hotplug: Add _EJ0 handling
>
> ---
> docs/specs/acpi_mem_hotplug.txt | 3 ++-
> hw/acpi/memory_hotplug.c | 9 +++------
> 2 files changed, 5 insertions(+), 7 deletions(-)
>
> diff --git a/docs/specs/acpi_mem_hotplug.txt b/docs/specs/acpi_mem_hotplug.txt
> index 1290994..4501774 100644
> --- a/docs/specs/acpi_mem_hotplug.txt
> +++ b/docs/specs/acpi_mem_hotplug.txt
> @@ -35,7 +35,8 @@ Memory hot-plug interface (IO port 0xa00-0xa17, 1-4 byte access):
> 1: if set to 1 clears device insert event, set by OSPM
> after it has emitted device check event for the
> selected memory device
> - 2-7: reserved, OSPM must clear them before writing to register
> + 2: Device no longer used by guest, can be safely removed
> + 3-7: reserved, OSPM must clear them before writing to register
>
> Selecting memory device slot beyond present range has no effect on platform:
> - write accesses to memory hot-plug registers not documented above are
> diff --git a/hw/acpi/memory_hotplug.c b/hw/acpi/memory_hotplug.c
> index 8aa829d..24989d3 100644
> --- a/hw/acpi/memory_hotplug.c
> +++ b/hw/acpi/memory_hotplug.c
> @@ -93,9 +93,6 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
> case 0x03: /* EJECT */
> switch (mdev->ost_status) {
> case 0x0: /* SUCCESS */
> - object_unparent(OBJECT(mdev->dimm));
> - mdev->is_removing = false;
> - mdev->dimm = NULL;
> break;
> case 0x1: /* FAILURE */
> case 0x2: /* UNRECOGNIZED NOTIFY */
> @@ -115,9 +112,6 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
> case 0x103: /* OSPM EJECT */
> switch (mdev->ost_status) {
> case 0x0: /* SUCCESS */
> - object_unparent(OBJECT(mdev->dimm));
> - mdev->is_removing = false;
> - mdev->dimm = NULL;
> break;
> case 0x84: /* EJECTION IN PROGRESS */
> mdev->is_enabled = false;
> @@ -135,6 +129,9 @@ static void acpi_memory_hotplug_write(void *opaque, hwaddr addr, uint64_t data,
> trace_mhp_acpi_clear_insert_evt(mem_st->selector);
> } else if (data & 4) { /* MRMV */
> mdev->is_enabled = false;
> + object_unparent(OBJECT(mdev->dimm));
> + mdev->is_removing = false;
> + mdev->dimm = NULL;
> }
> break;
> }
>
>
--
Regards,
Igor
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
