All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
@ 2018-07-30 14:09 Christian Borntraeger
  2018-07-30 14:34 ` David Hildenbrand
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Christian Borntraeger @ 2018-07-30 14:09 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: qemu-devel, qemu-s390x, Alexander Graf, Thomas Huth,
	David Hildenbrand, Richard Henderson, Janosch Frank, Halil Pasic,
	imbrenda, Christian Borntraeger

We clamp down ram_size to match the sclp increment size. We do
not do the same for maxram_size, which means for large guests
with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
This can break other code (e.g. CMMA migration) which uses maxram_size
to calculate the number of pages and then throws some errors.

Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
CC: qemu-stable@nongnu.org
CC: David Hildenbrand <david@redhat.com>
---
 hw/s390x/sclp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index bd2a024..4510a80 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
     initial_mem = initial_mem >> increment_size << increment_size;
 
     machine->ram_size = initial_mem;
+    machine->maxram_size = initial_mem;
     /* let's propagate the changed ram size into the global variable. */
     ram_size = initial_mem;
 }
-- 
1.8.3.1

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

* Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 14:09 [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation Christian Borntraeger
@ 2018-07-30 14:34 ` David Hildenbrand
  2018-07-30 15:00   ` [Qemu-devel] [qemu-s390x] " Christian Borntraeger
  2018-07-30 15:31 ` [Qemu-devel] " Christian Borntraeger
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 15+ messages in thread
From: David Hildenbrand @ 2018-07-30 14:34 UTC (permalink / raw)
  To: Christian Borntraeger, Cornelia Huck
  Cc: qemu-devel, qemu-s390x, Alexander Graf, Thomas Huth,
	Richard Henderson, Janosch Frank, Halil Pasic, imbrenda

On 30.07.2018 16:09, Christian Borntraeger wrote:
> We clamp down ram_size to match the sclp increment size. We do
> not do the same for maxram_size, which means for large guests
> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
> This can break other code (e.g. CMMA migration) which uses maxram_size
> to calculate the number of pages and then throws some errors.

So the only problem is that the buffer size between source and target
differ?

> 
> Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> CC: qemu-stable@nongnu.org
> CC: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/sclp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index bd2a024..4510a80 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
>      initial_mem = initial_mem >> increment_size << increment_size;
>  
>      machine->ram_size = initial_mem;
> +    machine->maxram_size = initial_mem;
>      /* let's propagate the changed ram size into the global variable. */
>      ram_size = initial_mem;
>  }
> 

I even have a private patch for that already :)

-- 

Thanks,

David / dhildenb

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 14:34 ` David Hildenbrand
@ 2018-07-30 15:00   ` Christian Borntraeger
  2018-07-30 15:17     ` David Hildenbrand
  0 siblings, 1 reply; 15+ messages in thread
From: Christian Borntraeger @ 2018-07-30 15:00 UTC (permalink / raw)
  To: David Hildenbrand, Cornelia Huck
  Cc: Janosch Frank, Thomas Huth, qemu-devel, Alexander Graf,
	qemu-s390x, Halil Pasic, imbrenda, Richard Henderson



On 07/30/2018 04:34 PM, David Hildenbrand wrote:
> On 30.07.2018 16:09, Christian Borntraeger wrote:
>> We clamp down ram_size to match the sclp increment size. We do
>> not do the same for maxram_size, which means for large guests
>> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
>> This can break other code (e.g. CMMA migration) which uses maxram_size
>> to calculate the number of pages and then throws some errors.
> 
> So the only problem is that the buffer size between source and target
> differ?

The problem is that the target tries to access a non-existing buffer when 
committing all cmma value, so the kernel returns with EFAULT.
> 
>>
>> Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> CC: qemu-stable@nongnu.org
>> CC: David Hildenbrand <david@redhat.com>
>> ---
>>  hw/s390x/sclp.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index bd2a024..4510a80 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>      initial_mem = initial_mem >> increment_size << increment_size;
>>  
>>      machine->ram_size = initial_mem;
>> +    machine->maxram_size = initial_mem;
>>      /* let's propagate the changed ram size into the global variable. */
>>      ram_size = initial_mem;
>>  }
>>
> 
> I even have a private patch for that already :)

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 15:00   ` [Qemu-devel] [qemu-s390x] " Christian Borntraeger
@ 2018-07-30 15:17     ` David Hildenbrand
  2018-07-30 15:20       ` Christian Borntraeger
  0 siblings, 1 reply; 15+ messages in thread
