diff mbox

[25/74] pc: acpi: memhp: prepare context in SSDT for moving memhp DSDT code

Message ID 1449704528-289297-26-git-send-email-imammedo@redhat.com
State New
Headers show

Commit Message

Igor Mammedov Dec. 9, 2015, 11:41 p.m. UTC
Signed-off-by: Igor Mammedov <imammedo@redhat.com>
---
 hw/acpi/Makefile.objs               |  2 +-
 hw/acpi/memory_hotplug_acpi_table.c | 40 +++++++++++++++++++++++++++++++++++++
 hw/i386/acpi-build.c                |  3 +++
 include/hw/acpi/memory_hotplug.h    |  4 ++++
 4 files changed, 48 insertions(+), 1 deletion(-)
 create mode 100644 hw/acpi/memory_hotplug_acpi_table.c

Comments

Marcel Apfelbaum Dec. 10, 2015, 12:45 p.m. UTC | #1
On 12/10/2015 01:41 AM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   hw/acpi/Makefile.objs               |  2 +-
>   hw/acpi/memory_hotplug_acpi_table.c | 40 +++++++++++++++++++++++++++++++++++++
>   hw/i386/acpi-build.c                |  3 +++
>   include/hw/acpi/memory_hotplug.h    |  4 ++++
>   4 files changed, 48 insertions(+), 1 deletion(-)
>   create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
>
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 7d3230c..c04064e 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -1,7 +1,7 @@
>   common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
>   common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>   common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
>   common-obj-$(CONFIG_ACPI) += acpi_interface.o
>   common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>   common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c
> new file mode 100644
> index 0000000..25bbf5e
> --- /dev/null
> +++ b/hw/acpi/memory_hotplug_acpi_table.c
> @@ -0,0 +1,40 @@
> +/*
> + * Memory hotplug AML code of DSDT ACPI table
> + *
> + * Copyright (C) 2013 Red Hat Inc

You mean 2015, right :) ?


> + *
> + * Author: Igor Mammedov <imammedo@redhat.com>
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * 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, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <stdbool.h>
> +#include "hw/acpi/memory_hotplug.h"
> +#include "include/hw/acpi/pc-hotplug.h"
> +#include "hw/boards.h"
> +
> +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> +                              uint16_t io_base, uint16_t io_len)
> +{
> +    Aml *pci_scope;
> +    Aml *ctrl_dev;
> +
> +    /* scope for memory hotplug controller device node */
> +    pci_scope = aml_scope("_SB.PCI0");
> +    ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE));
> +    {
> +    }
> +    aml_append(pci_scope, ctrl_dev);
> +    aml_append(ctx, pci_scope);
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index bbd37e9..1b609e6 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -928,6 +928,9 @@ build_ssdt(GArray *table_data, GArray *linker,
>       /* Reserve space for header */
>       acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>
> +    build_memory_hotplug_aml(ssdt, nr_mem, pm->mem_hp_io_base,
> +                             pm->mem_hp_io_len);
> +
>       /* Extra PCI root buses are implemented  only for i440fx */
>       bus = find_i440fx();
>       if (bus) {
> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> index 1342adb..b6e9f50 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -4,6 +4,7 @@
>   #include "hw/qdev-core.h"
>   #include "hw/acpi/acpi.h"
>   #include "migration/vmstate.h"
> +#include "hw/acpi/aml-build.h"
>
>   /**
>    * MemStatus:
> @@ -45,4 +46,7 @@ extern const VMStateDescription vmstate_memory_hotplug;
>                      vmstate_memory_hotplug, MemHotplugState)
>
>   void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
> +
> +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> +                              uint16_t io_base, uint16_t io_len);
>   #endif
>
Igor Mammedov Dec. 10, 2015, 1:43 p.m. UTC | #2
On Thu, 10 Dec 2015 14:45:32 +0200
Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:

> On 12/10/2015 01:41 AM, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >   hw/acpi/Makefile.objs               |  2 +-
> >   hw/acpi/memory_hotplug_acpi_table.c | 40
> > +++++++++++++++++++++++++++++++++++++
> > hw/i386/acpi-build.c                |  3 +++
> > include/hw/acpi/memory_hotplug.h    |  4 ++++ 4 files changed, 48
> > insertions(+), 1 deletion(-) create mode 100644
> > hw/acpi/memory_hotplug_acpi_table.c
> >
> > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> > index 7d3230c..c04064e 100644
> > --- a/hw/acpi/Makefile.objs
> > +++ b/hw/acpi/Makefile.objs
> > @@ -1,7 +1,7 @@
> >   common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> >   common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
> >   common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> > -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> > +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> > memory_hotplug_acpi_table.o common-obj-$(CONFIG_ACPI) +=
> > acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
> >   common-obj-$(CONFIG_ACPI) += aml-build.o
> > diff --git a/hw/acpi/memory_hotplug_acpi_table.c
> > b/hw/acpi/memory_hotplug_acpi_table.c new file mode 100644
> > index 0000000..25bbf5e
> > --- /dev/null
> > +++ b/hw/acpi/memory_hotplug_acpi_table.c
> > @@ -0,0 +1,40 @@
> > +/*
> > + * Memory hotplug AML code of DSDT ACPI table
> > + *
> > + * Copyright (C) 2013 Red Hat Inc
> 
> You mean 2015, right :) ?
yep, I'll fix it.

> 
> 
> > + *
> > + * Author: Igor Mammedov <imammedo@redhat.com>
> > + *
> > + * 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; either version 2 of the License,
> > or
> > + * (at your option) any later version.
> > +
> > + * 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, see <http://www.gnu.org/licenses/>.
> > + */
> > +
> > +#include <stdbool.h>
> > +#include "hw/acpi/memory_hotplug.h"
> > +#include "include/hw/acpi/pc-hotplug.h"
> > +#include "hw/boards.h"
> > +
> > +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> > +                              uint16_t io_base, uint16_t io_len)
> > +{
> > +    Aml *pci_scope;
> > +    Aml *ctrl_dev;
> > +
> > +    /* scope for memory hotplug controller device node */
> > +    pci_scope = aml_scope("_SB.PCI0");
> > +    ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE));
> > +    {
> > +    }
> > +    aml_append(pci_scope, ctrl_dev);
> > +    aml_append(ctx, pci_scope);
> > +}
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index bbd37e9..1b609e6 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -928,6 +928,9 @@ build_ssdt(GArray *table_data, GArray *linker,
> >       /* Reserve space for header */
> >       acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
> >
> > +    build_memory_hotplug_aml(ssdt, nr_mem, pm->mem_hp_io_base,
> > +                             pm->mem_hp_io_len);
> > +
> >       /* Extra PCI root buses are implemented  only for i440fx */
> >       bus = find_i440fx();
> >       if (bus) {
> > diff --git a/include/hw/acpi/memory_hotplug.h
> > b/include/hw/acpi/memory_hotplug.h index 1342adb..b6e9f50 100644
> > --- a/include/hw/acpi/memory_hotplug.h
> > +++ b/include/hw/acpi/memory_hotplug.h
> > @@ -4,6 +4,7 @@
> >   #include "hw/qdev-core.h"
> >   #include "hw/acpi/acpi.h"
> >   #include "migration/vmstate.h"
> > +#include "hw/acpi/aml-build.h"
> >
> >   /**
> >    * MemStatus:
> > @@ -45,4 +46,7 @@ extern const VMStateDescription
> > vmstate_memory_hotplug; vmstate_memory_hotplug, MemHotplugState)
> >
> >   void acpi_memory_ospm_status(MemHotplugState *mem_st,
> > ACPIOSTInfoList ***list); +
> > +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> > +                              uint16_t io_base, uint16_t io_len);
> >   #endif
> >
>
Marcel Apfelbaum Dec. 16, 2015, 1:25 p.m. UTC | #3
On 12/10/2015 01:41 AM, Igor Mammedov wrote:
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   hw/acpi/Makefile.objs               |  2 +-
>   hw/acpi/memory_hotplug_acpi_table.c | 40 +++++++++++++++++++++++++++++++++++++
>   hw/i386/acpi-build.c                |  3 +++
>   include/hw/acpi/memory_hotplug.h    |  4 ++++
>   4 files changed, 48 insertions(+), 1 deletion(-)
>   create mode 100644 hw/acpi/memory_hotplug_acpi_table.c
>
> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> index 7d3230c..c04064e 100644
> --- a/hw/acpi/Makefile.objs
> +++ b/hw/acpi/Makefile.objs
> @@ -1,7 +1,7 @@
>   common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
>   common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>   common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
>   common-obj-$(CONFIG_ACPI) += acpi_interface.o
>   common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>   common-obj-$(CONFIG_ACPI) += aml-build.o
> diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c
> new file mode 100644
> index 0000000..25bbf5e
> --- /dev/null
> +++ b/hw/acpi/memory_hotplug_acpi_table.c
> @@ -0,0 +1,40 @@
> +/*
> + * Memory hotplug AML code of DSDT ACPI table

You are advertising as part of the DSDT code, but you are putting in SSDT, right?
By the way, maybe the answer is clear, but why are you moving it to SSDT?

Last thing, from this patch forward (until maybe the last one) make check fails, right?
(Specifically the acpi tests)


> + *
> + * Copyright (C) 2013 Red Hat Inc
> + *
> + * Author: Igor Mammedov <imammedo@redhat.com>
> + *
> + * 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; either version 2 of the License, or
> + * (at your option) any later version.
> +
> + * 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, see <http://www.gnu.org/licenses/>.
> + */

Regarding the license, I was advised to just add a pointer to the main license
instead of copying it in each file, something like:

    This work is licensed under the terms of the GNU GPL, version 2 or later.
    See the COPYING file in the top-level directory.

Thanks,
Marcel

> +
> +#include <stdbool.h>
> +#include "hw/acpi/memory_hotplug.h"
> +#include "include/hw/acpi/pc-hotplug.h"
> +#include "hw/boards.h"
> +
> +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> +                              uint16_t io_base, uint16_t io_len)
> +{
> +    Aml *pci_scope;
> +    Aml *ctrl_dev;
> +
> +    /* scope for memory hotplug controller device node */
> +    pci_scope = aml_scope("_SB.PCI0");
> +    ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE));
> +    {
> +    }
> +    aml_append(pci_scope, ctrl_dev);
> +    aml_append(ctx, pci_scope);
> +}
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index bbd37e9..1b609e6 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -928,6 +928,9 @@ build_ssdt(GArray *table_data, GArray *linker,
>       /* Reserve space for header */
>       acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>
> +    build_memory_hotplug_aml(ssdt, nr_mem, pm->mem_hp_io_base,
> +                             pm->mem_hp_io_len);
> +
>       /* Extra PCI root buses are implemented  only for i440fx */
>       bus = find_i440fx();
>       if (bus) {
> diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
> index 1342adb..b6e9f50 100644
> --- a/include/hw/acpi/memory_hotplug.h
> +++ b/include/hw/acpi/memory_hotplug.h
> @@ -4,6 +4,7 @@
>   #include "hw/qdev-core.h"
>   #include "hw/acpi/acpi.h"
>   #include "migration/vmstate.h"
> +#include "hw/acpi/aml-build.h"
>
>   /**
>    * MemStatus:
> @@ -45,4 +46,7 @@ extern const VMStateDescription vmstate_memory_hotplug;
>                      vmstate_memory_hotplug, MemHotplugState)
>
>   void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
> +
> +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> +                              uint16_t io_base, uint16_t io_len);
>   #endif
>
Igor Mammedov Dec. 16, 2015, 2:25 p.m. UTC | #4
On Wed, 16 Dec 2015 15:25:46 +0200
Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:

> On 12/10/2015 01:41 AM, Igor Mammedov wrote:
> > Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> > ---
> >   hw/acpi/Makefile.objs               |  2 +-
> >   hw/acpi/memory_hotplug_acpi_table.c | 40
> > +++++++++++++++++++++++++++++++++++++
> > hw/i386/acpi-build.c                |  3 +++
> > include/hw/acpi/memory_hotplug.h    |  4 ++++ 4 files changed, 48
> > insertions(+), 1 deletion(-) create mode 100644
> > hw/acpi/memory_hotplug_acpi_table.c
> >
> > diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> > index 7d3230c..c04064e 100644
> > --- a/hw/acpi/Makefile.objs
> > +++ b/hw/acpi/Makefile.objs
> > @@ -1,7 +1,7 @@
> >   common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> >   common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
> >   common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> > -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> > +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> > memory_hotplug_acpi_table.o common-obj-$(CONFIG_ACPI) +=
> > acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
> >   common-obj-$(CONFIG_ACPI) += aml-build.o
> > diff --git a/hw/acpi/memory_hotplug_acpi_table.c
> > b/hw/acpi/memory_hotplug_acpi_table.c new file mode 100644
> > index 0000000..25bbf5e
> > --- /dev/null
> > +++ b/hw/acpi/memory_hotplug_acpi_table.c
> > @@ -0,0 +1,40 @@
> > +/*
> > + * Memory hotplug AML code of DSDT ACPI table
> 
> You are advertising as part of the DSDT code, but you are putting in
> SSDT, right? By the way, maybe the answer is clear, but why are you
> moving it to SSDT?
There could be only one instance of DSDT and I temporarily put
converted DSDT bits in SSDT until conversion is complete.

> 
> Last thing, from this patch forward (until maybe the last one) make
> check fails, right? (Specifically the acpi tests)
yes, starting from this patch and upto 72nd acpi test fails since
patches move DSDT bits into SSDT.
But patch 73, when precompiled DSDT is empty and provides only header,
moves converted DSDT content out of SSDT into QEMU
generated DSDT, and drops ASL template.  

> 
> 
> > + *
> > + * Copyright (C) 2013 Red Hat Inc
> > + *
> > + * Author: Igor Mammedov <imammedo@redhat.com>
> > + *
> > + * 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; either version 2 of the License,
> > or
> > + * (at your option) any later version.
> > +
> > + * 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, see <http://www.gnu.org/licenses/>.
> > + */
> 
> Regarding the license, I was advised to just add a pointer to the
> main license instead of copying it in each file, something like:
> 
>     This work is licensed under the terms of the GNU GPL, version 2
> or later. See the COPYING file in the top-level directory.
> 
> Thanks,
> Marcel
> 
> > +
> > +#include <stdbool.h>
> > +#include "hw/acpi/memory_hotplug.h"
> > +#include "include/hw/acpi/pc-hotplug.h"
> > +#include "hw/boards.h"
> > +
> > +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> > +                              uint16_t io_base, uint16_t io_len)
> > +{
> > +    Aml *pci_scope;
> > +    Aml *ctrl_dev;
> > +
> > +    /* scope for memory hotplug controller device node */
> > +    pci_scope = aml_scope("_SB.PCI0");
> > +    ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE));
> > +    {
> > +    }
> > +    aml_append(pci_scope, ctrl_dev);
> > +    aml_append(ctx, pci_scope);
> > +}
> > diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> > index bbd37e9..1b609e6 100644
> > --- a/hw/i386/acpi-build.c
> > +++ b/hw/i386/acpi-build.c
> > @@ -928,6 +928,9 @@ build_ssdt(GArray *table_data, GArray *linker,
> >       /* Reserve space for header */
> >       acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
> >
> > +    build_memory_hotplug_aml(ssdt, nr_mem, pm->mem_hp_io_base,
> > +                             pm->mem_hp_io_len);
> > +
> >       /* Extra PCI root buses are implemented  only for i440fx */
> >       bus = find_i440fx();
> >       if (bus) {
> > diff --git a/include/hw/acpi/memory_hotplug.h
> > b/include/hw/acpi/memory_hotplug.h index 1342adb..b6e9f50 100644
> > --- a/include/hw/acpi/memory_hotplug.h
> > +++ b/include/hw/acpi/memory_hotplug.h
> > @@ -4,6 +4,7 @@
> >   #include "hw/qdev-core.h"
> >   #include "hw/acpi/acpi.h"
> >   #include "migration/vmstate.h"
> > +#include "hw/acpi/aml-build.h"
> >
> >   /**
> >    * MemStatus:
> > @@ -45,4 +46,7 @@ extern const VMStateDescription
> > vmstate_memory_hotplug; vmstate_memory_hotplug, MemHotplugState)
> >
> >   void acpi_memory_ospm_status(MemHotplugState *mem_st,
> > ACPIOSTInfoList ***list); +
> > +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> > +                              uint16_t io_base, uint16_t io_len);
> >   #endif
> >
>
Marcel Apfelbaum Dec. 17, 2015, 12:14 p.m. UTC | #5
On 12/16/2015 04:25 PM, Igor Mammedov wrote:
> On Wed, 16 Dec 2015 15:25:46 +0200
> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
>
>> On 12/10/2015 01:41 AM, Igor Mammedov wrote:
>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>> ---
>>>    hw/acpi/Makefile.objs               |  2 +-
>>>    hw/acpi/memory_hotplug_acpi_table.c | 40
>>> +++++++++++++++++++++++++++++++++++++
>>> hw/i386/acpi-build.c                |  3 +++
>>> include/hw/acpi/memory_hotplug.h    |  4 ++++ 4 files changed, 48
>>> insertions(+), 1 deletion(-) create mode 100644
>>> hw/acpi/memory_hotplug_acpi_table.c
>>>
>>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>>> index 7d3230c..c04064e 100644
>>> --- a/hw/acpi/Makefile.objs
>>> +++ b/hw/acpi/Makefile.objs
>>> @@ -1,7 +1,7 @@
>>>    common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
>>>    common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>>>    common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>>> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>>> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>>> memory_hotplug_acpi_table.o common-obj-$(CONFIG_ACPI) +=
>>> acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>>>    common-obj-$(CONFIG_ACPI) += aml-build.o
>>> diff --git a/hw/acpi/memory_hotplug_acpi_table.c
>>> b/hw/acpi/memory_hotplug_acpi_table.c new file mode 100644
>>> index 0000000..25bbf5e
>>> --- /dev/null
>>> +++ b/hw/acpi/memory_hotplug_acpi_table.c
>>> @@ -0,0 +1,40 @@
>>> +/*
>>> + * Memory hotplug AML code of DSDT ACPI table
>>
>> You are advertising as part of the DSDT code, but you are putting in
>> SSDT, right? By the way, maybe the answer is clear, but why are you
>> moving it to SSDT?
> There could be only one instance of DSDT and I temporarily put
> converted DSDT bits in SSDT until conversion is complete.

I understand there can only be one DSDT table, but can't we have a "hybrid"
DSDT made from both asl files and C code?

>
>>
>> Last thing, from this patch forward (until maybe the last one) make
>> check fails, right? (Specifically the acpi tests)
> yes, starting from this patch and upto 72nd acpi test fails since
> patches move DSDT bits into SSDT.

This can be a problem.
I am thinking of 2 solutions:
1. Disable the iasl tests on patch 1 and re-enable it on patch 73.
2. Go for a "hybrid" DSDT.

If (2) is too complicated, the implication is that all this series
must be taken atomically. Of course, (1) or some other idea is needed.

What do you think?

Thanks,
Marcel

> But patch 73, when precompiled DSDT is empty and provides only header,
> moves converted DSDT content out of SSDT into QEMU
> generated DSDT, and drops ASL template.
>
>>
>>
>>> + *
>>> + * Copyright (C) 2013 Red Hat Inc
>>> + *
>>> + * Author: Igor Mammedov <imammedo@redhat.com>
>>> + *
>>> + * 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; either version 2 of the License,
>>> or
>>> + * (at your option) any later version.
>>> +
>>> + * 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, see <http://www.gnu.org/licenses/>.
>>> + */
>>
>> Regarding the license, I was advised to just add a pointer to the
>> main license instead of copying it in each file, something like:
>>
>>      This work is licensed under the terms of the GNU GPL, version 2
>> or later. See the COPYING file in the top-level directory.
>>
>> Thanks,
>> Marcel
>>
>>> +
>>> +#include <stdbool.h>
>>> +#include "hw/acpi/memory_hotplug.h"
>>> +#include "include/hw/acpi/pc-hotplug.h"
>>> +#include "hw/boards.h"
>>> +
>>> +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
>>> +                              uint16_t io_base, uint16_t io_len)
>>> +{
>>> +    Aml *pci_scope;
>>> +    Aml *ctrl_dev;
>>> +
>>> +    /* scope for memory hotplug controller device node */
>>> +    pci_scope = aml_scope("_SB.PCI0");
>>> +    ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE));
>>> +    {
>>> +    }
>>> +    aml_append(pci_scope, ctrl_dev);
>>> +    aml_append(ctx, pci_scope);
>>> +}
>>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>>> index bbd37e9..1b609e6 100644
>>> --- a/hw/i386/acpi-build.c
>>> +++ b/hw/i386/acpi-build.c
>>> @@ -928,6 +928,9 @@ build_ssdt(GArray *table_data, GArray *linker,
>>>        /* Reserve space for header */
>>>        acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
>>>
>>> +    build_memory_hotplug_aml(ssdt, nr_mem, pm->mem_hp_io_base,
>>> +                             pm->mem_hp_io_len);
>>> +
>>>        /* Extra PCI root buses are implemented  only for i440fx */
>>>        bus = find_i440fx();
>>>        if (bus) {
>>> diff --git a/include/hw/acpi/memory_hotplug.h
>>> b/include/hw/acpi/memory_hotplug.h index 1342adb..b6e9f50 100644
>>> --- a/include/hw/acpi/memory_hotplug.h
>>> +++ b/include/hw/acpi/memory_hotplug.h
>>> @@ -4,6 +4,7 @@
>>>    #include "hw/qdev-core.h"
>>>    #include "hw/acpi/acpi.h"
>>>    #include "migration/vmstate.h"
>>> +#include "hw/acpi/aml-build.h"
>>>
>>>    /**
>>>     * MemStatus:
>>> @@ -45,4 +46,7 @@ extern const VMStateDescription
>>> vmstate_memory_hotplug; vmstate_memory_hotplug, MemHotplugState)
>>>
>>>    void acpi_memory_ospm_status(MemHotplugState *mem_st,
>>> ACPIOSTInfoList ***list); +
>>> +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
>>> +                              uint16_t io_base, uint16_t io_len);
>>>    #endif
>>>
>>
>
Igor Mammedov Dec. 17, 2015, 1:47 p.m. UTC | #6
On Thu, 17 Dec 2015 14:14:28 +0200
Marcel Apfelbaum <marcel@redhat.com> wrote:

> On 12/16/2015 04:25 PM, Igor Mammedov wrote:
> > On Wed, 16 Dec 2015 15:25:46 +0200
> > Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
> >
> >> On 12/10/2015 01:41 AM, Igor Mammedov wrote:
> >>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >>> ---
> >>>    hw/acpi/Makefile.objs               |  2 +-
> >>>    hw/acpi/memory_hotplug_acpi_table.c | 40
> >>> +++++++++++++++++++++++++++++++++++++
> >>> hw/i386/acpi-build.c                |  3 +++
> >>> include/hw/acpi/memory_hotplug.h    |  4 ++++ 4 files changed, 48
> >>> insertions(+), 1 deletion(-) create mode 100644
> >>> hw/acpi/memory_hotplug_acpi_table.c
> >>>
> >>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> >>> index 7d3230c..c04064e 100644
> >>> --- a/hw/acpi/Makefile.objs
> >>> +++ b/hw/acpi/Makefile.objs
> >>> @@ -1,7 +1,7 @@
> >>>    common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> >>>    common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
> >>>    common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> >>> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> >>> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> >>> memory_hotplug_acpi_table.o common-obj-$(CONFIG_ACPI) +=
> >>> acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
> >>>    common-obj-$(CONFIG_ACPI) += aml-build.o
> >>> diff --git a/hw/acpi/memory_hotplug_acpi_table.c
> >>> b/hw/acpi/memory_hotplug_acpi_table.c new file mode 100644
> >>> index 0000000..25bbf5e
> >>> --- /dev/null
> >>> +++ b/hw/acpi/memory_hotplug_acpi_table.c
> >>> @@ -0,0 +1,40 @@
> >>> +/*
> >>> + * Memory hotplug AML code of DSDT ACPI table
> >>
> >> You are advertising as part of the DSDT code, but you are putting in
> >> SSDT, right? By the way, maybe the answer is clear, but why are you
> >> moving it to SSDT?
> > There could be only one instance of DSDT and I temporarily put
> > converted DSDT bits in SSDT until conversion is complete.
> 
> I understand there can only be one DSDT table, but can't we have a "hybrid"
> DSDT made from both asl files and C code?
I've considered that and it's a little painful but possible
to do for some parts of DSDT if ASL moved out from beginning/end
of template. However when it comes to moving parts of ASL from
the middle of template due to dependencies it all becomes way to
too complicated or I'll have to convert almost ALL of DSDT in one patch,
but that, even though verifiable via ACPI tests, won't be human
review-able chunk.


> 
> >
> >>
> >> Last thing, from this patch forward (until maybe the last one) make
> >> check fails, right? (Specifically the acpi tests)
> > yes, starting from this patch and upto 72nd acpi test fails since
> > patches move DSDT bits into SSDT.
> 
> This can be a problem.
> I am thinking of 2 solutions:
> 1. Disable the iasl tests on patch 1 and re-enable it on patch 73.
> 2. Go for a "hybrid" DSDT.
> 
> If (2) is too complicated, the implication is that all this series
> must be taken atomically. Of course, (1) or some other idea is needed.
> 
> What do you think?
as #2 too complicated, we could go with #1 but I don't think
it will gain us much if anything.

Yep, untill whole series is applied ACPI tests will not pass,
that will affect bisection with 'make check' on each step
but developers can deal with it and just ignore failed tests.

Main goal was to make conversion and make sure that at the end
of day it hasn't regressed tests.
I don't think that putting much more efforts to keep interseries
ACPI tests check passing is worth of an effort.

> 
> Thanks,
> Marcel
> 
> > But patch 73, when precompiled DSDT is empty and provides only header,
> > moves converted DSDT content out of SSDT into QEMU
> > generated DSDT, and drops ASL template.
> >
> >>
> >>
> >>> + *
> >>> + * Copyright (C) 2013 Red Hat Inc
> >>> + *
> >>> + * Author: Igor Mammedov <imammedo@redhat.com>
> >>> + *
> >>> + * 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; either version 2 of the License,
> >>> or
> >>> + * (at your option) any later version.
> >>> +
> >>> + * 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, see <http://www.gnu.org/licenses/>.
> >>> + */
> >>
> >> Regarding the license, I was advised to just add a pointer to the
> >> main license instead of copying it in each file, something like:
> >>
> >>      This work is licensed under the terms of the GNU GPL, version 2
> >> or later. See the COPYING file in the top-level directory.
> >>
> >> Thanks,
> >> Marcel
> >>
> >>> +
> >>> +#include <stdbool.h>
> >>> +#include "hw/acpi/memory_hotplug.h"
> >>> +#include "include/hw/acpi/pc-hotplug.h"
> >>> +#include "hw/boards.h"
> >>> +
> >>> +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> >>> +                              uint16_t io_base, uint16_t io_len)
> >>> +{
> >>> +    Aml *pci_scope;
> >>> +    Aml *ctrl_dev;
> >>> +
> >>> +    /* scope for memory hotplug controller device node */
> >>> +    pci_scope = aml_scope("_SB.PCI0");
> >>> +    ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE));
> >>> +    {
> >>> +    }
> >>> +    aml_append(pci_scope, ctrl_dev);
> >>> +    aml_append(ctx, pci_scope);
> >>> +}
> >>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> >>> index bbd37e9..1b609e6 100644
> >>> --- a/hw/i386/acpi-build.c
> >>> +++ b/hw/i386/acpi-build.c
> >>> @@ -928,6 +928,9 @@ build_ssdt(GArray *table_data, GArray *linker,
> >>>        /* Reserve space for header */
> >>>        acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
> >>>
> >>> +    build_memory_hotplug_aml(ssdt, nr_mem, pm->mem_hp_io_base,
> >>> +                             pm->mem_hp_io_len);
> >>> +
> >>>        /* Extra PCI root buses are implemented  only for i440fx */
> >>>        bus = find_i440fx();
> >>>        if (bus) {
> >>> diff --git a/include/hw/acpi/memory_hotplug.h
> >>> b/include/hw/acpi/memory_hotplug.h index 1342adb..b6e9f50 100644
> >>> --- a/include/hw/acpi/memory_hotplug.h
> >>> +++ b/include/hw/acpi/memory_hotplug.h
> >>> @@ -4,6 +4,7 @@
> >>>    #include "hw/qdev-core.h"
> >>>    #include "hw/acpi/acpi.h"
> >>>    #include "migration/vmstate.h"
> >>> +#include "hw/acpi/aml-build.h"
> >>>
> >>>    /**
> >>>     * MemStatus:
> >>> @@ -45,4 +46,7 @@ extern const VMStateDescription
> >>> vmstate_memory_hotplug; vmstate_memory_hotplug, MemHotplugState)
> >>>
> >>>    void acpi_memory_ospm_status(MemHotplugState *mem_st,
> >>> ACPIOSTInfoList ***list); +
> >>> +void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
> >>> +                              uint16_t io_base, uint16_t io_len);
> >>>    #endif
> >>>
> >>
> >
>
Marcel Apfelbaum Dec. 17, 2015, 2:12 p.m. UTC | #7
On 12/17/2015 03:47 PM, Igor Mammedov wrote:
> On Thu, 17 Dec 2015 14:14:28 +0200
> Marcel Apfelbaum <marcel@redhat.com> wrote:
>
>> On 12/16/2015 04:25 PM, Igor Mammedov wrote:
>>> On Wed, 16 Dec 2015 15:25:46 +0200
>>> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
>>>
>>>> On 12/10/2015 01:41 AM, Igor Mammedov wrote:
>>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
>>>>> ---
>>>>>     hw/acpi/Makefile.objs               |  2 +-
>>>>>     hw/acpi/memory_hotplug_acpi_table.c | 40
>>>>> +++++++++++++++++++++++++++++++++++++
>>>>> hw/i386/acpi-build.c                |  3 +++
>>>>> include/hw/acpi/memory_hotplug.h    |  4 ++++ 4 files changed, 48
>>>>> insertions(+), 1 deletion(-) create mode 100644
>>>>> hw/acpi/memory_hotplug_acpi_table.c
>>>>>
>>>>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>>>>> index 7d3230c..c04064e 100644
>>>>> --- a/hw/acpi/Makefile.objs
>>>>> +++ b/hw/acpi/Makefile.objs
>>>>> @@ -1,7 +1,7 @@
>>>>>     common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
>>>>>     common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
>>>>>     common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
>>>>> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>>>>> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
>>>>> memory_hotplug_acpi_table.o common-obj-$(CONFIG_ACPI) +=
>>>>> acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
>>>>>     common-obj-$(CONFIG_ACPI) += aml-build.o
>>>>> diff --git a/hw/acpi/memory_hotplug_acpi_table.c
>>>>> b/hw/acpi/memory_hotplug_acpi_table.c new file mode 100644
>>>>> index 0000000..25bbf5e
>>>>> --- /dev/null
>>>>> +++ b/hw/acpi/memory_hotplug_acpi_table.c
>>>>> @@ -0,0 +1,40 @@
>>>>> +/*
>>>>> + * Memory hotplug AML code of DSDT ACPI table
>>>>
>>>> You are advertising as part of the DSDT code, but you are putting in
>>>> SSDT, right? By the way, maybe the answer is clear, but why are you
>>>> moving it to SSDT?
>>> There could be only one instance of DSDT and I temporarily put
>>> converted DSDT bits in SSDT until conversion is complete.
>>
>> I understand there can only be one DSDT table, but can't we have a "hybrid"
>> DSDT made from both asl files and C code?
> I've considered that and it's a little painful but possible
> to do for some parts of DSDT if ASL moved out from beginning/end
> of template. However when it comes to moving parts of ASL from
> the middle of template due to dependencies it all becomes way to
> too complicated or I'll have to convert almost ALL of DSDT in one patch,
> but that, even though verifiable via ACPI tests, won't be human
> review-able chunk.

Yes, I thought it would not be easy.

>
>
>>
>>>
>>>>
>>>> Last thing, from this patch forward (until maybe the last one) make
>>>> check fails, right? (Specifically the acpi tests)
>>> yes, starting from this patch and upto 72nd acpi test fails since
>>> patches move DSDT bits into SSDT.
>>
>> This can be a problem.
>> I am thinking of 2 solutions:
>> 1. Disable the iasl tests on patch 1 and re-enable it on patch 73.
>> 2. Go for a "hybrid" DSDT.
>>
>> If (2) is too complicated, the implication is that all this series
>> must be taken atomically. Of course, (1) or some other idea is needed.
>>
>> What do you think?
> as #2 too complicated, we could go with #1 but I don't think
> it will gain us much if anything.
>
> Yep, untill whole series is applied ACPI tests will not pass,
> that will affect bisection with 'make check' on each step
> but developers can deal with it and just ignore failed tests.

Here I don't agree, think about different tests not working from
different reasons, and make check failing because all kind of series,
how the developer would know not to take it in consideration? (and what to take?)

>
> Main goal was to make conversion and make sure that at the end
> of day it hasn't regressed tests.
> I don't think that putting much more efforts to keep interseries
> ACPI tests check passing is worth of an effort.

I personally think that is a little effort to disable the acpi test
in order to keep 'make check' clean.
However, we can ask for another opinion, I CC-ed Michael as the PC maintainer.

Thanks,
Marcel

[...]
Igor Mammedov Dec. 17, 2015, 4:59 p.m. UTC | #8
On Thu, 17 Dec 2015 16:12:55 +0200
Marcel Apfelbaum <marcel@redhat.com> wrote:

> On 12/17/2015 03:47 PM, Igor Mammedov wrote:
> > On Thu, 17 Dec 2015 14:14:28 +0200
> > Marcel Apfelbaum <marcel@redhat.com> wrote:
> >
> >> On 12/16/2015 04:25 PM, Igor Mammedov wrote:
> >>> On Wed, 16 Dec 2015 15:25:46 +0200
> >>> Marcel Apfelbaum <marcel.apfelbaum@gmail.com> wrote:
> >>>
> >>>> On 12/10/2015 01:41 AM, Igor Mammedov wrote:
> >>>>> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> >>>>> ---
> >>>>>     hw/acpi/Makefile.objs               |  2 +-
> >>>>>     hw/acpi/memory_hotplug_acpi_table.c | 40
> >>>>> +++++++++++++++++++++++++++++++++++++
> >>>>> hw/i386/acpi-build.c                |  3 +++
> >>>>> include/hw/acpi/memory_hotplug.h    |  4 ++++ 4 files changed, 48
> >>>>> insertions(+), 1 deletion(-) create mode 100644
> >>>>> hw/acpi/memory_hotplug_acpi_table.c
> >>>>>
> >>>>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
> >>>>> index 7d3230c..c04064e 100644
> >>>>> --- a/hw/acpi/Makefile.objs
> >>>>> +++ b/hw/acpi/Makefile.objs
> >>>>> @@ -1,7 +1,7 @@
> >>>>>     common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
> >>>>>     common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
> >>>>>     common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
> >>>>> -common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> >>>>> +common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
> >>>>> memory_hotplug_acpi_table.o common-obj-$(CONFIG_ACPI) +=
> >>>>> acpi_interface.o common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
> >>>>>     common-obj-$(CONFIG_ACPI) += aml-build.o
> >>>>> diff --git a/hw/acpi/memory_hotplug_acpi_table.c
> >>>>> b/hw/acpi/memory_hotplug_acpi_table.c new file mode 100644
> >>>>> index 0000000..25bbf5e
> >>>>> --- /dev/null
> >>>>> +++ b/hw/acpi/memory_hotplug_acpi_table.c
> >>>>> @@ -0,0 +1,40 @@
> >>>>> +/*
> >>>>> + * Memory hotplug AML code of DSDT ACPI table
> >>>>
> >>>> You are advertising as part of the DSDT code, but you are putting in
> >>>> SSDT, right? By the way, maybe the answer is clear, but why are you
> >>>> moving it to SSDT?
> >>> There could be only one instance of DSDT and I temporarily put
> >>> converted DSDT bits in SSDT until conversion is complete.
> >>
> >> I understand there can only be one DSDT table, but can't we have a "hybrid"
> >> DSDT made from both asl files and C code?
> > I've considered that and it's a little painful but possible
> > to do for some parts of DSDT if ASL moved out from beginning/end
> > of template. However when it comes to moving parts of ASL from
> > the middle of template due to dependencies it all becomes way to
> > too complicated or I'll have to convert almost ALL of DSDT in one patch,
> > but that, even though verifiable via ACPI tests, won't be human
> > review-able chunk.
> 
> Yes, I thought it would not be easy.
> 
> >
> >
> >>
> >>>
> >>>>
> >>>> Last thing, from this patch forward (until maybe the last one) make
> >>>> check fails, right? (Specifically the acpi tests)
> >>> yes, starting from this patch and upto 72nd acpi test fails since
> >>> patches move DSDT bits into SSDT.
> >>
> >> This can be a problem.
> >> I am thinking of 2 solutions:
> >> 1. Disable the iasl tests on patch 1 and re-enable it on patch 73.
> >> 2. Go for a "hybrid" DSDT.
> >>
> >> If (2) is too complicated, the implication is that all this series
> >> must be taken atomically. Of course, (1) or some other idea is needed.
> >>
> >> What do you think?
> > as #2 too complicated, we could go with #1 but I don't think
> > it will gain us much if anything.
> >
> > Yep, untill whole series is applied ACPI tests will not pass,
> > that will affect bisection with 'make check' on each step
> > but developers can deal with it and just ignore failed tests.
> 
> Here I don't agree, think about different tests not working from
> different reasons, and make check failing because all kind of series,
> how the developer would know not to take it in consideration? (and what to take?)
> 
> >
> > Main goal was to make conversion and make sure that at the end
> > of day it hasn't regressed tests.
> > I don't think that putting much more efforts to keep interseries
> > ACPI tests check passing is worth of an effort.
> 
> I personally think that is a little effort to disable the acpi test
> in order to keep 'make check' clean.
> However, we can ask for another opinion, I CC-ed Michael as the PC maintainer.
So far with patches/series, that are changing SSDT or DSDT and obviously
breaking ACPI test, policy was submit it without updating test and let
maintainer to update ACPI blobs in test after series has been applied,
i.e. not disabling it.

I sometimes post an additional patch, that updates ACPI blobs in test
as the last in series. But that patch however could be easily discarded
or be obsoleted because some another patch changing ACPI tables was
committed before and maintainer has to update test himself.

> 
> Thanks,
> Marcel
> 
> [...]
>
diff mbox

Patch

diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
index 7d3230c..c04064e 100644
--- a/hw/acpi/Makefile.objs
+++ b/hw/acpi/Makefile.objs
@@ -1,7 +1,7 @@ 
 common-obj-$(CONFIG_ACPI_X86) += core.o piix4.o pcihp.o
 common-obj-$(CONFIG_ACPI_X86_ICH) += ich9.o tco.o
 common-obj-$(CONFIG_ACPI_CPU_HOTPLUG) += cpu_hotplug.o
-common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o
+common-obj-$(CONFIG_ACPI_MEMORY_HOTPLUG) += memory_hotplug.o memory_hotplug_acpi_table.o
 common-obj-$(CONFIG_ACPI) += acpi_interface.o
 common-obj-$(CONFIG_ACPI) += bios-linker-loader.o
 common-obj-$(CONFIG_ACPI) += aml-build.o
diff --git a/hw/acpi/memory_hotplug_acpi_table.c b/hw/acpi/memory_hotplug_acpi_table.c
new file mode 100644
index 0000000..25bbf5e
--- /dev/null
+++ b/hw/acpi/memory_hotplug_acpi_table.c
@@ -0,0 +1,40 @@ 
+/*
+ * Memory hotplug AML code of DSDT ACPI table
+ *
+ * Copyright (C) 2013 Red Hat Inc
+ *
+ * Author: Igor Mammedov <imammedo@redhat.com>
+ *
+ * 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; either version 2 of the License, or
+ * (at your option) any later version.
+
+ * 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, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <stdbool.h>
+#include "hw/acpi/memory_hotplug.h"
+#include "include/hw/acpi/pc-hotplug.h"
+#include "hw/boards.h"
+
+void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
+                              uint16_t io_base, uint16_t io_len)
+{
+    Aml *pci_scope;
+    Aml *ctrl_dev;
+
+    /* scope for memory hotplug controller device node */
+    pci_scope = aml_scope("_SB.PCI0");
+    ctrl_dev = aml_scope(stringify(MEMORY_HOTPLUG_DEVICE));
+    {
+    }
+    aml_append(pci_scope, ctrl_dev);
+    aml_append(ctx, pci_scope);
+}
diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
index bbd37e9..1b609e6 100644
--- a/hw/i386/acpi-build.c
+++ b/hw/i386/acpi-build.c
@@ -928,6 +928,9 @@  build_ssdt(GArray *table_data, GArray *linker,
     /* Reserve space for header */
     acpi_data_push(ssdt->buf, sizeof(AcpiTableHeader));
 
+    build_memory_hotplug_aml(ssdt, nr_mem, pm->mem_hp_io_base,
+                             pm->mem_hp_io_len);
+
     /* Extra PCI root buses are implemented  only for i440fx */
     bus = find_i440fx();
     if (bus) {
diff --git a/include/hw/acpi/memory_hotplug.h b/include/hw/acpi/memory_hotplug.h
index 1342adb..b6e9f50 100644
--- a/include/hw/acpi/memory_hotplug.h
+++ b/include/hw/acpi/memory_hotplug.h
@@ -4,6 +4,7 @@ 
 #include "hw/qdev-core.h"
 #include "hw/acpi/acpi.h"
 #include "migration/vmstate.h"
+#include "hw/acpi/aml-build.h"
 
 /**
  * MemStatus:
@@ -45,4 +46,7 @@  extern const VMStateDescription vmstate_memory_hotplug;
                    vmstate_memory_hotplug, MemHotplugState)
 
 void acpi_memory_ospm_status(MemHotplugState *mem_st, ACPIOSTInfoList ***list);
+
+void build_memory_hotplug_aml(Aml *ctx, uint32_t nr_mem,
+                              uint16_t io_base, uint16_t io_len);
 #endif