On Fri, 11 Nov 2022 12:40:59 +0100 Gerd Hoffmann <kra...@redhat.com> wrote:
> On Fri, Nov 11, 2022 at 11:51:23AM +0100, Igor Mammedov wrote: > > On Tue, 8 Nov 2022 12:21:11 +0100 > > Gerd Hoffmann <kra...@redhat.com> wrote: > > > > > > >> > diff --git a/hw/i386/pc.c b/hw/i386/pc.c > > > > >> > index 566accf7e6..5bf5465a21 100644 > > > > >> > --- a/hw/i386/pc.c > > > > >> > +++ b/hw/i386/pc.c > > > > >> > @@ -1061,7 +1061,6 @@ void pc_memory_init(PCMachineState *pcms, > > > > >> > hwaddr cxl_size = MiB; > > > > >> > > > > > >> > cxl_base = pc_get_cxl_range_start(pcms); > > > > >> > - e820_add_entry(cxl_base, cxl_size, E820_RESERVED); > > > > > > Just dropping it doesn't look like a good plan to me. > > > > > > You can try set etc/reserved-memory-end fw_cfg file instead. Firmware > > > (both seabios and ovmf) read it and will make sure the 64bit pci mmio > > > window is placed above that address, i.e. this effectively reserves > > > address space. Right now used by memory hotplug code, but should work > > > for cxl too I think (disclaimer: don't know much about cxl ...). > > > > As far as I know CXL impl. in QEMU isn't using etc/reserved-memory-end > > at all, it' has its own mapping. > > This should be changed. cxl should make sure the highest address used > is stored in etc/reserved-memory-end to avoid the firmware mapping pci > resources there. if (pcmc->has_reserved_memory && machine->device_memory->base) { [...] if (pcms->cxl_devices_state.is_enabled) { res_mem_end = cxl_resv_end; that should be handled by this line } *val = cpu_to_le64(ROUND_UP(res_mem_end, 1 * GiB)); fw_cfg_add_file(fw_cfg, "etc/reserved-memory-end", val, sizeof(*val)); } so SeaBIOS shouldn't intrude into CXL address space (I assume EDK2 behave similarly here) > > so dropping reserved entries looks reasonable from ACPI spec point of view. > > > > Yep, I don't want dispute that. > > I suspect the reason for these entries to exist in the first place is to > inform the firmware that it should not place stuff there, and if we ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ just to educate me, can you point out what SeaBIOS code does with reservations. > remove that to conform with the spec we need some alternative way for > that ... with etc/reserved-memory-end set as above, is E820_RESERVED really needed here? (my understanding was that E820_RESERVED weren't accounted for when initializing PCI devices) > > take care, > Gerd >