On Fri, Nov 11, 2022 at 02:36:02PM +0100, Gerd Hoffmann wrote:
> > 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
>
> Yes, looks good, so with this in place already everyting should be fine.
>
> > (I assume EDK2 behave similarly here)
>
> Correct, ovmf reads that fw_cfg file too.
>
> > > 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.
>
> They are added to the e820 map which gets passed on to the OS. seabios
> uses (and updateas) the e820 map too, when allocating memory for
> example. While thinking about it I'm not fully sure it actually looks
> at reservations, maybe it only uses (and updates) ram entries when
> allocating memory.
>
> > > 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?
>
> No. Setting etc/reserved-memory-end is enough.
>
> So for the original patch:
> Acked-by: Gerd Hoffmann <kra...@redhat.com>
>
> take care,
> Gerd
It's upstream already, sorry I can't add your tag.
--
MST
