| Message ID | 20220802064529.547361-3-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/arm/virt: Improve address assignment for highmem IO regions | expand |
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); > } > } >
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
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"; + } + + warn_report("Disabled %s high memory region due to PA limit", + region_name); } }
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(+)