| Message ID | 20231011105335.42296-1-anisinha@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range | expand |
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); > --- > 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 {
> 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. > >> --- >> 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 { >
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 { > > >
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 {
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> --- hw/i386/pc.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)