[Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[liguang]

These patches try to add ACPI Embedded Controller (EC),
refer-to:
ACPI SPEC v5 chapter 5 
"ACPI Embedded Controller Interface Specification"

EC is a standard ACPI device, it plays flexible roles,
e.g. 
power controller, it can control power sequence for
platform to enter or leave system state(0,1,3,4,5),
it can controller CPU fan by the temperature of sensor.
event carrier, it can pass events between platform
and OS, so OS can execute _Qxx method which defined
by yourself.

So, I want to deliver CPU online/offline event between
OS and QEMU for CPU hotplug feature, then we will don't
need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
again after 'cpu-add'.

patches for online/offline event handler of QEUM and 
linux kernel are writing, and will send once finished.

since EC is a common device, so I send pathes separately.

[Qemu-devel] [PATCH 1/4] acpi: add ACPI Embedded Controller support

[liguang]

this work implemented Embedded Controller chip emulation
which was defined at ACPI SEPC v5 chapter 12:
"ACPI Embedded Controller Interface Specification"

commonly Embedded Controller will emulate keyboard,
mouse, handle ACPI defined operations and some
low-speed devices like SMbus.

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 default-configs/x86_64-softmmu.mak |    1 +
 hw/acpi/Makefile.objs              |    1 +
 hw/acpi/ec.c                       |  192 ++++++++++++++++++++++++++++++++++++
 include/hw/acpi/ec.h               |   29 ++++++
 4 files changed, 223 insertions(+), 0 deletions(-)
 create mode 100644 hw/acpi/ec.c
 create mode 100644 include/hw/acpi/ec.h

diff --git a/default-configs/x86_64-softmmu.mak b/default-configs/x86_64-softmmu.mak
index 599b630..c654ae9 100644
--- a/default-configs/x86_64-softmmu.mak
+++ b/default-configs/x86_64-softmmu.mak
@@ -46,3 +46,4 @@ CONFIG_APIC=y
 CONFIG_IOAPIC=y
 CONFIG_ICC_BUS=y
 CONFIG_PVPANIC=y
+CONFIG_ACPI_EC=y
diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index a0b63b5..0936f17 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,2 +1,3 @@
 common-obj-$(CONFIG_ACPI) += core.o piix4.o ich9.o
+common-obj-$(CONFIG_ACPI_EC) += ec.o
 
diff --git a/hw/acpi/ec.c b/hw/acpi/ec.c
new file mode 100644
index 0000000..24c2d5a
--- /dev/null
+++ b/hw/acpi/ec.c
@@ -0,0 +1,192 @@
+#include "hw/acpi/ec.h"
+#include "hw/hw.h"
+#include "hw/isa/isa.h"
+#include "sysemu/sysemu.h"
+
+#define TYPE_EC_DEV "ACPI Embedded Controller"
+#define EC_DEV(obj) \
+    OBJECT_CHECK(ECState, (obj), TYPE_EC_DEV)
+
+static uint8_t ec_acpi_space[EC_ACPI_SPACE_SIZE] = {0};
+static uint8_t sci_event;
+static bool ec_enabled = false;
+
+typedef struct ECState {
+    ISADevice parent_obj;
+
+    uint8_t cmd;
+    uint8_t status;
+    uint8_t data;
+    uint8_t buf;
+    qemu_irq irq;
+    MemoryRegion io;
+} ECState;
+
+static NotifierList ec_sci_notifiers =
+    NOTIFIER_LIST_INITIALIZER(ec_sci_notifiers);
+void qemu_register_ec_sci_notifier(Notifier *notifier)
+{
+    notifier_list_add(&ec_sci_notifiers, notifier);
+}
+
+bool qemu_ec_enabled(void)
+{
+    return ec_enabled;
+}
+
+void ec_dev_init(ISABus *isabus)
+{
+    isa_create_simple(isabus, TYPE_EC_DEV);
+}
+
+void ec_acpi_event(uint8_t evt)
+{
+    sci_event = evt;
+}
+
+static void ec_generate_sci(void)
+{
+    notifier_list_notify(&ec_sci_notifiers, NULL);
+}
+
+static void acpi_data_write(void *opaque, hwaddr addr, uint64_t val,
+                            unsigned size)
+{
+    ECState *s = opaque;
+    uint8_t tmp = val & 0xff;
+
+    if (s->status & EC_ACPI_CMD) {
+        s->buf = tmp & 0xff;
+        s->status &= ~EC_ACPI_CMD;
+    } else {
+        switch (tmp) {
+        case EC_ACPI_CMD_READ:
+            if (tmp < EC_ACPI_SPACE_SIZE) {
+                s->data = ec_acpi_space[tmp];
+            }
+            s->status |= EC_ACPI_OBF;
+            break;
+        case EC_ACPI_CMD_WRITE:
+            if (tmp < EC_ACPI_SPACE_SIZE) {
+                ec_acpi_space[s->buf] = tmp;
+            }
+            break;
+        case EC_ACPI_CMD_QUERY:
+            s->data = sci_event;
+        default:
+            break;
+        }
+    }
+    ec_generate_sci();
+}
+
+static uint64_t acpi_data_read(void *opaque, hwaddr addr, unsigned size)
+{
+    ECState *s = opaque;
+
+    s->status &= ~(EC_ACPI_OBF | EC_ACPI_SCI);
+
+    return s->data;
+}
+
+static void acpi_cmd_write(void *opaque, hwaddr addr, uint64_t val, unsigned size)
+{
+    ECState *s = opaque;
+    bool is_cmd = false;
+
+    s->status |= EC_ACPI_CMD;
+
+    switch (val & 0xff) {
+    case EC_ACPI_CMD_READ:
+    case EC_ACPI_CMD_WRITE:
+        is_cmd = true;
+        break;
+    case EC_ACPI_BURST_EN:
+        s->status |= EC_ACPI_BST;
+        s->data = 0x90;
+        is_cmd = true;
+        break;
+    case EC_ACPI_BURST_DN:
+        s->status &= ~EC_ACPI_BST;
+        is_cmd = true;
+        break;
+    case EC_ACPI_CMD_QUERY:
+        is_cmd = true;
+    default:
+        break;
+    }
+    if (is_cmd) {
+        s->cmd = val & 0xff;
+        ec_generate_sci();
+        s->status |= EC_ACPI_SCI;
+    } else {
+        s->cmd = 0;
+    }
+}
+
+static uint64_t acpi_status_read(void *opaque, hwaddr addr, unsigned size)
+{
+    ECState *s = opaque;
+
+    return s->status;
+}
+
+static const MemoryRegionOps io62_io_ops = {
+    .write = acpi_data_write,
+    .read = acpi_data_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static const MemoryRegionOps io66_io_ops = {
+    .write = acpi_cmd_write,
+    .read = acpi_status_read,
+    .endianness = DEVICE_NATIVE_ENDIAN,
+    .impl = {
+        .min_access_size = 1,
+        .max_access_size = 1,
+    },
+};
+
+static int ec_dev_initfn(ISADevice *dev)
+{
+    ISADevice *isadev = dev;
+    ECState *s = EC_DEV(dev);
+
+/*    isa_init_irq(isadev, &s->irq, 0xb); */
+
+    memory_region_init_io(&s->io, &io62_io_ops, NULL, "ec-acpi-data", 1);
+    isa_register_ioport(isadev, &s->io, 0x62);
+
+    memory_region_init_io(&s->io + 1, &io66_io_ops, NULL, "ec-acpi-cmd", 1);
+    isa_register_ioport(isadev, &s->io + 1, 0x66);
+
+    return 0;
+}
+
+static void ec_class_initfn(ObjectClass *oc, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(oc);
+    ISADeviceClass *ic = ISA_DEVICE_CLASS(oc);
+
+    dc->no_user = 1;
+    ic->init = ec_dev_initfn;
+    ec_enabled = true;
+}
+
+static TypeInfo acpi_ec_info = {
+    .name = TYPE_EC_DEV,
+    .parent = TYPE_ISA_DEVICE,
+    .class_init = ec_class_initfn,
+    .instance_size = sizeof(ECState),
+};
+
+static void ec_register_type(void)
+{
+    type_register_static(&acpi_ec_info);
+}
+
+type_init(ec_register_type);
diff --git a/include/hw/acpi/ec.h b/include/hw/acpi/ec.h
new file mode 100644
index 0000000..3556acd
--- /dev/null
+++ b/include/hw/acpi/ec.h
@@ -0,0 +1,29 @@
+#ifndef __EC_H
+#define __EC_H
+
+#include "hw/i386/pc.h"
+#include "qemu/notify.h"
+
+#define EC_ACPI_SPACE_SIZE 0x80
+
+#define EC_ACPI_CMD_PORT 0x66
+#define EC_ACPI_DATA_PORT 0x62
+
+#define EC_ACPI_OBF 0x1
+#define EC_ACPI_IBF 0x2
+#define EC_ACPI_CMD 0x8
+#define EC_ACPI_BST 0x10
+#define EC_ACPI_SCI 0x20
+
+#define EC_ACPI_CMD_READ 0x80
+#define EC_ACPI_CMD_WRITE 0x81
+#define EC_ACPI_BURST_EN 0x82
+#define EC_ACPI_BURST_DN 0x83
+#define EC_ACPI_CMD_QUERY 0x84
+
+void qemu_register_ec_sci_notifier(Notifier *notifier);
+bool qemu_ec_enabled(void);
+void ec_dev_init(ISABus *isabus);
+void ec_acpi_event(uint8_t evt);
+
+#endif
-- 
1.7.2.5

[Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[liguang]

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 hw/acpi/core.c         |    8 ++++----
 hw/acpi/ich9.c         |    2 +-
 hw/acpi/piix4.c        |    2 +-
 include/hw/acpi/acpi.h |    4 ++--
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/hw/acpi/core.c b/hw/acpi/core.c
index 42eeace..38ddeb8 100644
--- a/hw/acpi/core.c
+++ b/hw/acpi/core.c
@@ -575,9 +575,9 @@ void acpi_gpe_reset(ACPIREGS *ar)
     memset(ar->gpe.en, 0, ar->gpe.len / 2);
 }
 
-static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
+static uint32_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
 {
-    uint8_t *cur = NULL;
+    uint32_t *cur = NULL;
 
     if (addr < ar->gpe.len / 2) {
         cur = ar->gpe.sts + addr;
@@ -592,7 +592,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
 
 void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
 {
-    uint8_t *cur;
+    uint32_t *cur;
 
     cur = acpi_gpe_ioport_get_ptr(ar, addr);
     if (addr < ar->gpe.len / 2) {
@@ -608,7 +608,7 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
 
 uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
 {
-    uint8_t *cur;
+    uint32_t *cur;
     uint32_t val;
 
     cur = acpi_gpe_ioport_get_ptr(ar, addr);
diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 4a17f32..582dbec 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -153,7 +153,7 @@ static int ich9_pm_post_load(void *opaque, int version_id)
      .info       = &vmstate_info_uint8,                              \
      .size       = sizeof(uint8_t),                                  \
      .flags      = VMS_ARRAY | VMS_POINTER,                          \
-     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
+     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
  }
 
 const VMStateDescription vmstate_ich9_pm = {
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index c4af1cc..3a7b669 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -205,7 +205,7 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)
      .info       = &vmstate_info_uint16,                             \
      .size       = sizeof(uint16_t),                                 \
      .flags      = VMS_SINGLE | VMS_POINTER,                         \
-     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
+     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
  }
 
 static const VMStateDescription vmstate_gpe = {
diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
index 635be7b..deca3ce 100644
--- a/include/hw/acpi/acpi.h
+++ b/include/hw/acpi/acpi.h
@@ -112,8 +112,8 @@ struct ACPIPM1CNT {
 struct ACPIGPE {
     uint8_t len;
 
-    uint8_t *sts;
-    uint8_t *en;
+    uint32_t *sts;
+    uint32_t *en;
 };
 
 struct ACPIREGS {
-- 
1.7.2.5

[Qemu-devel] [PATCH 3/4] ich9: add notifer for ec to generate sci

[liguang]

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 hw/acpi/ich9.c         |   15 +++++++++++++++
 include/hw/acpi/ich9.h |    1 +
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 582dbec..2ecde32 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -33,6 +33,7 @@
 #include "exec/address-spaces.h"
 
 #include "hw/i386/ich9.h"
+#include "hw/acpi/ec.h"
 
 //#define DEBUG
 
@@ -43,6 +44,8 @@ do { printf("%s "fmt, __func__, ## __VA_ARGS__); } while (0)
 #define ICH9_DEBUG(fmt, ...)    do { } while (0)
 #endif
 
+#define GPE_EC_SCI_STATUS (0x1 << (16 + 1))
+
 static void pm_update_sci(ICH9LPCPMRegs *pm)
 {
     int sci_level, pm1a_sts;
@@ -202,6 +205,15 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
     acpi_pm1_evt_power_down(&pm->acpi_regs);
 }
 
+static void pm_ec_sci_req(Notifier *n, void *opaque)
+{
+    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, ec_sci_notifier);
+    ACPIGPE *gpe = &pm->acpi_regs.gpe;
+
+    *gpe->sts |= GPE_EC_SCI_STATUS;
+    pm_update_sci(pm);
+}
+
 void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
                   qemu_irq sci_irq)
 {
@@ -227,4 +239,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
     qemu_register_reset(pm_reset, pm);
     pm->powerdown_notifier.notify = pm_powerdown_req;
     qemu_register_powerdown_notifier(&pm->powerdown_notifier);
+
+    pm->ec_sci_notifier.notify = pm_ec_sci_req;
+    qemu_register_ec_sci_notifier(&pm->ec_sci_notifier);
 }
diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
index b1fe71f..f358deb 100644
--- a/include/hw/acpi/ich9.h
+++ b/include/hw/acpi/ich9.h
@@ -42,6 +42,7 @@ typedef struct ICH9LPCPMRegs {
 
     uint32_t pm_io_base;
     Notifier powerdown_notifier;
+    Notifier ec_sci_notifier;
 } ICH9LPCPMRegs;
 
 void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
-- 
1.7.2.5

[Qemu-devel] [PATCH 4/4][seabios] ec: add ASL for ACPI Embedded Controller

[liguang]

Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
---
 src/acpi-dsdt.dsl     |    1 +
 src/ec.dsl            |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/q35-acpi-dsdt.dsl |    1 +
 3 files changed, 53 insertions(+), 0 deletions(-)
 create mode 100644 src/ec.dsl

diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
index 158f6b4..4fa2b9a 100644
--- a/src/acpi-dsdt.dsl
+++ b/src/acpi-dsdt.dsl
@@ -112,6 +112,7 @@ DefinitionBlock (
             }
             Name(FDEN, 1)
         }
+#include "ec.dsl"
     }
 
 #include "acpi-dsdt-isa.dsl"
diff --git a/src/ec.dsl b/src/ec.dsl
new file mode 100644
index 0000000..c8be420
--- /dev/null
+++ b/src/ec.dsl
@@ -0,0 +1,51 @@
+/*
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+Device (EC0)
+{
+	Name(_HID, EISAID ("PNP0C09"))
+	Name(_UID, 1)
+
+	Method(_CRS, 0)
+	{
+		Name(BFFR, ResourceTemplate()
+		{
+			IO(Decode16, 0x62, 0x62, 0, 1)		// ACPI DATA IN/OUT
+			IO(Decode16, 0x66, 0x66, 0, 1)		// CMD/STS
+		})
+		Return(BFFR)
+	}
+
+	OperationRegion(ECFD, EmbeddedControl, 0, 0xFF)
+	Field(ECFD, ByteAcc, Lock, Preserve)
+	{	Offset(1),
+		CPUS,   8,		// 1, CPU status
+		CPUN,	8,		// 2, CPU index
+	}
+
+	Name(_GPE, 17)
+
+	Method(_Q01)
+	{
+		If(LEqual(1, CPUS))
+		{
+			Store(0, CPUS)
+		}
+		Else
+		{
+			Store(1, CPUS)
+		}
+	}
+}
diff --git a/src/q35-acpi-dsdt.dsl b/src/q35-acpi-dsdt.dsl
index c031d83..056b4f8 100644
--- a/src/q35-acpi-dsdt.dsl
+++ b/src/q35-acpi-dsdt.dsl
@@ -167,6 +167,7 @@ DefinitionBlock (
                 FDEN,   1
             }
         }
+#include "ec.dsl"
     }
 
 #include "acpi-dsdt-isa.dsl"
-- 
1.7.2.5

Re: [Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[Isaku Yamahata]

Why?
And it breaks pointer operation like

>          cur = ar->gpe.sts + addr;

thanks,

On Wed, May 22, 2013 at 11:46:35AM +0800, liguang wrote:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  hw/acpi/core.c         |    8 ++++----
>  hw/acpi/ich9.c         |    2 +-
>  hw/acpi/piix4.c        |    2 +-
>  include/hw/acpi/acpi.h |    4 ++--
>  4 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> index 42eeace..38ddeb8 100644
> --- a/hw/acpi/core.c
> +++ b/hw/acpi/core.c
> @@ -575,9 +575,9 @@ void acpi_gpe_reset(ACPIREGS *ar)
>      memset(ar->gpe.en, 0, ar->gpe.len / 2);
>  }
>  
> -static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> +static uint32_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
>  {
> -    uint8_t *cur = NULL;
> +    uint32_t *cur = NULL;
>  
>      if (addr < ar->gpe.len / 2) {
>          cur = ar->gpe.sts + addr;
> @@ -592,7 +592,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
>  
>  void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
>  {
> -    uint8_t *cur;
> +    uint32_t *cur;
>  
>      cur = acpi_gpe_ioport_get_ptr(ar, addr);
>      if (addr < ar->gpe.len / 2) {
> @@ -608,7 +608,7 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
>  
>  uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
>  {
> -    uint8_t *cur;
> +    uint32_t *cur;
>      uint32_t val;
>  
>      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 4a17f32..582dbec 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -153,7 +153,7 @@ static int ich9_pm_post_load(void *opaque, int version_id)
>       .info       = &vmstate_info_uint8,                              \
>       .size       = sizeof(uint8_t),                                  \
>       .flags      = VMS_ARRAY | VMS_POINTER,                          \
> -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
>   }
>  
>  const VMStateDescription vmstate_ich9_pm = {
> diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> index c4af1cc..3a7b669 100644
> --- a/hw/acpi/piix4.c
> +++ b/hw/acpi/piix4.c
> @@ -205,7 +205,7 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)
>       .info       = &vmstate_info_uint16,                             \
>       .size       = sizeof(uint16_t),                                 \
>       .flags      = VMS_SINGLE | VMS_POINTER,                         \
> -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
>   }
>  
>  static const VMStateDescription vmstate_gpe = {
> diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> index 635be7b..deca3ce 100644
> --- a/include/hw/acpi/acpi.h
> +++ b/include/hw/acpi/acpi.h
> @@ -112,8 +112,8 @@ struct ACPIPM1CNT {
>  struct ACPIGPE {
>      uint8_t len;
>  
> -    uint8_t *sts;
> -    uint8_t *en;
> +    uint32_t *sts;
> +    uint32_t *en;
>  };
>  
>  struct ACPIREGS {
> -- 
> 1.7.2.5
> 

-- 
yamahata

Re: [Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[li guang]

在 2013-05-22三的 14:28 +0900，Isaku Yamahata写道：
> Why?
> And it breaks pointer operation like

the fact is I can't guess why gpe->sts is defined uint8_t 
but the real hardware is 32-bit width.
I expand it to 32 because the future usage for me will 
access bit beyond 8.
of course, I can keep it, but I don't if I can
violate the bit usage indication of this register.

> 
> >          cur = ar->gpe.sts + addr;
> 
> thanks,
> 
> On Wed, May 22, 2013 at 11:46:35AM +0800, liguang wrote:
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> >  hw/acpi/core.c         |    8 ++++----
> >  hw/acpi/ich9.c         |    2 +-
> >  hw/acpi/piix4.c        |    2 +-
> >  include/hw/acpi/acpi.h |    4 ++--
> >  4 files changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> > index 42eeace..38ddeb8 100644
> > --- a/hw/acpi/core.c
> > +++ b/hw/acpi/core.c
> > @@ -575,9 +575,9 @@ void acpi_gpe_reset(ACPIREGS *ar)
> >      memset(ar->gpe.en, 0, ar->gpe.len / 2);
> >  }
> >  
> > -static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > +static uint32_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> >  {
> > -    uint8_t *cur = NULL;
> > +    uint32_t *cur = NULL;
> >  
> >      if (addr < ar->gpe.len / 2) {
> >          cur = ar->gpe.sts + addr;
> > @@ -592,7 +592,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> >  
> >  void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
> >  {
> > -    uint8_t *cur;
> > +    uint32_t *cur;
> >  
> >      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> >      if (addr < ar->gpe.len / 2) {
> > @@ -608,7 +608,7 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
> >  
> >  uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
> >  {
> > -    uint8_t *cur;
> > +    uint32_t *cur;
> >      uint32_t val;
> >  
> >      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 4a17f32..582dbec 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -153,7 +153,7 @@ static int ich9_pm_post_load(void *opaque, int version_id)
> >       .info       = &vmstate_info_uint8,                              \
> >       .size       = sizeof(uint8_t),                                  \
> >       .flags      = VMS_ARRAY | VMS_POINTER,                          \
> > -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> > +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
> >   }
> >  
> >  const VMStateDescription vmstate_ich9_pm = {
> > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > index c4af1cc..3a7b669 100644
> > --- a/hw/acpi/piix4.c
> > +++ b/hw/acpi/piix4.c
> > @@ -205,7 +205,7 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)
> >       .info       = &vmstate_info_uint16,                             \
> >       .size       = sizeof(uint16_t),                                 \
> >       .flags      = VMS_SINGLE | VMS_POINTER,                         \
> > -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> > +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
> >   }
> >  
> >  static const VMStateDescription vmstate_gpe = {
> > diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> > index 635be7b..deca3ce 100644
> > --- a/include/hw/acpi/acpi.h
> > +++ b/include/hw/acpi/acpi.h
> > @@ -112,8 +112,8 @@ struct ACPIPM1CNT {
> >  struct ACPIGPE {
> >      uint8_t len;
> >  
> > -    uint8_t *sts;
> > -    uint8_t *en;
> > +    uint32_t *sts;
> > +    uint32_t *en;
> >  };
> >  
> >  struct ACPIREGS {
> > -- 
> > 1.7.2.5
> > 
> 

Re: [Qemu-devel] [PATCH 4/4][seabios] ec: add ASL for ACPI Embedded Controller

[li guang]

Add seabios mail-list

在 2013-05-22三的 11:46 +0800，liguang写道：
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  src/acpi-dsdt.dsl     |    1 +
>  src/ec.dsl            |   51 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/q35-acpi-dsdt.dsl |    1 +
>  3 files changed, 53 insertions(+), 0 deletions(-)
>  create mode 100644 src/ec.dsl
> 
> diff --git a/src/acpi-dsdt.dsl b/src/acpi-dsdt.dsl
> index 158f6b4..4fa2b9a 100644
> --- a/src/acpi-dsdt.dsl
> +++ b/src/acpi-dsdt.dsl
> @@ -112,6 +112,7 @@ DefinitionBlock (
>              }
>              Name(FDEN, 1)
>          }
> +#include "ec.dsl"
>      }
>  
>  #include "acpi-dsdt-isa.dsl"
> diff --git a/src/ec.dsl b/src/ec.dsl
> new file mode 100644
> index 0000000..c8be420
> --- /dev/null
> +++ b/src/ec.dsl
> @@ -0,0 +1,51 @@
> +/*
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
> + */
> +
> +Device (EC0)
> +{
> +	Name(_HID, EISAID ("PNP0C09"))
> +	Name(_UID, 1)
> +
> +	Method(_CRS, 0)
> +	{
> +		Name(BFFR, ResourceTemplate()
> +		{
> +			IO(Decode16, 0x62, 0x62, 0, 1)		// ACPI DATA IN/OUT
> +			IO(Decode16, 0x66, 0x66, 0, 1)		// CMD/STS
> +		})
> +		Return(BFFR)
> +	}
> +
> +	OperationRegion(ECFD, EmbeddedControl, 0, 0xFF)
> +	Field(ECFD, ByteAcc, Lock, Preserve)
> +	{	Offset(1),
> +		CPUS,   8,		// 1, CPU status
> +		CPUN,	8,		// 2, CPU index
> +	}
> +
> +	Name(_GPE, 17)
> +
> +	Method(_Q01)
> +	{
> +		If(LEqual(1, CPUS))
> +		{
> +			Store(0, CPUS)
> +		}
> +		Else
> +		{
> +			Store(1, CPUS)
> +		}
> +	}
> +}
> diff --git a/src/q35-acpi-dsdt.dsl b/src/q35-acpi-dsdt.dsl
> index c031d83..056b4f8 100644
> --- a/src/q35-acpi-dsdt.dsl
> +++ b/src/q35-acpi-dsdt.dsl
> @@ -167,6 +167,7 @@ DefinitionBlock (
>                  FDEN,   1
>              }
>          }
> +#include "ec.dsl"
>      }
>  
>  #include "acpi-dsdt-isa.dsl"

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

Hi, Andreas, Paolo

As you have commented on the raw RFC patch before,
can you also give some comments on these?

and all who are interested in this topic(ACPI EC),

waiting for you comments

Thanks a lot!


在 2013-05-22三的 11:46 +0800，liguang写道：
> These patches try to add ACPI Embedded Controller (EC),
> refer-to:
> ACPI SPEC v5 chapter 5 
> "ACPI Embedded Controller Interface Specification"
> 
> EC is a standard ACPI device, it plays flexible roles,
> e.g. 
> power controller, it can control power sequence for
> platform to enter or leave system state(0,1,3,4,5),
> it can controller CPU fan by the temperature of sensor.
> event carrier, it can pass events between platform
> and OS, so OS can execute _Qxx method which defined
> by yourself.
> 
> So, I want to deliver CPU online/offline event between
> OS and QEMU for CPU hotplug feature, then we will don't
> need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> again after 'cpu-add'.
> 
> patches for online/offline event handler of QEUM and 
> linux kernel are writing, and will send once finished.
> 
> since EC is a common device, so I send pathes separately.
> 
> 

Re: [Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[Isaku Yamahata]

On Wed, May 22, 2013 at 01:37:41PM +0800, li guang wrote:
> 在 2013-05-22三的 14:28 +0900，Isaku Yamahata写道：
> > Why?
> > And it breaks pointer operation like
> 
> the fact is I can't guess why gpe->sts is defined uint8_t 
> but the real hardware is 32-bit width.

Which section of ACPI spec?


> I expand it to 32 because the future usage for me will 
> access bit beyond 8.
> of course, I can keep it, but I don't if I can
> violate the bit usage indication of this register.

I don't have strong opinion wheter uint8_t or uint32_t.
But the current patch is broken as the pointer offset 
operation needs to be fixed.

thanks,

> > >          cur = ar->gpe.sts + addr;
> > 
> > thanks,
> > 
> > On Wed, May 22, 2013 at 11:46:35AM +0800, liguang wrote:
> > > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > > ---
> > >  hw/acpi/core.c         |    8 ++++----
> > >  hw/acpi/ich9.c         |    2 +-
> > >  hw/acpi/piix4.c        |    2 +-
> > >  include/hw/acpi/acpi.h |    4 ++--
> > >  4 files changed, 8 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> > > index 42eeace..38ddeb8 100644
> > > --- a/hw/acpi/core.c
> > > +++ b/hw/acpi/core.c
> > > @@ -575,9 +575,9 @@ void acpi_gpe_reset(ACPIREGS *ar)
> > >      memset(ar->gpe.en, 0, ar->gpe.len / 2);
> > >  }
> > >  
> > > -static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > > +static uint32_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > >  {
> > > -    uint8_t *cur = NULL;
> > > +    uint32_t *cur = NULL;
> > >  
> > >      if (addr < ar->gpe.len / 2) {
> > >          cur = ar->gpe.sts + addr;
> > > @@ -592,7 +592,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > >  
> > >  void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
> > >  {
> > > -    uint8_t *cur;
> > > +    uint32_t *cur;
> > >  
> > >      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> > >      if (addr < ar->gpe.len / 2) {
> > > @@ -608,7 +608,7 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
> > >  
> > >  uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
> > >  {
> > > -    uint8_t *cur;
> > > +    uint32_t *cur;
> > >      uint32_t val;
> > >  
> > >      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > > index 4a17f32..582dbec 100644
> > > --- a/hw/acpi/ich9.c
> > > +++ b/hw/acpi/ich9.c
> > > @@ -153,7 +153,7 @@ static int ich9_pm_post_load(void *opaque, int version_id)
> > >       .info       = &vmstate_info_uint8,                              \
> > >       .size       = sizeof(uint8_t),                                  \
> > >       .flags      = VMS_ARRAY | VMS_POINTER,                          \
> > > -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> > > +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
> > >   }
> > >  
> > >  const VMStateDescription vmstate_ich9_pm = {
> > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > index c4af1cc..3a7b669 100644
> > > --- a/hw/acpi/piix4.c
> > > +++ b/hw/acpi/piix4.c
> > > @@ -205,7 +205,7 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)
> > >       .info       = &vmstate_info_uint16,                             \
> > >       .size       = sizeof(uint16_t),                                 \
> > >       .flags      = VMS_SINGLE | VMS_POINTER,                         \
> > > -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> > > +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
> > >   }
> > >  
> > >  static const VMStateDescription vmstate_gpe = {
> > > diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> > > index 635be7b..deca3ce 100644
> > > --- a/include/hw/acpi/acpi.h
> > > +++ b/include/hw/acpi/acpi.h
> > > @@ -112,8 +112,8 @@ struct ACPIPM1CNT {
> > >  struct ACPIGPE {
> > >      uint8_t len;
> > >  
> > > -    uint8_t *sts;
> > > -    uint8_t *en;
> > > +    uint32_t *sts;
> > > +    uint32_t *en;
> > >  };
> > >  
> > >  struct ACPIREGS {
> > > -- 
> > > 1.7.2.5
> > > 
> > 
> 
> 

-- 
yamahata

Re: [Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[li guang]

在 2013-05-24五的 10:24 +0900，Isaku Yamahata写道：
> On Wed, May 22, 2013 at 01:37:41PM +0800, li guang wrote:
> > 在 2013-05-22三的 14:28 +0900，Isaku Yamahata写道：
> > > Why?
> > > And it breaks pointer operation like
> > 
> > the fact is I can't guess why gpe->sts is defined uint8_t 
> > but the real hardware is 32-bit width.
> 
> Which section of ACPI spec?

seems ACPI SPEC does not define the length,
but, real hardware does, e.g. ICH9.

> 
> 
> > I expand it to 32 because the future usage for me will 
> > access bit beyond 8.
> > of course, I can keep it, but I don't if I can
> > violate the bit usage indication of this register.
> 
> I don't have strong opinion wheter uint8_t or uint32_t.
> But the current patch is broken as the pointer offset 
> operation needs to be fixed.

Yes, I'll fix later

Thanks!

> 
> > > >          cur = ar->gpe.sts + addr;
> > > 
> > > thanks,
> > > 
> > > On Wed, May 22, 2013 at 11:46:35AM +0800, liguang wrote:
> > > > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > > > ---
> > > >  hw/acpi/core.c         |    8 ++++----
> > > >  hw/acpi/ich9.c         |    2 +-
> > > >  hw/acpi/piix4.c        |    2 +-
> > > >  include/hw/acpi/acpi.h |    4 ++--
> > > >  4 files changed, 8 insertions(+), 8 deletions(-)
> > > > 
> > > > diff --git a/hw/acpi/core.c b/hw/acpi/core.c
> > > > index 42eeace..38ddeb8 100644
> > > > --- a/hw/acpi/core.c
> > > > +++ b/hw/acpi/core.c
> > > > @@ -575,9 +575,9 @@ void acpi_gpe_reset(ACPIREGS *ar)
> > > >      memset(ar->gpe.en, 0, ar->gpe.len / 2);
> > > >  }
> > > >  
> > > > -static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > > > +static uint32_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > > >  {
> > > > -    uint8_t *cur = NULL;
> > > > +    uint32_t *cur = NULL;
> > > >  
> > > >      if (addr < ar->gpe.len / 2) {
> > > >          cur = ar->gpe.sts + addr;
> > > > @@ -592,7 +592,7 @@ static uint8_t *acpi_gpe_ioport_get_ptr(ACPIREGS *ar, uint32_t addr)
> > > >  
> > > >  void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
> > > >  {
> > > > -    uint8_t *cur;
> > > > +    uint32_t *cur;
> > > >  
> > > >      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> > > >      if (addr < ar->gpe.len / 2) {
> > > > @@ -608,7 +608,7 @@ void acpi_gpe_ioport_writeb(ACPIREGS *ar, uint32_t addr, uint32_t val)
> > > >  
> > > >  uint32_t acpi_gpe_ioport_readb(ACPIREGS *ar, uint32_t addr)
> > > >  {
> > > > -    uint8_t *cur;
> > > > +    uint32_t *cur;
> > > >      uint32_t val;
> > > >  
> > > >      cur = acpi_gpe_ioport_get_ptr(ar, addr);
> > > > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > > > index 4a17f32..582dbec 100644
> > > > --- a/hw/acpi/ich9.c
> > > > +++ b/hw/acpi/ich9.c
> > > > @@ -153,7 +153,7 @@ static int ich9_pm_post_load(void *opaque, int version_id)
> > > >       .info       = &vmstate_info_uint8,                              \
> > > >       .size       = sizeof(uint8_t),                                  \
> > > >       .flags      = VMS_ARRAY | VMS_POINTER,                          \
> > > > -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> > > > +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
> > > >   }
> > > >  
> > > >  const VMStateDescription vmstate_ich9_pm = {
> > > > diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
> > > > index c4af1cc..3a7b669 100644
> > > > --- a/hw/acpi/piix4.c
> > > > +++ b/hw/acpi/piix4.c
> > > > @@ -205,7 +205,7 @@ static int vmstate_acpi_post_load(void *opaque, int version_id)
> > > >       .info       = &vmstate_info_uint16,                             \
> > > >       .size       = sizeof(uint16_t),                                 \
> > > >       .flags      = VMS_SINGLE | VMS_POINTER,                         \
> > > > -     .offset     = vmstate_offset_pointer(_state, _field, uint8_t),  \
> > > > +     .offset     = vmstate_offset_pointer(_state, _field, uint32_t),  \
> > > >   }
> > > >  
> > > >  static const VMStateDescription vmstate_gpe = {
> > > > diff --git a/include/hw/acpi/acpi.h b/include/hw/acpi/acpi.h
> > > > index 635be7b..deca3ce 100644
> > > > --- a/include/hw/acpi/acpi.h
> > > > +++ b/include/hw/acpi/acpi.h
> > > > @@ -112,8 +112,8 @@ struct ACPIPM1CNT {
> > > >  struct ACPIGPE {
> > > >      uint8_t len;
> > > >  
> > > > -    uint8_t *sts;
> > > > -    uint8_t *en;
> > > > +    uint32_t *sts;
> > > > +    uint32_t *en;
> > > >  };
> > > >  
> > > >  struct ACPIREGS {
> > > > -- 
> > > > 1.7.2.5
> > > > 
> > > 
> > 
> > 
> 

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Michael S. Tsirkin]

On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> These patches try to add ACPI Embedded Controller (EC),
> refer-to:
> ACPI SPEC v5 chapter 5 
> "ACPI Embedded Controller Interface Specification"
> 
> EC is a standard ACPI device, it plays flexible roles,
> e.g. 
> power controller, it can control power sequence for
> platform to enter or leave system state(0,1,3,4,5),
> it can controller CPU fan by the temperature of sensor.
> event carrier, it can pass events between platform
> and OS, so OS can execute _Qxx method which defined
> by yourself.
> 
> So, I want to deliver CPU online/offline event between
> OS and QEMU for CPU hotplug feature, then we will don't
> need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> again after 'cpu-add'.
> 
> patches for online/offline event handler of QEUM and 
> linux kernel are writing, and will send once finished.
> 
> since EC is a common device, so I send pathes separately.

Do any non-linux guests support this device?

-- 
MST

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > These patches try to add ACPI Embedded Controller (EC),
> > refer-to:
> > ACPI SPEC v5 chapter 5 
> > "ACPI Embedded Controller Interface Specification"
> > 
> > EC is a standard ACPI device, it plays flexible roles,
> > e.g. 
> > power controller, it can control power sequence for
> > platform to enter or leave system state(0,1,3,4,5),
> > it can controller CPU fan by the temperature of sensor.
> > event carrier, it can pass events between platform
> > and OS, so OS can execute _Qxx method which defined
> > by yourself.
> > 
> > So, I want to deliver CPU online/offline event between
> > OS and QEMU for CPU hotplug feature, then we will don't
> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > again after 'cpu-add'.
> > 
> > patches for online/offline event handler of QEUM and 
> > linux kernel are writing, and will send once finished.
> > 
> > since EC is a common device, so I send pathes separately.
> 
> Do any non-linux guests support this device?
> 

In fact, any OSes support ACPI will support this device.
so, windows will.

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Anthony Liguori]

li guang <lig.fnst@cn.fujitsu.com> writes:

> 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
>> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
>> > These patches try to add ACPI Embedded Controller (EC),
>> > refer-to:
>> > ACPI SPEC v5 chapter 5 
>> > "ACPI Embedded Controller Interface Specification"
>> > 
>> > EC is a standard ACPI device, it plays flexible roles,
>> > e.g. 
>> > power controller, it can control power sequence for
>> > platform to enter or leave system state(0,1,3,4,5),
>> > it can controller CPU fan by the temperature of sensor.
>> > event carrier, it can pass events between platform
>> > and OS, so OS can execute _Qxx method which defined
>> > by yourself.
>> > 
>> > So, I want to deliver CPU online/offline event between
>> > OS and QEMU for CPU hotplug feature, then we will don't
>> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
>> > again after 'cpu-add'.
>> > 
>> > patches for online/offline event handler of QEUM and 
>> > linux kernel are writing, and will send once finished.
>> > 
>> > since EC is a common device, so I send pathes separately.
>> 
>> Do any non-linux guests support this device?
>> 
>
> In fact, any OSes support ACPI will support this device.
> so, windows will.

When you say "any OSes supporting ACPI" I think what you really mean is
that we can provide bytecode that interacts with the embedded
controller.

There is not explicit driver in Linux or Windows AFAIK.

I still don't get the point of this.  We can make ACPI hotplug work
without introducing a new device like this.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> li guang <lig.fnst@cn.fujitsu.com> writes:
> 
> > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> >> > These patches try to add ACPI Embedded Controller (EC),
> >> > refer-to:
> >> > ACPI SPEC v5 chapter 5 
> >> > "ACPI Embedded Controller Interface Specification"
> >> > 
> >> > EC is a standard ACPI device, it plays flexible roles,
> >> > e.g. 
> >> > power controller, it can control power sequence for
> >> > platform to enter or leave system state(0,1,3,4,5),
> >> > it can controller CPU fan by the temperature of sensor.
> >> > event carrier, it can pass events between platform
> >> > and OS, so OS can execute _Qxx method which defined
> >> > by yourself.
> >> > 
> >> > So, I want to deliver CPU online/offline event between
> >> > OS and QEMU for CPU hotplug feature, then we will don't
> >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> >> > again after 'cpu-add'.
> >> > 
> >> > patches for online/offline event handler of QEUM and 
> >> > linux kernel are writing, and will send once finished.
> >> > 
> >> > since EC is a common device, so I send pathes separately.
> >> 
> >> Do any non-linux guests support this device?
> >> 
> >
> > In fact, any OSes support ACPI will support this device.
> > so, windows will.
> 
> When you say "any OSes supporting ACPI" I think what you really mean is
> that we can provide bytecode that interacts with the embedded
> controller.
> 
> There is not explicit driver in Linux or Windows AFAIK.

Hmm, yep, mostly there's no special driver for EC,
because it is directly embedded in code for ACPI
for linux kernel, it's drivers/acpi/ec.c

> 
> I still don't get the point of this.  We can make ACPI hotplug work
> without introducing a new device like this.
> 

when you 'cpu-add' a cpu, acpi driver for cpu will not
trigger 'cpu_up' for linux kernel AFAIK, unless you add
a user space program to listen it's uevent and tigger 'cpu_up'.

and EC is not only for ACPI hotplug
for example, the 'pvpanic', if there's a EC,
we can pass pvpanic event by EC's ACPI space or Q-event,
then we will not need this kind of special devices anymore.

Re: [Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[Gerd Hoffmann]

On 05/24/13 04:02, li guang wrote:
> 在 2013-05-24五的 10:24 +0900，Isaku Yamahata写道：
>> On Wed, May 22, 2013 at 01:37:41PM +0800, li guang wrote:
>>> 在 2013-05-22三的 14:28 +0900，Isaku Yamahata写道：
>>>> Why?
>>>> And it breaks pointer operation like
>>>
>>> the fact is I can't guess why gpe->sts is defined uint8_t 
>>> but the real hardware is 32-bit width.
>>
>> Which section of ACPI spec?
> 
> seems ACPI SPEC does not define the length,
> but, real hardware does, e.g. ICH9.

Still fail to see what exactly you are trying to fix.

qemu can continue work with uint8_t internally just fine.  memory api
will split dword access by guests into 4 byte accesses.  ich9_gpe_ops
looks correct.

>>> I expand it to 32 because the future usage for me will 
>>> access bit beyond 8.

gpe->sts is an array, so you already have more than 8 bits.

>> I don't have strong opinion wheter uint8_t or uint32_t.
>> But the current patch is broken as the pointer offset 
>> operation needs to be fixed.

Oh, and it breaks live migration too.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[Gerd Hoffmann]

On 05/24/13 04:02, li guang wrote:
> 在 2013-05-24五的 10:24 +0900，Isaku Yamahata写道：
>> On Wed, May 22, 2013 at 01:37:41PM +0800, li guang wrote:
>>> 在 2013-05-22三的 14:28 +0900，Isaku Yamahata写道：
>>>> Why?
>>>> And it breaks pointer operation like
>>>
>>> the fact is I can't guess why gpe->sts is defined uint8_t 
>>> but the real hardware is 32-bit width.
>>
>> Which section of ACPI spec?
> 
> seems ACPI SPEC does not define the length,
> but, real hardware does, e.g. ICH9.

Still fail to see what exactly you are trying to fix.

qemu can continue work with uint8_t internally just fine.  memory api
will split dword access by guests into 4 byte accesses.  ich9_gpe_ops
looks correct.

>>> I expand it to 32 because the future usage for me will 
>>> access bit beyond 8.

gpe->sts is an array, so you already have more than 8 bits.

>> I don't have strong opinion wheter uint8_t or uint32_t.
>> But the current patch is broken as the pointer offset 
>> operation needs to be fixed.

Oh, and it breaks live migration too.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Igor Mammedov]

On Mon, 27 May 2013 09:22:59 +0800
li guang <lig.fnst@cn.fujitsu.com> wrote:

> 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > li guang <lig.fnst@cn.fujitsu.com> writes:
> > 
> > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > >> > These patches try to add ACPI Embedded Controller (EC),
> > >> > refer-to:
> > >> > ACPI SPEC v5 chapter 5 
> > >> > "ACPI Embedded Controller Interface Specification"
> > >> > 
> > >> > EC is a standard ACPI device, it plays flexible roles,
> > >> > e.g. 
> > >> > power controller, it can control power sequence for
> > >> > platform to enter or leave system state(0,1,3,4,5),
> > >> > it can controller CPU fan by the temperature of sensor.
> > >> > event carrier, it can pass events between platform
> > >> > and OS, so OS can execute _Qxx method which defined
> > >> > by yourself.
> > >> > 
> > >> > So, I want to deliver CPU online/offline event between
> > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > >> > again after 'cpu-add'.
> > >> > 
> > >> > patches for online/offline event handler of QEUM and 
> > >> > linux kernel are writing, and will send once finished.
> > >> > 
> > >> > since EC is a common device, so I send pathes separately.
> > >> 
> > >> Do any non-linux guests support this device?
> > >> 
> > >
> > > In fact, any OSes support ACPI will support this device.
> > > so, windows will.
> > 
> > When you say "any OSes supporting ACPI" I think what you really mean is
> > that we can provide bytecode that interacts with the embedded
> > controller.
> > 
> > There is not explicit driver in Linux or Windows AFAIK.
> 
> Hmm, yep, mostly there's no special driver for EC,
> because it is directly embedded in code for ACPI
> for linux kernel, it's drivers/acpi/ec.c
> 
> > 
> > I still don't get the point of this.  We can make ACPI hotplug work
> > without introducing a new device like this.
> > 
> 
> when you 'cpu-add' a cpu, acpi driver for cpu will not
> trigger 'cpu_up' for linux kernel AFAIK, unless you add
> a user space program to listen it's uevent and tigger 'cpu_up'.
it is up to guest OS policy to decide if CPU should be onlined or not,
at least I've seen this rationale on LKML when topic was discussed and
automatic cpu_up on hotplug was rejected. 

Re: [Qemu-devel] [PATCH 3/4] ich9: add notifer for ec to generate sci

[Michael S. Tsirkin]

On Wed, May 22, 2013 at 11:46:36AM +0800, liguang wrote:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  hw/acpi/ich9.c         |   15 +++++++++++++++
>  include/hw/acpi/ich9.h |    1 +
>  2 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 582dbec..2ecde32 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -33,6 +33,7 @@
>  #include "exec/address-spaces.h"
>  
>  #include "hw/i386/ich9.h"
> +#include "hw/acpi/ec.h"
>  
>  //#define DEBUG
>  

Which symbols does this patch use from ec.h?

> @@ -43,6 +44,8 @@ do { printf("%s "fmt, __func__, ## __VA_ARGS__); } while (0)
>  #define ICH9_DEBUG(fmt, ...)    do { } while (0)
>  #endif
>  
> +#define GPE_EC_SCI_STATUS (0x1 << (16 + 1))
> +
>  static void pm_update_sci(ICH9LPCPMRegs *pm)
>  {
>      int sci_level, pm1a_sts;
> @@ -202,6 +205,15 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
>      acpi_pm1_evt_power_down(&pm->acpi_regs);
>  }
>  
> +static void pm_ec_sci_req(Notifier *n, void *opaque)
> +{
> +    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, ec_sci_notifier);
> +    ACPIGPE *gpe = &pm->acpi_regs.gpe;
> +
> +    *gpe->sts |= GPE_EC_SCI_STATUS;
> +    pm_update_sci(pm);
> +}
> +
>  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>                    qemu_irq sci_irq)
>  {
> @@ -227,4 +239,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>      qemu_register_reset(pm_reset, pm);
>      pm->powerdown_notifier.notify = pm_powerdown_req;
>      qemu_register_powerdown_notifier(&pm->powerdown_notifier);
> +
> +    pm->ec_sci_notifier.notify = pm_ec_sci_req;
> +    qemu_register_ec_sci_notifier(&pm->ec_sci_notifier);
>  }
> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index b1fe71f..f358deb 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -42,6 +42,7 @@ typedef struct ICH9LPCPMRegs {
>  
>      uint32_t pm_io_base;
>      Notifier powerdown_notifier;
> +    Notifier ec_sci_notifier;
>  } ICH9LPCPMRegs;
>  
>  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> -- 
> 1.7.2.5

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Michael S. Tsirkin]

On Mon, May 27, 2013 at 09:22:59AM +0800, li guang wrote:
> 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > li guang <lig.fnst@cn.fujitsu.com> writes:
> > 
> > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > >> > These patches try to add ACPI Embedded Controller (EC),
> > >> > refer-to:
> > >> > ACPI SPEC v5 chapter 5 
> > >> > "ACPI Embedded Controller Interface Specification"
> > >> > 
> > >> > EC is a standard ACPI device, it plays flexible roles,
> > >> > e.g. 
> > >> > power controller, it can control power sequence for
> > >> > platform to enter or leave system state(0,1,3,4,5),
> > >> > it can controller CPU fan by the temperature of sensor.
> > >> > event carrier, it can pass events between platform
> > >> > and OS, so OS can execute _Qxx method which defined
> > >> > by yourself.
> > >> > 
> > >> > So, I want to deliver CPU online/offline event between
> > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > >> > again after 'cpu-add'.
> > >> > 
> > >> > patches for online/offline event handler of QEUM and 
> > >> > linux kernel are writing, and will send once finished.
> > >> > 
> > >> > since EC is a common device, so I send pathes separately.
> > >> 
> > >> Do any non-linux guests support this device?
> > >> 
> > >
> > > In fact, any OSes support ACPI will support this device.
> > > so, windows will.
> > 
> > When you say "any OSes supporting ACPI" I think what you really mean is
> > that we can provide bytecode that interacts with the embedded
> > controller.
> > 
> > There is not explicit driver in Linux or Windows AFAIK.
> 
> Hmm, yep, mostly there's no special driver for EC,
> because it is directly embedded in code for ACPI
> for linux kernel, it's drivers/acpi/ec.c
> 
> > 
> > I still don't get the point of this.  We can make ACPI hotplug work
> > without introducing a new device like this.
> > 
> 
> when you 'cpu-add' a cpu, acpi driver for cpu will not
> trigger 'cpu_up' for linux kernel AFAIK, unless you add
> a user space program to listen it's uevent and tigger 'cpu_up'.
> 
> and EC is not only for ACPI hotplug
> for example, the 'pvpanic', if there's a EC,
> we can pass pvpanic event by EC's ACPI space or Q-event,
> then we will not need this kind of special devices anymore.
> 
> 


So just to clarify: this patchset doesn't do anything useful
itself, it's some infrastructure that you want to build on
top of?

-- 
MST

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

在 2013-05-27一的 23:23 +0300，Michael S. Tsirkin写道：
> On Mon, May 27, 2013 at 09:22:59AM +0800, li guang wrote:
> > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > 
> > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > >> > refer-to:
> > > >> > ACPI SPEC v5 chapter 5 
> > > >> > "ACPI Embedded Controller Interface Specification"
> > > >> > 
> > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > >> > e.g. 
> > > >> > power controller, it can control power sequence for
> > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > >> > it can controller CPU fan by the temperature of sensor.
> > > >> > event carrier, it can pass events between platform
> > > >> > and OS, so OS can execute _Qxx method which defined
> > > >> > by yourself.
> > > >> > 
> > > >> > So, I want to deliver CPU online/offline event between
> > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > >> > again after 'cpu-add'.
> > > >> > 
> > > >> > patches for online/offline event handler of QEUM and 
> > > >> > linux kernel are writing, and will send once finished.
> > > >> > 
> > > >> > since EC is a common device, so I send pathes separately.
> > > >> 
> > > >> Do any non-linux guests support this device?
> > > >> 
> > > >
> > > > In fact, any OSes support ACPI will support this device.
> > > > so, windows will.
> > > 
> > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > that we can provide bytecode that interacts with the embedded
> > > controller.
> > > 
> > > There is not explicit driver in Linux or Windows AFAIK.
> > 
> > Hmm, yep, mostly there's no special driver for EC,
> > because it is directly embedded in code for ACPI
> > for linux kernel, it's drivers/acpi/ec.c
> > 
> > > 
> > > I still don't get the point of this.  We can make ACPI hotplug work
> > > without introducing a new device like this.
> > > 
> > 
> > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > a user space program to listen it's uevent and tigger 'cpu_up'.
> > 
> > and EC is not only for ACPI hotplug
> > for example, the 'pvpanic', if there's a EC,
> > we can pass pvpanic event by EC's ACPI space or Q-event,
> > then we will not need this kind of special devices anymore.
> > 
> > 
> 
> 
> So just to clarify: this patchset doesn't do anything useful
> itself, 

the first patch is pure EC implementation which is useful.

> it's some infrastructure that you want to build on
> top of?

Yes, I'd like to use EC as infrastructure for my further development.

Re: [Qemu-devel] [PATCH 3/4] ich9: add notifer for ec to generate sci

[li guang]

在 2013-05-27一的 23:21 +0300，Michael S. Tsirkin写道：
> On Wed, May 22, 2013 at 11:46:36AM +0800, liguang wrote:
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> >  hw/acpi/ich9.c         |   15 +++++++++++++++
> >  include/hw/acpi/ich9.h |    1 +
> >  2 files changed, 16 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 582dbec..2ecde32 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -33,6 +33,7 @@
> >  #include "exec/address-spaces.h"
> >  
> >  #include "hw/i386/ich9.h"
> > +#include "hw/acpi/ec.h"
> >  
> >  //#define DEBUG
> >  
> 
> Which symbols does this patch use from ec.h?

qemu_register_ec_sci_notifier

> 
> > @@ -43,6 +44,8 @@ do { printf("%s "fmt, __func__, ## __VA_ARGS__); } while (0)
> >  #define ICH9_DEBUG(fmt, ...)    do { } while (0)
> >  #endif
> >  
> > +#define GPE_EC_SCI_STATUS (0x1 << (16 + 1))
> > +
> >  static void pm_update_sci(ICH9LPCPMRegs *pm)
> >  {
> >      int sci_level, pm1a_sts;
> > @@ -202,6 +205,15 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
> >      acpi_pm1_evt_power_down(&pm->acpi_regs);
> >  }
> >  
> > +static void pm_ec_sci_req(Notifier *n, void *opaque)
> > +{
> > +    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, ec_sci_notifier);
> > +    ACPIGPE *gpe = &pm->acpi_regs.gpe;
> > +
> > +    *gpe->sts |= GPE_EC_SCI_STATUS;
> > +    pm_update_sci(pm);
> > +}
> > +
> >  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> >                    qemu_irq sci_irq)
> >  {
> > @@ -227,4 +239,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> >      qemu_register_reset(pm_reset, pm);
> >      pm->powerdown_notifier.notify = pm_powerdown_req;
> >      qemu_register_powerdown_notifier(&pm->powerdown_notifier);
> > +
> > +    pm->ec_sci_notifier.notify = pm_ec_sci_req;
> > +    qemu_register_ec_sci_notifier(&pm->ec_sci_notifier);
> >  }
> > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> > index b1fe71f..f358deb 100644
> > --- a/include/hw/acpi/ich9.h
> > +++ b/include/hw/acpi/ich9.h
> > @@ -42,6 +42,7 @@ typedef struct ICH9LPCPMRegs {
> >  
> >      uint32_t pm_io_base;
> >      Notifier powerdown_notifier;
> > +    Notifier ec_sci_notifier;
> >  } ICH9LPCPMRegs;
> >  
> >  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> > -- 
> > 1.7.2.5

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

在 2013-05-27一的 13:45 +0200，Igor Mammedov写道：
> On Mon, 27 May 2013 09:22:59 +0800
> li guang <lig.fnst@cn.fujitsu.com> wrote:
> 
> > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > 
> > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > >> > refer-to:
> > > >> > ACPI SPEC v5 chapter 5 
> > > >> > "ACPI Embedded Controller Interface Specification"
> > > >> > 
> > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > >> > e.g. 
> > > >> > power controller, it can control power sequence for
> > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > >> > it can controller CPU fan by the temperature of sensor.
> > > >> > event carrier, it can pass events between platform
> > > >> > and OS, so OS can execute _Qxx method which defined
> > > >> > by yourself.
> > > >> > 
> > > >> > So, I want to deliver CPU online/offline event between
> > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > >> > again after 'cpu-add'.
> > > >> > 
> > > >> > patches for online/offline event handler of QEUM and 
> > > >> > linux kernel are writing, and will send once finished.
> > > >> > 
> > > >> > since EC is a common device, so I send pathes separately.
> > > >> 
> > > >> Do any non-linux guests support this device?
> > > >> 
> > > >
> > > > In fact, any OSes support ACPI will support this device.
> > > > so, windows will.
> > > 
> > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > that we can provide bytecode that interacts with the embedded
> > > controller.
> > > 
> > > There is not explicit driver in Linux or Windows AFAIK.
> > 
> > Hmm, yep, mostly there's no special driver for EC,
> > because it is directly embedded in code for ACPI
> > for linux kernel, it's drivers/acpi/ec.c
> > 
> > > 
> > > I still don't get the point of this.  We can make ACPI hotplug work
> > > without introducing a new device like this.
> > > 
> > 
> > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > a user space program to listen it's uevent and tigger 'cpu_up'.
> it is up to guest OS policy to decide if CPU should be onlined or not,

Yep, but I think it's a favor to do cpu online.

> at least I've seen this rationale on LKML when topic was discussed and
> automatic cpu_up on hotplug was rejected. 

Oh, and the reason is?
can you give me a link?

Oh, Igor,
I remember that you said you can give me some your considerations
on the further development of cpu hotplug feature, but I haven't got
them :-)

Thanks!

Re: [Qemu-devel] [PATCH 2/4] acpi/gpe: expand bits of gpe register

[li guang]

在 2013-05-27一的 11:00 +0200，Gerd Hoffmann写道：
> On 05/24/13 04:02, li guang wrote:
> > 在 2013-05-24五的 10:24 +0900，Isaku Yamahata写道：
> >> On Wed, May 22, 2013 at 01:37:41PM +0800, li guang wrote:
> >>> 在 2013-05-22三的 14:28 +0900，Isaku Yamahata写道：
> >>>> Why?
> >>>> And it breaks pointer operation like
> >>>
> >>> the fact is I can't guess why gpe->sts is defined uint8_t 
> >>> but the real hardware is 32-bit width.
> >>
> >> Which section of ACPI spec?
> > 
> > seems ACPI SPEC does not define the length,
> > but, real hardware does, e.g. ICH9.
> 
> Still fail to see what exactly you are trying to fix.
> 
> qemu can continue work with uint8_t internally just fine.  memory api
> will split dword access by guests into 4 byte accesses.  ich9_gpe_ops
> looks correct.
> 
> >>> I expand it to 32 because the future usage for me will 
> >>> access bit beyond 8.
> 
> gpe->sts is an array, so you already have more than 8 bits.
> 
> >> I don't have strong opinion wheter uint8_t or uint32_t.
> >> But the current patch is broken as the pointer offset 
> >> operation needs to be fixed.
> 
> Oh, and it breaks live migration too.
> 

Yes, I will re-consider this.

Thanks!

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Michael S. Tsirkin]

On Tue, May 28, 2013 at 08:21:24AM +0800, li guang wrote:
> 在 2013-05-27一的 23:23 +0300，Michael S. Tsirkin写道：
> > On Mon, May 27, 2013 at 09:22:59AM +0800, li guang wrote:
> > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > 
> > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > >> > refer-to:
> > > > >> > ACPI SPEC v5 chapter 5 
> > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > >> > 
> > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > >> > e.g. 
> > > > >> > power controller, it can control power sequence for
> > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > >> > event carrier, it can pass events between platform
> > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > >> > by yourself.
> > > > >> > 
> > > > >> > So, I want to deliver CPU online/offline event between
> > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > >> > again after 'cpu-add'.
> > > > >> > 
> > > > >> > patches for online/offline event handler of QEUM and 
> > > > >> > linux kernel are writing, and will send once finished.
> > > > >> > 
> > > > >> > since EC is a common device, so I send pathes separately.
> > > > >> 
> > > > >> Do any non-linux guests support this device?
> > > > >> 
> > > > >
> > > > > In fact, any OSes support ACPI will support this device.
> > > > > so, windows will.
> > > > 
> > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > that we can provide bytecode that interacts with the embedded
> > > > controller.
> > > > 
> > > > There is not explicit driver in Linux or Windows AFAIK.
> > > 
> > > Hmm, yep, mostly there's no special driver for EC,
> > > because it is directly embedded in code for ACPI
> > > for linux kernel, it's drivers/acpi/ec.c
> > > 
> > > > 
> > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > without introducing a new device like this.
> > > > 
> > > 
> > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > > 
> > > and EC is not only for ACPI hotplug
> > > for example, the 'pvpanic', if there's a EC,
> > > we can pass pvpanic event by EC's ACPI space or Q-event,
> > > then we will not need this kind of special devices anymore.
> > > 
> > > 
> > 
> > 
> > So just to clarify: this patchset doesn't do anything useful
> > itself, 
> 
> the first patch is pure EC implementation which is useful.

Sorry, I didn't mean to imply that this is not useful,
merely that it is not yet directly useful for users.

> > it's some infrastructure that you want to build on
> > top of?
> 
> Yes, I'd like to use EC as infrastructure for my further development.
> 

I (and apparently others in this thread) would like to know some more
about planned use for this code before deciding on inclusion.

-- 
MST

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

Hi, Michael

在 2013-05-28二的 09:31 +0300，Michael S. Tsirkin写道：
> On Tue, May 28, 2013 at 08:21:24AM +0800, li guang wrote:
> > 在 2013-05-27一的 23:23 +0300，Michael S. Tsirkin写道：
> > > On Mon, May 27, 2013 at 09:22:59AM +0800, li guang wrote:
> > > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > > 
> > > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > > >> > refer-to:
> > > > > >> > ACPI SPEC v5 chapter 5 
> > > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > > >> > 
> > > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > > >> > e.g. 
> > > > > >> > power controller, it can control power sequence for
> > > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > > >> > event carrier, it can pass events between platform
> > > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > > >> > by yourself.
> > > > > >> > 
> > > > > >> > So, I want to deliver CPU online/offline event between
> > > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > > >> > again after 'cpu-add'.
> > > > > >> > 
> > > > > >> > patches for online/offline event handler of QEUM and 
> > > > > >> > linux kernel are writing, and will send once finished.
> > > > > >> > 
> > > > > >> > since EC is a common device, so I send pathes separately.
> > > > > >> 
> > > > > >> Do any non-linux guests support this device?
> > > > > >> 
> > > > > >
> > > > > > In fact, any OSes support ACPI will support this device.
> > > > > > so, windows will.
> > > > > 
> > > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > > that we can provide bytecode that interacts with the embedded
> > > > > controller.
> > > > > 
> > > > > There is not explicit driver in Linux or Windows AFAIK.
> > > > 
> > > > Hmm, yep, mostly there's no special driver for EC,
> > > > because it is directly embedded in code for ACPI
> > > > for linux kernel, it's drivers/acpi/ec.c
> > > > 
> > > > > 
> > > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > > without introducing a new device like this.
> > > > > 
> > > > 
> > > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > > > 
> > > > and EC is not only for ACPI hotplug
> > > > for example, the 'pvpanic', if there's a EC,
> > > > we can pass pvpanic event by EC's ACPI space or Q-event,
> > > > then we will not need this kind of special devices anymore.
> > > > 
> > > > 
> > > 
> > > 
> > > So just to clarify: this patchset doesn't do anything useful
> > > itself, 
> > 
> > the first patch is pure EC implementation which is useful.
> 
> Sorry, I didn't mean to imply that this is not useful,
> merely that it is not yet directly useful for users.

Yep, by now, of course, only 1 user, that's me. :-)

> 
> > > it's some infrastructure that you want to build on
> > > top of?
> > 
> > Yes, I'd like to use EC as infrastructure for my further development.
> > 
> 
> I (and apparently others in this thread) would like to know some more
> about planned use for this code before deciding on inclusion.
> 

OK, 
currently, I am using EC to pass cpu hotplug event between QEMU and
linux kernel, and I'll post these patches later.
then, I'll query usages like this in QEMU, and convert them to use
EC's acpi approach if possible.

Thanks!

Re: [Qemu-devel] [PATCH 3/4] ich9: add notifer for ec to generate sci

[Michael S. Tsirkin]

On Wed, May 22, 2013 at 11:46:36AM +0800, liguang wrote:
> Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> ---
>  hw/acpi/ich9.c         |   15 +++++++++++++++
>  include/hw/acpi/ich9.h |    1 +
>  2 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> index 582dbec..2ecde32 100644
> --- a/hw/acpi/ich9.c
> +++ b/hw/acpi/ich9.c
> @@ -33,6 +33,7 @@
>  #include "exec/address-spaces.h"
>  
>  #include "hw/i386/ich9.h"
> +#include "hw/acpi/ec.h"
>  
>  //#define DEBUG
>  
> @@ -43,6 +44,8 @@ do { printf("%s "fmt, __func__, ## __VA_ARGS__); } while (0)
>  #define ICH9_DEBUG(fmt, ...)    do { } while (0)
>  #endif
>  
> +#define GPE_EC_SCI_STATUS (0x1 << (16 + 1))
> +
>  static void pm_update_sci(ICH9LPCPMRegs *pm)
>  {
>      int sci_level, pm1a_sts;
> @@ -202,6 +205,15 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
>      acpi_pm1_evt_power_down(&pm->acpi_regs);
>  }
>  
> +static void pm_ec_sci_req(Notifier *n, void *opaque)
> +{
> +    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, ec_sci_notifier);
> +    ACPIGPE *gpe = &pm->acpi_regs.gpe;
> +
> +    *gpe->sts |= GPE_EC_SCI_STATUS;
> +    pm_update_sci(pm);
> +}
> +
>  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>                    qemu_irq sci_irq)
>  {
> @@ -227,4 +239,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
>      qemu_register_reset(pm_reset, pm);
>      pm->powerdown_notifier.notify = pm_powerdown_req;
>      qemu_register_powerdown_notifier(&pm->powerdown_notifier);
> +
> +    pm->ec_sci_notifier.notify = pm_ec_sci_req;
> +    qemu_register_ec_sci_notifier(&pm->ec_sci_notifier);
>  }

So an EC is using ICH9 to send SCI to guest,
but ICH9 should not worry about EC that's connected to it.
So it should be something like
qemu_register_sci_notifier
and not make ich9 depend on ec.

> diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> index b1fe71f..f358deb 100644
> --- a/include/hw/acpi/ich9.h
> +++ b/include/hw/acpi/ich9.h
> @@ -42,6 +42,7 @@ typedef struct ICH9LPCPMRegs {
>  
>      uint32_t pm_io_base;
>      Notifier powerdown_notifier;
> +    Notifier ec_sci_notifier;
>  } ICH9LPCPMRegs;
>  
>  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> -- 
> 1.7.2.5

Re: [Qemu-devel] [PATCH 3/4] ich9: add notifer for ec to generate sci

[li guang]

在 2013-05-28二的 09:40 +0300，Michael S. Tsirkin写道：
> On Wed, May 22, 2013 at 11:46:36AM +0800, liguang wrote:
> > Signed-off-by: liguang <lig.fnst@cn.fujitsu.com>
> > ---
> >  hw/acpi/ich9.c         |   15 +++++++++++++++
> >  include/hw/acpi/ich9.h |    1 +
> >  2 files changed, 16 insertions(+), 0 deletions(-)
> > 
> > diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
> > index 582dbec..2ecde32 100644
> > --- a/hw/acpi/ich9.c
> > +++ b/hw/acpi/ich9.c
> > @@ -33,6 +33,7 @@
> >  #include "exec/address-spaces.h"
> >  
> >  #include "hw/i386/ich9.h"
> > +#include "hw/acpi/ec.h"
> >  
> >  //#define DEBUG
> >  
> > @@ -43,6 +44,8 @@ do { printf("%s "fmt, __func__, ## __VA_ARGS__); } while (0)
> >  #define ICH9_DEBUG(fmt, ...)    do { } while (0)
> >  #endif
> >  
> > +#define GPE_EC_SCI_STATUS (0x1 << (16 + 1))
> > +
> >  static void pm_update_sci(ICH9LPCPMRegs *pm)
> >  {
> >      int sci_level, pm1a_sts;
> > @@ -202,6 +205,15 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
> >      acpi_pm1_evt_power_down(&pm->acpi_regs);
> >  }
> >  
> > +static void pm_ec_sci_req(Notifier *n, void *opaque)
> > +{
> > +    ICH9LPCPMRegs *pm = container_of(n, ICH9LPCPMRegs, ec_sci_notifier);
> > +    ACPIGPE *gpe = &pm->acpi_regs.gpe;
> > +
> > +    *gpe->sts |= GPE_EC_SCI_STATUS;
> > +    pm_update_sci(pm);
> > +}
> > +
> >  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> >                    qemu_irq sci_irq)
> >  {
> > @@ -227,4 +239,7 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> >      qemu_register_reset(pm_reset, pm);
> >      pm->powerdown_notifier.notify = pm_powerdown_req;
> >      qemu_register_powerdown_notifier(&pm->powerdown_notifier);
> > +
> > +    pm->ec_sci_notifier.notify = pm_ec_sci_req;
> > +    qemu_register_ec_sci_notifier(&pm->ec_sci_notifier);
> >  }
> 
> So an EC is using ICH9 to send SCI to guest,
> but ICH9 should not worry about EC that's connected to it.
> So it should be something like
> qemu_register_sci_notifier
> and not make ich9 depend on ec.

OK, let me think about it.

Thanks!

> 
> > diff --git a/include/hw/acpi/ich9.h b/include/hw/acpi/ich9.h
> > index b1fe71f..f358deb 100644
> > --- a/include/hw/acpi/ich9.h
> > +++ b/include/hw/acpi/ich9.h
> > @@ -42,6 +42,7 @@ typedef struct ICH9LPCPMRegs {
> >  
> >      uint32_t pm_io_base;
> >      Notifier powerdown_notifier;
> > +    Notifier ec_sci_notifier;
> >  } ICH9LPCPMRegs;
> >  
> >  void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
> > -- 
> > 1.7.2.5

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Michael S. Tsirkin]

On Tue, May 28, 2013 at 02:40:30PM +0800, li guang wrote:
> Hi, Michael
> 
> 在 2013-05-28二的 09:31 +0300，Michael S. Tsirkin写道：
> > On Tue, May 28, 2013 at 08:21:24AM +0800, li guang wrote:
> > > 在 2013-05-27一的 23:23 +0300，Michael S. Tsirkin写道：
> > > > On Mon, May 27, 2013 at 09:22:59AM +0800, li guang wrote:
> > > > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > > > 
> > > > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > > > >> > refer-to:
> > > > > > >> > ACPI SPEC v5 chapter 5 
> > > > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > > > >> > 
> > > > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > > > >> > e.g. 
> > > > > > >> > power controller, it can control power sequence for
> > > > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > > > >> > event carrier, it can pass events between platform
> > > > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > > > >> > by yourself.
> > > > > > >> > 
> > > > > > >> > So, I want to deliver CPU online/offline event between
> > > > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > > > >> > again after 'cpu-add'.
> > > > > > >> > 
> > > > > > >> > patches for online/offline event handler of QEUM and 
> > > > > > >> > linux kernel are writing, and will send once finished.
> > > > > > >> > 
> > > > > > >> > since EC is a common device, so I send pathes separately.
> > > > > > >> 
> > > > > > >> Do any non-linux guests support this device?
> > > > > > >> 
> > > > > > >
> > > > > > > In fact, any OSes support ACPI will support this device.
> > > > > > > so, windows will.
> > > > > > 
> > > > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > > > that we can provide bytecode that interacts with the embedded
> > > > > > controller.
> > > > > > 
> > > > > > There is not explicit driver in Linux or Windows AFAIK.
> > > > > 
> > > > > Hmm, yep, mostly there's no special driver for EC,
> > > > > because it is directly embedded in code for ACPI
> > > > > for linux kernel, it's drivers/acpi/ec.c
> > > > > 
> > > > > > 
> > > > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > > > without introducing a new device like this.
> > > > > > 
> > > > > 
> > > > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > > > > 
> > > > > and EC is not only for ACPI hotplug
> > > > > for example, the 'pvpanic', if there's a EC,
> > > > > we can pass pvpanic event by EC's ACPI space or Q-event,
> > > > > then we will not need this kind of special devices anymore.
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > > So just to clarify: this patchset doesn't do anything useful
> > > > itself, 
> > > 
> > > the first patch is pure EC implementation which is useful.
> > 
> > Sorry, I didn't mean to imply that this is not useful,
> > merely that it is not yet directly useful for users.
> 
> Yep, by now, of course, only 1 user, that's me. :-)
> 
> > 
> > > > it's some infrastructure that you want to build on
> > > > top of?
> > > 
> > > Yes, I'd like to use EC as infrastructure for my further development.
> > > 
> > 
> > I (and apparently others in this thread) would like to know some more
> > about planned use for this code before deciding on inclusion.
> > 
> 
> OK, 
> currently, I am using EC to pass cpu hotplug event between QEMU and
> linux kernel, and I'll post these patches later.
> then, I'll query usages like this in QEMU, and convert them to use
> EC's acpi approach if possible.
> 
> Thanks!
> 

Okay, that needs an ack from Igor then I think :)

-- 
MST

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Igor Mammedov]

On Tue, 28 May 2013 08:28:09 +0800
li guang <lig.fnst@cn.fujitsu.com> wrote:

> 在 2013-05-27一的 13:45 +0200，Igor Mammedov写道：
> > On Mon, 27 May 2013 09:22:59 +0800
> > li guang <lig.fnst@cn.fujitsu.com> wrote:
> > 
> > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > 
> > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > >> > refer-to:
> > > > >> > ACPI SPEC v5 chapter 5 
> > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > >> > 
> > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > >> > e.g. 
> > > > >> > power controller, it can control power sequence for
> > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > >> > event carrier, it can pass events between platform
> > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > >> > by yourself.
> > > > >> > 
> > > > >> > So, I want to deliver CPU online/offline event between
> > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > >> > again after 'cpu-add'.
> > > > >> > 
> > > > >> > patches for online/offline event handler of QEUM and 
> > > > >> > linux kernel are writing, and will send once finished.
> > > > >> > 
> > > > >> > since EC is a common device, so I send pathes separately.
> > > > >> 
> > > > >> Do any non-linux guests support this device?
> > > > >> 
> > > > >
> > > > > In fact, any OSes support ACPI will support this device.
> > > > > so, windows will.
> > > > 
> > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > that we can provide bytecode that interacts with the embedded
> > > > controller.
> > > > 
> > > > There is not explicit driver in Linux or Windows AFAIK.
> > > 
> > > Hmm, yep, mostly there's no special driver for EC,
> > > because it is directly embedded in code for ACPI
> > > for linux kernel, it's drivers/acpi/ec.c
> > > 
> > > > 
> > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > without introducing a new device like this.
> > > > 
> > > 
> > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > it is up to guest OS policy to decide if CPU should be onlined or not,
> 
> Yep, but I think it's a favor to do cpu online.
Then you have to teach kernel driver (EC) to do cpu_up on _Q02 event,
the question is why would you do this when there is ACPI processor driver
already that handles CPU hotplug in kernel.
You can try add cpu_up() there and perhaps with good enough reasoning it
would be accepted.

> 
> > at least I've seen this rationale on LKML when topic was discussed and
> > automatic cpu_up on hotplug was rejected. 
> 
> Oh, and the reason is?
Reason was to let guest decide whether online new CPU or nor instead of
doing it unconditionally.

> can you give me a link?
I'm sorry but I can't find link quickly.

> 
> Oh, Igor,
> I remember that you said you can give me some your considerations
> on the further development of cpu hotplug feature, but I haven't got
> them :-)
I'm sorry, I haven't time yet to update TODO list on wiki page:

But here some items that need some attention:

* try to introduce generic QOM interface for CPU topology introspection
    something like /machine/node/socket[xxx]/{(core[yyy]/thread[zzz])|thread[zzz]}

* allow to specify at CLI specific CPUs to be created at start-up time
   - important for hot-adding/removing an arbitrary CPU
   - probably depends on previous item so that each CPU could be specified by socket/core/thread numbers

* extend/rework -numa CLI option to support socket to node binding
   - goal is to obsolete node to thread biding which allows specify incorrect topology.

> 
> Thanks!
> 
> 
> 

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Igor Mammedov]

On Tue, 28 May 2013 14:40:30 +0800
li guang <lig.fnst@cn.fujitsu.com> wrote:

> Hi, Michael
> 
> 在 2013-05-28二的 09:31 +0300，Michael S. Tsirkin写道：
> > On Tue, May 28, 2013 at 08:21:24AM +0800, li guang wrote:
> > > 在 2013-05-27一的 23:23 +0300，Michael S. Tsirkin写道：
> > > > On Mon, May 27, 2013 at 09:22:59AM +0800, li guang wrote:
> > > > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > > > 
> > > > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > > > >> > refer-to:
> > > > > > >> > ACPI SPEC v5 chapter 5 
> > > > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > > > >> > 
> > > > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > > > >> > e.g. 
> > > > > > >> > power controller, it can control power sequence for
> > > > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > > > >> > event carrier, it can pass events between platform
> > > > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > > > >> > by yourself.
> > > > > > >> > 
> > > > > > >> > So, I want to deliver CPU online/offline event between
> > > > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > > > >> > again after 'cpu-add'.
> > > > > > >> > 
> > > > > > >> > patches for online/offline event handler of QEUM and 
> > > > > > >> > linux kernel are writing, and will send once finished.
> > > > > > >> > 
> > > > > > >> > since EC is a common device, so I send pathes separately.
> > > > > > >> 
> > > > > > >> Do any non-linux guests support this device?
> > > > > > >> 
> > > > > > >
> > > > > > > In fact, any OSes support ACPI will support this device.
> > > > > > > so, windows will.
> > > > > > 
> > > > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > > > that we can provide bytecode that interacts with the embedded
> > > > > > controller.
> > > > > > 
> > > > > > There is not explicit driver in Linux or Windows AFAIK.
> > > > > 
> > > > > Hmm, yep, mostly there's no special driver for EC,
> > > > > because it is directly embedded in code for ACPI
> > > > > for linux kernel, it's drivers/acpi/ec.c
> > > > > 
> > > > > > 
> > > > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > > > without introducing a new device like this.
> > > > > > 
> > > > > 
> > > > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > > > > 
> > > > > and EC is not only for ACPI hotplug
> > > > > for example, the 'pvpanic', if there's a EC,
> > > > > we can pass pvpanic event by EC's ACPI space or Q-event,
> > > > > then we will not need this kind of special devices anymore.
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > > So just to clarify: this patchset doesn't do anything useful
> > > > itself, 
> > > 
> > > the first patch is pure EC implementation which is useful.
> > 
> > Sorry, I didn't mean to imply that this is not useful,
> > merely that it is not yet directly useful for users.
> 
> Yep, by now, of course, only 1 user, that's me. :-)
> 
> > 
> > > > it's some infrastructure that you want to build on
> > > > top of?
> > > 
> > > Yes, I'd like to use EC as infrastructure for my further development.
> > > 
> > 
> > I (and apparently others in this thread) would like to know some more
> > about planned use for this code before deciding on inclusion.
> > 
> 
> OK, 
> currently, I am using EC to pass cpu hotplug event between QEMU and
> linux kernel, and I'll post these patches later.
> then, I'll query usages like this in QEMU, and convert them to use
> EC's acpi approach if possible.
When I see these patches for qemu/seebios/kernel, I probably would be able
to comment on them. But right now I don't see benefits in switching
CPU hot-plug to EC.

> 
> Thanks!
> 
> 
> 
> 

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

在 2013-05-28二的 10:16 +0200，Igor Mammedov写道：
> On Tue, 28 May 2013 08:28:09 +0800
> li guang <lig.fnst@cn.fujitsu.com> wrote:
> 
> > 在 2013-05-27一的 13:45 +0200，Igor Mammedov写道：
> > > On Mon, 27 May 2013 09:22:59 +0800
> > > li guang <lig.fnst@cn.fujitsu.com> wrote:
> > > 
> > > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > > 
> > > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > > >> > refer-to:
> > > > > >> > ACPI SPEC v5 chapter 5 
> > > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > > >> > 
> > > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > > >> > e.g. 
> > > > > >> > power controller, it can control power sequence for
> > > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > > >> > event carrier, it can pass events between platform
> > > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > > >> > by yourself.
> > > > > >> > 
> > > > > >> > So, I want to deliver CPU online/offline event between
> > > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > > >> > again after 'cpu-add'.
> > > > > >> > 
> > > > > >> > patches for online/offline event handler of QEUM and 
> > > > > >> > linux kernel are writing, and will send once finished.
> > > > > >> > 
> > > > > >> > since EC is a common device, so I send pathes separately.
> > > > > >> 
> > > > > >> Do any non-linux guests support this device?
> > > > > >> 
> > > > > >
> > > > > > In fact, any OSes support ACPI will support this device.
> > > > > > so, windows will.
> > > > > 
> > > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > > that we can provide bytecode that interacts with the embedded
> > > > > controller.
> > > > > 
> > > > > There is not explicit driver in Linux or Windows AFAIK.
> > > > 
> > > > Hmm, yep, mostly there's no special driver for EC,
> > > > because it is directly embedded in code for ACPI
> > > > for linux kernel, it's drivers/acpi/ec.c
> > > > 
> > > > > 
> > > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > > without introducing a new device like this.
> > > > > 
> > > > 
> > > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > > it is up to guest OS policy to decide if CPU should be onlined or not,
> > 
> > Yep, but I think it's a favor to do cpu online.
> Then you have to teach kernel driver (EC) to do cpu_up on _Q02 event,

I think it's not necessary to do this.
we can also add another platform driver to listen cpu plug/unplug event
and query EC's ACPI space to decide what to do next.

> the question is why would you do this when there is ACPI processor driver
> already that handles CPU hotplug in kernel.
> You can try add cpu_up() there and perhaps with good enough reasoning it
> would be accepted.

it's not so convenient to trigger cpu_up/down process for ACPI processor
driver as far as I can see, it seems like informal or hack there.
so I'd like to make a bridge between them.

but, anyway, it's a good point to think about or even try to do.

> 
> > 
> > > at least I've seen this rationale on LKML when topic was discussed and
> > > automatic cpu_up on hotplug was rejected. 
> > 
> > Oh, and the reason is?
> Reason was to let guest decide whether online new CPU or nor instead of
> doing it unconditionally.
> 

can this be a Kconfig option?
or it's strongly recommended to let guest decide? 

> > can you give me a link?
> I'm sorry but I can't find link quickly.
> 
> > 
> > Oh, Igor,
> > I remember that you said you can give me some your considerations
> > on the further development of cpu hotplug feature, but I haven't got
> > them :-)
> I'm sorry, I haven't time yet to update TODO list on wiki page:
> 
> But here some items that need some attention:
> 
> * try to introduce generic QOM interface for CPU topology introspection
>     something like /machine/node/socket[xxx]/{(core[yyy]/thread[zzz])|thread[zzz]}
> 
> * allow to specify at CLI specific CPUs to be created at start-up time
>    - important for hot-adding/removing an arbitrary CPU
>    - probably depends on previous item so that each CPU could be specified by socket/core/thread numbers
> 
> * extend/rework -numa CLI option to support socket to node binding
>    - goal is to obsolete node to thread biding which allows specify incorrect topology.
> 

OK, thank you so much!

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

Hi, Igor

在 2013-05-28二的 10:26 +0200，Igor Mammedov写道：
> On Tue, 28 May 2013 14:40:30 +0800
> li guang <lig.fnst@cn.fujitsu.com> wrote:
> 
> > Hi, Michael
> > 
> > 在 2013-05-28二的 09:31 +0300，Michael S. Tsirkin写道：
> > > On Tue, May 28, 2013 at 08:21:24AM +0800, li guang wrote:
> > > > 在 2013-05-27一的 23:23 +0300，Michael S. Tsirkin写道：
> > > > > On Mon, May 27, 2013 at 09:22:59AM +0800, li guang wrote:
> > > > > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > > > > 
> > > > > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > > > > >> > refer-to:
> > > > > > > >> > ACPI SPEC v5 chapter 5 
> > > > > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > > > > >> > 
> > > > > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > > > > >> > e.g. 
> > > > > > > >> > power controller, it can control power sequence for
> > > > > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > > > > >> > event carrier, it can pass events between platform
> > > > > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > > > > >> > by yourself.
> > > > > > > >> > 
> > > > > > > >> > So, I want to deliver CPU online/offline event between
> > > > > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > > > > >> > again after 'cpu-add'.
> > > > > > > >> > 
> > > > > > > >> > patches for online/offline event handler of QEUM and 
> > > > > > > >> > linux kernel are writing, and will send once finished.
> > > > > > > >> > 
> > > > > > > >> > since EC is a common device, so I send pathes separately.
> > > > > > > >> 
> > > > > > > >> Do any non-linux guests support this device?
> > > > > > > >> 
> > > > > > > >
> > > > > > > > In fact, any OSes support ACPI will support this device.
> > > > > > > > so, windows will.
> > > > > > > 
> > > > > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > > > > that we can provide bytecode that interacts with the embedded
> > > > > > > controller.
> > > > > > > 
> > > > > > > There is not explicit driver in Linux or Windows AFAIK.
> > > > > > 
> > > > > > Hmm, yep, mostly there's no special driver for EC,
> > > > > > because it is directly embedded in code for ACPI
> > > > > > for linux kernel, it's drivers/acpi/ec.c
> > > > > > 
> > > > > > > 
> > > > > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > > > > without introducing a new device like this.
> > > > > > > 
> > > > > > 
> > > > > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > > > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > > > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > > > > > 
> > > > > > and EC is not only for ACPI hotplug
> > > > > > for example, the 'pvpanic', if there's a EC,
> > > > > > we can pass pvpanic event by EC's ACPI space or Q-event,
> > > > > > then we will not need this kind of special devices anymore.
> > > > > > 
> > > > > > 
> > > > > 
> > > > > 
> > > > > So just to clarify: this patchset doesn't do anything useful
> > > > > itself, 
> > > > 
> > > > the first patch is pure EC implementation which is useful.
> > > 
> > > Sorry, I didn't mean to imply that this is not useful,
> > > merely that it is not yet directly useful for users.
> > 
> > Yep, by now, of course, only 1 user, that's me. :-)
> > 
> > > 
> > > > > it's some infrastructure that you want to build on
> > > > > top of?
> > > > 
> > > > Yes, I'd like to use EC as infrastructure for my further development.
> > > > 
> > > 
> > > I (and apparently others in this thread) would like to know some more
> > > about planned use for this code before deciding on inclusion.
> > > 
> > 
> > OK, 
> > currently, I am using EC to pass cpu hotplug event between QEMU and
> > linux kernel, and I'll post these patches later.
> > then, I'll query usages like this in QEMU, and convert them to use
> > EC's acpi approach if possible.
> When I see these patches for qemu/seebios/kernel, I probably would be able
> to comment on them. 

I'll send a RFC patch-set which will include all my work tomorrow.

> But right now I don't see benefits in switching
> CPU hot-plug to EC.

sorry, maybe a mistake here, I am not trying to switching
CPU hot-plug to EC, but just make EC as a helper to do
CPU hot-plug, like pass plug/unplug event between OS and
platform, it's convenient.

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[Igor Mammedov]

On Tue, 28 May 2013 16:34:42 +0800
li guang <lig.fnst@cn.fujitsu.com> wrote:

> 在 2013-05-28二的 10:16 +0200，Igor Mammedov写道：
> > On Tue, 28 May 2013 08:28:09 +0800
> > li guang <lig.fnst@cn.fujitsu.com> wrote:
> > 
> > > 在 2013-05-27一的 13:45 +0200，Igor Mammedov写道：
> > > > On Mon, 27 May 2013 09:22:59 +0800
> > > > li guang <lig.fnst@cn.fujitsu.com> wrote:
> > > > 
> > > > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > > > 
> > > > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > > > >> > refer-to:
> > > > > > >> > ACPI SPEC v5 chapter 5 
> > > > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > > > >> > 
> > > > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > > > >> > e.g. 
> > > > > > >> > power controller, it can control power sequence for
> > > > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > > > >> > event carrier, it can pass events between platform
> > > > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > > > >> > by yourself.
> > > > > > >> > 
> > > > > > >> > So, I want to deliver CPU online/offline event between
> > > > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > > > >> > again after 'cpu-add'.
> > > > > > >> > 
> > > > > > >> > patches for online/offline event handler of QEUM and 
> > > > > > >> > linux kernel are writing, and will send once finished.
> > > > > > >> > 
> > > > > > >> > since EC is a common device, so I send pathes separately.
> > > > > > >> 
> > > > > > >> Do any non-linux guests support this device?
> > > > > > >> 
> > > > > > >
> > > > > > > In fact, any OSes support ACPI will support this device.
> > > > > > > so, windows will.
> > > > > > 
> > > > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > > > that we can provide bytecode that interacts with the embedded
> > > > > > controller.
> > > > > > 
> > > > > > There is not explicit driver in Linux or Windows AFAIK.
> > > > > 
> > > > > Hmm, yep, mostly there's no special driver for EC,
> > > > > because it is directly embedded in code for ACPI
> > > > > for linux kernel, it's drivers/acpi/ec.c
> > > > > 
> > > > > > 
> > > > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > > > without introducing a new device like this.
> > > > > > 
> > > > > 
> > > > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > > > it is up to guest OS policy to decide if CPU should be onlined or not,
> > > 
> > > Yep, but I think it's a favor to do cpu online.
> > Then you have to teach kernel driver (EC) to do cpu_up on _Q02 event,
> 
> I think it's not necessary to do this.
> we can also add another platform driver to listen cpu plug/unplug event
> and query EC's ACPI space to decide what to do next.
> 
> > the question is why would you do this when there is ACPI processor driver
> > already that handles CPU hotplug in kernel.
> > You can try add cpu_up() there and perhaps with good enough reasoning it
> > would be accepted.
> 
> it's not so convenient to trigger cpu_up/down process for ACPI processor
> driver as far as I can see, it seems like informal or hack there.
> so I'd like to make a bridge between them.
> 
> but, anyway, it's a good point to think about or even try to do.
> 
> > 
> > > 
> > > > at least I've seen this rationale on LKML when topic was discussed and
> > > > automatic cpu_up on hotplug was rejected. 
> > > 
> > > Oh, and the reason is?
> > Reason was to let guest decide whether online new CPU or nor instead of
> > doing it unconditionally.
> > 
> 
> can this be a Kconfig option?
> or it's strongly recommended to let guest decide?

Probably there is no harm in trying to post patch to LKML and get feedback.

Re: [Qemu-devel] [PATCH 0/4] add ACPI Embedded Controller

[li guang]

在 2013-05-28二的 11:38 +0200，Igor Mammedov写道：
> On Tue, 28 May 2013 16:34:42 +0800
> li guang <lig.fnst@cn.fujitsu.com> wrote:
> 
> > 在 2013-05-28二的 10:16 +0200，Igor Mammedov写道：
> > > On Tue, 28 May 2013 08:28:09 +0800
> > > li guang <lig.fnst@cn.fujitsu.com> wrote:
> > > 
> > > > 在 2013-05-27一的 13:45 +0200，Igor Mammedov写道：
> > > > > On Mon, 27 May 2013 09:22:59 +0800
> > > > > li guang <lig.fnst@cn.fujitsu.com> wrote:
> > > > > 
> > > > > > 在 2013-05-26日的 19:51 -0500，Anthony Liguori写道：
> > > > > > > li guang <lig.fnst@cn.fujitsu.com> writes:
> > > > > > > 
> > > > > > > > 在 2013-05-24五的 14:45 +0300，Michael S. Tsirkin写道：
> > > > > > > >> On Wed, May 22, 2013 at 11:46:33AM +0800, liguang wrote:
> > > > > > > >> > These patches try to add ACPI Embedded Controller (EC),
> > > > > > > >> > refer-to:
> > > > > > > >> > ACPI SPEC v5 chapter 5 
> > > > > > > >> > "ACPI Embedded Controller Interface Specification"
> > > > > > > >> > 
> > > > > > > >> > EC is a standard ACPI device, it plays flexible roles,
> > > > > > > >> > e.g. 
> > > > > > > >> > power controller, it can control power sequence for
> > > > > > > >> > platform to enter or leave system state(0,1,3,4,5),
> > > > > > > >> > it can controller CPU fan by the temperature of sensor.
> > > > > > > >> > event carrier, it can pass events between platform
> > > > > > > >> > and OS, so OS can execute _Qxx method which defined
> > > > > > > >> > by yourself.
> > > > > > > >> > 
> > > > > > > >> > So, I want to deliver CPU online/offline event between
> > > > > > > >> > OS and QEMU for CPU hotplug feature, then we will don't
> > > > > > > >> > need to "echo 1 > /sys/devices/system/cpu/cpu1/online"
> > > > > > > >> > again after 'cpu-add'.
> > > > > > > >> > 
> > > > > > > >> > patches for online/offline event handler of QEUM and 
> > > > > > > >> > linux kernel are writing, and will send once finished.
> > > > > > > >> > 
> > > > > > > >> > since EC is a common device, so I send pathes separately.
> > > > > > > >> 
> > > > > > > >> Do any non-linux guests support this device?
> > > > > > > >> 
> > > > > > > >
> > > > > > > > In fact, any OSes support ACPI will support this device.
> > > > > > > > so, windows will.
> > > > > > > 
> > > > > > > When you say "any OSes supporting ACPI" I think what you really mean is
> > > > > > > that we can provide bytecode that interacts with the embedded
> > > > > > > controller.
> > > > > > > 
> > > > > > > There is not explicit driver in Linux or Windows AFAIK.
> > > > > > 
> > > > > > Hmm, yep, mostly there's no special driver for EC,
> > > > > > because it is directly embedded in code for ACPI
> > > > > > for linux kernel, it's drivers/acpi/ec.c
> > > > > > 
> > > > > > > 
> > > > > > > I still don't get the point of this.  We can make ACPI hotplug work
> > > > > > > without introducing a new device like this.
> > > > > > > 
> > > > > > 
> > > > > > when you 'cpu-add' a cpu, acpi driver for cpu will not
> > > > > > trigger 'cpu_up' for linux kernel AFAIK, unless you add
> > > > > > a user space program to listen it's uevent and tigger 'cpu_up'.
> > > > > it is up to guest OS policy to decide if CPU should be onlined or not,
> > > > 
> > > > Yep, but I think it's a favor to do cpu online.
> > > Then you have to teach kernel driver (EC) to do cpu_up on _Q02 event,
> > 
> > I think it's not necessary to do this.
> > we can also add another platform driver to listen cpu plug/unplug event
> > and query EC's ACPI space to decide what to do next.
> > 
> > > the question is why would you do this when there is ACPI processor driver
> > > already that handles CPU hotplug in kernel.
> > > You can try add cpu_up() there and perhaps with good enough reasoning it
> > > would be accepted.
> > 
> > it's not so convenient to trigger cpu_up/down process for ACPI processor
> > driver as far as I can see, it seems like informal or hack there.
> > so I'd like to make a bridge between them.
> > 
> > but, anyway, it's a good point to think about or even try to do.
> > 
> > > 
> > > > 
> > > > > at least I've seen this rationale on LKML when topic was discussed and
> > > > > automatic cpu_up on hotplug was rejected. 
> > > > 
> > > > Oh, and the reason is?
> > > Reason was to let guest decide whether online new CPU or nor instead of
> > > doing it unconditionally.
> > > 
> > 
> > can this be a Kconfig option?
> > or it's strongly recommended to let guest decide?
> 
> Probably there is no harm in trying to post patch to LKML and get feedback.

Yep, I'll do that,

Thanks!