From: David Hildenbrand @ 2018-07-30 15:17 UTC (permalink / raw)
  To: Christian Borntraeger, Cornelia Huck
  Cc: Janosch Frank, Thomas Huth, qemu-devel, Alexander Graf,
	qemu-s390x, Halil Pasic, imbrenda, Richard Henderson

On 30.07.2018 17:00, Christian Borntraeger wrote:
> 
> 
> On 07/30/2018 04:34 PM, David Hildenbrand wrote:
>> On 30.07.2018 16:09, Christian Borntraeger wrote:
>>> We clamp down ram_size to match the sclp increment size. We do
>>> not do the same for maxram_size, which means for large guests
>>> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
>>> This can break other code (e.g. CMMA migration) which uses maxram_size
>>> to calculate the number of pages and then throws some errors.
>>
>> So the only problem is that the buffer size between source and target
>> differ?
> 
> The problem is that the target tries to access a non-existing buffer when 
> committing all cmma value, so the kernel returns with EFAULT.
>>

Am I wrong or does CMMA migration code really not care about which parts
of maxram are actually used (== which memory regions are actually defined)?

If so, this looks broken to me and the right fix is to use ramsize for
now, because it simply does not support maxram.

(I assume using some -m X,maxmem=X+Y would make it fail in the same way)

(this patch still makes sense and should be done)


-- 

Thanks,

David / dhildenb

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 15:17     ` David Hildenbrand
@ 2018-07-30 15:20       ` Christian Borntraeger
  2018-07-30 15:28         ` David Hildenbrand
  2018-07-30 15:32         ` Cornelia Huck
  0 siblings, 2 replies; 15+ messages in thread
From: Christian Borntraeger @ 2018-07-30 15:20 UTC (permalink / raw)
  To: David Hildenbrand, Cornelia Huck
  Cc: Janosch Frank, Thomas Huth, qemu-devel, Alexander Graf,
	qemu-s390x, Halil Pasic, imbrenda, Richard Henderson



On 07/30/2018 05:17 PM, David Hildenbrand wrote:
> On 30.07.2018 17:00, Christian Borntraeger wrote:
>>
>>
>> On 07/30/2018 04:34 PM, David Hildenbrand wrote:
>>> On 30.07.2018 16:09, Christian Borntraeger wrote:
>>>> We clamp down ram_size to match the sclp increment size. We do
>>>> not do the same for maxram_size, which means for large guests
>>>> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
>>>> This can break other code (e.g. CMMA migration) which uses maxram_size
>>>> to calculate the number of pages and then throws some errors.
>>>
>>> So the only problem is that the buffer size between source and target
>>> differ?
>>
>> The problem is that the target tries to access a non-existing buffer when 
>> committing all cmma value, so the kernel returns with EFAULT.
>>>
> 
> Am I wrong or does CMMA migration code really not care about which parts
> of maxram are actually used (== which memory regions are actually defined)?
> 
> If so, this looks broken to me and the right fix is to use ramsize for
> now, because it simply does not support maxram.
> 
> (I assume using some -m X,maxmem=X+Y would make it fail in the same way)
> 
> (this patch still makes sense and should be done)

I am looking for the minimal fix for 2.13 and ideally even for 2.12.1.

