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

[Michael S. Tsirkin]

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 <kraxel@redhat.com>
> 
> take care,
>   Gerd

It's upstream already, sorry I can't add your tag.

-- 
MST