Re: [BUG] hw/i386/pc.c: CXL Fixed Memory Window should not reserve e820 in bios

[Gerd Hoffmann]

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.

> 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
remove that to conform with the spec we need some alternative way for
that ...

take care,
  Gerd