* [PATCH/RFC] vl/s390: fixup ram sizes for compat machines
@ 2020-03-31 12:02 Christian Borntraeger
2020-03-31 12:13 ` David Hildenbrand
2020-03-31 12:15 ` Cornelia Huck
0 siblings, 2 replies; 6+ messages in thread
From: Christian Borntraeger @ 2020-03-31 12:02 UTC (permalink / raw)
To: qemu-devel, Cornelia Huck
Cc: Lukáš Doktor, Thomas Huth, Janosch Frank,
David Hildenbrand, Dr . David Alan Gilbert, Halil Pasic,
Christian Borntraeger, qemu-s390x, Igor Mammedov,
Richard Henderson
compat machines did fixup the ram size to match what can be reported via
sclp. We need to mimic those for machines 4.2 and older to not fail on
inbound migration.
Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
Reported-by: Lukáš Doktor <ldoktor@redhat.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
hw/s390x/s390-virtio-ccw.c | 12 ++++++++++++
hw/s390x/sclp.c | 10 ----------
include/hw/boards.h | 1 +
softmmu/vl.c | 3 +++
4 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3cf19c99f3..748c08b157 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -579,6 +579,17 @@ static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
s390_cpu_restart(S390_CPU(cs));
}
+#define MAX_STORAGE_INCREMENTS 1020
+static ram_addr_t s390_align_ram(ram_addr_t sz)
+{
+ /* same logic as in sclp.c */
+ int increment_size = 20;
+ while ((sz >> increment_size) > MAX_STORAGE_INCREMENTS) {
+ increment_size++;
+ }
+ return sz >> increment_size << increment_size;
+}
+
static void ccw_machine_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
@@ -808,6 +819,7 @@ static void ccw_machine_4_2_instance_options(MachineState *machine)
static void ccw_machine_4_2_class_options(MachineClass *mc)
{
ccw_machine_5_0_class_options(mc);
+ mc->machine_align_ram = s390_align_ram;
compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);
}
DEFINE_CCW_MACHINE(4_2, "4.2", false);
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index d8ae207731..0a6ff2be82 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -372,16 +372,6 @@ static void sclp_memory_init(SCLPDevice *sclp)
increment_size++;
}
sclp->increment_size = increment_size;
-
- /* The core memory area needs to be aligned with the increment size.
- * In effect, this can cause the user-specified memory size to be rounded
- * down to align with the nearest increment boundary. */
- 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;
}
static void sclp_init(Object *obj)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 236d239c19..e3574f4b5f 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -218,6 +218,7 @@ struct MachineClass {
unsigned cpu_index);
const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
+ ram_addr_t (*machine_align_ram)(ram_addr_t size);
};
/**
diff --git a/softmmu/vl.c b/softmmu/vl.c
index 1d33a28340..12b5758d12 100644
--- a/softmmu/vl.c
+++ b/softmmu/vl.c
@@ -2601,6 +2601,9 @@ static bool set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
}
sz = QEMU_ALIGN_UP(sz, 8192);
+ if (mc->machine_align_ram) {
+ sz = mc->machine_align_ram(sz);
+ }
ram_size = sz;
if (ram_size != sz) {
error_report("ram size too large");
--
2.25.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] vl/s390: fixup ram sizes for compat machines
2020-03-31 12:02 [PATCH/RFC] vl/s390: fixup ram sizes for compat machines Christian Borntraeger
@ 2020-03-31 12:13 ` David Hildenbrand
2020-03-31 12:16 ` Christian Borntraeger
2020-03-31 12:15 ` Cornelia Huck
1 sibling, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2020-03-31 12:13 UTC (permalink / raw)
To: Christian Borntraeger, qemu-devel, Cornelia Huck
Cc: Lukáš Doktor, Thomas Huth, Janosch Frank,
Dr . David Alan Gilbert, Halil Pasic, qemu-s390x, Igor Mammedov,
Richard Henderson
On 31.03.20 14:02, Christian Borntraeger wrote:
> compat machines did fixup the ram size to match what can be reported via
> sclp. We need to mimic those for machines 4.2 and older to not fail on
> inbound migration.
>
> Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
> Reported-by: Lukáš Doktor <ldoktor@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> hw/s390x/s390-virtio-ccw.c | 12 ++++++++++++
> hw/s390x/sclp.c | 10 ----------
> include/hw/boards.h | 1 +
> softmmu/vl.c | 3 +++
> 4 files changed, 16 insertions(+), 10 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 3cf19c99f3..748c08b157 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -579,6 +579,17 @@ static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
> s390_cpu_restart(S390_CPU(cs));
> }
>
> +#define MAX_STORAGE_INCREMENTS 1020
> +static ram_addr_t s390_align_ram(ram_addr_t sz)
> +{
> + /* same logic as in sclp.c */
> + int increment_size = 20;
> + while ((sz >> increment_size) > MAX_STORAGE_INCREMENTS) {
> + increment_size++;
> + }
> + return sz >> increment_size << increment_size;
> +}
> +
> static void ccw_machine_class_init(ObjectClass *oc, void *data)
> {
> MachineClass *mc = MACHINE_CLASS(oc);
> @@ -808,6 +819,7 @@ static void ccw_machine_4_2_instance_options(MachineState *machine)
> static void ccw_machine_4_2_class_options(MachineClass *mc)
> {
> ccw_machine_5_0_class_options(mc);
> + mc->machine_align_ram = s390_align_ram;
> compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);
> }
> DEFINE_CCW_MACHINE(4_2, "4.2", false);
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index d8ae207731..0a6ff2be82 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -372,16 +372,6 @@ static void sclp_memory_init(SCLPDevice *sclp)
> increment_size++;
> }
> sclp->increment_size = increment_size;
> -
> - /* The core memory area needs to be aligned with the increment size.
> - * In effect, this can cause the user-specified memory size to be rounded
> - * down to align with the nearest increment boundary. */
> - 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;
> }
>
> static void sclp_init(Object *obj)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 236d239c19..e3574f4b5f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -218,6 +218,7 @@ struct MachineClass {
> unsigned cpu_index);
> const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
> + ram_addr_t (*machine_align_ram)(ram_addr_t size);
> };
>
> /**
> diff --git a/softmmu/vl.c b/softmmu/vl.c
> index 1d33a28340..12b5758d12 100644
> --- a/softmmu/vl.c
> +++ b/softmmu/vl.c
> @@ -2601,6 +2601,9 @@ static bool set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
> }
>
> sz = QEMU_ALIGN_UP(sz, 8192);
> + if (mc->machine_align_ram) {
> + sz = mc->machine_align_ram(sz);
> + }
> ram_size = sz;
> if (ram_size != sz) {
> error_report("ram size too large");
>
1. You're missing the maxram changes from my patch.
2. Shouldn't we error out in case ram_size is not aligned in sclp.c (new
machines)?
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] vl/s390: fixup ram sizes for compat machines
2020-03-31 12:02 [PATCH/RFC] vl/s390: fixup ram sizes for compat machines Christian Borntraeger
2020-03-31 12:13 ` David Hildenbrand
@ 2020-03-31 12:15 ` Cornelia Huck
1 sibling, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2020-03-31 12:15 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Lukáš Doktor, Thomas Huth, Janosch Frank,
David Hildenbrand, qemu-devel, Dr . David Alan Gilbert,
Halil Pasic, qemu-s390x, Igor Mammedov, Richard Henderson
On Tue, 31 Mar 2020 08:02:38 -0400
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> compat machines did fixup the ram size to match what can be reported via
> sclp. We need to mimic those for machines 4.2 and older to not fail on
> inbound migration.
>
> Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
> Reported-by: Lukáš Doktor <ldoktor@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
> hw/s390x/s390-virtio-ccw.c | 12 ++++++++++++
> hw/s390x/sclp.c | 10 ----------
> include/hw/boards.h | 1 +
> softmmu/vl.c | 3 +++
> 4 files changed, 16 insertions(+), 10 deletions(-)
(...)
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 236d239c19..e3574f4b5f 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -218,6 +218,7 @@ struct MachineClass {
> unsigned cpu_index);
> const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
> int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
> + ram_addr_t (*machine_align_ram)(ram_addr_t size);
Maybe add a comment:
/* For compat machine usage only. Don't use this in new machines. */
?
> };
>
> /**
(...)
(Didn't really read the patch yet.)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] vl/s390: fixup ram sizes for compat machines
2020-03-31 12:13 ` David Hildenbrand
@ 2020-03-31 12:16 ` Christian Borntraeger
2020-03-31 12:19 ` David Hildenbrand
0 siblings, 1 reply; 6+ messages in thread
From: Christian Borntraeger @ 2020-03-31 12:16 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel, Cornelia Huck
Cc: Lukáš Doktor, Thomas Huth, Janosch Frank,
Dr . David Alan Gilbert, Halil Pasic, qemu-s390x, Igor Mammedov,
Richard Henderson
On 31.03.20 14:13, David Hildenbrand wrote:
> On 31.03.20 14:02, Christian Borntraeger wrote:
>> compat machines did fixup the ram size to match what can be reported via
>> sclp. We need to mimic those for machines 4.2 and older to not fail on
>> inbound migration.
>>
>> Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
>> Reported-by: Lukáš Doktor <ldoktor@redhat.com>
>> Cc: Igor Mammedov <imammedo@redhat.com>
>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> ---
>> hw/s390x/s390-virtio-ccw.c | 12 ++++++++++++
>> hw/s390x/sclp.c | 10 ----------
>> include/hw/boards.h | 1 +
>> softmmu/vl.c | 3 +++
>> 4 files changed, 16 insertions(+), 10 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 3cf19c99f3..748c08b157 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -579,6 +579,17 @@ static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
>> s390_cpu_restart(S390_CPU(cs));
>> }
>>
>> +#define MAX_STORAGE_INCREMENTS 1020
>> +static ram_addr_t s390_align_ram(ram_addr_t sz)
>> +{
>> + /* same logic as in sclp.c */
>> + int increment_size = 20;
>> + while ((sz >> increment_size) > MAX_STORAGE_INCREMENTS) {
>> + increment_size++;
>> + }
>> + return sz >> increment_size << increment_size;
>> +}
>> +
>> static void ccw_machine_class_init(ObjectClass *oc, void *data)
>> {
>> MachineClass *mc = MACHINE_CLASS(oc);
>> @@ -808,6 +819,7 @@ static void ccw_machine_4_2_instance_options(MachineState *machine)
>> static void ccw_machine_4_2_class_options(MachineClass *mc)
>> {
>> ccw_machine_5_0_class_options(mc);
>> + mc->machine_align_ram = s390_align_ram;
>> compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);
>> }
>> DEFINE_CCW_MACHINE(4_2, "4.2", false);
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index d8ae207731..0a6ff2be82 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -372,16 +372,6 @@ static void sclp_memory_init(SCLPDevice *sclp)
>> increment_size++;
>> }
>> sclp->increment_size = increment_size;
>> -
>> - /* The core memory area needs to be aligned with the increment size.
>> - * In effect, this can cause the user-specified memory size to be rounded
>> - * down to align with the nearest increment boundary. */
>> - 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;
>> }
>>
>> static void sclp_init(Object *obj)
>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>> index 236d239c19..e3574f4b5f 100644
>> --- a/include/hw/boards.h
>> +++ b/include/hw/boards.h
>> @@ -218,6 +218,7 @@ struct MachineClass {
>> unsigned cpu_index);
>> const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
>> int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
>> + ram_addr_t (*machine_align_ram)(ram_addr_t size);
>> };
>>
>> /**
>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>> index 1d33a28340..12b5758d12 100644
>> --- a/softmmu/vl.c
>> +++ b/softmmu/vl.c
>> @@ -2601,6 +2601,9 @@ static bool set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
>> }
>>
>> sz = QEMU_ALIGN_UP(sz, 8192);
>> + if (mc->machine_align_ram) {
>> + sz = mc->machine_align_ram(sz);
>> + }
>> ram_size = sz;
>> if (ram_size != sz) {
>> error_report("ram size too large");
>>
>
> 1. You're missing the maxram changes from my patch.
Yes, will fixup the remaining things.
>
> 2. Shouldn't we error out in case ram_size is not aligned in sclp.c (new
> machines)?
I think its perfectly fine to have slightly larger ram than what sclp reports. Maybe a future
sclp extension will improve this? (In fact since we no longer have sclp memory hotplug we COULD
use more than 1020 increments)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] vl/s390: fixup ram sizes for compat machines
2020-03-31 12:16 ` Christian Borntraeger
@ 2020-03-31 12:19 ` David Hildenbrand
2020-03-31 12:26 ` Christian Borntraeger
0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2020-03-31 12:19 UTC (permalink / raw)
To: Christian Borntraeger, qemu-devel, Cornelia Huck
Cc: Lukáš Doktor, Thomas Huth, Janosch Frank,
Dr . David Alan Gilbert, Halil Pasic, qemu-s390x, Igor Mammedov,
Richard Henderson
On 31.03.20 14:16, Christian Borntraeger wrote:
>
>
> On 31.03.20 14:13, David Hildenbrand wrote:
>> On 31.03.20 14:02, Christian Borntraeger wrote:
>>> compat machines did fixup the ram size to match what can be reported via
>>> sclp. We need to mimic those for machines 4.2 and older to not fail on
>>> inbound migration.
>>>
>>> Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
>>> Reported-by: Lukáš Doktor <ldoktor@redhat.com>
>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> ---
>>> hw/s390x/s390-virtio-ccw.c | 12 ++++++++++++
>>> hw/s390x/sclp.c | 10 ----------
>>> include/hw/boards.h | 1 +
>>> softmmu/vl.c | 3 +++
>>> 4 files changed, 16 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 3cf19c99f3..748c08b157 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -579,6 +579,17 @@ static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
>>> s390_cpu_restart(S390_CPU(cs));
>>> }
>>>
>>> +#define MAX_STORAGE_INCREMENTS 1020
>>> +static ram_addr_t s390_align_ram(ram_addr_t sz)
>>> +{
>>> + /* same logic as in sclp.c */
>>> + int increment_size = 20;
>>> + while ((sz >> increment_size) > MAX_STORAGE_INCREMENTS) {
>>> + increment_size++;
>>> + }
>>> + return sz >> increment_size << increment_size;
>>> +}
>>> +
>>> static void ccw_machine_class_init(ObjectClass *oc, void *data)
>>> {
>>> MachineClass *mc = MACHINE_CLASS(oc);
>>> @@ -808,6 +819,7 @@ static void ccw_machine_4_2_instance_options(MachineState *machine)
>>> static void ccw_machine_4_2_class_options(MachineClass *mc)
>>> {
>>> ccw_machine_5_0_class_options(mc);
>>> + mc->machine_align_ram = s390_align_ram;
>>> compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);
>>> }
>>> DEFINE_CCW_MACHINE(4_2, "4.2", false);
>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>>> index d8ae207731..0a6ff2be82 100644
>>> --- a/hw/s390x/sclp.c
>>> +++ b/hw/s390x/sclp.c
>>> @@ -372,16 +372,6 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>> increment_size++;
>>> }
>>> sclp->increment_size = increment_size;
>>> -
>>> - /* The core memory area needs to be aligned with the increment size.
>>> - * In effect, this can cause the user-specified memory size to be rounded
>>> - * down to align with the nearest increment boundary. */
>>> - 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;
>>> }
>>>
>>> static void sclp_init(Object *obj)
>>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>>> index 236d239c19..e3574f4b5f 100644
>>> --- a/include/hw/boards.h
>>> +++ b/include/hw/boards.h
>>> @@ -218,6 +218,7 @@ struct MachineClass {
>>> unsigned cpu_index);
>>> const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
>>> int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
>>> + ram_addr_t (*machine_align_ram)(ram_addr_t size);
>>> };
>>>
>>> /**
>>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>>> index 1d33a28340..12b5758d12 100644
>>> --- a/softmmu/vl.c
>>> +++ b/softmmu/vl.c
>>> @@ -2601,6 +2601,9 @@ static bool set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
>>> }
>>>
>>> sz = QEMU_ALIGN_UP(sz, 8192);
>>> + if (mc->machine_align_ram) {
>>> + sz = mc->machine_align_ram(sz);
>>> + }
>>> ram_size = sz;
>>> if (ram_size != sz) {
>>> error_report("ram size too large");
>>>
>>
>> 1. You're missing the maxram changes from my patch.
>
> Yes, will fixup the remaining things.
>>
>> 2. Shouldn't we error out in case ram_size is not aligned in sclp.c (new
>> machines)?
>
> I think its perfectly fine to have slightly larger ram than what sclp reports. Maybe a future
> sclp extension will improve this? (In fact since we no longer have sclp memory hotplug we COULD
> use more than 1020 increments)
I'd say if the guest cannot detect it *right now*, we should error out
(because that's not what the user asked for).
Anyhow, whatever we do, we should add it to the changelog.
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH/RFC] vl/s390: fixup ram sizes for compat machines
2020-03-31 12:19 ` David Hildenbrand
@ 2020-03-31 12:26 ` Christian Borntraeger
0 siblings, 0 replies; 6+ messages in thread
From: Christian Borntraeger @ 2020-03-31 12:26 UTC (permalink / raw)
To: David Hildenbrand, qemu-devel, Cornelia Huck
Cc: Lukáš Doktor, Thomas Huth, Janosch Frank,
Dr . David Alan Gilbert, Halil Pasic, qemu-s390x, Igor Mammedov,
Richard Henderson
On 31.03.20 14:19, David Hildenbrand wrote:
> On 31.03.20 14:16, Christian Borntraeger wrote:
>>
>>
>> On 31.03.20 14:13, David Hildenbrand wrote:
>>> On 31.03.20 14:02, Christian Borntraeger wrote:
>>>> compat machines did fixup the ram size to match what can be reported via
>>>> sclp. We need to mimic those for machines 4.2 and older to not fail on
>>>> inbound migration.
>>>>
>>>> Fixes: 3a12fc61af5c ("390x/s390-virtio-ccw: use memdev for RAM")
>>>> Reported-by: Lukáš Doktor <ldoktor@redhat.com>
>>>> Cc: Igor Mammedov <imammedo@redhat.com>
>>>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>>> Signed-off-by: David Hildenbrand <david@redhat.com>
>>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>>> ---
>>>> hw/s390x/s390-virtio-ccw.c | 12 ++++++++++++
>>>> hw/s390x/sclp.c | 10 ----------
>>>> include/hw/boards.h | 1 +
>>>> softmmu/vl.c | 3 +++
>>>> 4 files changed, 16 insertions(+), 10 deletions(-)
>>>>
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index 3cf19c99f3..748c08b157 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -579,6 +579,17 @@ static void s390_nmi(NMIState *n, int cpu_index, Error **errp)
>>>> s390_cpu_restart(S390_CPU(cs));
>>>> }
>>>>
>>>> +#define MAX_STORAGE_INCREMENTS 1020
>>>> +static ram_addr_t s390_align_ram(ram_addr_t sz)
>>>> +{
>>>> + /* same logic as in sclp.c */
>>>> + int increment_size = 20;
>>>> + while ((sz >> increment_size) > MAX_STORAGE_INCREMENTS) {
>>>> + increment_size++;
>>>> + }
>>>> + return sz >> increment_size << increment_size;
>>>> +}
>>>> +
>>>> static void ccw_machine_class_init(ObjectClass *oc, void *data)
>>>> {
>>>> MachineClass *mc = MACHINE_CLASS(oc);
>>>> @@ -808,6 +819,7 @@ static void ccw_machine_4_2_instance_options(MachineState *machine)
>>>> static void ccw_machine_4_2_class_options(MachineClass *mc)
>>>> {
>>>> ccw_machine_5_0_class_options(mc);
>>>> + mc->machine_align_ram = s390_align_ram;
>>>> compat_props_add(mc->compat_props, hw_compat_4_2, hw_compat_4_2_len);
>>>> }
>>>> DEFINE_CCW_MACHINE(4_2, "4.2", false);
>>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>>>> index d8ae207731..0a6ff2be82 100644
>>>> --- a/hw/s390x/sclp.c
>>>> +++ b/hw/s390x/sclp.c
>>>> @@ -372,16 +372,6 @@ static void sclp_memory_init(SCLPDevice *sclp)
>>>> increment_size++;
>>>> }
>>>> sclp->increment_size = increment_size;
>>>> -
>>>> - /* The core memory area needs to be aligned with the increment size.
>>>> - * In effect, this can cause the user-specified memory size to be rounded
>>>> - * down to align with the nearest increment boundary. */
>>>> - 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;
>>>> }
>>>>
>>>> static void sclp_init(Object *obj)
>>>> diff --git a/include/hw/boards.h b/include/hw/boards.h
>>>> index 236d239c19..e3574f4b5f 100644
>>>> --- a/include/hw/boards.h
>>>> +++ b/include/hw/boards.h
>>>> @@ -218,6 +218,7 @@ struct MachineClass {
>>>> unsigned cpu_index);
>>>> const CPUArchIdList *(*possible_cpu_arch_ids)(MachineState *machine);
>>>> int64_t (*get_default_cpu_node_id)(const MachineState *ms, int idx);
>>>> + ram_addr_t (*machine_align_ram)(ram_addr_t size);
>>>> };
>>>>
>>>> /**
>>>> diff --git a/softmmu/vl.c b/softmmu/vl.c
>>>> index 1d33a28340..12b5758d12 100644
>>>> --- a/softmmu/vl.c
>>>> +++ b/softmmu/vl.c
>>>> @@ -2601,6 +2601,9 @@ static bool set_memory_options(uint64_t *ram_slots, ram_addr_t *maxram_size,
>>>> }
>>>>
>>>> sz = QEMU_ALIGN_UP(sz, 8192);
>>>> + if (mc->machine_align_ram) {
>>>> + sz = mc->machine_align_ram(sz);
>>>> + }
>>>> ram_size = sz;
>>>> if (ram_size != sz) {
>>>> error_report("ram size too large");
>>>>
>>>
>>> 1. You're missing the maxram changes from my patch.
>>
>> Yes, will fixup the remaining things.
>>>
>>> 2. Shouldn't we error out in case ram_size is not aligned in sclp.c (new
>>> machines)?
>>
>> I think its perfectly fine to have slightly larger ram than what sclp reports. Maybe a future
>> sclp extension will improve this? (In fact since we no longer have sclp memory hotplug we COULD
>> use more than 1020 increments)
>
> I'd say if the guest cannot detect it *right now*, we should error out
> (because that's not what the user asked for).
I am not a big fan for erroring out, but if this is only happending for 5.0 and later
this might be ok (as the xml will keep the version).
As an alternative I couldd certainly extend this rounding to ALL machines (including 5.0).
Regarding guest detection. The guest can detect the last memory (by using tprot for example)
just not via todays sclp. Let me look into sclp, I think without sclp memory hotplug we can
have 64k increments (we did have that before we had memory hotplug IIRC).
>
> Anyhow, whatever we do, we should add it to the changelog.
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-03-31 12:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-31 12:02 [PATCH/RFC] vl/s390: fixup ram sizes for compat machines Christian Borntraeger
2020-03-31 12:13 ` David Hildenbrand
2020-03-31 12:16 ` Christian Borntraeger
2020-03-31 12:19 ` David Hildenbrand
2020-03-31 12:26 ` Christian Borntraeger
2020-03-31 12:15 ` 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.