Re: [PATCH 2/2] hw/arm/virt: Warn when high memory region is disabled

[Eric Auger]

Hi Gavin,

On 8/2/22 08:45, Gavin Shan wrote:
> When one specific high memory region is disabled due to the PA
> limit, it'd better to warn user about that. The warning messages
> help to identify the cause in some cases. For example, PCIe device
> that has large MMIO bar, to be covered by PCIE_MMIO high memory
> region, won't work properly if PCIE_MMIO high memory region is
> disabled due to the PA limit.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  hw/arm/virt.c | 18 ++++++++++++++++++
>  1 file changed, 18 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index bc0cd218f9..c91756e33d 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -1691,6 +1691,7 @@ static uint64_t virt_cpu_mp_affinity(VirtMachineState 
> *vms, int idx)
>  static void virt_memmap_fits(VirtMachineState *vms, int index,
>                               bool *enabled, hwaddr *base, int pa_bits)
>  {
> +    const char *region_name;
>      hwaddr size = extended_memmap[index].size;
>  
>      /* The region will be disabled if its size isn't given */
> @@ -1713,6 +1714,23 @@ static void virt_memmap_fits(VirtMachineState *vms, 
> int index,
>          vms->highest_gpa = *base + size - 1;
>  
>       *base = *base + size;
> +    } else {
> +        switch (index) {
> +        case VIRT_HIGH_GIC_REDIST2:
> +            region_name = "GIC_REDIST2";
> +            break;
> +        case VIRT_HIGH_PCIE_ECAM:
> +            region_name = "PCIE_ECAM";
> +            break;
> +        case VIRT_HIGH_PCIE_MMIO:
> +            region_name = "PCIE_MMIO";
> +            break;
> +        default:
> +            region_name = "unknown";
> +        }
when highmem is turned off I don't think we want those warnings because
it is obvious that highmem regions are not meant to be used.

On the other hand I am afraid some users may complain about warnings
that do not affect them. If you miss high MMIO don't you get a warning
on guest side?

Thanks

Eric
> +
> +        warn_report("Disabled %s high memory region due to PA limit",
> +                    region_name);
>      }
>  }
>