On Mon, Sep 18, 2023 at 7:25 PM Ani Sinha <anisi...@redhat.com> wrote:
>
> 32-bit systems do not have a reserved memory for hole64 but they may have a
> reserved memory space for memory hotplug. Since, hole64 starts after the
> reserved hotplug memory, the unaligned hole64 start address gives us the
> end address for this memory hotplug region that the processor may use.
> Fix this. This ensures that the physical address space bound checking works
> correctly for 32-bit systems as well.
This patch breaks some unit tests. I am not sure why it did not catch
it when I tested it before sending.
Will have to resend after fixing the tests.
>
> Suggested-by: David Hildenbrand <da...@redhat.com>
> Signed-off-by: Ani Sinha <anisi...@redhat.com>
> ---
> hw/i386/pc.c | 60 ++++++++++++++++++++++++++++++----------------------
> 1 file changed, 35 insertions(+), 25 deletions(-)
>
> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> index 54838c0c41..c8abcabd53 100644
> --- a/hw/i386/pc.c
> +++ b/hw/i386/pc.c
> @@ -904,13 +904,43 @@ static uint64_t pc_get_cxl_range_end(PCMachineState
> *pcms)
> return start;
> }
>
> +/*
> + * The 64bit pci hole starts after "above 4G RAM" and
> + * potentially the space reserved for memory hotplug.
> + * This function returns unaligned start address.
> + */
> +static uint64_t pc_pci_hole64_start_unaligned(void)
> +{
> + PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> + PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> + MachineState *ms = MACHINE(pcms);
> + uint64_t hole64_start = 0;
> + ram_addr_t size = 0;
> +
> + if (pcms->cxl_devices_state.is_enabled) {
> + hole64_start = pc_get_cxl_range_end(pcms);
> + } else if (pcmc->has_reserved_memory && (ms->ram_size <
> ms->maxram_size)) {
> + pc_get_device_memory_range(pcms, &hole64_start, &size);
> + if (!pcmc->broken_reserved_end) {
> + hole64_start += size;
> + }
> + } else {
> + hole64_start = pc_above_4g_end(pcms);
> + }
> +
> + return hole64_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;
> + /*
> + * 32-bit systems don't have hole64, but we might have a region for
> + * memory hotplug.
> + */
> + if (!(cpu->env.features[FEAT_8000_0001_EDX] & CPUID_EXT2_LM)) {
> + return pc_pci_hole64_start_unaligned() - 1;
> }
>
> return pc_pci_hole64_start() + pci_hole64_size - 1;
> @@ -1147,30 +1177,10 @@ void pc_memory_init(PCMachineState *pcms,
> pcms->memhp_io_base = ACPI_MEMORY_HOTPLUG_BASE;
> }
>
> -/*
> - * The 64bit pci hole starts after "above 4G RAM" and
> - * potentially the space reserved for memory hotplug.
> - */
> +/* returns 1 GiB aligned hole64 start address */
> uint64_t pc_pci_hole64_start(void)
> {
> - PCMachineState *pcms = PC_MACHINE(qdev_get_machine());
> - PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> - MachineState *ms = MACHINE(pcms);
> - uint64_t hole64_start = 0;
> - ram_addr_t size = 0;
> -
> - if (pcms->cxl_devices_state.is_enabled) {
> - hole64_start = pc_get_cxl_range_end(pcms);
> - } else if (pcmc->has_reserved_memory && (ms->ram_size <
> ms->maxram_size)) {
> - pc_get_device_memory_range(pcms, &hole64_start, &size);
> - if (!pcmc->broken_reserved_end) {
> - hole64_start += size;
> - }
> - } else {
> - hole64_start = pc_above_4g_end(pcms);
> - }
> -
> - return ROUND_UP(hole64_start, 1 * GiB);
> + return ROUND_UP(pc_pci_hole64_start_unaligned(), 1 * GiB);
> }
>
> DeviceState *pc_vga_init(ISABus *isa_bus, PCIBus *pci_bus)
> --
> 2.39.1
>
