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

[Gavin Shan]

Hi Eric,

On 8/2/22 7:49 PM, Eric Auger wrote:
> 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?

Yes, there is error message from guest to complain 64-bits PCI BAR can't
be settled. In this regard, I do think the error messages are duplicate
to some extent.

Lets drop this patch in next revision.

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

Thanks,
Gavin