Can we agree on this fix and do the remaining thing later?

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 15:20       ` Christian Borntraeger
@ 2018-07-30 15:28         ` David Hildenbrand
  2018-07-30 15:32         ` Cornelia Huck
  1 sibling, 0 replies; 15+ messages in thread
From: David Hildenbrand @ 2018-07-30 15:28 UTC (permalink / raw)
  To: Christian Borntraeger, Cornelia Huck
  Cc: Janosch Frank, Thomas Huth, qemu-devel, Alexander Graf,
	qemu-s390x, Halil Pasic, imbrenda, Richard Henderson

On 30.07.2018 17:20, Christian Borntraeger wrote:
> 
> 
> On 07/30/2018 05:17 PM, David Hildenbrand wrote:
>> On 30.07.2018 17:00, Christian Borntraeger wrote:
>>>
>>>
>>> On 07/30/2018 04:34 PM, David Hildenbrand wrote:
>>>> On 30.07.2018 16:09, Christian Borntraeger wrote:
>>>>> We clamp down ram_size to match the sclp increment size. We do
>>>>> not do the same for maxram_size, which means for large guests
>>>>> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
>>>>> This can break other code (e.g. CMMA migration) which uses maxram_size
>>>>> to calculate the number of pages and then throws some errors.
>>>>
>>>> So the only problem is that the buffer size between source and target
>>>> differ?
>>>
>>> The problem is that the target tries to access a non-existing buffer when 
>>> committing all cmma value, so the kernel returns with EFAULT.
>>>>
>>
>> Am I wrong or does CMMA migration code really not care about which parts
>> of maxram are actually used (== which memory regions are actually defined)?
>>
>> If so, this looks broken to me and the right fix is to use ramsize for
>> now, because it simply does not support maxram.
>>
>> (I assume using some -m X,maxmem=X+Y would make it fail in the same way)
>>
>> (this patch still makes sense and should be done)
> 
> I am looking for the minimal fix for 2.13 and ideally even for 2.12.1.
> 
> Can we agree on this fix and do the remaining thing later?
> 

Yes. The clean fix should then really only consider mapped memory
regions (so the sending side should somehow iterate over them and also
only access that memory).

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David / dhildenb

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

* Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 14:09 [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation Christian Borntraeger
  2018-07-30 14:34 ` David Hildenbrand
@ 2018-07-30 15:31 ` Christian Borntraeger
  2018-07-30 16:58   ` Michael Roth
  2018-07-30 15:43 ` David Hildenbrand
  2018-07-30 15:55 ` [Qemu-devel] " Cornelia Huck
  3 siblings, 1 reply; 15+ messages in thread
From: Christian Borntraeger @ 2018-07-30 15:31 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: qemu-devel, qemu-s390x, Alexander Graf, Thomas Huth,
	David Hildenbrand, Richard Henderson, Janosch Frank, Halil Pasic,
	imbrenda, qemu-stable, Michael Roth

Are we still able to get things into 2.12.1 or are we too late?


On 07/30/2018 04:09 PM, Christian Borntraeger wrote:
> We clamp down ram_size to match the sclp increment size. We do
> not do the same for maxram_size, which means for large guests
> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
> This can break other code (e.g. CMMA migration) which uses maxram_size
> to calculate the number of pages and then throws some errors.
> 
> Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> CC: qemu-stable@nongnu.org
> CC: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/sclp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index bd2a024..4510a80 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
>      initial_mem = initial_mem >> increment_size << increment_size;
>  
>      machine->ram_size = initial_mem;
> +    machine->maxram_size = initial_mem;
>      /* let's propagate the changed ram size into the global variable. */
>      ram_size = initial_mem;
>  }
> 

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 15:20       ` Christian Borntraeger
  2018-07-30 15:28         ` David Hildenbrand
