Re: [Qemu-devel] [PATCH v2 repost 8/9] i386: generate pc guest info

2013-07-24 Thread Laszlo Ersek 
On 07/17/13 17:07, Laszlo Ersek wrote:
 On 07/10/13 15:51, Michael S. Tsirkin wrote:
 This fills in guest info table with misc
 information of interest to the guest.
 Will be used by ACPI table generation code.

 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/acpi/ich9.c |  7 ++-
  hw/acpi/piix4.c| 44 +++-
  hw/i386/Makefile.objs  |  2 ++
  hw/i386/pc.c   | 41 +++--
  hw/i386/pc_piix.c  | 15 ---
  hw/i386/pc_q35.c   | 10 +++---
  hw/isa/lpc_ich9.c  | 11 +--
  hw/mips/mips_malta.c   |  2 +-
  hw/misc/pvpanic.c  | 12 +++-
  hw/pci-host/q35.c  |  1 +
  include/hw/acpi/ich9.h |  2 +-
  include/hw/i386/ich9.h |  3 ++-
  include/hw/i386/pc.h   | 37 ++---
  13 files changed, 164 insertions(+), 23 deletions(-)
 
 So we won't be fishing in a global pool of information at ACPI table
 creation time as I had expected / advertized before. Instead any
 required bits are gradually collected into the guest info structure
 while creating / configuring the machine.
 
 This is likely a better approach; the set of dependencies for all ACPI
 tables together are tracked explicitly in guest info. Also, we don't
 collect the bits from the outside, breaching encapsulation of devices;
 devices publish the bits.
 
 Reviewed-by: Laszlo Ersek ler...@redhat.com

If I understand correctly, based on the recent comments you got from
Gerd and Andreas for this series (in the other, non-repost thread),
fishing in the global pool it should be after all, just with a different
fishing rod than what I used in my original patch (hw/i386: build ACPI
MADT (APIC) for fw_cfg clients).

These U-turns in design have proved that I'm not qualified to review
this work. So I won't; there's no use in my repeated saying yeah why
not to both approaches (which are polar opposites). My apologies.

I applaud your perseverance in this matter.

Laszlo

Re: [Qemu-devel] [PATCH v2 repost 8/9] i386: generate pc guest info

2013-07-24 Thread Michael S. Tsirkin 
On Wed, Jul 24, 2013 at 05:36:58PM +0200, Laszlo Ersek wrote:
 On 07/17/13 17:07, Laszlo Ersek wrote:
  On 07/10/13 15:51, Michael S. Tsirkin wrote:
  This fills in guest info table with misc
  information of interest to the guest.
  Will be used by ACPI table generation code.
 
  Signed-off-by: Michael S. Tsirkin m...@redhat.com
  ---
   hw/acpi/ich9.c |  7 ++-
   hw/acpi/piix4.c| 44 +++-
   hw/i386/Makefile.objs  |  2 ++
   hw/i386/pc.c   | 41 +++--
   hw/i386/pc_piix.c  | 15 ---
   hw/i386/pc_q35.c   | 10 +++---
   hw/isa/lpc_ich9.c  | 11 +--
   hw/mips/mips_malta.c   |  2 +-
   hw/misc/pvpanic.c  | 12 +++-
   hw/pci-host/q35.c  |  1 +
   include/hw/acpi/ich9.h |  2 +-
   include/hw/i386/ich9.h |  3 ++-
   include/hw/i386/pc.h   | 37 ++---
   13 files changed, 164 insertions(+), 23 deletions(-)
  
  So we won't be fishing in a global pool of information at ACPI table
  creation time as I had expected / advertized before. Instead any
  required bits are gradually collected into the guest info structure
  while creating / configuring the machine.
  
  This is likely a better approach; the set of dependencies for all ACPI
  tables together are tracked explicitly in guest info. Also, we don't
  collect the bits from the outside, breaching encapsulation of devices;
  devices publish the bits.
  
  Reviewed-by: Laszlo Ersek ler...@redhat.com
 
 If I understand correctly, based on the recent comments you got from
 Gerd and Andreas for this series (in the other, non-repost thread),
 fishing in the global pool it should be after all, just with a different
 fishing rod than what I used in my original patch (hw/i386: build ACPI
 MADT (APIC) for fw_cfg clients).
 
 These U-turns in design have proved that I'm not qualified to review
 this work. So I won't; there's no use in my repeated saying yeah why
 not to both approaches (which are polar opposites). My apologies.
 
 I applaud your perseverance in this matter.
 
 Laszlo

Thanks for all your comments so far,

-- 
MST

