|
From: | Gavin Shan |
Subject: | Re: [PATCH v3 5/5] hw/arm/virt: Add 'highmem-compact' property |
Date: | Thu, 29 Sep 2022 21:21:35 +1000 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.2.0 |
Hi Cornelia, On 9/29/22 8:27 PM, Cornelia Huck wrote:
On Thu, Sep 29 2022, Gavin Shan <gshan@redhat.com> wrote: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_MMIOs/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 beI would rather rename the property into compact-highmem even if the vms field is name highmem_compact to align with other highmem fieldsOk, 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.)
Ok. 'compact-highmem' is also fine to me. I'm really bad at naming :)
explicitly enabled or disabled on all machine types by users. Signed-off-by: Gavin Shan <gshan@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.
Ok, lets do as Eric suggested. There are existing comments about @extended_memmap[] in hw/arm/virt.c. We need to explain the legacy/modern laoyout and 'compact-highmem' property there. Thanks, Gavin
[Prev in Thread] | Current Thread | [Next in Thread] |