On Mon, 1 Jun 2026 17:17:06 +0900
Akihiko Odaki <[email protected]> wrote:

> On 2026/05/29 18:39, Igor Mammedov wrote:
> > On Thu, 28 May 2026 00:05:44 +0900
> > Akihiko Odaki <[email protected]> wrote:
> >   
> >> Relax the lower bound of the HighMem MMIO region size to make it more
> >> likely that the region fits in the PA space.
> >>
> >> The lower bound was especially problematic on Apple M2. The current
> >> lower bound, 512 GiB, is too big to fit in the limited PA space on the
> >> hardware. That makes the HighMem MMIO region unavailable and limits the
> >> PCIe MMIO space to the base MMIO region. The base MMIO region is not big
> >> enough to contain a large hostmem window configured for virtio-gpu-gl-pci
> >> with DRM native context, for example.
> >>
> >> Relax the lower bound to 64 KiB, matching the memory map comment that
> >> says "devices should generally be placed at multiples of 0x10000, to
> >> accommodate guests using 64K pages." Also, automatically lower the
> >> default HighMem MMIO region size to fit in the PA space for the latest
> >> machine version; the older machine versions retain the previous default
> >> HighMem MMIO region size for compatibility.  
> > 
> > How useful/practical 64K window would be?
> > Should it be reasonably large (like 64Mb or similar), or mirror lower 
> > windows size?
> > CCing Gerd,
> > to look at it from firmware POV.  
> 
> Practically it's not useful. I just followed the existing convention 
> described in the comment instead of inventing another lower bound.
> 
> > 
> > wrt 64K magic value, it opencoded in several places. I'd use a macro for 
> > that.  
> 
> The new occurrences are:
> - One numeric literal
> - One string literal
> - One documentation
> 
> They cannot be cleanly shared. The existing memory map contains many 
> hardcoded values that are aligned by 64 KiB, but I think they are better 
> to stay as is. For example, the size of VIRT_CXL_HOST can be rewritten 
> as MEMMAP_GRANULARITY * 16 instead of 64 * KiB * 16, but if you do so, 
> that size will vary when changing the granularity, which is probably not 
> what we want. We actually need a fixed value that satisfies the 
> requirement of CXL and is consistent with old versions of the machine.


ok, I guess I'm hair splitting at this point

Acked-by: Igor Mammedov <[email protected]>

