Re: [Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface

2013-12-16 Thread Michael S. Tsirkin 
On Mon, Nov 25, 2013 at 03:39:13PM +0100, Igor Mammedov wrote:
 On Thu, 21 Nov 2013 11:37:01 +0200
 Michael S. Tsirkin m...@redhat.com wrote:
 
  On Thu, Nov 21, 2013 at 03:38:44AM +0100, Igor Mammedov wrote:
   - provides static SSDT object for memory hotplug
   - SSDT template for memory devices and runtime generator
 of them in SSDT table.
   
   Signed-off-by: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com
   Signed-off-by: Igor Mammedov imamm...@redhat.com
   ---
 [...]
 
   diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
   index a4484b8..b0c581e 100644
   --- a/hw/i386/ssdt-misc.dsl
   +++ b/hw/i386/ssdt-misc.dsl
   @@ -116,4 +116,183 @@ DefinitionBlock (ssdt-misc.aml, SSDT, 0x01, 
   BXPC, BXSSDTSUSP, 0x1)
}
}
}
   +
   +External(MTFY, MethodObj)
   +
   +Scope(\_SB) {
   +Device(MHPD) {
   +Name(_HID, EISAID(PNP0C08))
   +
   +ACPI_EXTRACT_NAME_WORD_CONST ssdt_mctrl_port
   +Name(MHPP, 0x)
   +
   +ACPI_EXTRACT_NAME_DWORD_CONST ssdt_mctrl_nr_slots
   +Name(MDNR, 0x12345678)
   +
   +/* Memory hotplug IO registers */
   +OperationRegion(HPMR, SystemIO, MHPP, 24)
   +
   +Method(_CRS, 0, Serialized) {
   +Name(CRS, ResourceTemplate() {
   +IO(Decode16, 0x00, 0x00, 0x01, 24, IO)
   +})
  
  
  Declaring name makes us serialize it.
  Can't we use a local variable?
 If that works, I'll change it.
 
 But I have a question of my own perhaps to Paolo or Gerd,
 Do we really need this _CRS, because if you look at next hunk
 _STA should report not present but functioning to avoid windows BSOD.
 So there is not guaranties that OSPM would care or even query it
 and honor _CRS provided range.
 

Yes this worries me too.
Making _STA non present looks wrong.
I suspect we need to look for some other way to make
windows not crash that will make it respect the _CRS.
Also which windows? All of them?


 [...]
   +
   +Method(_STA, 0) {
   +If (LEqual(MDNR, Zero)) {
   +Return(0x0)
   +}
   +/* Leave bit 0 cleared to avoid Windows BSOD */
   +Return(0xA)
   +}
 [...]

Re: [Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface

2013-12-16 Thread Igor Mammedov 
On Mon, 16 Dec 2013 21:50:53 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Mon, Nov 25, 2013 at 03:39:13PM +0100, Igor Mammedov wrote:
  On Thu, 21 Nov 2013 11:37:01 +0200
  Michael S. Tsirkin m...@redhat.com wrote:
  
   On Thu, Nov 21, 2013 at 03:38:44AM +0100, Igor Mammedov wrote:
- provides static SSDT object for memory hotplug
- SSDT template for memory devices and runtime generator
  of them in SSDT table.

Signed-off-by: Vasilis Liaskovitis 
vasilis.liaskovi...@profitbricks.com
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
  [...]
  
diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
index a4484b8..b0c581e 100644
--- a/hw/i386/ssdt-misc.dsl
+++ b/hw/i386/ssdt-misc.dsl
@@ -116,4 +116,183 @@ DefinitionBlock (ssdt-misc.aml, SSDT, 0x01, 
BXPC, BXSSDTSUSP, 0x1)
 }
 }
 }
+
+External(MTFY, MethodObj)
+
+Scope(\_SB) {
+Device(MHPD) {
+Name(_HID, EISAID(PNP0C08))
+
+ACPI_EXTRACT_NAME_WORD_CONST ssdt_mctrl_port
+Name(MHPP, 0x)
+
+ACPI_EXTRACT_NAME_DWORD_CONST ssdt_mctrl_nr_slots
+Name(MDNR, 0x12345678)
+
+/* Memory hotplug IO registers */
+OperationRegion(HPMR, SystemIO, MHPP, 24)
+
+Method(_CRS, 0, Serialized) {
+Name(CRS, ResourceTemplate() {
+IO(Decode16, 0x00, 0x00, 0x01, 24, IO)
+})
   
   
   Declaring name makes us serialize it.
   Can't we use a local variable?
  If that works, I'll change it.
  
  But I have a question of my own perhaps to Paolo or Gerd,
  Do we really need this _CRS, because if you look at next hunk
  _STA should report not present but functioning to avoid windows BSOD.
  So there is not guaranties that OSPM would care or even query it
  and honor _CRS provided range.
  
 
 Yes this worries me too.
 Making _STA non present looks wrong.
I've made status as not present and enabled/decoding and functioning, but that
doesn't mean that OSPM respects its _CRS.
 
 I suspect we need to look for some other way to make
 windows not crash that will make it respect the _CRS.
 Also which windows? All of them?
I've tested with WS2012DCx64,

PV panic doesn't crash windows because it's showed as Unknown device, which
also doesn't guaranty that windows would respects it's _CRS.

OSPM already knows that range is used by OperationRegion.
Question is do we really need to expose OperationRegion as _CRS?


 
 
  [...]
+
+Method(_STA, 0) {
+If (LEqual(MDNR, Zero)) {
+Return(0x0)
+}
+/* Leave bit 0 cleared to avoid Windows BSOD */
+Return(0xA)
+}
  [...]


-- 
Regards,
  Igor

Re: [Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface

2013-11-25 Thread Paolo Bonzini 
Il 21/11/2013 03:38, Igor Mammedov ha scritto:
 +// 64-bit math: MAX = MIN + LEN - 1
 +Add(MINL, LENL, MAXL)
 +Add(MINH, LENH, MAXH)
 +If (Or(LLess(MAXL, MINL), LLess(MAXL, LENL))) {

The Or is not necessary, LLess(MAXL, MINL) is enough.

 +Add(MAXH, 1, MAXH)

One?  (Just for consistency, iasl does it already).

 +}

 +// Ignore (MAXL == 0  MAXH == 0) case
 +If (LEqual(MAXL, Zero)) {
 +Subtract(MAXH, One, MAXH)
 +Store(0x, MAXL)
 +} Else {
 +Subtract(MAXL, One, MAXL)
 +}
 +

Why not this:

   If (LLess(MAXL, One)) {
   Subtract(MAXH, One, MAXH)
   }
   Subtract(MAXL, One, MAXL)

Paolo

Re: [Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface

2013-11-25 Thread Igor Mammedov 
On Mon, 25 Nov 2013 14:42:44 +0100
Paolo Bonzini pbonz...@redhat.com wrote:

 Il 21/11/2013 03:38, Igor Mammedov ha scritto:
  +// 64-bit math: MAX = MIN + LEN - 1
  +Add(MINL, LENL, MAXL)
  +Add(MINH, LENH, MAXH)
  +If (Or(LLess(MAXL, MINL), LLess(MAXL, LENL))) {
 
 The Or is not necessary, LLess(MAXL, MINL) is enough.
ok, I'll fix it.

 
  +Add(MAXH, 1, MAXH)
 
 One?  (Just for consistency, iasl does it already).
sure

 
  +}
 
  +// Ignore (MAXL == 0  MAXH == 0) case
  +If (LEqual(MAXL, Zero)) {
  +Subtract(MAXH, One, MAXH)
  +Store(0x, MAXL)
  +} Else {
  +Subtract(MAXL, One, MAXL)
  +}
  +
 
 Why not this:
 
If (LLess(MAXL, One)) {
Subtract(MAXH, One, MAXH)
}
Subtract(MAXL, One, MAXL)
I'll try it, thanks!

 
 Paolo

Re: [Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface

2013-11-25 Thread Igor Mammedov 
On Thu, 21 Nov 2013 11:37:01 +0200
Michael S. Tsirkin m...@redhat.com wrote:

 On Thu, Nov 21, 2013 at 03:38:44AM +0100, Igor Mammedov wrote:
  - provides static SSDT object for memory hotplug
  - SSDT template for memory devices and runtime generator
of them in SSDT table.
  
  Signed-off-by: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com
  Signed-off-by: Igor Mammedov imamm...@redhat.com
  ---
[...]

  diff --git a/hw/i386/ssdt-misc.dsl b/hw/i386/ssdt-misc.dsl
  index a4484b8..b0c581e 100644
  --- a/hw/i386/ssdt-misc.dsl
  +++ b/hw/i386/ssdt-misc.dsl
  @@ -116,4 +116,183 @@ DefinitionBlock (ssdt-misc.aml, SSDT, 0x01, 
  BXPC, BXSSDTSUSP, 0x1)
   }
   }
   }
  +
  +External(MTFY, MethodObj)
  +
  +Scope(\_SB) {
  +Device(MHPD) {
  +Name(_HID, EISAID(PNP0C08))
  +
  +ACPI_EXTRACT_NAME_WORD_CONST ssdt_mctrl_port
  +Name(MHPP, 0x)
  +
  +ACPI_EXTRACT_NAME_DWORD_CONST ssdt_mctrl_nr_slots
  +Name(MDNR, 0x12345678)
  +
  +/* Memory hotplug IO registers */
  +OperationRegion(HPMR, SystemIO, MHPP, 24)
  +
  +Method(_CRS, 0, Serialized) {
  +Name(CRS, ResourceTemplate() {
  +IO(Decode16, 0x00, 0x00, 0x01, 24, IO)
  +})
 
 
 Declaring name makes us serialize it.
 Can't we use a local variable?
If that works, I'll change it.

But I have a question of my own perhaps to Paolo or Gerd,
Do we really need this _CRS, because if you look at next hunk
_STA should report not present but functioning to avoid windows BSOD.
So there is not guaranties that OSPM would care or even query it
and honor _CRS provided range.


[...]
  +
  +Method(_STA, 0) {
  +If (LEqual(MDNR, Zero)) {
  +Return(0x0)
  +}
  +/* Leave bit 0 cleared to avoid Windows BSOD */
  +Return(0xA)
  +}
[...]

Re: [Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface

2013-11-21 Thread Michael S. Tsirkin 
On Thu, Nov 21, 2013 at 03:38:44AM +0100, Igor Mammedov wrote:
 - provides static SSDT object for memory hotplug
 - SSDT template for memory devices and runtime generator
   of them in SSDT table.
 
 Signed-off-by: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com
 Signed-off-by: Igor Mammedov imamm...@redhat.com
 ---
 mst/pci tree specific change:
acpi-build: s/build_append_notify/build_append_notify_method/
 introduced by acpi-build: enable hotplug for PCI bridges
 should be moved to separate patch since it's generic internal API rename.
 ---
  hw/i386/Makefile.objs |3 +-
  hw/i386/acpi-build.c  |   36 ++
  hw/i386/ssdt-mem.dsl  |   75 
  hw/i386/ssdt-misc.dsl |  179 
 +
  4 files changed, 292 insertions(+), 1 deletions(-)
  create mode 100644 hw/i386/ssdt-mem.dsl
 
 diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
 index 185aacb..f9262a1 100644
 --- a/hw/i386/Makefile.objs
 +++ b/hw/i386/Makefile.objs
 @@ -9,7 +9,8 @@ obj-y += acpi-build.o
  obj-y += bios-linker-loader.o
  hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
   hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
 - hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
 + hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
 + hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex
  
  iasl-option=$(shell if test -z `$(1) $(2) 21  /dev/null` \
  ; then echo $(2); else echo $(3); fi ;)
 diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
 index 93ffb17..d41fd81 100644
 --- a/hw/i386/acpi-build.c
 +++ b/hw/i386/acpi-build.c
 @@ -36,6 +36,7 @@
  #include hw/nvram/fw_cfg.h
  #include bios-linker-loader.h
  #include hw/loader.h
 +#include qemu/config-file.h
  
  /* Supported chipsets: */
  #include hw/acpi/piix4.h
 @@ -69,6 +70,7 @@ typedef struct AcpiPmInfo {
  uint32_t gpe0_blk;
  uint32_t gpe0_blk_len;
  uint32_t io_base;
 +uint16_t mem_hotplug_io_base;
  } AcpiPmInfo;
  
  typedef struct AcpiMiscInfo {
 @@ -176,6 +178,8 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
 NULL);
  pm-gpe0_blk_len = object_property_get_int(obj, 
 ACPI_PM_PROP_GPE0_BLK_LEN,
 NULL);
 +pm-mem_hotplug_io_base =
 +object_property_get_int(obj, ACPI_MEMORY_HOTPLUG_IO_BASE_PROP, NULL);
  }
  
  static void acpi_get_misc_info(AcpiMiscInfo *info)
 @@ -632,6 +636,14 @@ static inline char acpi_get_hex(uint32_t val)
  #define ACPI_PCIHP_SIZEOF (*ssdt_pcihp_end - *ssdt_pcihp_start)
  #define ACPI_PCIHP_AML (ssdp_pcihp_aml + *ssdt_pcihp_start)
  
 +#include hw/i386/ssdt-mem.hex
 +
 +/* 0x5B 0x82 DeviceOp PkgLength NameString DimmID */
 +#define ACPI_MEM_OFFSET_HEX (*ssdt_mem_name - *ssdt_mem_start + 2)
 +#define ACPI_MEM_OFFSET_ID (*ssdt_mem_id - *ssdt_mem_start + 7)
 +#define ACPI_MEM_SIZEOF (*ssdt_mem_end - *ssdt_mem_start)
 +#define ACPI_MEM_AML (ssdm_mem_aml + *ssdt_mem_start)
 +
  #define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */
  #define ACPI_SSDT_HEADER_LENGTH 36
  
 @@ -897,6 +909,8 @@ build_ssdt(GArray *table_data, GArray *linker,
 AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
 PcPciInfo *pci, PcGuestInfo *guest_info)
  {
 +QemuOpts *opts = qemu_opts_find(qemu_find_opts(memory-opts), NULL);
 +uint32_t nr_mem = qemu_opt_get_number(opts, slots, 0);
  int acpi_cpus = MIN(0xff, guest_info-apic_id_limit);
  int ssdt_start = table_data-len;
  uint8_t *ssdt_ptr;
 @@ -920,6 +934,10 @@ build_ssdt(GArray *table_data, GArray *linker,
  *(uint16_t *)(ssdt_ptr + *ssdt_isa_pest) =
  cpu_to_le16(misc-pvpanic_port);
  
 +*(uint16_t *)(ssdt_ptr + *ssdt_mctrl_port) =
 +cpu_to_le16(pm-mem_hotplug_io_base);
 +*(int32_t *)(ssdt_ptr + *ssdt_mctrl_nr_slots) = cpu_to_le32(nr_mem);
 +
  {
  GArray *sb_scope = build_alloc_array();
  uint8_t op = 0x10; /* ScopeOp */
 @@ -961,6 +979,24 @@ build_ssdt(GArray *table_data, GArray *linker,
  build_free_array(package);
  }
  
 +if (nr_mem) {
 +/* build memory devices */
 +for (i = 0; i  nr_mem; i++) {
 +char id[3];
 +uint8_t *mem = acpi_data_push(sb_scope, ACPI_MEM_SIZEOF);
 +
 +snprintf(id, sizeof(id), %02x, i);
 +memcpy(mem, ACPI_MEM_AML, ACPI_MEM_SIZEOF);
 +memcpy(mem + ACPI_MEM_OFFSET_HEX, id, 2);
 +memcpy(mem + ACPI_MEM_OFFSET_ID, id, 2);
 +}
 +
 +/* build Method(MTFY, 2) {
 + * If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ...
 + */
 +build_append_notify_method(sb_scope, MTFY, MP%0.02X, nr_mem);
 +}
 +
  {
  AcpiBuildPciBusHotplugState hotplug_state;
  PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
 diff --git a/hw/i386/ssdt-mem.dsl

[Qemu-devel] [PATCH 23/27] pc: ACPI BIOS: implement memory hotplug interface

2013-11-20 Thread Igor Mammedov 
- provides static SSDT object for memory hotplug
- SSDT template for memory devices and runtime generator
  of them in SSDT table.

Signed-off-by: Vasilis Liaskovitis vasilis.liaskovi...@profitbricks.com
Signed-off-by: Igor Mammedov imamm...@redhat.com
---
mst/pci tree specific change:
   acpi-build: s/build_append_notify/build_append_notify_method/
introduced by acpi-build: enable hotplug for PCI bridges
should be moved to separate patch since it's generic internal API rename.
---
 hw/i386/Makefile.objs |3 +-
 hw/i386/acpi-build.c  |   36 ++
 hw/i386/ssdt-mem.dsl  |   75 
 hw/i386/ssdt-misc.dsl |  179 +
 4 files changed, 292 insertions(+), 1 deletions(-)
 create mode 100644 hw/i386/ssdt-mem.dsl

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 185aacb..f9262a1 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -9,7 +9,8 @@ obj-y += acpi-build.o
 obj-y += bios-linker-loader.o
 hw/i386/acpi-build.o: hw/i386/acpi-build.c hw/i386/acpi-dsdt.hex \
hw/i386/ssdt-proc.hex hw/i386/ssdt-pcihp.hex hw/i386/ssdt-misc.hex \
-   hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex
+   hw/i386/acpi-dsdt.hex hw/i386/q35-acpi-dsdt.hex \
+   hw/i386/q35-acpi-dsdt.hex hw/i386/ssdt-mem.hex
 
 iasl-option=$(shell if test -z `$(1) $(2) 21  /dev/null` \
 ; then echo $(2); else echo $(3); fi ;)
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index 93ffb17..d41fd81 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -36,6 +36,7 @@
 #include hw/nvram/fw_cfg.h
 #include bios-linker-loader.h
 #include hw/loader.h
+#include qemu/config-file.h
 
 /* Supported chipsets: */
 #include hw/acpi/piix4.h
@@ -69,6 +70,7 @@ typedef struct AcpiPmInfo {
 uint32_t gpe0_blk;
 uint32_t gpe0_blk_len;
 uint32_t io_base;
+uint16_t mem_hotplug_io_base;
 } AcpiPmInfo;
 
 typedef struct AcpiMiscInfo {
@@ -176,6 +178,8 @@ static void acpi_get_pm_info(AcpiPmInfo *pm)
NULL);
 pm-gpe0_blk_len = object_property_get_int(obj, ACPI_PM_PROP_GPE0_BLK_LEN,
NULL);
+pm-mem_hotplug_io_base =
+object_property_get_int(obj, ACPI_MEMORY_HOTPLUG_IO_BASE_PROP, NULL);
 }
 
 static void acpi_get_misc_info(AcpiMiscInfo *info)
@@ -632,6 +636,14 @@ static inline char acpi_get_hex(uint32_t val)
 #define ACPI_PCIHP_SIZEOF (*ssdt_pcihp_end - *ssdt_pcihp_start)
 #define ACPI_PCIHP_AML (ssdp_pcihp_aml + *ssdt_pcihp_start)
 
+#include hw/i386/ssdt-mem.hex
+
+/* 0x5B 0x82 DeviceOp PkgLength NameString DimmID */
+#define ACPI_MEM_OFFSET_HEX (*ssdt_mem_name - *ssdt_mem_start + 2)
+#define ACPI_MEM_OFFSET_ID (*ssdt_mem_id - *ssdt_mem_start + 7)
+#define ACPI_MEM_SIZEOF (*ssdt_mem_end - *ssdt_mem_start)
+#define ACPI_MEM_AML (ssdm_mem_aml + *ssdt_mem_start)
+
 #define ACPI_SSDT_SIGNATURE 0x54445353 /* SSDT */
 #define ACPI_SSDT_HEADER_LENGTH 36
 
@@ -897,6 +909,8 @@ build_ssdt(GArray *table_data, GArray *linker,
AcpiCpuInfo *cpu, AcpiPmInfo *pm, AcpiMiscInfo *misc,
PcPciInfo *pci, PcGuestInfo *guest_info)
 {
+QemuOpts *opts = qemu_opts_find(qemu_find_opts(memory-opts), NULL);
+uint32_t nr_mem = qemu_opt_get_number(opts, slots, 0);
 int acpi_cpus = MIN(0xff, guest_info-apic_id_limit);
 int ssdt_start = table_data-len;
 uint8_t *ssdt_ptr;
@@ -920,6 +934,10 @@ build_ssdt(GArray *table_data, GArray *linker,
 *(uint16_t *)(ssdt_ptr + *ssdt_isa_pest) =
 cpu_to_le16(misc-pvpanic_port);
 
+*(uint16_t *)(ssdt_ptr + *ssdt_mctrl_port) =
+cpu_to_le16(pm-mem_hotplug_io_base);
+*(int32_t *)(ssdt_ptr + *ssdt_mctrl_nr_slots) = cpu_to_le32(nr_mem);
+
 {
 GArray *sb_scope = build_alloc_array();
 uint8_t op = 0x10; /* ScopeOp */
@@ -961,6 +979,24 @@ build_ssdt(GArray *table_data, GArray *linker,
 build_free_array(package);
 }
 
+if (nr_mem) {
+/* build memory devices */
+for (i = 0; i  nr_mem; i++) {
+char id[3];
+uint8_t *mem = acpi_data_push(sb_scope, ACPI_MEM_SIZEOF);
+
+snprintf(id, sizeof(id), %02x, i);
+memcpy(mem, ACPI_MEM_AML, ACPI_MEM_SIZEOF);
+memcpy(mem + ACPI_MEM_OFFSET_HEX, id, 2);
+memcpy(mem + ACPI_MEM_OFFSET_ID, id, 2);
+}
+
+/* build Method(MTFY, 2) {
+ * If (LEqual(Arg0, 0x00)) {Notify(MP00, Arg1)} ...
+ */
+build_append_notify_method(sb_scope, MTFY, MP%0.02X, nr_mem);
+}
+
 {
 AcpiBuildPciBusHotplugState hotplug_state;
 PCIBus *bus = find_i440fx(); /* TODO: Q35 support */
diff --git a/hw/i386/ssdt-mem.dsl b/hw/i386/ssdt-mem.dsl
new file mode 100644
index 000..7f68750
--- /dev/null
+++ b/hw/i386/ssdt-mem.dsl
@@ -0,0 +1,75 @@
+/*
+ * Memory hotplug ACPI DSDT static objects