From: Wei Yang <richardw.yang@linux.intel.com>
To: Igor Mammedov <imammedo@redhat.com>
Cc: yang.zhong@intel.com, peter.maydell@linaro.org, mst@redhat.com,
qemu-devel@nongnu.org, shannon.zhaosl@gmail.com,
qemu-arm@nongnu.org, Wei Yang <richardw.yang@linux.intel.com>
Subject: Re: [Qemu-arm] [PATCH v3] hw/acpi: extract acpi_add_rom_blob()
Date: Tue, 19 Mar 2019 08:45:23 +0800 [thread overview]
Message-ID: <20190319004523.GA5032@richard> (raw)
In-Reply-To: <20190318133912.1c67a304@redhat.com>
On Mon, Mar 18, 2019 at 01:39:12PM +0100, Igor Mammedov wrote:
>On Fri, 15 Mar 2019 08:44:32 +0800
>Wei Yang <richardw.yang@linux.intel.com> wrote:
>
>in subject: s/extract/generalize/
>
>> arm and i386 has almost the same function acpi_add_rom_blob(), except
>> giving different FWCfgCallback function.
>>
>> This patch extract acpi_add_rom_blob() to utils.c by passing
> s/extract/moves/
>
>> FWCfgCallback to it.
>>
>> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>
>otherwise patch looks fine to me,
>but checkpatch complains about comment style so that needs to be fixed as well
>
BTW, the checkpatch also says
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
Do I need to fix this?
>>
>> ---
>> v3:
>> * put acpi_add_rom_blob() to hw/acpi/utils.c
>> v2:
>> * remove unused header in original source file
>> ---
>> hw/acpi/Makefile.objs | 2 +-
>> hw/acpi/utils.c | 35 +++++++++++++++++++++++++++++++++++
>> hw/arm/virt-acpi-build.c | 26 ++++++++++----------------
>> hw/i386/acpi-build.c | 26 ++++++++++----------------
>> include/hw/acpi/utils.h | 9 +++++++++
>> 5 files changed, 65 insertions(+), 33 deletions(-)
>> create mode 100644 hw/acpi/utils.c
>> create mode 100644 include/hw/acpi/utils.h
>>
>> diff --git a/hw/acpi/Makefile.objs b/hw/acpi/Makefile.objs
>> index 2d46e3789a..ba93c5b64a 100644
>> --- a/hw/acpi/Makefile.objs
>> +++ b/hw/acpi/Makefile.objs
>> @@ -10,7 +10,7 @@ common-obj-$(call lnot,$(CONFIG_ACPI_X86)) += acpi-stub.o
>>
>> common-obj-y += acpi_interface.o
>> common-obj-y += bios-linker-loader.o
>> -common-obj-y += aml-build.o
>> +common-obj-y += aml-build.o utils.o
>> common-obj-$(CONFIG_TPM) += tpm.o
>>
>> common-obj-$(CONFIG_IPMI) += ipmi.o
>> diff --git a/hw/acpi/utils.c b/hw/acpi/utils.c
>> new file mode 100644
>> index 0000000000..ab0e064a0e
>> --- /dev/null
>> +++ b/hw/acpi/utils.c
>> @@ -0,0 +1,35 @@
>> +/* Utilities for generating ACPI tables and passing them to Guests
>> + *
>> + * Copyright (C) 2019 Intel Corporation
>> + * Copyright (C) 2019 Red Hat Inc
>> + *
>> + * Author: Wei Yang <richardw.yang@linux.intel.com>
>> + * 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 "qemu/osdep.h"
>> +#include <glib/gprintf.h>
>> +#include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/utils.h"
>> +
>> +MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque,
>> + GArray *blob, const char *name,
>> + uint64_t max_size)
>> +{
>> + return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
>> + name, update, opaque, NULL, true);
>> +}
>> +
>> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
>> index 57679a89bf..a846f74a14 100644
>> --- a/hw/arm/virt-acpi-build.c
>> +++ b/hw/arm/virt-acpi-build.c
>> @@ -37,9 +37,9 @@
>> #include "hw/acpi/acpi.h"
>> #include "hw/nvram/fw_cfg.h"
>> #include "hw/acpi/bios-linker-loader.h"
>> -#include "hw/loader.h"
>> #include "hw/hw.h"
>> #include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/utils.h"
>> #include "hw/pci/pcie_host.h"
>> #include "hw/pci/pci.h"
>> #include "hw/arm/virt.h"
>> @@ -881,14 +881,6 @@ static void virt_acpi_build_reset(void *build_opaque)
>> build_state->patched = false;
>> }
>>
>> -static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
>> - GArray *blob, const char *name,
>> - uint64_t max_size)
>> -{
>> - return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
>> - name, virt_acpi_build_update, build_state, NULL, true);
>> -}
>> -
>> static const VMStateDescription vmstate_virt_acpi_build = {
>> .name = "virt_acpi_build",
>> .version_id = 1,
>> @@ -920,20 +912,22 @@ void virt_acpi_setup(VirtMachineState *vms)
>> virt_acpi_build(vms, &tables);
>>
>> /* Now expose it all to Guest */
>> - build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
>> - ACPI_BUILD_TABLE_FILE,
>> - ACPI_BUILD_TABLE_MAX_SIZE);
>> + build_state->table_mr = acpi_add_rom_blob(virt_acpi_build_update,
>> + build_state, tables.table_data,
>> + ACPI_BUILD_TABLE_FILE,
>> + ACPI_BUILD_TABLE_MAX_SIZE);
>> assert(build_state->table_mr != NULL);
>>
>> build_state->linker_mr =
>> - acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
>> - "etc/table-loader", 0);
>> + acpi_add_rom_blob(virt_acpi_build_update, build_state,
>> + tables.linker->cmd_blob, "etc/table-loader", 0);
>>
>> fw_cfg_add_file(vms->fw_cfg, ACPI_BUILD_TPMLOG_FILE, tables.tcpalog->data,
>> acpi_data_len(tables.tcpalog));
>>
>> - build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
>> - ACPI_BUILD_RSDP_FILE, 0);
>> + build_state->rsdp_mr = acpi_add_rom_blob(virt_acpi_build_update,
>> + build_state, tables.rsdp,
>> + ACPI_BUILD_RSDP_FILE, 0);
>>
>> qemu_register_reset(virt_acpi_build_reset, build_state);
>> virt_acpi_build_reset(build_state);
>> diff --git a/hw/i386/acpi-build.c b/hw/i386/acpi-build.c
>> index 416da318ae..b17d4a711d 100644
>> --- a/hw/i386/acpi-build.c
>> +++ b/hw/i386/acpi-build.c
>> @@ -37,7 +37,6 @@
>> #include "hw/acpi/cpu.h"
>> #include "hw/nvram/fw_cfg.h"
>> #include "hw/acpi/bios-linker-loader.h"
>> -#include "hw/loader.h"
>> #include "hw/isa/isa.h"
>> #include "hw/block/fdc.h"
>> #include "hw/acpi/memory_hotplug.h"
>> @@ -58,6 +57,7 @@
>> #include "hw/i386/x86-iommu.h"
>>
>> #include "hw/acpi/aml-build.h"
>> +#include "hw/acpi/utils.h"
>>
>> #include "qom/qom-qobject.h"
>> #include "hw/i386/amd_iommu.h"
>> @@ -2842,14 +2842,6 @@ static void acpi_build_reset(void *build_opaque)
>> build_state->patched = 0;
>> }
>>
>> -static MemoryRegion *acpi_add_rom_blob(AcpiBuildState *build_state,
>> - GArray *blob, const char *name,
>> - uint64_t max_size)
>> -{
>> - return rom_add_blob(name, blob->data, acpi_data_len(blob), max_size, -1,
>> - name, acpi_build_update, build_state, NULL, true);
>> -}
>> -
>> static const VMStateDescription vmstate_acpi_build = {
>> .name = "acpi_build",
>> .version_id = 1,
>> @@ -2891,14 +2883,15 @@ void acpi_setup(void)
>> acpi_build(&tables, MACHINE(pcms));
>>
>> /* Now expose it all to Guest */
>> - build_state->table_mr = acpi_add_rom_blob(build_state, tables.table_data,
>> - ACPI_BUILD_TABLE_FILE,
>> - ACPI_BUILD_TABLE_MAX_SIZE);
>> + build_state->table_mr = acpi_add_rom_blob(acpi_build_update,
>> + build_state, tables.table_data,
>> + ACPI_BUILD_TABLE_FILE,
>> + ACPI_BUILD_TABLE_MAX_SIZE);
>> assert(build_state->table_mr != NULL);
>>
>> build_state->linker_mr =
>> - acpi_add_rom_blob(build_state, tables.linker->cmd_blob,
>> - "etc/table-loader", 0);
>> + acpi_add_rom_blob(acpi_build_update, build_state,
>> + tables.linker->cmd_blob, "etc/table-loader", 0);
>>
>> fw_cfg_add_file(pcms->fw_cfg, ACPI_BUILD_TPMLOG_FILE,
>> tables.tcpalog->data, acpi_data_len(tables.tcpalog));
>> @@ -2935,8 +2928,9 @@ void acpi_setup(void)
>> build_state->rsdp_mr = NULL;
>> } else {
>> build_state->rsdp = NULL;
>> - build_state->rsdp_mr = acpi_add_rom_blob(build_state, tables.rsdp,
>> - ACPI_BUILD_RSDP_FILE, 0);
>> + build_state->rsdp_mr = acpi_add_rom_blob(acpi_build_update,
>> + build_state, tables.rsdp,
>> + ACPI_BUILD_RSDP_FILE, 0);
>> }
>>
>> qemu_register_reset(acpi_build_reset, build_state);
>> diff --git a/include/hw/acpi/utils.h b/include/hw/acpi/utils.h
>> new file mode 100644
>> index 0000000000..5212113b7d
>> --- /dev/null
>> +++ b/include/hw/acpi/utils.h
>> @@ -0,0 +1,9 @@
>> +#ifndef HW_ACPI_UTILS_H
>> +#define HW_ACPI_UTILS_H
>> +
>> +#include "hw/loader.h"
>> +
>> +MemoryRegion *acpi_add_rom_blob(FWCfgCallback update, void *opaque,
>> + GArray *blob, const char *name,
>> + uint64_t max_size);
>> +#endif
--
Wei Yang
Help you, Help me
next prev parent reply other threads:[~2019-03-19 0:46 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-15 0:44 [Qemu-arm] [PATCH v3] hw/acpi: extract acpi_add_rom_blob() Wei Yang
2019-03-18 12:39 ` [Qemu-devel] " Igor Mammedov
2019-03-18 13:51 ` [Qemu-arm] " Wei Yang
2019-03-19 0:45 ` Wei Yang [this message]
2019-03-19 9:00 ` [Qemu-arm] " Igor Mammedov
2019-03-20 0:37 ` Wei Yang
2019-03-20 10:53 ` Igor Mammedov
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=20190319004523.GA5032@richard \
--to=richardw.yang@linux.intel.com \
--cc=imammedo@redhat.com \
--cc=mst@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=shannon.zhaosl@gmail.com \
--cc=yang.zhong@intel.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.