Re: [Qemu-devel] [RFC PATCH 3/3] hw/acpi: Extract build_mcfg

[Wei Yang <richardw.yang@linux.intel.com>]

On Sat, Mar 16, 2019 at 10:31:24AM +0000, Wei Yang wrote:
>On Thu, Mar 14, 2019 at 10:18:30AM +0100, Igor Mammedov wrote:
>>On Wed, 13 Mar 2019 21:31:37 +0000
>>Wei Yang <richard.weiyang@gmail.com> wrote:
>>
>>> On Wed, Mar 13, 2019 at 05:09:43PM +0100, Igor Mammedov wrote:
>>> >On Wed, 13 Mar 2019 13:33:59 +0000
>>> >Wei Yang <richard.weiyang@gmail.com> wrote:
>>> >  
>>> >> On Wed, Mar 13, 2019 at 01:23:00PM +0100, Igor Mammedov wrote:  
>>> >> >On Wed, 13 Mar 2019 12:42:53 +0800
>>> >> >Wei Yang <richardw.yang@linux.intel.com> wrote:
>>> >> >    
>>> >> >> Now we have two identical build_mcfg function.
>>> >> >> 
>>> >> >> Extract them to aml-build.c.
>>> >> >> 
>>> >> >> Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>
>>> >> >> ---
>>> >> >>  hw/acpi/aml-build.c         | 30 ++++++++++++++++++++++++++++++
>>> >> >>  hw/arm/virt-acpi-build.c    | 16 ----------------
>>> >> >>  hw/i386/acpi-build.c        | 31 +------------------------------
>>> >> >>  include/hw/acpi/aml-build.h |  1 +
>>> >> >>  4 files changed, 32 insertions(+), 46 deletions(-)
>>> >> >> 
>>> >> >> diff --git a/hw/acpi/aml-build.c b/hw/acpi/aml-build.c
>>> >> >> index 555c24f21d..58d3b8f31d 100644
>>> >> >> --- a/hw/acpi/aml-build.c
>>> >> >> +++ b/hw/acpi/aml-build.c    
>>> >> >
>
>[...]
>
>>> >I mean do not call build_mcfg() at all when you don't have to.
>>> >
>>> >And when you need to keep table_blob the same size (for old machines)
>>> >using acpi_data_push() to reserve space instead of build_mcfg(sig="QEMU")
>>> >might just work as well. it's still hack but it can live in x86 specific
>>> >acpi_build() keeping build_mcfg() generic.
>>> >  
>>> 
>>> Seems got your idea.
>>> 
>>> >As for defining what to use as criteria to decide when we need to keep
>>> >table_blob size the same, I don't remember history of it, so I'd suggest
>>> >to look at commit a1666142, study history of acpi_ram_update() and
>>> >legacy_acpi_table_size to figure out since which machine type one doesn't
>>> >have to keep table_blob size the same.
>>> >  
>>> 
>>> OK, let me study the history first.
>>> 
>>> BTW, the legacy here is hardware specification level or qemu software
>>> design level?
>>it's QEMU only, you need to find a version of QEMU (machine type)
>>which didn't have re-sizable MemoryRegion and the next version most likely
>>would have a knob somewhere in machine class definition saying that we don't
>>care about sizes anymore or care about sizes only for previous machine types.
>
>I have got something, but not sure this is correct. So I'd like to
>summarize them here first.
>
>The following commits are ordered in timeline.
>
>1. First we introduced resizable MR.
>
>    a1666142db 2015-01-08 acpi-build: make ROMs RAM blocks resizeable
>    60786ef339 2015-01-08 memory: API to allocate resizeable RAM MR
>
>2. Then acpi_ram_update in introduced
>
>    339240b5cd 2015-04-27 acpi-build: remove dependency from ram_addr.h
>    42d859001d 2015-02-26 acpi-build: fix ACPI RAM management
>
>3. After that we introduce 2.4 machine type
>
>    5cb50e0acc 2015-04-27 pc: add 2.4 machine types
>
>But I don't find a comment stating we don't care about sizes anymore.
>
>Below is my understanding of the dummy table history. 
>
>Before resizable MR was introduced, the dummy table is used to be a
>place holder so that we could add real table in acpi_build_update().
>Othersize the new ACPI table is bigger to be placed into
>build_state->table_mr.
>
>If this is correct, after commit a1666142db ACPI table has the
>capability to expand itself and not necessary to pre-allocate the dummy
>table. Can I say after commit a1666142db, we don't need to put a fake
>table in ACPI?
>
>Another confusion is in the comment of build_mcfg(). In which aspect, it
>relates to migration? Is this before resizable MR introduced?
>

Hi, Igor

I guess you may be busy with other stuff. I'd appreciate it a lot if you could
spare some time slot to take a look at this one. Thanks :-)

Have a great day~

>-- 
>Wei Yang
>Help you, Help me

-- 
Wei Yang
Help you, Help me