qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range
@ 2023-10-11 10:53 Ani Sinha
  2023-10-11 16:31 ` Jonathan Cameron via
  0 siblings, 1 reply; 6+ messages in thread
From: Ani Sinha @ 2023-10-11 10:53 UTC (permalink / raw)
  To: Paolo Bonzini, Richard Henderson, Eduardo Habkost,
	Michael S. Tsirkin, Marcel Apfelbaum
  Cc: Ani Sinha, qemu-devel

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(-)

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 {
-- 
2.42.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range
  2023-10-11 10:53 [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range Ani Sinha
@ 2023-10-11 16:31 ` Jonathan Cameron via
  2023-10-11 16:31   ` Jonathan Cameron
  2023-10-12  4:40   ` Ani Sinha
  0 siblings, 2 replies; 6+ messages in thread
From: Jonathan Cameron via @ 2023-10-11 16:31 UTC (permalink / raw)
  To: Ani Sinha
  Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost,
	Michael S. Tsirkin, Marcel Apfelbaum, qemu-devel

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 {



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range
  2023-10-11 16:31 ` Jonathan Cameron via
@ 2023-10-11 16:31   ` Jonathan Cameron
  2023-10-12  4:40   ` Ani Sinha
  1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2023-10-11 16:31 UTC (permalink / raw)
  To: Ani Sinha
  Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost,
	Michael S. Tsirkin, Marcel Apfelbaum, qemu-devel

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 {



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range
  2023-10-11 16:31 ` Jonathan Cameron via
  2023-10-11 16:31   ` Jonathan Cameron
@ 2023-10-12  4:40   ` Ani Sinha
  2023-10-12  9:09     ` Jonathan Cameron via
  1 sibling, 1 reply; 6+ messages in thread
From: Ani Sinha @ 2023-10-12  4:40 UTC (permalink / raw)
  To: Jonathan Cameron
  Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost,
	Michael S. Tsirkin, Marcel Apfelbaum, qemu-devel



> 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 {
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range
  2023-10-12  4:40   ` Ani Sinha
@ 2023-10-12  9:09     ` Jonathan Cameron via
  2023-10-12  9:09       ` Jonathan Cameron
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Cameron via @ 2023-10-12  9:09 UTC (permalink / raw)
  To: Ani Sinha
  Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost,
	Michael S. Tsirkin, Marcel Apfelbaum, qemu-devel

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 {  
> >   
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range
  2023-10-12  9:09     ` Jonathan Cameron via
@ 2023-10-12  9:09       ` Jonathan Cameron
  0 siblings, 0 replies; 6+ messages in thread
From: Jonathan Cameron @ 2023-10-12  9:09 UTC (permalink / raw)
  To: Ani Sinha
  Cc: Paolo Bonzini, Richard Henderson, Eduardo Habkost,
	Michael S. Tsirkin, Marcel Apfelbaum, qemu-devel

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 {  
> >   
> 



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-10-12  9:10 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-11 10:53 [PATCH] hw/i386/cxl: ensure maxram is greater than ram size for calculating cxl range Ani Sinha
2023-10-11 16:31 ` Jonathan Cameron via
2023-10-11 16:31   ` Jonathan Cameron
2023-10-12  4:40   ` Ani Sinha
2023-10-12  9:09     ` Jonathan Cameron via
2023-10-12  9:09       ` Jonathan Cameron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).