All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergio Lopez <slp@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>,
	Peter Maydell <peter.maydell@linaro.org>,
	Thomas Huth <thuth@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	qemu-devel@nongnu.org, Shannon Zhao <shannon.zhaosl@gmail.com>,
	qemu-arm@nongnu.org, Igor Mammedov <imammedo@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Richard Henderson <rth@twiddle.net>
Subject: Re: [PATCH v6 14/20] x86: move acpi_dev from pc/microvm
Date: Thu, 27 Aug 2020 16:51:02 +0200	[thread overview]
Message-ID: <20200827145102.nxzgmfumncqkbkuy@mhamilton> (raw)
In-Reply-To: <20200826105254.28496-15-kraxel@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 13018 bytes --]

On Wed, Aug 26, 2020 at 12:52:48PM +0200, Gerd Hoffmann wrote:
> Both pc and microvm machine types have a acpi_dev field.
> Move it to the common base type.
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/hw/i386/microvm.h |  1 -
>  include/hw/i386/pc.h      |  1 -
>  include/hw/i386/x86.h     |  1 +
>  hw/i386/acpi-build.c      |  2 +-
>  hw/i386/acpi-microvm.c    |  5 +++--
>  hw/i386/microvm.c         | 10 ++++++----
>  hw/i386/pc.c              | 34 +++++++++++++++++++---------------
>  hw/i386/pc_piix.c         |  2 +-
>  hw/i386/pc_q35.c          |  2 +-
>  9 files changed, 32 insertions(+), 26 deletions(-)

Reviewed-by: Sergio Lopez <slp@redhat.com>


