[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [Qemu-devel] [RFC PATCH 3/3] hw/acpi: Extract build_mcfg
From: |
Igor Mammedov |
Subject: |
Re: [Qemu-arm] [Qemu-devel] [RFC PATCH 3/3] hw/acpi: Extract build_mcfg |
Date: |
Fri, 12 Apr 2019 10:14:18 +0200 |
On Fri, 12 Apr 2019 13:44:07 +0800
Wei Yang <address@hidden> wrote:
> On Tue, Apr 09, 2019 at 04:54:15PM +0200, Igor Mammedov wrote:
> >>
> >> Let's see a normal hotplug case first.
> >>
> >> I did one test to see the how a guest with hot-plug cpu migrate to
> >> destination. It looks the migration fails if I just do hot-plug at
> >> source. So I have to do hot-plug both at source and at destination.
> >> This
> >> will expand the table_mr both at source and destination.
> >>
> >> Then let's see the effect of hotplug more devices to exceed original padded
> >> size. There are two cases, before re-sizable MemoryRegion and after.
> >>
> >> 1) Before re-sizable MemoryRegion introduced
> >>
> >> Before re-sizable MemoryRegion introduced, we just pad table_mr to 4K.
> >> And
> >> this size never gets bigger, if I am right. To be accurate, the
> >> table_blob
> >> would grow to next padded size if we hot-add more cpus/pci bridge, but
> >> we
> >> just copy the original size of MemoryRegion. Even without migration,
> >> the
> >> ACPI table is corrupted when we expand to next padded size.
> >>
> >> Is my understanding correct here?
> >>
> >> 2) After re-sizable MemoryRegion introduced
> >>
> >> This time both tabl_blob and MemoryRegion grows when it expand to next
> >> padded size. Since we need to hot-add device at both side, ACPI table
> >> grows at the same pace.
> >>
> >> Every thing looks good, until one of it exceed the resizable
> >> MemoryRegion's max size. (Not sure this is possible in reality, while
> >> possible in theory). Actually, this looks like case 1) when resizable
> >> MemoryRegion is not introduced. The too big ACPI table get corrupted.
> >>
> >> So if my understanding is correct, the procedure you mentioned "expand from
> >> initial padded size to next padded size" only applies to two different max
> >> size resizable MemoryRegion. For other cases, the procedure corrupt the
> >> ACPI
> >> table itself.
> >>
> >> Then when we look at
> >>
> >> commit 07fb61760cdea7c3f1b9c897513986945bca8e89
> >> Author: Paolo Bonzini <address@hidden>
> >> Date: Mon Jul 28 17:34:15 2014 +0200
> >>
> >> pc: hack for migration compatibility from QEMU 2.0
> >>
> >> This fix ACPI migration issue before resizable MemoryRegion is
> >> introduced(introduced in 2015-01-08). This looks expand to next padded size
> >> always corrupt ACPI table at that time. And it make me think expand to next
> >> padded size is not the procedure we should do?
> >>
> >> And my colleague Wei Wang(in cc) mentioned, to make migration succeed, the
> >> MemoryRegion has to be the same size at both side. So I guess the problem
> >> doesn't lie in hotplug but in "main table" size difference.
> >
> >It's true only for pre-resizable MemoryRegion QEMU versions,
> >after that size doesn't affect migration anymore.
> >
> >
> >> For example, we have two version of Qemu: v1 and v2. Their "main table"
> >> size
> >> is:
> >>
> >> v1: 3990
> >> v2: 4020
> >>
> >> At this point, their ACPI table all padded to 4k, which is the same.
> >>
> >> Then we create a machine with 1 more vcpu by these two versions. This will
> >> expand the table to:
> >>
> >> v1: 4095
> >> v2: 4125
> >>
> >> After padding, v1's ACPI table size is still 4k but v2's is 8k. Now the
> >> migration is broken.
> >>
> >> If this analysis is correct, the relationship between migration failure and
> >> ACPI table is "the change of ACPI table size". Any size change of any
> >you should make distinction between used_length and max_length here.
> >Migration puts on wire used_length and that's what matter for keeping
> >migration
> >working.
> >
> >> ACPI table would break migration. While of course, since we pad the table,
> >> only some combinations of tables would result in a visible real size
> >> change in
> >> MemoryRegion.
> >>
> >> Then the principle for future ACPI development is to keep all ACPI table
> >> size
> >> unchanged.
> >once again it applies only for QEMU (versions < 2.1) and that was
> >the problem (i.e. there always would be configurations that would create
> >differently sized tables regardless of arbitrary size we would preallocate)
> >resizable MemoryRegions solved.
> >
> >> Now let's back to mcfg table. As the comment mentioned, guest could
> >> enable/disable MCFG, so the code here reserve table no matter it is
> >> enabled or
> >> not. This behavior ensures ACPI table size not changed. So do we need to
> >> find
> >> the machine type as you suggested before?
> >We should be able to drop mcgf 'padding' hack since machine version
> >which was introduced in the QEMU version that introduced resizable
> >MemoryRegion
> >as well.
> >
> >I'll send a patch to address that
>
> Hi, Igor,
>
> We have found the qemu version 2.1 which is with resizable MemoryRegion
> enabled and q35 will stop support version before 2.3. The concern about ACPI
> mcfg table breaking live migration is solved, right?
>
> If so, I would prepare mcfg refactor patch based on your cleanup.
yes, just base your patches on top of
"[PATCH for-4.1] q35: acpi: do not create dummy MCFG table"