@ 2018-07-30 15:32         ` Cornelia Huck
  1 sibling, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2018-07-30 15:32 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: David Hildenbrand, Janosch Frank, Thomas Huth, qemu-devel,
	Alexander Graf, qemu-s390x, Halil Pasic, imbrenda,
	Richard Henderson

On Mon, 30 Jul 2018 17:20:25 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 07/30/2018 05:17 PM, David Hildenbrand wrote:
> > On 30.07.2018 17:00, Christian Borntraeger wrote:  
> >>
> >>
> >> On 07/30/2018 04:34 PM, David Hildenbrand wrote:  
> >>> On 30.07.2018 16:09, Christian Borntraeger wrote:  
> >>>> We clamp down ram_size to match the sclp increment size. We do
> >>>> not do the same for maxram_size, which means for large guests
> >>>> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
> >>>> This can break other code (e.g. CMMA migration) which uses maxram_size
> >>>> to calculate the number of pages and then throws some errors.  
> >>>
> >>> So the only problem is that the buffer size between source and target
> >>> differ?  
> >>
> >> The problem is that the target tries to access a non-existing buffer when 
> >> committing all cmma value, so the kernel returns with EFAULT.  
> >>>  
> > 
> > Am I wrong or does CMMA migration code really not care about which parts
> > of maxram are actually used (== which memory regions are actually defined)?
> > 
> > If so, this looks broken to me and the right fix is to use ramsize for
> > now, because it simply does not support maxram.
> > 
> > (I assume using some -m X,maxmem=X+Y would make it fail in the same way)
> > 
> > (this patch still makes sense and should be done)  
> 
> I am looking for the minimal fix for 2.13 and ideally even for 2.12.1.

2.13 is not the QEMU version you are looking for :)

> Can we agree on this fix and do the remaining thing later?

If you guys agree, I'm happy to queue it to s390-fixes for 3.0-rc3.
Just be quick, as I'd need to send it tomorrow morning the latest :)

(FWIW, patch looks sane and small enough to me.)

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

* Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 14:09 [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation Christian Borntraeger
  2018-07-30 14:34 ` David Hildenbrand
  2018-07-30 15:31 ` [Qemu-devel] " Christian Borntraeger
@ 2018-07-30 15:43 ` David Hildenbrand
  2018-07-30 15:47   ` Cornelia Huck
  2018-07-30 15:55 ` [Qemu-devel] " Cornelia Huck
  3 siblings, 1 reply; 15+ messages in thread
From: David Hildenbrand @ 2018-07-30 15:43 UTC (permalink / raw)
  To: Christian Borntraeger, Cornelia Huck
  Cc: qemu-devel, qemu-s390x, Alexander Graf, Thomas Huth,
	Richard Henderson, Janosch Frank, Halil Pasic, imbrenda

On 30.07.2018 16:09, Christian Borntraeger wrote:
> We clamp down ram_size to match the sclp increment size. We do
> not do the same for maxram_size, which means for large guests
> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
> This can break other code (e.g. CMMA migration) which uses maxram_size
> to calculate the number of pages and then throws some errors.
> 
> Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> CC: qemu-stable@nongnu.org
> CC: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/sclp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index bd2a024..4510a80 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
>      initial_mem = initial_mem >> increment_size << increment_size;
>  
>      machine->ram_size = initial_mem;
> +    machine->maxram_size = initial_mem;
>      /* let's propagate the changed ram size into the global variable. */
>      ram_size = initial_mem;
>  }
> 

BTW, I handle it in may private patch like this

 static inline SCLPDevice *get_sclp_device(void)
 {
@@ -319,9 +321,12 @@ static void sclp_memory_init(SCLPDevice *sclp)
      * down to align with the nearest increment boundary. */
     initial_mem = initial_mem >> increment_size << increment_size;

-    machine->ram_size = initial_mem;
-    /* let's propagate the changed ram size into the global variable. */
-    ram_size = initial_mem;
+    /* propagate the changed ram size into the different places */
+    if (initial_mem != machine->ram_size) {
+        machine->maxram_size -= machine->ram_size - initial_mem;
+        machine->ram_size = initial_mem;
+        ram_size = initial_mem;
+    }
 }

