Re: [PATCH v6 7/7] hw/arm/virt: Add properties to disable high memory regions

[Cornelia Huck <cohuck@redhat.com>]

On Mon, Oct 24 2022, Gavin Shan <gshan@redhat.com> wrote:

> These 3 high memory regions are usually enabled by default, but

s/These 3/The/ ?

> they may be not used. For example, VIRT_HIGH_GIC_REDIST2 isn't
> needed by GICv2. This leads to waste in the PA space.

When building the command line, do we have enough information on when
the regions provide something useful, and when they just waste space?

>
> Add properties to allow users selectively disable them if needed:
> "highmem-redists", "highmem-ecam", "highmem-mmio".
>
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  docs/system/arm/virt.rst | 12 ++++++++
>  hw/arm/virt.c            | 64 ++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+)
>
> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> index 4454706392..a1668a969d 100644
> --- a/docs/system/arm/virt.rst
> +++ b/docs/system/arm/virt.rst
> @@ -98,6 +98,18 @@ compact-highmem
>    Set ``on``/``off`` to enable/disable the compact layout for high memory regions.
>    The default is ``on`` for machine types later than ``virt-7.2``.
>  
> +highmem-redists
> +  Set ``on``/``off`` to enable/disable the high memry region for GICv3/4

s/memry/memory/

> +  redistributor. The default is ``on``.

Do we need to add a note about what effects setting this to "off" may
have, e.g. "Setting this to ``off`` may limit the maximum number of
cpus." or so? And/or "Setting this to ``off`` when using GICv2 will save
some space."?

> +
> +highmem-ecam
> +  Set ``on``/``off`` to enable/disable the high memry region for PCI ECAM.

s/memry/memory/

> +  The default is ``on`` for machine types later than ``virt-3.0``.
> +
> +highmem-mmio
> +  Set ``on``/``off`` to enable/disable the high memry region for PCI MMIO.

s/memry/memory/

> +  The default is ``on``.
> +
>  gic-version
>    Specify the version of the Generic Interrupt Controller (GIC) to provide.
>    Valid values are: