Re: [PATCH v6 09/10] i386/pc: relocate 4g start to 1T where applicable

[Joao Martins]

On 7/1/22 17:10, Joao Martins wrote:
> +    /*
> +     * The HyperTransport range close to the 1T boundary is unique to AMD
> +     * hosts with IOMMUs enabled. Restrict the ram-above-4g relocation
> +     * to above 1T to AMD vCPUs only.
> +     */
> +    if (IS_AMD_CPU(&cpu->env)) {
> +        pc_set_amd_above_4g_mem_start(pcms, pci_hole64_size);
> +
> +        /*
> +         * Advertise the HT region if address space covers the reserved
> +         * region or if we relocate.
> +         */
> +        if (x86ms->above_4g_mem_start == AMD_ABOVE_1TB_START ||
> +            cpu->phys_bits >= 40) {
> +            e820_add_entry(AMD_HT_START, AMD_HT_SIZE, E820_RESERVED);
> +        }
> +    }
> +

[As part of Alex discussion in previous version there's this other case where 
VMs with
memory less than 1T but having enough GPUs (say each having 40G to state an 
example) can
have PCI devices placed within reserved HT region.]

Changing fwcfg 'reserved-memory-end' to 1T (bearing that phys_bits is correctly
configured) without triggering above-4g relocation ... fixes the case above. As
'reserved-memory-end' is ultimately what virtual firmware uses (SeaBIOS and 
OVMF) for
hole64 start. Though, I am at odds whether to include this. Meaning, whether 
this is the
VMM going around a fw bug[*] even after e820 is described accurately, or if 
this is the
right to do in the VMM?

Part of the reason I haven't done this was because the issue doesn't happen if 
VMM user
describes the correct pci-hole64-size in q35/pc that's big enough to cover all 
VFIO
devices (which is ultimately correct). Thoughts?

[*] as it should look at *all* reserved ranges including those above ram.

>      /*
>       * Split single memory region and use aliases to address portions of it,
>       * done for backwards compatibility with older qemus.
> @@ -938,6 +1038,7 @@ void pc_memory_init(PCMachineState *pcms,
>                               0, x86ms->below_4g_mem_size);
>      memory_region_add_subregion(system_memory, 0, ram_below_4g);
>      e820_add_entry(0, x86ms->below_4g_mem_size, E820_RAM);
> +

Spurious new line here that I will fix on v7.

>      if (x86ms->above_4g_mem_size > 0) {
>          ram_above_4g = g_malloc(sizeof(*ram_above_4g));
>          memory_region_init_alias(ram_above_4g, NULL, "ram-above-4g",