You would right now overwrite any maxmem setting (which might be ok as
we don't support it yet).

-- 

Thanks,

David / dhildenb

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

* Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 15:43 ` David Hildenbrand
@ 2018-07-30 15:47   ` Cornelia Huck
  2018-07-31  8:34     ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
  0 siblings, 1 reply; 15+ messages in thread
From: Cornelia Huck @ 2018-07-30 15:47 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Christian Borntraeger, qemu-devel, qemu-s390x, Alexander Graf,
	Thomas Huth, Richard Henderson, Janosch Frank, Halil Pasic,
	imbrenda

On Mon, 30 Jul 2018 17:43:42 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 30.07.2018 16:09, Christian Borntraeger wrote:
> > We clamp down ram_size to match the sclp increment size. We do
> > not do the same for maxram_size, which means for large guests
> > with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
> > This can break other code (e.g. CMMA migration) which uses maxram_size
> > to calculate the number of pages and then throws some errors.
> > 
> > Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > CC: qemu-stable@nongnu.org
> > CC: David Hildenbrand <david@redhat.com>
> > ---
> >  hw/s390x/sclp.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index bd2a024..4510a80 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
> >      initial_mem = initial_mem >> increment_size << increment_size;
> >  
> >      machine->ram_size = initial_mem;
> > +    machine->maxram_size = initial_mem;
> >      /* let's propagate the changed ram size into the global variable. */
> >      ram_size = initial_mem;
> >  }
> >   
> 
> BTW, I handle it in may private patch like this
> 
>  static inline SCLPDevice *get_sclp_device(void)
>  {
> @@ -319,9 +321,12 @@ static void sclp_memory_init(SCLPDevice *sclp)
>       * down to align with the nearest increment boundary. */
>      initial_mem = initial_mem >> increment_size << increment_size;
> 
> -    machine->ram_size = initial_mem;
> -    /* let's propagate the changed ram size into the global variable. */
> -    ram_size = initial_mem;
> +    /* propagate the changed ram size into the different places */
> +    if (initial_mem != machine->ram_size) {
> +        machine->maxram_size -= machine->ram_size - initial_mem;
> +        machine->ram_size = initial_mem;
> +        ram_size = initial_mem;
> +    }
>  }
> 
> You would right now overwrite any maxmem setting (which might be ok as
> we don't support it yet).
> 

So, will you (for whatever value of 'you') submit more patches for 3.1?

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

* Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 14:09 [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation Christian Borntraeger
                   ` (2 preceding siblings ...)
  2018-07-30 15:43 ` David Hildenbrand
@ 2018-07-30 15:55 ` Cornelia Huck
  3 siblings, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2018-07-30 15:55 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: qemu-devel, qemu-s390x, Alexander Graf, Thomas Huth,
	David Hildenbrand, Richard Henderson, Janosch Frank, Halil Pasic,
	imbrenda

On Mon, 30 Jul 2018 16:09:26 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> We clamp down ram_size to match the sclp increment size. We do
> not do the same for maxram_size, which means for large guests
> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
> This can break other code (e.g. CMMA migration) which uses maxram_size
> to calculate the number of pages and then throws some errors.
> 
> Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> CC: qemu-stable@nongnu.org
> CC: David Hildenbrand <david@redhat.com>
> ---
>  hw/s390x/sclp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index bd2a024..4510a80 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
>      initial_mem = initial_mem >> increment_size << increment_size;
>  
>      machine->ram_size = initial_mem;
> +    machine->maxram_size = initial_mem;
>      /* let's propagate the changed ram size into the global variable. */
>      ram_size = initial_mem;
>  }

Thanks, queued to s390-fixes.

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

* Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 15:31 ` [Qemu-devel] " Christian Borntraeger
@ 2018-07-30 16:58   ` Michael Roth
  2018-07-31  6:52     ` Cornelia Huck
  0 siblings, 1 reply; 15+ messages in thread
From: Michael Roth @ 2018-07-30 16:58 UTC (permalink / raw)
  To: Christian Borntraeger, Cornelia Huck
  Cc: qemu-devel, qemu-s390x, Alexander Graf, Thomas Huth,
	David Hildenbrand, Richard Henderson, Janosch Frank, Halil Pasic,
	imbrenda, qemu-stable

Quoting Christian Borntraeger (2018-07-30 10:31:12)
> Are we still able to get things into 2.12.1 or are we too late?

Freeze is EOD today, but I can grab them if they hit master/rc3 tomorrow.

> 
> 
> On 07/30/2018 04:09 PM, Christian Borntraeger wrote:
> > We clamp down ram_size to match the sclp increment size. We do
> > not do the same for maxram_size, which means for large guests
> > with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
> > This can break other code (e.g. CMMA migration) which uses maxram_size
> > to calculate the number of pages and then throws some errors.
> > 
> > Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
> > Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > CC: qemu-stable@nongnu.org
> > CC: David Hildenbrand <david@redhat.com>
> > ---
> >  hw/s390x/sclp.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index bd2a024..4510a80 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
> >      initial_mem = initial_mem >> increment_size << increment_size;
> >  
> >      machine->ram_size = initial_mem;
> > +    machine->maxram_size = initial_mem;
> >      /* let's propagate the changed ram size into the global variable. */
> >      ram_size = initial_mem;
> >  }
> > 
> 

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

* Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 16:58   ` Michael Roth
@ 2018-07-31  6:52     ` Cornelia Huck
  2018-07-31 10:48       ` Cornelia Huck
  0 siblings, 1 reply; 15+ messages in thread
From: Cornelia Huck @ 2018-07-31  6:52 UTC (permalink / raw)
  To: Michael Roth
  Cc: Christian Borntraeger, qemu-devel, qemu-s390x, Alexander Graf,
	Thomas Huth, David Hildenbrand, Richard Henderson, Janosch Frank,
	Halil Pasic, imbrenda, qemu-stable

On Mon, 30 Jul 2018 11:58:13 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:

> Quoting Christian Borntraeger (2018-07-30 10:31:12)
> > Are we still able to get things into 2.12.1 or are we too late?  
> 
> Freeze is EOD today, but I can grab them if they hit master/rc3 tomorrow.

OK, I just sent a pull request with this patch.

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

* Re: [Qemu-devel] [qemu-s390x] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-30 15:47   ` Cornelia Huck
@ 2018-07-31  8:34     ` David Hildenbrand
  0 siblings, 0 replies; 15+ messages in thread
From: David Hildenbrand @ 2018-07-31  8:34 UTC (permalink / raw)
  To: Cornelia Huck
  Cc: Janosch Frank, Thomas Huth, qemu-devel, Alexander Graf,
	Christian Borntraeger, qemu-s390x, Halil Pasic, imbrenda,
	Richard Henderson

On 30.07.2018 17:47, Cornelia Huck wrote:
> On Mon, 30 Jul 2018 17:43:42 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 30.07.2018 16:09, Christian Borntraeger wrote:
>>> We clamp down ram_size to match the sclp increment size. We do
>>> not do the same for maxram_size, which means for large guests
>>> with some sizes (e.g. -m 50000) maxram_size differs from ram_size.
>>> This can break other code (e.g. CMMA migration) which uses maxram_size
>>> to calculate the number of pages and then throws some errors.
>>>
>>> Fixes: 82fab5c5b90e468f3e9d54c ("s390x/sclp: remove memory hotplug support")
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> CC: qemu-stable@nongnu.org
>>> CC: David Hildenbrand <david@redhat.com>
>>> ---
>>>  hw/s390x/sclp.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>>> index bd2a024..4510a80 100644
>>> --- a/hw/s390x/sclp.c
>>> +++ b/hw/s390x/sclp.c
>>> @@ -320,6 +320,7 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>>      initial_mem = initial_mem >> increment_size << increment_size;
>>>  
>>>      machine->ram_size = initial_mem;
>>> +    machine->maxram_size = initial_mem;
>>>      /* let's propagate the changed ram size into the global variable. */
>>>      ram_size = initial_mem;
>>>  }
>>>   
>>
>> BTW, I handle it in may private patch like this
>>
>>  static inline SCLPDevice *get_sclp_device(void)
>>  {
>> @@ -319,9 +321,12 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>       * down to align with the nearest increment boundary. */
>>      initial_mem = initial_mem >> increment_size << increment_size;
>>
>> -    machine->ram_size = initial_mem;
>> -    /* let's propagate the changed ram size into the global variable. */
>> -    ram_size = initial_mem;
>> +    /* propagate the changed ram size into the different places */
>> +    if (initial_mem != machine->ram_size) {
>> +        machine->maxram_size -= machine->ram_size - initial_mem;
>> +        machine->ram_size = initial_mem;
>> +        ram_size = initial_mem;
>> +    }
>>  }
>>
>> You would right now overwrite any maxmem setting (which might be ok as
>> we don't support it yet).
>>
> 
> So, will you (for whatever value of 'you') submit more patches for 3.1?
> 

Once we have memory device support for s390x that will be needed. But
the person that implemented cmma migration should fix the handling and
only try to migrate memory that is actually there. This was broken
before my patch, just never happened as Linux always onlines all memory
it sees via SCLP (thereby creating the memory regions).

-- 

Thanks,

David / dhildenb

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

* Re: [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation
  2018-07-31  6:52     ` Cornelia Huck
@ 2018-07-31 10:48       ` Cornelia Huck
  0 siblings, 0 replies; 15+ messages in thread
From: Cornelia Huck @ 2018-07-31 10:48 UTC (permalink / raw)
  To: Michael Roth
  Cc: Christian Borntraeger, qemu-devel, qemu-s390x, Alexander Graf,
	Thomas Huth, David Hildenbrand, Richard Henderson, Janosch Frank,
	Halil Pasic, imbrenda, qemu-stable

On Tue, 31 Jul 2018 08:52:05 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Mon, 30 Jul 2018 11:58:13 -0500
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> 
> > Quoting Christian Borntraeger (2018-07-30 10:31:12)  
> > > Are we still able to get things into 2.12.1 or are we too late?    
> > 
> > Freeze is EOD today, but I can grab them if they hit master/rc3 tomorrow.  
> 
> OK, I just sent a pull request with this patch.

...and it is in master now.

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

end of thread, other threads:[~2018-07-31 10:48 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-30 14:09 [Qemu-devel] [PATCH 1/1] s390x/sclp: fix maxram calculation Christian Borntraeger
2018-07-30 14:34 ` David Hildenbrand
2018-07-30 15:00   ` [Qemu-devel] [qemu-s390x] " Christian Borntraeger
2018-07-30 15:17     ` David Hildenbrand
2018-07-30 15:20       ` Christian Borntraeger
2018-07-30 15:28         ` David Hildenbrand
2018-07-30 15:32         ` Cornelia Huck
2018-07-30 15:31 ` [Qemu-devel] " Christian Borntraeger
2018-07-30 16:58   ` Michael Roth
2018-07-31  6:52     ` Cornelia Huck
2018-07-31 10:48       ` Cornelia Huck
2018-07-30 15:43 ` David Hildenbrand
2018-07-30 15:47   ` Cornelia Huck
2018-07-31  8:34     ` [Qemu-devel] [qemu-s390x] " David Hildenbrand
2018-07-30 15:55 ` [Qemu-devel] " Cornelia Huck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.