Patchwork [2/3] acpi_piix4: Add stub functions for CPU eject callback

login
register
mail settings
Submitter Vasilis Liaskovitis
Date Jan. 13, 2012, 11:11 a.m.
Message ID <1326453092-4256-3-git-send-email-vasilis.liaskovitis@profitbricks.com>
Download mbox | patch
Permalink /patch/135833/
State New
Headers show

Comments

Vasilis Liaskovitis - Jan. 13, 2012, 11:11 a.m.
Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
---
 hw/acpi_piix4.c |   15 +++++++++++++++
 1 files changed, 15 insertions(+), 0 deletions(-)
Avi Kivity - Jan. 15, 2012, 12:38 p.m.
On 01/13/2012 01:11 PM, Vasilis Liaskovitis wrote:
> Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
> ---
>  hw/acpi_piix4.c |   15 +++++++++++++++
>  1 files changed, 15 insertions(+), 0 deletions(-)
>
> diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> index d5743b6..8bf30dd 100644
> --- a/hw/acpi_piix4.c
> +++ b/hw/acpi_piix4.c
> @@ -37,6 +37,7 @@
>  
>  #define GPE_BASE 0xafe0
>  #define PROC_BASE 0xaf00
> +#define PROC_EJ_BASE 0xaf20
>

We're adding stuff to piix4 which was never there.  At a minimum this
needs to be documented.  Also needs to be -M pc-1.1 and later only.
Vasilis Liaskovitis - Jan. 16, 2012, 11:32 a.m.
On Sun, Jan 15, 2012 at 02:38:52PM +0200, Avi Kivity wrote:
> On 01/13/2012 01:11 PM, Vasilis Liaskovitis wrote:
> > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
> > ---
> >  hw/acpi_piix4.c |   15 +++++++++++++++
> >  1 files changed, 15 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> > index d5743b6..8bf30dd 100644
> > --- a/hw/acpi_piix4.c
> > +++ b/hw/acpi_piix4.c
> > @@ -37,6 +37,7 @@
> >  
> >  #define GPE_BASE 0xafe0
> >  #define PROC_BASE 0xaf00
> > +#define PROC_EJ_BASE 0xaf20
> >
> 
> We're adding stuff to piix4 which was never there.  At a minimum this
> needs to be documented.  Also needs to be -M pc-1.1 and later only.

Where should this be documented? PCI/ACPI hotplug addresses are documented in
docs/specs/acpi_pci_hotplug.txt but for CPU hotplug documentation (i.e.
for the existing PROC_BASE) I don't see relevant documentation. I will
create a docs/specs/acpi_cpu_hotplug.txt if that sounds reasonable.

For pc-1.1, a new QEMUmachine type will be needed I assume. Should a check be
made against the machine version in the piix4 code? any relevant examples? 

thanks,

- Vasilis
Avi Kivity - Jan. 16, 2012, 12:26 p.m.
On 01/16/2012 01:32 PM, Vasilis Liaskovitis wrote:
> On Sun, Jan 15, 2012 at 02:38:52PM +0200, Avi Kivity wrote:
> > On 01/13/2012 01:11 PM, Vasilis Liaskovitis wrote:
> > > Signed-off-by: Vasilis Liaskovitis <vasilis.liaskovitis@profitbricks.com>
> > > ---
> > >  hw/acpi_piix4.c |   15 +++++++++++++++
> > >  1 files changed, 15 insertions(+), 0 deletions(-)
> > >
> > > diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
> > > index d5743b6..8bf30dd 100644
> > > --- a/hw/acpi_piix4.c
> > > +++ b/hw/acpi_piix4.c
> > > @@ -37,6 +37,7 @@
> > >  
> > >  #define GPE_BASE 0xafe0
> > >  #define PROC_BASE 0xaf00
> > > +#define PROC_EJ_BASE 0xaf20
> > >
> > 
> > We're adding stuff to piix4 which was never there.  At a minimum this
> > needs to be documented.  Also needs to be -M pc-1.1 and later only.
>
> Where should this be documented? PCI/ACPI hotplug addresses are documented in
> docs/specs/acpi_pci_hotplug.txt 

A pleasant surprise

> but for CPU hotplug documentation (i.e.
> for the existing PROC_BASE) I don't see relevant documentation. I will
> create a docs/specs/acpi_cpu_hotplug.txt if that sounds reasonable.

I suggest renaming it to acpi_hotplug.txt, so it covers both cases.

> For pc-1.1, a new QEMUmachine type will be needed I assume. Should a check be
> made against the machine version in the piix4 code? any relevant examples? 
>

The standard practice is to set a property.  See for example
pc_machine_v0_14 in hw/pc_piix.c, it autosets properties for devices
(erroneously called "drivers" in the code).

btw, I notice the I/O ports are write only and don't remember their
state.  I can't think offhand if there's anything bad about it (in fact
not having state makes live migration more robust), but perhaps someone
else will.

Patch

diff --git a/hw/acpi_piix4.c b/hw/acpi_piix4.c
index d5743b6..8bf30dd 100644
--- a/hw/acpi_piix4.c
+++ b/hw/acpi_piix4.c
@@ -37,6 +37,7 @@ 
 
 #define GPE_BASE 0xafe0
 #define PROC_BASE 0xaf00
+#define PROC_EJ_BASE 0xaf20
 #define GPE_LEN 4
 #define PCI_BASE 0xae00
 #define PCI_EJ_BASE 0xae08
@@ -493,6 +494,17 @@  static void pcihotplug_write(void *opaque, uint32_t addr, uint32_t val)
     PIIX4_DPRINTF("pcihotplug write %x <== %d\n", addr, val);
 }
 
+static uint32_t cpuej_read(void *opaque, uint32_t addr)
+{
+    PIIX4_DPRINTF("cpuej read %x\n", addr);
+    return 0;
+}
+
+static void cpuej_write(void *opaque, uint32_t addr, uint32_t val)
+{
+    PIIX4_DPRINTF("cpuej write %x <== %d\n", addr, val);
+}
+
 static uint32_t pciej_read(void *opaque, uint32_t addr)
 {
     PIIX4_DPRINTF("pciej read %x\n", addr);
@@ -553,6 +565,9 @@  static void piix4_acpi_system_hot_add_init(PCIBus *bus, PIIX4PMState *s)
     register_ioport_write(PROC_BASE, 32, 1, gpe_writeb, s);
     register_ioport_read(PROC_BASE, 32, 1,  gpe_readb, s);
 
+    register_ioport_write(PROC_EJ_BASE, 32, 1, cpuej_write, s);
+    register_ioport_read(PROC_EJ_BASE, 32, 1,  cpuej_read, s);
+
     register_ioport_write(PCI_BASE, 8, 4, pcihotplug_write, pci0_status);
     register_ioport_read(PCI_BASE, 8, 4,  pcihotplug_read, pci0_status);