[PATCH v8 09/11] i386/pc: bounds check phys-bits against max used GPA

Joao Martins posted 11 patches 3 years, 6 months ago
There is a newer version of this series
[PATCH v8 09/11] i386/pc: bounds check phys-bits against max used GPA
Posted by Joao Martins 3 years, 6 months ago
Calculate max *used* GPA against the CPU maximum possible address
and error out if the former surprasses the latter. This ensures
max used GPA is reacheable by configured phys-bits. Default phys-bits
on Qemu is TCG_PHYS_ADDR_BITS (40) which is enough for the CPU to
address 1Tb (0xff ffff ffff) or 1010G (0xfc ffff ffff) in AMD hosts
with IOMMU.

This is preparation for AMD guests with >1010G, where it will want relocate
ram-above-4g to be after 1Tb instead of 4G.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 hw/i386/pc.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/hw/i386/pc.c b/hw/i386/pc.c
index cda435e3baeb..f30661b7f1a2 100644
--- a/hw/i386/pc.c
+++ b/hw/i386/pc.c
@@ -880,6 +880,18 @@ static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
     return start;
 }
 
+static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t pci_hole64_size)
+{
+    X86CPU *cpu = X86_CPU(first_cpu);
+
+    /* 32-bit systems don't have hole64 thus return max CPU address */
+    if (cpu->phys_bits <= 32) {
+        return ((hwaddr)1 << cpu->phys_bits) - 1;
+    }
+
+    return pc_pci_hole64_start() + pci_hole64_size - 1;
+}
+
 void pc_memory_init(PCMachineState *pcms,
                     MemoryRegion *system_memory,
                     MemoryRegion *rom_memory,
@@ -894,13 +906,28 @@ void pc_memory_init(PCMachineState *pcms,
     MachineClass *mc = MACHINE_GET_CLASS(machine);
     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
     X86MachineState *x86ms = X86_MACHINE(pcms);
+    hwaddr maxphysaddr, maxusedaddr;
     hwaddr cxl_base, cxl_resv_end = 0;
+    X86CPU *cpu = X86_CPU(first_cpu);
 
     assert(machine->ram_size == x86ms->below_4g_mem_size +
                                 x86ms->above_4g_mem_size);
 
     linux_boot = (machine->kernel_filename != NULL);
 
+    /*
+     * phys-bits is required to be appropriately configured
+     * to make sure max used GPA is reachable.
+     */
+    maxusedaddr = pc_max_used_gpa(pcms, pci_hole64_size);
+    maxphysaddr = ((hwaddr)1 << cpu->phys_bits) - 1;
+    if (maxphysaddr < maxusedaddr) {
+        error_report("Address space limit 0x%"PRIx64" < 0x%"PRIx64
+                     " phys-bits too low (%u)",
+                     maxphysaddr, maxusedaddr, cpu->phys_bits);
+        exit(EXIT_FAILURE);
+    }
+
     /*
      * Split single memory region and use aliases to address portions of it,
      * done for backwards compatibility with older qemus.
-- 
2.17.2
Re: [PATCH v8 09/11] i386/pc: bounds check phys-bits against max used GPA
Posted by Igor Mammedov 3 years, 6 months ago
On Fri, 15 Jul 2022 18:16:26 +0100
Joao Martins <joao.m.martins@oracle.com> wrote:

> Calculate max *used* GPA against the CPU maximum possible address
> and error out if the former surprasses the latter. This ensures
> max used GPA is reacheable by configured phys-bits. Default phys-bits
> on Qemu is TCG_PHYS_ADDR_BITS (40) which is enough for the CPU to
> address 1Tb (0xff ffff ffff) or 1010G (0xfc ffff ffff) in AMD hosts
> with IOMMU.
> 
> This is preparation for AMD guests with >1010G, where it will want relocate
> ram-above-4g to be after 1Tb instead of 4G.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>

Acked-by: Igor Mammedov <imammedo@redhat.com>

> ---
>  hw/i386/pc.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index cda435e3baeb..f30661b7f1a2 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -880,6 +880,18 @@ static uint64_t pc_get_cxl_range_end(PCMachineState *pcms)
>      return start;
>  }
>  
> +static hwaddr pc_max_used_gpa(PCMachineState *pcms, uint64_t pci_hole64_size)
> +{
> +    X86CPU *cpu = X86_CPU(first_cpu);
> +
> +    /* 32-bit systems don't have hole64 thus return max CPU address */
> +    if (cpu->phys_bits <= 32) {
> +        return ((hwaddr)1 << cpu->phys_bits) - 1;
> +    }
> +
> +    return pc_pci_hole64_start() + pci_hole64_size - 1;
> +}
> +
>  void pc_memory_init(PCMachineState *pcms,
>                      MemoryRegion *system_memory,
>                      MemoryRegion *rom_memory,
> @@ -894,13 +906,28 @@ void pc_memory_init(PCMachineState *pcms,
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
>      PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
>      X86MachineState *x86ms = X86_MACHINE(pcms);
> +    hwaddr maxphysaddr, maxusedaddr;
>      hwaddr cxl_base, cxl_resv_end = 0;
> +    X86CPU *cpu = X86_CPU(first_cpu);
>  
>      assert(machine->ram_size == x86ms->below_4g_mem_size +
>                                  x86ms->above_4g_mem_size);
>  
>      linux_boot = (machine->kernel_filename != NULL);
>  
> +    /*
> +     * phys-bits is required to be appropriately configured
> +     * to make sure max used GPA is reachable.
> +     */
> +    maxusedaddr = pc_max_used_gpa(pcms, pci_hole64_size);
> +    maxphysaddr = ((hwaddr)1 << cpu->phys_bits) - 1;
> +    if (maxphysaddr < maxusedaddr) {
> +        error_report("Address space limit 0x%"PRIx64" < 0x%"PRIx64
> +                     " phys-bits too low (%u)",
> +                     maxphysaddr, maxusedaddr, cpu->phys_bits);
> +        exit(EXIT_FAILURE);
> +    }
> +
>      /*
>       * Split single memory region and use aliases to address portions of it,
>       * done for backwards compatibility with older qemus.
Re: [PATCH v8 09/11] i386/pc: bounds check phys-bits against max used GPA
Posted by Igor Mammedov 3 years, 6 months ago
On Mon, 18 Jul 2022 15:16:22 +0200
Igor Mammedov <imammedo@redhat.com> wrote:

> On Fri, 15 Jul 2022 18:16:26 +0100
> Joao Martins <joao.m.martins@oracle.com> wrote:
> 
> > Calculate max *used* GPA against the CPU maximum possible address
> > and error out if the former surprasses the latter. This ensures
> > max used GPA is reacheable by configured phys-bits. Default phys-bits
> > on Qemu is TCG_PHYS_ADDR_BITS (40) which is enough for the CPU to
> > address 1Tb (0xff ffff ffff) or 1010G (0xfc ffff ffff) in AMD hosts
> > with IOMMU.
> > 
> > This is preparation for AMD guests with >1010G, where it will want relocate
> > ram-above-4g to be after 1Tb instead of 4G.
> > 
> > Signed-off-by: Joao Martins <joao.m.martins@oracle.com>  
> 
> Acked-by: Igor Mammedov <imammedo@redhat.com>
[...]

> > +    return pc_pci_hole64_start() + pci_hole64_size - 1;

off by 1?

> > +}
> > +
[...]
Re: [PATCH v8 09/11] i386/pc: bounds check phys-bits against max used GPA
Posted by Joao Martins 3 years, 6 months ago

On 7/18/22 14:56, Igor Mammedov wrote:
> On Mon, 18 Jul 2022 15:16:22 +0200
> Igor Mammedov <imammedo@redhat.com> wrote:
> 
>> On Fri, 15 Jul 2022 18:16:26 +0100
>> Joao Martins <joao.m.martins@oracle.com> wrote:
>>
>>> Calculate max *used* GPA against the CPU maximum possible address
>>> and error out if the former surprasses the latter. This ensures
>>> max used GPA is reacheable by configured phys-bits. Default phys-bits
>>> on Qemu is TCG_PHYS_ADDR_BITS (40) which is enough for the CPU to
>>> address 1Tb (0xff ffff ffff) or 1010G (0xfc ffff ffff) in AMD hosts
>>> with IOMMU.
>>>
>>> This is preparation for AMD guests with >1010G, where it will want relocate
>>> ram-above-4g to be after 1Tb instead of 4G.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>  
>>
>> Acked-by: Igor Mammedov <imammedo@redhat.com>
> [...]
> 
>>> +    return pc_pci_hole64_start() + pci_hole64_size - 1;
> 
> off by 1?
> 

If you add a size to a start of range, you get past the end, not
the actual end address. And given that we are supposed to return the
end address ... or am I seeing a non issue here?

[Also this was new in v8 predecessor patches didn't have it.]

>>> +}
>>> +
> [...]
>