On Thu, Sep 29 2022, Gavin Shan <gs...@redhat.com> wrote: > Hi Eric, > > On 9/28/22 10:22 PM, Eric Auger wrote: >> On 9/22/22 01:13, Gavin Shan wrote: >>> After the improvement to high memory region address assignment is >>> applied, the memory layout is changed. For example, VIRT_HIGH_PCIE_MMIO >> s/the memory layout is changed./the memory layout is changed, >> introducing possible migration breakage. > > Ok, much clearer. > >>> memory region is enabled when the improvement is applied, but it's >>> disabled if the improvement isn't applied. >>> >>> pa_bits = 40; >>> vms->highmem_redists = false; >>> vms->highmem_ecam = false; >>> vms->highmem_mmio = true; >>> >>> # qemu-system-aarch64 -accel kvm -cpu host \ >>> -machine virt-7.2 -m 4G,maxmem=511G \ >>> -monitor stdio >>> >>> In order to keep backwords compatibility, we need to disable the >>> optimization on machines, which is virt-7.1 or ealier than it. It >>> means the optimization is enabled by default from virt-7.2. Besides, >>> 'highmem-compact' property is added so that the optimization can be >> I would rather rename the property into compact-highmem even if the vms >> field is name highmem_compact to align with other highmem fields > > Ok, but I would love to know why. Note that we already have > 'highmem=on|off'. 'highmem_compact=on|off' seems consistent > to me.
FWIW, I initially misread 'highmem_compact' as 'highmem_compat' (and had to re-read because I got confused). At least to me, 'compact_highmem' has less chance of being parsed incorrectly :) (although that is probably a personal thing.) > >>> explicitly enabled or disabled on all machine types by users. >>> >>> Signed-off-by: Gavin Shan <gs...@redhat.com> >>> --- >>> docs/system/arm/virt.rst | 4 ++++ >>> hw/arm/virt.c | 33 +++++++++++++++++++++++++++++++++ >>> include/hw/arm/virt.h | 2 ++ >>> 3 files changed, 39 insertions(+) >>> >>> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst >>> index 20442ea2c1..f05ec2253b 100644 >>> --- a/docs/system/arm/virt.rst >>> +++ b/docs/system/arm/virt.rst >>> @@ -94,6 +94,10 @@ highmem >>> address space above 32 bits. The default is ``on`` for machine types >>> later than ``virt-2.12``. >>> >>> +highmem-compact >>> + Set ``on``/``off`` to enable/disable compact space for high memory >>> regions. >>> + The default is ``on`` for machine types later than ``virt-7.2`` >> I think you should document what is compact layout versus legacy one, >> both in the commit msg and maybe as a comment in a code along with the >> comment in hw/arm/virt.c starting with 'Highmem IO Regions: ' > > Ok, I will add this into the commit log in v4. I don't think it's necessary > to add duplicate comment in the code. People can check the commit log for > details if needed. Rather explain it in this file here, maybe? I'd prefer to be able to find out what 'compact' means without digging through the commit log.