> 
> Regards,
> Akihiko Odaki
> 
> >   
> >>
> >> Signed-off-by: Akihiko Odaki <[email protected]>
> >> ---
> >>   docs/system/arm/virt.rst |  2 +-
> >>   include/hw/arm/virt.h    |  1 +
> >>   hw/arm/virt.c            | 22 ++++++++++++++--------
> >>   3 files changed, 16 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/docs/system/arm/virt.rst b/docs/system/arm/virt.rst
> >> index f811e662d68d..c8284ce2fa0e 100644
> >> --- a/docs/system/arm/virt.rst
> >> +++ b/docs/system/arm/virt.rst
> >> @@ -149,7 +149,7 @@ highmem-mmio
> >>   
> >>   highmem-mmio-size
> >>     Set the high memory region size for PCI MMIO. Must be a power of 2 and
> >> -  greater than or equal to the default size (512G).
> >> +  greater than or equal to 64 KiB.
> >>   
> >>   gic-version
> >>     Specify the version of the Generic Interrupt Controller (GIC) to 
> >> provide.
> >> diff --git a/include/hw/arm/virt.h b/include/hw/arm/virt.h
> >> index 6c8ba8f3185b..7aa289d3936f 100644
> >> --- a/include/hw/arm/virt.h
> >> +++ b/include/hw/arm/virt.h
> >> @@ -143,6 +143,7 @@ typedef enum VirtGICType {
> >>   struct VirtMachineClass {
> >>       MachineClass parent;
> >>       hwaddr min_highmem_base;
> >> +    hwaddr high_pcie_mmio_size;
> >>       bool no_tcg_its;
> >>       bool no_highmem_compact;
> >>       bool no_kvm_steal_time;
> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> >> index 78a85a966ad7..dd97b0dd6a42 100644
> >> --- a/hw/arm/virt.c
> >> +++ b/hw/arm/virt.c
> >> @@ -222,8 +222,7 @@ static const MemMapEntry base_memmap[] = {
> >>   };
> >>   
> >>   /* Update the docs for highmem-mmio-size when changing this default */
> >> -#define DEFAULT_HIGH_PCIE_MMIO_SIZE_GB 512
> >> -#define DEFAULT_HIGH_PCIE_MMIO_SIZE (DEFAULT_HIGH_PCIE_MMIO_SIZE_GB * GiB)
> >> +#define MAX_DEFAULT_HIGH_PCIE_MMIO_SIZE (512 * GiB)
> >>   
> >>   /*
> >>    * Highmem IO Regions: This memory map is floating, located after the 
> >> RAM.
> >> @@ -249,7 +248,7 @@ static MemMapEntry extended_memmap[] = {
> >>       [VIRT_CXL_HOST] =           { 0x0, 64 * KiB * 16 }, /* 16 UID */
> >>       [VIRT_HIGH_PCIE_ECAM] =     { 0x0, 256 * MiB },
> >>       /* Second PCIe window */
> >> -    [VIRT_HIGH_PCIE_MMIO] =     { 0x0, DEFAULT_HIGH_PCIE_MMIO_SIZE },
> >> +    [VIRT_HIGH_PCIE_MMIO] =     { 0x0, 0 },
> >>       /* Any CXL Fixed memory windows come here */
> >>   };
> >>   
> >> @@ -2448,9 +2447,14 @@ static void virt_set_high_memmap(VirtMachineState 
> >> *vms,
> >>   
> >>       for (i = VIRT_LOWMEMMAP_LAST; i < ARRAY_SIZE(extended_memmap); i++) {
> >>           region_enabled = virt_get_high_memmap_enabled(vms, i);
> >> -        region_base = ROUND_UP(base, extended_memmap[i].size);
> >>           region_size = extended_memmap[i].size;
> >>   
> >> +        if (i == VIRT_HIGH_PCIE_MMIO && !region_size) {
> >> +            region_size = CLAMP(pow2floor(BIT_ULL(pa_bits) - base),
> >> +                                64 * KiB, 
> >> MAX_DEFAULT_HIGH_PCIE_MMIO_SIZE);
> >> +        }
> >> +
> >> +        region_base = ROUND_UP(base, region_size);
> >>           vms->memmap[i].base = region_base;
> >>           vms->memmap[i].size = region_size;
> >>   
> >> @@ -3340,11 +3344,9 @@ static void virt_set_highmem_mmio_size(Object *obj, 
> >> Visitor *v,
> >>           return;
> >>       }
> >>   
> >> -    if (size < DEFAULT_HIGH_PCIE_MMIO_SIZE) {
> >> -        char *sz = size_to_str(DEFAULT_HIGH_PCIE_MMIO_SIZE);
> >> +    if (size < 64 * KiB) {
> >>           error_setg(errp, "highmem-mmio-size cannot be set to a lower 
> >> value "
> >> -                         "than the default (%s)", sz);
> >> -        g_free(sz);
> >> +                         "than 64 KiB");
> >>           return;
> >>       }
> >>   
> >> @@ -4271,6 +4273,8 @@ static void virt_instance_init(Object *obj)
> >>       VirtMachineState *vms = VIRT_MACHINE(obj);
> >>       VirtMachineClass *vmc = VIRT_MACHINE_GET_CLASS(vms);
> >>   
> >> +    extended_memmap[VIRT_HIGH_PCIE_MMIO].size = vmc->high_pcie_mmio_size;
> >> +
> >>       /* EL3 is disabled by default on virt: this makes us consistent
> >>        * between KVM and TCG for this board, and it also allows us to
> >>        * boot UEFI blobs which assume no TrustZone support.
> >> @@ -4372,6 +4376,8 @@ static void virt_machine_11_0_options(MachineClass 
> >> *mc)
> >>        * < 255GiB we keep the legacy memory map.
> >>        */
> >>       vmc->min_highmem_base = base_memmap[VIRT_MEM].base + 
> >> LEGACY_RAMLIMIT_BYTES;
> >> +
> >> +    vmc->high_pcie_mmio_size = MAX_DEFAULT_HIGH_PCIE_MMIO_SIZE;
> >>   }
> >>   DEFINE_VIRT_MACHINE(11, 0)
> >>   
> >>  
> >   
> 


Reply via email to