Re: [Qemu-devel] [PATCH v2 repost 8/9] i386: generate pc guest info

2013-07-18 Thread Hu Tao 
...

 +void ich9_lpc_set_guest_info(PcGuestInfo *guest_info)
 +{
 +guest_info-sci_int = 9;
 +guest_info-acpi_enable_cmd = ICH9_APM_ACPI_ENABLE;
 +guest_info-acpi_disable_cmd = ICH9_APM_ACPI_DISABLE;
 +}
 +

This function has to be called somewhere(ich9_lpc_pm_init?) to setup
acpi_enable_cmd and acpi_disable_cmd, or guest Linux will report:

ACPI Error: No ACPI mode transition supported in this system (enable/disable 
both zero)

and disable ACPI.

Re: [Qemu-devel] [PATCH v2 repost 8/9] i386: generate pc guest info

2013-07-17 Thread Laszlo Ersek 
On 07/10/13 15:51, Michael S. Tsirkin wrote:
 This fills in guest info table with misc
 information of interest to the guest.
 Will be used by ACPI table generation code.
 
 Signed-off-by: Michael S. Tsirkin m...@redhat.com
 ---
  hw/acpi/ich9.c |  7 ++-
  hw/acpi/piix4.c| 44 +++-
  hw/i386/Makefile.objs  |  2 ++
  hw/i386/pc.c   | 41 +++--
  hw/i386/pc_piix.c  | 15 ---
  hw/i386/pc_q35.c   | 10 +++---
  hw/isa/lpc_ich9.c  | 11 +--
  hw/mips/mips_malta.c   |  2 +-
  hw/misc/pvpanic.c  | 12 +++-
  hw/pci-host/q35.c  |  1 +
  include/hw/acpi/ich9.h |  2 +-
  include/hw/i386/ich9.h |  3 ++-
  include/hw/i386/pc.h   | 37 ++---
  13 files changed, 164 insertions(+), 23 deletions(-)

So we won't be fishing in a global pool of information at ACPI table
creation time as I had expected / advertized before. Instead any
required bits are gradually collected into the guest info structure
while creating / configuring the machine.

This is likely a better approach; the set of dependencies for all ACPI
tables together are tracked explicitly in guest info. Also, we don't
collect the bits from the outside, breaching encapsulation of devices;
devices publish the bits.

Reviewed-by: Laszlo Ersek ler...@redhat.com

[Qemu-devel] [PATCH v2 repost 8/9] i386: generate pc guest info

2013-07-10 Thread Michael S. Tsirkin 
This fills in guest info table with misc
information of interest to the guest.
Will be used by ACPI table generation code.

Signed-off-by: Michael S. Tsirkin m...@redhat.com
---
 hw/acpi/ich9.c |  7 ++-
 hw/acpi/piix4.c| 44 +++-
 hw/i386/Makefile.objs  |  2 ++
 hw/i386/pc.c   | 41 +++--
 hw/i386/pc_piix.c  | 15 ---
 hw/i386/pc_q35.c   | 10 +++---
 hw/isa/lpc_ich9.c  | 11 +--
 hw/mips/mips_malta.c   |  2 +-
 hw/misc/pvpanic.c  | 12 +++-
 hw/pci-host/q35.c  |  1 +
 include/hw/acpi/ich9.h |  2 +-
 include/hw/i386/ich9.h |  3 ++-
 include/hw/i386/pc.h   | 37 ++---
 13 files changed, 164 insertions(+), 23 deletions(-)

diff --git a/hw/acpi/ich9.c b/hw/acpi/ich9.c
index 4a17f32..764e27f 100644
--- a/hw/acpi/ich9.c
+++ b/hw/acpi/ich9.c
@@ -203,7 +203,7 @@ static void pm_powerdown_req(Notifier *n, void *opaque)
 }
 
 void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
