Re: [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range

[Jonathan Cameron]

On Thu, 12 Oct 2023 10:10:36 +0530
Ani Sinha <anisinha@redhat.com> wrote:

> > On 11-Oct-2023, at 10:01 PM, Jonathan Cameron <Jonathan.Cameron@huawei.com> 
> > wrote:
> > 
> > On Wed, 11 Oct 2023 16:23:35 +0530
> > Ani Sinha <anisinha@redhat.com> wrote:
> >   
> >> pc_get_device_memory_range() finds the device memory size by calculating 
> >> the
> >> difference between maxram and ram sizes. This calculation makes sense only 
> >> when
> >> maxram is greater than the ram size. Make sure we check for that before 
> >> calling
> >> pc_get_device_memory_range().
> >> 
> >> Signed-off-by: Ani Sinha <anisinha@redhat.com>  
> > 
> > Whilst this is similar to other cases, I can't remember or quickly work
> > out if the 'else' path here is appropriate.  Can we add something to the
> > patch description to talk about that?
> > 
> > For reference it's:
> > 
> > cxl_base = pc_above_4g_end(pcms);  
> 
> Leaving the alignment adjustments aside, the hotplugged memory devices are 
> mapped starting from where the “above_4g” memory ends.
> The cxl memory starts after the region reserved for hot plugged memory 
> devices. If there is no hot plugged memory device region (maxmem == mem), the 
> cxl memory can start right after where “above_4g” memory ends.
> See also pc_pci_hole64_start() and pc_max_used_gpa().
> I did not want to add any alignment adjustments because I was not sure if it 
> would add regression and incompatibility with older machine types. 
> 

Thanks for explanation.

Acked-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> >   
> >> ---
> >> hw/i386/pc.c | 4 +++-
> >> 1 file changed, 3 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/hw/i386/pc.c b/hw/i386/pc.c
> >> index f72e2c3b35..948c58171c 100644
> >> --- a/hw/i386/pc.c
> >> +++ b/hw/i386/pc.c
> >> @@ -820,10 +820,12 @@ static void 
> >> pc_get_device_memory_range(PCMachineState *pcms,
> >> static uint64_t pc_get_cxl_range_start(PCMachineState *pcms)
> >> {
> >>     PCMachineClass *pcmc = PC_MACHINE_GET_CLASS(pcms);
> >> +    MachineState *ms = MACHINE(pcms);
> >>     hwaddr cxl_base;
> >>     ram_addr_t size;
> >> 
> >> -    if (pcmc->has_reserved_memory) {
> >> +    if (pcmc->has_reserved_memory &&
> >> +        (ms->ram_size < ms->maxram_size)) {
> >>         pc_get_device_memory_range(pcms, &cxl_base, &size);
> >>         cxl_base += size;
> >>     } else {  
> >   
>