[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [BUG] hw/i386/pc.c: CXL Fixed Memory Window should not reserve e820
From: |
Igor Mammedov |
Subject: |
Re: [BUG] hw/i386/pc.c: CXL Fixed Memory Window should not reserve e820 in bios |
Date: |
Fri, 11 Nov 2022 14:24:11 +0100 |
On Fri, 11 Nov 2022 12:40:59 +0100
Gerd Hoffmann <kraxel@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 <kraxel@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
>