-  qemu_irq sci_irq)
+  qemu_irq sci_irq, PcGuestInfo *guest_info)
 {
 memory_region_init(pm-io, ich9-pm, ICH9_PMIO_SIZE);
 memory_region_set_enabled(pm-io, false);
@@ -219,6 +219,11 @@ void ich9_pm_init(PCIDevice *lpc_pci, ICH9LPCPMRegs *pm,
   ICH9_PMIO_GPE0_LEN);
 memory_region_add_subregion(pm-io, ICH9_PMIO_GPE0_STS, pm-io_gpe);
 
+guest_info-gpe0_blk = PC_GUEST_PORT_ACPI_PM_BASE + ICH9_PMIO_GPE0_STS;
+guest_info-gpe0_blk_len = ICH9_PMIO_GPE0_LEN;
+guest_info-fix_rtc = true;
+guest_info-platform_timer = false;
+
 memory_region_init_io(pm-io_smi, ich9_smi_ops, pm, apci-smi,
   8);
 memory_region_add_subregion(pm-io, ICH9_PMIO_SMI_EN, pm-io_smi);
diff --git a/hw/acpi/piix4.c b/hw/acpi/piix4.c
index 756df3b..c077a7a 100644
--- a/hw/acpi/piix4.c
+++ b/hw/acpi/piix4.c
@@ -94,6 +94,8 @@ typedef struct PIIX4PMState {
 
 CPUStatus gpe_cpu;
 Notifier cpu_added_notifier;
+
+PcGuestInfo *guest_info;
 } PIIX4PMState;
 
 static void piix4_acpi_system_hot_add_init(MemoryRegion *parent,
@@ -380,6 +382,27 @@ static void piix4_pm_powerdown_req(Notifier *n, void 
*opaque)
 acpi_pm1_evt_power_down(s-ar);
 }
 
+static void piix4_update_guest_info(PIIX4PMState *s)
+{
+PCIDevice *dev = s-dev;
+BusState *bus = qdev_get_parent_bus(dev-qdev);
+BusChild *kid, *next;
+
+memset(s-guest_info-slot_hotplug_enable, 0xff,
+   DIV_ROUND_UP(PCI_SLOT_MAX, BITS_PER_BYTE));
+
+QTAILQ_FOREACH_SAFE(kid, bus-children, sibling, next) {
+DeviceState *qdev = kid-child;
+PCIDevice *pdev = PCI_DEVICE(qdev);
+PCIDeviceClass *pc = PCI_DEVICE_GET_CLASS(pdev);
+int slot = PCI_SLOT(pdev-devfn);
+
+if (pc-no_hotplug) {
+clear_bit(slot, s-guest_info-slot_hotplug_enable);
+}
+}
+}
+
 static void piix4_pm_machine_ready(Notifier *n, void *opaque)
 {
 PIIX4PMState *s = container_of(n, PIIX4PMState, machine_ready);
@@ -391,6 +414,9 @@ static void piix4_pm_machine_ready(Notifier *n, void 
*opaque)
 pci_conf[0x67] = (isa_is_ioport_assigned(0x3f8) ? 0x08 : 0) |
(isa_is_ioport_assigned(0x2f8) ? 0x90 : 0);
 
+if (s-guest_info) {
+piix4_update_guest_info(s);
+}
 }
 
 static int piix4_pm_initfn(PCIDevice *dev)
@@ -447,7 +473,8 @@ static int piix4_pm_initfn(PCIDevice *dev)
 
 i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t smb_io_base,
qemu_irq sci_irq, qemu_irq smi_irq,
-   int kvm_enabled, FWCfgState *fw_cfg)
+   int kvm_enabled, FWCfgState *fw_cfg,
+   PcGuestInfo *guest_info)
 {
 PCIDevice *dev;
 PIIX4PMState *s;
@@ -470,6 +497,21 @@ i2c_bus *piix4_pm_init(PCIBus *bus, int devfn, uint32_t 
smb_io_base,
 fw_cfg_add_file(fw_cfg, etc/system-states, g_memdup(suspend, 6), 6);
 }
 
+if (guest_info) {
+s-guest_info = guest_info;
+
+guest_info-s3_disabled = s-disable_s3;
+guest_info-s4_disabled = s-disable_s4;
+guest_info-s4_val = s-s4_val;
+
+guest_info-acpi_enable_cmd = ACPI_ENABLE;
+guest_info-acpi_disable_cmd = ACPI_DISABLE;
+guest_info-gpe0_blk = GPE_BASE;
+guest_info-gpe0_blk_len = GPE_LEN;
+guest_info-fix_rtc = false;
+guest_info-platform_timer = true;
+}
+
 return s-smb.smbus;
 }
 
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 71be2da..e783050 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -5,6 +5,8 @@ obj-$(CONFIG_XEN) += xen_domainbuild.o xen_machine_pv.o
 
 obj-y += kvmvapic.o
 obj-y += bios-linker-loader.o
+hw/i386/pc_piix.o: hw/i386/pc_piix.c hw/i386/acpi-dsdt.hex
+hw/i386/pc_q35.o: hw/i386/pc_q35.c hw/i386/q35-acpi-dsdt.hex
 
 iasl-option=$(shell if test -z `$(1) $(2) 21  /dev/null` \
 ; then echo $(2); else echo $(3);