Hi Gavin,
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.
> 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
> 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: '
> +
> gic-version
> Specify the version of the Generic Interrupt Controller (GIC) to provide.
> Valid values are:
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b702f8f2b5..a4fbdaef91 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1734,6 +1734,13 @@ static void virt_set_high_memmap(VirtMachineState *vms,
> base = region_base + region_size;
> } else {
> *region_enabled = false;
> +
> + if (!vms->highmem_compact) {
this snippet should be already present in previous patch otherwise this
will break bisectability.
> + base = region_base + region_size;
> + if (fits) {
> + vms->highest_gpa = region_base + region_size - 1;
> + }
> + }
> }
> }
> }
> @@ -2348,6 +2355,20 @@ static void virt_set_highmem(Object *obj, bool value,
> Error **errp)
> vms->highmem = value;
> }
>
> +static bool virt_get_highmem_compact(Object *obj, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> + return vms->highmem_compact;
> +}
> +
> +static void virt_set_highmem_compact(Object *obj, bool value, Error **errp)
> +{
> + VirtMachineState *vms = VIRT_MACHINE(obj);
> +
> + vms->highmem_compact = value;
> +}
> +
> static bool virt_get_its(Object *obj, Error **errp)
> {
> VirtMachineState *vms = VIRT_MACHINE(obj);
> @@ -2966,6 +2987,13 @@ static void virt_machine_class_init(ObjectClass *oc,
> void *data)
> "Set on/off to enable/disable
> using "
> "physical address space above 32
> bits");
>
> + object_class_property_add_bool(oc, "highmem-compact",
> + virt_get_highmem_compact,
> + virt_set_highmem_compact);
> + object_class_property_set_description(oc, "highmem-compact",
> + "Set on/off to enable/disable
> compact "
> + "space for high memory regions");
> +
> object_class_property_add_str(oc, "gic-version", virt_get_gic_version,
> virt_set_gic_version);
> object_class_property_set_description(oc, "gic-version",
> @@ -3050,6 +3078,7 @@ static void virt_instance_init(Object *obj)
>
> /* High memory is enabled by default */
> vms->highmem = true;
> + vms->highmem_compact = !vmc->no_highmem_compact;
> vms->gic_version = VIRT_GIC_VERSION_NOSEL;
>
> vms->highmem_ecam = !vmc->no_highmem_ecam;
> @@ -3119,8 +3148,12 @@ DEFINE_VIRT_MACHINE_AS_LATEST(7, 2)
>
> static void virt_machine_7_1_options(MachineClass *mc)
> {
> + VirtMachineClass *vmc = VIRT_MACHINE_CLASS(OBJECT_CLASS(mc));
> +
> virt_machine_7_2_options(mc);
> compat_props_add(mc->compat_props, hw_compat_7_1, hw_compat_7_1_len);
> + /* Compact space for high memory regions was introduced with 7.2 */
> + vmc->no_highmem_compact = true;
> }
> DEFINE_VIRT_MACHINE(7, 1)
>
> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> index 6ec479ca2b..c7dd59d7f1 100644
> --- a/include/hw/arm/virt.h
> +++ b/include/hw/arm/virt.h
> @@ -125,6 +125,7 @@ struct VirtMachineClass {
> bool no_pmu;
> bool claim_edge_triggered_timers;
> bool smbios_old_sys_ver;
> + bool no_highmem_compact;
> bool no_highmem_ecam;
> bool no_ged; /* Machines < 4.2 have no support for ACPI GED device */
> bool kvm_no_adjvtime;
> @@ -144,6 +145,7 @@ struct VirtMachineState {
> PFlashCFI01 *flash[2];
> bool secure;
> bool highmem;
> + bool highmem_compact;
> bool highmem_ecam;
> bool highmem_mmio;
> bool highmem_redists;
Thanks
Eric