> diff --git a/include/hw/i386/microvm.h b/include/hw/i386/microvm.h
> index b6e0d4395af7..b8ec99aeb051 100644
> --- a/include/hw/i386/microvm.h
> +++ b/include/hw/i386/microvm.h
> @@ -66,7 +66,6 @@ typedef struct {
>      bool kernel_cmdline_fixed;
>      Notifier machine_done;
>      Notifier powerdown_req;
> -    AcpiDeviceIf *acpi_dev;
>  } MicrovmMachineState;
>  
>  #define TYPE_MICROVM_MACHINE   MACHINE_TYPE_NAME("microvm")
> diff --git a/include/hw/i386/pc.h b/include/hw/i386/pc.h
> index fe52e165b27c..0f7da2329b0f 100644
> --- a/include/hw/i386/pc.h
> +++ b/include/hw/i386/pc.h
> @@ -29,7 +29,6 @@ struct PCMachineState {
>      Notifier machine_done;
>  
>      /* Pointers to devices and objects: */
> -    HotplugHandler *acpi_dev;
>      PCIBus *bus;
>      I2CBus *smbus;
>      PFlashCFI01 *flash[2];
> diff --git a/include/hw/i386/x86.h b/include/hw/i386/x86.h
> index a350ea3609f5..de74c831c3ab 100644
> --- a/include/hw/i386/x86.h
> +++ b/include/hw/i386/x86.h
> @@ -50,6 +50,7 @@ typedef struct {
>      FWCfgState *fw_cfg;
>      qemu_irq *gsi;
>      GMappedFile *initrd_mapped_file;
> +    HotplugHandler *acpi_dev;
>  
>      /* RAM information (sizes, addresses, configuration): */
>      ram_addr_t below_4g_mem_size, above_4g_mem_size;
> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
> index b7bcbbbb2a35..c356cc71fe08 100644
> --- a/hw/i386/acpi-build.c
> +++ b/hw/i386/acpi-build.c
> @@ -2431,7 +2431,7 @@ void acpi_build(AcpiBuildTables *tables, MachineState *machine)
>  
>      acpi_add_table(table_offsets, tables_blob);
>      acpi_build_madt(tables_blob, tables->linker, x86ms,
> -                    ACPI_DEVICE_IF(pcms->acpi_dev), true);
> +                    ACPI_DEVICE_IF(x86ms->acpi_dev), true);
>  
>      vmgenid_dev = find_vmgenid_dev();
>      if (vmgenid_dev) {
> diff --git a/hw/i386/acpi-microvm.c b/hw/i386/acpi-microvm.c
> index b9ce3768b263..df39c5d3bd90 100644
> --- a/hw/i386/acpi-microvm.c
> +++ b/hw/i386/acpi-microvm.c
> @@ -108,7 +108,7 @@ build_dsdt_microvm(GArray *table_data, BIOSLinker *linker,
>      sb_scope = aml_scope("_SB");
>      fw_cfg_add_acpi_dsdt(sb_scope, x86ms->fw_cfg);
>      isa_build_aml(ISA_BUS(isabus), sb_scope);
> -    build_ged_aml(sb_scope, GED_DEVICE, HOTPLUG_HANDLER(mms->acpi_dev),
> +    build_ged_aml(sb_scope, GED_DEVICE, x86ms->acpi_dev,
>                    GED_MMIO_IRQ, AML_SYSTEM_MEMORY, GED_MMIO_BASE);
>      acpi_dsdt_add_power_button(sb_scope);
>      acpi_dsdt_add_virtio(sb_scope, mms);
> @@ -136,6 +136,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
>                                 MicrovmMachineState *mms)
>  {
>      MachineState *machine = MACHINE(mms);
> +    X86MachineState *x86ms = X86_MACHINE(mms);
>      GArray *table_offsets;
>      GArray *tables_blob = tables->table_data;
>      unsigned dsdt, xsdt;
> @@ -183,7 +184,7 @@ static void acpi_build_microvm(AcpiBuildTables *tables,
>  
>      acpi_add_table(table_offsets, tables_blob);
>      acpi_build_madt(tables_blob, tables->linker, X86_MACHINE(machine),
> -                    mms->acpi_dev, false);
> +                    ACPI_DEVICE_IF(x86ms->acpi_dev), false);
>  
>      xsdt = tables_blob->len;
>      build_xsdt(tables_blob, tables->linker, table_offsets, NULL, NULL);
> diff --git a/hw/i386/microvm.c b/hw/i386/microvm.c
> index 04209eb38fbe..9df15354ce0f 100644
> --- a/hw/i386/microvm.c
> +++ b/hw/i386/microvm.c
> @@ -143,7 +143,7 @@ static void microvm_devices_init(MicrovmMachineState *mms)
>          sysbus_connect_irq(SYS_BUS_DEVICE(dev), 0,
>                             x86ms->gsi[GED_MMIO_IRQ]);
>          sysbus_realize(SYS_BUS_DEVICE(dev), &error_fatal);
> -        mms->acpi_dev = ACPI_DEVICE_IF(dev);
> +        x86ms->acpi_dev = HOTPLUG_HANDLER(dev);
>      }
>  
>      if (mms->pic == ON_OFF_AUTO_ON || mms->pic == ON_OFF_AUTO_AUTO) {
> @@ -469,11 +469,13 @@ static void microvm_powerdown_req(Notifier *notifier, void *data)
>  {
>      MicrovmMachineState *mms = container_of(notifier, MicrovmMachineState,
>                                              powerdown_req);
> +    X86MachineState *x86ms = X86_MACHINE(mms);
>  
> -    if (mms->acpi_dev) {
> -        Object *obj = OBJECT(mms->acpi_dev);
> +    if (x86ms->acpi_dev) {
> +        Object *obj = OBJECT(x86ms->acpi_dev);
>          AcpiDeviceIfClass *adevc = ACPI_DEVICE_IF_GET_CLASS(obj);
> -        adevc->send_event(mms->acpi_dev, ACPI_POWER_DOWN_STATUS);
> +        adevc->send_event(ACPI_DEVICE_IF(x86ms->acpi_dev),
> +                          ACPI_POWER_DOWN_STATUS);
>      }
>  }
>  
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 5d8d5ef8b373..0bd6dbbd7bf6 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -1274,6 +1274,7 @@ static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>                                 Error **errp)
>  {
>      const PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +    const X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
>      const PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>      const MachineState *ms = MACHINE(hotplug_dev);
>      const bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
> @@ -1285,7 +1286,7 @@ static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>       * but pcms->acpi_dev is still created. Check !acpi_enabled in
>       * addition to cover this case.
>       */
> -    if (!pcms->acpi_dev || !x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
> +    if (!x86ms->acpi_dev || !x86_machine_is_acpi_enabled(x86ms)) {
>          error_setg(errp,
>                     "memory hotplug is not enabled: missing acpi device or acpi disabled");
>          return;
> @@ -1296,7 +1297,7 @@ static void pc_memory_pre_plug(HotplugHandler *hotplug_dev, DeviceState *dev,
>          return;
>      }
>  
> -    hotplug_handler_pre_plug(pcms->acpi_dev, dev, &local_err);
> +    hotplug_handler_pre_plug(x86ms->acpi_dev, dev, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
> @@ -1311,6 +1312,7 @@ static void pc_memory_plug(HotplugHandler *hotplug_dev,
>  {
>      Error *local_err = NULL;
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +    X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
>      MachineState *ms = MACHINE(hotplug_dev);
>      bool is_nvdimm = object_dynamic_cast(OBJECT(dev), TYPE_NVDIMM);
>  
> @@ -1323,7 +1325,7 @@ static void pc_memory_plug(HotplugHandler *hotplug_dev,
>          nvdimm_plug(ms->nvdimms_state);
>      }
>  
> -    hotplug_handler_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &error_abort);
> +    hotplug_handler_plug(x86ms->acpi_dev, dev, &error_abort);
>  out:
>      error_propagate(errp, local_err);
>  }
> @@ -1331,14 +1333,14 @@ out:
>  static void pc_memory_unplug_request(HotplugHandler *hotplug_dev,
>                                       DeviceState *dev, Error **errp)
>  {
> -    PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +    X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
>  
>      /*
>       * When -no-acpi is used with Q35 machine type, no ACPI is built,
>       * but pcms->acpi_dev is still created. Check !acpi_enabled in
>       * addition to cover this case.
>       */
> -    if (!pcms->acpi_dev || !x86_machine_is_acpi_enabled(X86_MACHINE(pcms))) {
> +    if (!x86ms->acpi_dev || !x86_machine_is_acpi_enabled(x86ms)) {
>          error_setg(errp,
>                     "memory hotplug is not enabled: missing acpi device or acpi disabled");
>          return;
> @@ -1349,7 +1351,7 @@ static void pc_memory_unplug_request(HotplugHandler *hotplug_dev,
>          return;
>      }
>  
> -    hotplug_handler_unplug_request(HOTPLUG_HANDLER(pcms->acpi_dev), dev,
> +    hotplug_handler_unplug_request(x86ms->acpi_dev, dev,
>                                     errp);
>  }
>  
> @@ -1357,9 +1359,10 @@ static void pc_memory_unplug(HotplugHandler *hotplug_dev,
>                               DeviceState *dev, Error **errp)
>  {
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +    X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
>      Error *local_err = NULL;
>  
> -    hotplug_handler_unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
> +    hotplug_handler_unplug(x86ms->acpi_dev, dev, &local_err);
>      if (local_err) {
>          goto out;
>      }
> @@ -1403,10 +1406,10 @@ static void pc_cpu_plug(HotplugHandler *hotplug_dev,
>      Error *local_err = NULL;
>      X86CPU *cpu = X86_CPU(dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> -    X86MachineState *x86ms = X86_MACHINE(pcms);
> +    X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
>  
> -    if (pcms->acpi_dev) {
> -        hotplug_handler_plug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
> +    if (x86ms->acpi_dev) {
> +        hotplug_handler_plug(x86ms->acpi_dev, dev, &local_err);
>          if (local_err) {
>              goto out;
>          }
> @@ -1432,8 +1435,9 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
>      int idx = -1;
>      X86CPU *cpu = X86_CPU(dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> +    X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
>  
> -    if (!pcms->acpi_dev) {
> +    if (!x86ms->acpi_dev) {
>          error_setg(errp, "CPU hot unplug not supported without ACPI");
>          return;
>      }
> @@ -1445,7 +1449,7 @@ static void pc_cpu_unplug_request_cb(HotplugHandler *hotplug_dev,
>          return;
>      }
>  
> -    hotplug_handler_unplug_request(HOTPLUG_HANDLER(pcms->acpi_dev), dev,
> +    hotplug_handler_unplug_request(x86ms->acpi_dev, dev,
>                                     errp);
>  }
>  
> @@ -1456,9 +1460,9 @@ static void pc_cpu_unplug_cb(HotplugHandler *hotplug_dev,
>      Error *local_err = NULL;
>      X86CPU *cpu = X86_CPU(dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> -    X86MachineState *x86ms = X86_MACHINE(pcms);
> +    X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
>  
> -    hotplug_handler_unplug(HOTPLUG_HANDLER(pcms->acpi_dev), dev, &local_err);
> +    hotplug_handler_unplug(x86ms->acpi_dev, dev, &local_err);
>      if (local_err) {
>          goto out;
>      }
> @@ -1487,7 +1491,7 @@ static void pc_cpu_pre_plug(HotplugHandler *hotplug_dev,
>      CPUX86State *env = &cpu->env;
>      MachineState *ms = MACHINE(hotplug_dev);
>      PCMachineState *pcms = PC_MACHINE(hotplug_dev);
> -    X86MachineState *x86ms = X86_MACHINE(pcms);
> +    X86MachineState *x86ms = X86_MACHINE(hotplug_dev);
>      unsigned int smp_cores = ms->smp.cores;
>      unsigned int smp_threads = ms->smp.threads;
>      X86CPUTopoInfo topo_info;
> diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
> index 32b1453e6a82..759b4a97facb 100644
> --- a/hw/i386/pc_piix.c
> +++ b/hw/i386/pc_piix.c
> @@ -293,7 +293,7 @@ static void pc_init1(MachineState *machine,
>  
>          object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
>                                   TYPE_HOTPLUG_HANDLER,
> -                                 (Object **)&pcms->acpi_dev,
> +                                 (Object **)&x86ms->acpi_dev,
>                                   object_property_allow_set_link,
>                                   OBJ_PROP_LINK_STRONG);
>          object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
> diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
> index 0cb9c18cd44d..622d0397172a 100644
> --- a/hw/i386/pc_q35.c
> +++ b/hw/i386/pc_q35.c
> @@ -240,7 +240,7 @@ static void pc_q35_init(MachineState *machine)
>  
>      object_property_add_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
>                               TYPE_HOTPLUG_HANDLER,
> -                             (Object **)&pcms->acpi_dev,
> +                             (Object **)&x86ms->acpi_dev,
>                               object_property_allow_set_link,
>                               OBJ_PROP_LINK_STRONG);
>      object_property_set_link(OBJECT(machine), PC_MACHINE_ACPI_DEVICE_PROP,
> -- 
> 2.27.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2020-08-27 14:52 UTC|newest]

Thread overview: 42+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-26 10:52 [PATCH v6 00/20] microvm: add acpi support Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 01/20] microvm: name qboot binary qboot.rom Gerd Hoffmann
2020-08-27  8:25   ` Sergio Lopez
2020-08-26 10:52 ` [PATCH v6 02/20] seabios: add microvm config, update build rules Gerd Hoffmann
2020-08-27 14:35   ` Sergio Lopez
2020-08-26 10:52 ` [PATCH v6 03/20] seabios: add bios-microvm.bin binary Gerd Hoffmann
2020-08-27 14:48   ` Sergio Lopez
2020-08-28  5:02     ` Gerd Hoffmann
2020-08-28 10:57       ` Sergio Lopez
2020-08-28 12:12         ` Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 04/20] acpi: ged: add control regs Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 05/20] acpi: ged: add x86 device variant Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 06/20] acpi: move acpi_dsdt_add_power_button() to ged Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 07/20] microvm: make virtio irq base runtime configurable Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 08/20] microvm/acpi: add minimal acpi support Gerd Hoffmann
2020-08-31  8:06   ` Igor Mammedov
2020-08-26 10:52 ` [PATCH v6 09/20] microvm/acpi: add acpi_dsdt_add_virtio() for x86 Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 10/20] microvm/acpi: use GSI 16-23 for virtio Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 11/20] microvm/acpi: use seabios with acpi=on Gerd Hoffmann
2020-08-27 14:54   ` Sergio Lopez
2020-08-26 10:52 ` [PATCH v6 12/20] microvm/acpi: disable virtio-mmio cmdline hack Gerd Hoffmann
2020-08-27  8:26   ` Sergio Lopez
2020-08-26 10:52 ` [PATCH v6 13/20] x86: constify x86_machine_is_*_enabled Gerd Hoffmann
2020-08-27 14:56   ` Sergio Lopez
2020-08-31  7:55   ` Igor Mammedov
2020-08-26 10:52 ` [PATCH v6 14/20] x86: move acpi_dev from pc/microvm Gerd Hoffmann
2020-08-27 14:51   ` Sergio Lopez [this message]
2020-08-31  7:54   ` Igor Mammedov
2020-08-26 10:52 ` [PATCH v6 15/20] x86: move cpu hotplug from pc to x86 Gerd Hoffmann
2020-08-27 14:50   ` Sergio Lopez
2020-08-31  7:50   ` Igor Mammedov
2020-08-31 13:12     ` Babu Moger
2020-08-26 10:52 ` [PATCH v6 16/20] microvm: wire up hotplug Gerd Hoffmann
2020-08-27 14:54   ` Sergio Lopez
2020-08-26 10:52 ` [PATCH v6 17/20] tests/acpi: allow microvm test data updates Gerd Hoffmann
2020-08-26 10:52 ` [PATCH v6 18/20] tests/acpi: allow override blkdev Gerd Hoffmann
2020-08-27 14:55   ` Sergio Lopez
2020-08-26 10:52 ` [PATCH v6 19/20] tests/acpi: add microvm test Gerd Hoffmann
2020-08-31  7:43   ` Igor Mammedov
2020-08-31  9:46     ` Gerd Hoffmann
2020-08-31 15:00   ` Igor Mammedov
2020-08-26 10:52 ` [PATCH v6 20/20] tests/acpi: update expected data files for microvm Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200827145102.nxzgmfumncqkbkuy@mhamilton \
    --to=slp@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=peter.maydell@linaro.org \
    --cc=qemu-arm@nongnu.org \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=shannon.zhaosl@gmail.com \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.