* [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug
2014-05-07 18:05 [Qemu-devel] [PATCH v3 0/4] s390: Support for Hotplug of Standby Memory Matthew Rosato
@ 2014-05-07 18:05 ` Matthew Rosato
2014-05-07 18:50 ` Alexander Graf
2014-05-09 12:35 ` Christian Borntraeger
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 2/4] sclp-s390: Add device to manage s390 " Matthew Rosato
` (3 subsequent siblings)
4 siblings, 2 replies; 18+ messages in thread
From: Matthew Rosato @ 2014-05-07 18:05 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, borntraeger, aliguori, imammedo, cornelia.huck, pbonzini,
rth
From: Igor Mammedov <imammedo@redhat.com>
From: Igor Mammedov <imammedo@redhat.com>
Add following parameters:
"slots" - total number of hotplug memory slots
"maxmem" - maximum possible memory
"slots" and "maxmem" should go in pair and "maxmem" should be greater
than "mem" for memory hotplug to be enabled.
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
include/hw/boards.h | 2 ++
qemu-options.hx | 9 ++++++---
vl.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 59 insertions(+), 3 deletions(-)
diff --git a/include/hw/boards.h b/include/hw/boards.h
index 4345bd0..5b3a903 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -11,6 +11,8 @@
typedef struct QEMUMachineInitArgs {
const MachineClass *machine;
ram_addr_t ram_size;
+ ram_addr_t maxram_size;
+ uint64_t ram_slots;
const char *boot_order;
const char *kernel_filename;
const char *kernel_cmdline;
diff --git a/qemu-options.hx b/qemu-options.hx
index 781af14..c6411c4 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -210,17 +210,20 @@ use is discouraged as it may be removed from future versions.
ETEXI
DEF("m", HAS_ARG, QEMU_OPTION_m,
- "-m [size=]megs\n"
+ "-m[emory] [size=]megs[,slots=n,maxmem=size]\n"
" configure guest RAM\n"
" size: initial amount of guest memory (default: "
- stringify(DEFAULT_RAM_SIZE) "MiB)\n",
+ stringify(DEFAULT_RAM_SIZE) "MiB)\n"
+ " slots: number of hotplug slots (default: none)\n"
+ " maxmem: maximum amount of guest memory (default: none)\n",
QEMU_ARCH_ALL)
STEXI
@item -m [size=]@var{megs}
@findex -m
Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB. Optionally,
a suffix of ``M'' or ``G'' can be used to signify a value in megabytes or
-gigabytes respectively.
+gigabytes respectively. Optional pair @var{slots}, @var{maxmem} could be used
+to set amount of hotluggable memory slots and possible maximum amount of memory.
ETEXI
DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
diff --git a/vl.c b/vl.c
index 73e0661..36ad82c 100644
--- a/vl.c
+++ b/vl.c
@@ -520,6 +520,14 @@ static QemuOptsList qemu_mem_opts = {
.name = "size",
.type = QEMU_OPT_SIZE,
},
+ {
+ .name = "slots",
+ .type = QEMU_OPT_NUMBER,
+ },
+ {
+ .name = "maxmem",
+ .type = QEMU_OPT_SIZE,
+ },
{ /* end of list */ }
},
};
@@ -2988,6 +2996,8 @@ int main(int argc, char **argv, char **envp)
const char *trace_file = NULL;
const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
1024 * 1024;
+ ram_addr_t maxram_size = default_ram_size;
+ uint64_t ram_slots = 0;
atexit(qemu_run_exit_notifiers);
error_set_progname(argv[0]);
@@ -3324,6 +3334,7 @@ int main(int argc, char **argv, char **envp)
case QEMU_OPTION_m: {
uint64_t sz;
const char *mem_str;
+ const char *maxmem_str, *slots_str;
opts = qemu_opts_parse(qemu_find_opts("memory"),
optarg, 1);
@@ -3365,6 +3376,44 @@ int main(int argc, char **argv, char **envp)
error_report("ram size too large");
exit(EXIT_FAILURE);
}
+
+ maxmem_str = qemu_opt_get(opts, "maxmem");
+ slots_str = qemu_opt_get(opts, "slots");
+ if (maxmem_str && slots_str) {
+ uint64_t slots;
+
+ sz = qemu_opt_get_size(opts, "maxmem", 0);
+ if (sz < ram_size) {
+ fprintf(stderr, "qemu: invalid -m option value: maxmem "
+ "(%" PRIu64 ") <= initial memory (%"
+ PRIu64 ")\n", sz, ram_size);
+ exit(EXIT_FAILURE);
+ }
+
+ slots = qemu_opt_get_number(opts, "slots", 0);
+ if ((sz > ram_size) && !slots) {
+ fprintf(stderr, "qemu: invalid -m option value: maxmem "
+ "(%" PRIu64 ") more than initial memory (%"
+ PRIu64 ") but no hotplug slots where "
+ "specified\n", sz, ram_size);
+ exit(EXIT_FAILURE);
+ }
+
+ if ((sz <= ram_size) && slots) {
+ fprintf(stderr, "qemu: invalid -m option value: %"
+ PRIu64 " hotplug slots where specified but "
+ "maxmem (%" PRIu64 ") <= initial memory (%"
+ PRIu64 ")\n", slots, sz, ram_size);
+ exit(EXIT_FAILURE);
+ }
+ maxram_size = sz;
+ ram_slots = slots;
+ } else if ((!maxmem_str && slots_str) ||
+ (maxmem_str && !slots_str)) {
+ fprintf(stderr, "qemu: invalid -m option value: missing "
+ "'%s' option\n", slots_str ? "maxmem" : "slots");
+ exit(EXIT_FAILURE);
+ }
break;
}
#ifdef CONFIG_TPM
@@ -4424,6 +4473,8 @@ int main(int argc, char **argv, char **envp)
current_machine->init_args = (QEMUMachineInitArgs) {
.machine = machine_class,
.ram_size = ram_size,
+ .maxram_size = maxram_size,
+ .ram_slots = ram_slots,
.boot_order = boot_order,
.kernel_filename = kernel_filename,
.kernel_cmdline = kernel_cmdline,
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug Matthew Rosato
@ 2014-05-07 18:50 ` Alexander Graf
2014-05-07 19:00 ` Matthew Rosato
2014-05-09 12:35 ` Christian Borntraeger
1 sibling, 1 reply; 18+ messages in thread
From: Alexander Graf @ 2014-05-07 18:50 UTC (permalink / raw)
To: Matthew Rosato
Cc: qemu-devel, borntraeger, aliguori, imammedo, cornelia.huck,
pbonzini, rth
On 05/07/2014 08:05 PM, Matthew Rosato wrote:
> From: Igor Mammedov <imammedo@redhat.com>
>
> From: Igor Mammedov <imammedo@redhat.com>
>
> Add following parameters:
> "slots" - total number of hotplug memory slots
> "maxmem" - maximum possible memory
>
> "slots" and "maxmem" should go in pair and "maxmem" should be greater
> than "mem" for memory hotplug to be enabled.
>
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
If it's From: Igor, why doesn't the patch have his SoB line?
Alex
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug
2014-05-07 18:50 ` Alexander Graf
@ 2014-05-07 19:00 ` Matthew Rosato
2014-05-08 8:43 ` Alexander Graf
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Rosato @ 2014-05-07 19:00 UTC (permalink / raw)
To: Alexander Graf
Cc: qemu-devel, borntraeger, aliguori, imammedo, cornelia.huck,
pbonzini, rth
On 05/07/2014 02:50 PM, Alexander Graf wrote:
> On 05/07/2014 08:05 PM, Matthew Rosato wrote:
>> From: Igor Mammedov <imammedo@redhat.com>
>>
>> From: Igor Mammedov <imammedo@redhat.com>
>>
>> Add following parameters:
>> "slots" - total number of hotplug memory slots
>> "maxmem" - maximum possible memory
>>
>> "slots" and "maxmem" should go in pair and "maxmem" should be greater
>> than "mem" for memory hotplug to be enabled.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>
> If it's From: Igor, why doesn't the patch have his SoB line?
>
I needed to rebase a few lines from Igor's latest version, so I wasn't
sure if I should put his SoB line until he had a chance to look at it.
>
> Alex
>
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug
2014-05-07 19:00 ` Matthew Rosato
@ 2014-05-08 8:43 ` Alexander Graf
0 siblings, 0 replies; 18+ messages in thread
From: Alexander Graf @ 2014-05-08 8:43 UTC (permalink / raw)
To: Matthew Rosato
Cc: qemu-devel, borntraeger, aliguori, imammedo, cornelia.huck,
pbonzini, rth
On 05/07/2014 09:00 PM, Matthew Rosato wrote:
> On 05/07/2014 02:50 PM, Alexander Graf wrote:
>> On 05/07/2014 08:05 PM, Matthew Rosato wrote:
>>> From: Igor Mammedov <imammedo@redhat.com>
>>>
>>> From: Igor Mammedov <imammedo@redhat.com>
>>>
>>> Add following parameters:
>>> "slots" - total number of hotplug memory slots
>>> "maxmem" - maximum possible memory
>>>
>>> "slots" and "maxmem" should go in pair and "maxmem" should be greater
>>> than "mem" for memory hotplug to be enabled.
>>>
>>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>> If it's From: Igor, why doesn't the patch have his SoB line?
>>
> I needed to rebase a few lines from Igor's latest version, so I wasn't
> sure if I should put his SoB line until he had a chance to look at it.
Usually (unless you work inside the same company and/or have special
agreements in place) you maintain the original author's SoB line, put a
comment line with things you changed below and then add your SoB line.
Alex
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug Matthew Rosato
2014-05-07 18:50 ` Alexander Graf
@ 2014-05-09 12:35 ` Christian Borntraeger
2014-05-15 13:53 ` Igor Mammedov
1 sibling, 1 reply; 18+ messages in thread
From: Christian Borntraeger @ 2014-05-09 12:35 UTC (permalink / raw)
To: Matthew Rosato, qemu-devel
Cc: agraf, aliguori, imammedo, cornelia.huck, pbonzini, rth
On 07/05/14 20:05, Matthew Rosato wrote:
> From: Igor Mammedov <imammedo@redhat.com>
>
> From: Igor Mammedov <imammedo@redhat.com>
>
> Add following parameters:
> "slots" - total number of hotplug memory slots
> "maxmem" - maximum possible memory
>
> "slots" and "maxmem" should go in pair and "maxmem" should be greater
> than "mem" for memory hotplug to be enabled.
>
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
I would prefer if Igor could resend his latest version and one of the overall maintainer applies this as it is not s390-specific.
What happened to the x86 implementation?
Christian
> ---
> include/hw/boards.h | 2 ++
> qemu-options.hx | 9 ++++++---
> vl.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 59 insertions(+), 3 deletions(-)
>
> diff --git a/include/hw/boards.h b/include/hw/boards.h
> index 4345bd0..5b3a903 100644
> --- a/include/hw/boards.h
> +++ b/include/hw/boards.h
> @@ -11,6 +11,8 @@
> typedef struct QEMUMachineInitArgs {
> const MachineClass *machine;
> ram_addr_t ram_size;
> + ram_addr_t maxram_size;
> + uint64_t ram_slots;
> const char *boot_order;
> const char *kernel_filename;
> const char *kernel_cmdline;
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 781af14..c6411c4 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -210,17 +210,20 @@ use is discouraged as it may be removed from future versions.
> ETEXI
>
> DEF("m", HAS_ARG, QEMU_OPTION_m,
> - "-m [size=]megs\n"
> + "-m[emory] [size=]megs[,slots=n,maxmem=size]\n"
> " configure guest RAM\n"
> " size: initial amount of guest memory (default: "
> - stringify(DEFAULT_RAM_SIZE) "MiB)\n",
> + stringify(DEFAULT_RAM_SIZE) "MiB)\n"
> + " slots: number of hotplug slots (default: none)\n"
> + " maxmem: maximum amount of guest memory (default: none)\n",
> QEMU_ARCH_ALL)
> STEXI
> @item -m [size=]@var{megs}
> @findex -m
> Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB. Optionally,
> a suffix of ``M'' or ``G'' can be used to signify a value in megabytes or
> -gigabytes respectively.
> +gigabytes respectively. Optional pair @var{slots}, @var{maxmem} could be used
> +to set amount of hotluggable memory slots and possible maximum amount of memory.
> ETEXI
>
> DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
> diff --git a/vl.c b/vl.c
> index 73e0661..36ad82c 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -520,6 +520,14 @@ static QemuOptsList qemu_mem_opts = {
> .name = "size",
> .type = QEMU_OPT_SIZE,
> },
> + {
> + .name = "slots",
> + .type = QEMU_OPT_NUMBER,
> + },
> + {
> + .name = "maxmem",
> + .type = QEMU_OPT_SIZE,
> + },
> { /* end of list */ }
> },
> };
> @@ -2988,6 +2996,8 @@ int main(int argc, char **argv, char **envp)
> const char *trace_file = NULL;
> const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> 1024 * 1024;
> + ram_addr_t maxram_size = default_ram_size;
> + uint64_t ram_slots = 0;
>
> atexit(qemu_run_exit_notifiers);
> error_set_progname(argv[0]);
> @@ -3324,6 +3334,7 @@ int main(int argc, char **argv, char **envp)
> case QEMU_OPTION_m: {
> uint64_t sz;
> const char *mem_str;
> + const char *maxmem_str, *slots_str;
>
> opts = qemu_opts_parse(qemu_find_opts("memory"),
> optarg, 1);
> @@ -3365,6 +3376,44 @@ int main(int argc, char **argv, char **envp)
> error_report("ram size too large");
> exit(EXIT_FAILURE);
> }
> +
> + maxmem_str = qemu_opt_get(opts, "maxmem");
> + slots_str = qemu_opt_get(opts, "slots");
> + if (maxmem_str && slots_str) {
> + uint64_t slots;
> +
> + sz = qemu_opt_get_size(opts, "maxmem", 0);
> + if (sz < ram_size) {
> + fprintf(stderr, "qemu: invalid -m option value: maxmem "
> + "(%" PRIu64 ") <= initial memory (%"
> + PRIu64 ")\n", sz, ram_size);
> + exit(EXIT_FAILURE);
> + }
> +
> + slots = qemu_opt_get_number(opts, "slots", 0);
> + if ((sz > ram_size) && !slots) {
> + fprintf(stderr, "qemu: invalid -m option value: maxmem "
> + "(%" PRIu64 ") more than initial memory (%"
> + PRIu64 ") but no hotplug slots where "
> + "specified\n", sz, ram_size);
> + exit(EXIT_FAILURE);
> + }
> +
> + if ((sz <= ram_size) && slots) {
> + fprintf(stderr, "qemu: invalid -m option value: %"
> + PRIu64 " hotplug slots where specified but "
> + "maxmem (%" PRIu64 ") <= initial memory (%"
> + PRIu64 ")\n", slots, sz, ram_size);
> + exit(EXIT_FAILURE);
> + }
> + maxram_size = sz;
> + ram_slots = slots;
> + } else if ((!maxmem_str && slots_str) ||
> + (maxmem_str && !slots_str)) {
> + fprintf(stderr, "qemu: invalid -m option value: missing "
> + "'%s' option\n", slots_str ? "maxmem" : "slots");
> + exit(EXIT_FAILURE);
> + }
> break;
> }
> #ifdef CONFIG_TPM
> @@ -4424,6 +4473,8 @@ int main(int argc, char **argv, char **envp)
> current_machine->init_args = (QEMUMachineInitArgs) {
> .machine = machine_class,
> .ram_size = ram_size,
> + .maxram_size = maxram_size,
> + .ram_slots = ram_slots,
> .boot_order = boot_order,
> .kernel_filename = kernel_filename,
> .kernel_cmdline = kernel_cmdline,
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug
2014-05-09 12:35 ` Christian Borntraeger
@ 2014-05-15 13:53 ` Igor Mammedov
0 siblings, 0 replies; 18+ messages in thread
From: Igor Mammedov @ 2014-05-15 13:53 UTC (permalink / raw)
To: Christian Borntraeger
Cc: Matthew Rosato, qemu-devel, agraf, aliguori, cornelia.huck,
pbonzini, rth
On Fri, 09 May 2014 14:35:03 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> On 07/05/14 20:05, Matthew Rosato wrote:
> > From: Igor Mammedov <imammedo@redhat.com>
> >
> > From: Igor Mammedov <imammedo@redhat.com>
> >
> > Add following parameters:
> > "slots" - total number of hotplug memory slots
> > "maxmem" - maximum possible memory
> >
> > "slots" and "maxmem" should go in pair and "maxmem" should be greater
> > than "mem" for memory hotplug to be enabled.
> >
> > Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>
> I would prefer if Igor could resend his latest version and one of the overall maintainer applies this as it is not s390-specific.
> What happened to the x86 implementation?
I'm rebasing it on current master and chasing migration bug.
>
> Christian
> > ---
> > include/hw/boards.h | 2 ++
> > qemu-options.hx | 9 ++++++---
> > vl.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 59 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/hw/boards.h b/include/hw/boards.h
> > index 4345bd0..5b3a903 100644
> > --- a/include/hw/boards.h
> > +++ b/include/hw/boards.h
> > @@ -11,6 +11,8 @@
> > typedef struct QEMUMachineInitArgs {
> > const MachineClass *machine;
> > ram_addr_t ram_size;
> > + ram_addr_t maxram_size;
> > + uint64_t ram_slots;
> > const char *boot_order;
> > const char *kernel_filename;
> > const char *kernel_cmdline;
> > diff --git a/qemu-options.hx b/qemu-options.hx
> > index 781af14..c6411c4 100644
> > --- a/qemu-options.hx
> > +++ b/qemu-options.hx
> > @@ -210,17 +210,20 @@ use is discouraged as it may be removed from future versions.
> > ETEXI
> >
> > DEF("m", HAS_ARG, QEMU_OPTION_m,
> > - "-m [size=]megs\n"
> > + "-m[emory] [size=]megs[,slots=n,maxmem=size]\n"
> > " configure guest RAM\n"
> > " size: initial amount of guest memory (default: "
> > - stringify(DEFAULT_RAM_SIZE) "MiB)\n",
> > + stringify(DEFAULT_RAM_SIZE) "MiB)\n"
> > + " slots: number of hotplug slots (default: none)\n"
> > + " maxmem: maximum amount of guest memory (default: none)\n",
> > QEMU_ARCH_ALL)
> > STEXI
> > @item -m [size=]@var{megs}
> > @findex -m
> > Set virtual RAM size to @var{megs} megabytes. Default is 128 MiB. Optionally,
> > a suffix of ``M'' or ``G'' can be used to signify a value in megabytes or
> > -gigabytes respectively.
> > +gigabytes respectively. Optional pair @var{slots}, @var{maxmem} could be used
> > +to set amount of hotluggable memory slots and possible maximum amount of memory.
> > ETEXI
> >
> > DEF("mem-path", HAS_ARG, QEMU_OPTION_mempath,
> > diff --git a/vl.c b/vl.c
> > index 73e0661..36ad82c 100644
> > --- a/vl.c
> > +++ b/vl.c
> > @@ -520,6 +520,14 @@ static QemuOptsList qemu_mem_opts = {
> > .name = "size",
> > .type = QEMU_OPT_SIZE,
> > },
> > + {
> > + .name = "slots",
> > + .type = QEMU_OPT_NUMBER,
> > + },
> > + {
> > + .name = "maxmem",
> > + .type = QEMU_OPT_SIZE,
> > + },
> > { /* end of list */ }
> > },
> > };
> > @@ -2988,6 +2996,8 @@ int main(int argc, char **argv, char **envp)
> > const char *trace_file = NULL;
> > const ram_addr_t default_ram_size = (ram_addr_t)DEFAULT_RAM_SIZE *
> > 1024 * 1024;
> > + ram_addr_t maxram_size = default_ram_size;
> > + uint64_t ram_slots = 0;
> >
> > atexit(qemu_run_exit_notifiers);
> > error_set_progname(argv[0]);
> > @@ -3324,6 +3334,7 @@ int main(int argc, char **argv, char **envp)
> > case QEMU_OPTION_m: {
> > uint64_t sz;
> > const char *mem_str;
> > + const char *maxmem_str, *slots_str;
> >
> > opts = qemu_opts_parse(qemu_find_opts("memory"),
> > optarg, 1);
> > @@ -3365,6 +3376,44 @@ int main(int argc, char **argv, char **envp)
> > error_report("ram size too large");
> > exit(EXIT_FAILURE);
> > }
> > +
> > + maxmem_str = qemu_opt_get(opts, "maxmem");
> > + slots_str = qemu_opt_get(opts, "slots");
> > + if (maxmem_str && slots_str) {
> > + uint64_t slots;
> > +
> > + sz = qemu_opt_get_size(opts, "maxmem", 0);
> > + if (sz < ram_size) {
> > + fprintf(stderr, "qemu: invalid -m option value: maxmem "
> > + "(%" PRIu64 ") <= initial memory (%"
> > + PRIu64 ")\n", sz, ram_size);
> > + exit(EXIT_FAILURE);
> > + }
> > +
> > + slots = qemu_opt_get_number(opts, "slots", 0);
> > + if ((sz > ram_size) && !slots) {
> > + fprintf(stderr, "qemu: invalid -m option value: maxmem "
> > + "(%" PRIu64 ") more than initial memory (%"
> > + PRIu64 ") but no hotplug slots where "
> > + "specified\n", sz, ram_size);
> > + exit(EXIT_FAILURE);
> > + }
> > +
> > + if ((sz <= ram_size) && slots) {
> > + fprintf(stderr, "qemu: invalid -m option value: %"
> > + PRIu64 " hotplug slots where specified but "
> > + "maxmem (%" PRIu64 ") <= initial memory (%"
> > + PRIu64 ")\n", slots, sz, ram_size);
> > + exit(EXIT_FAILURE);
> > + }
> > + maxram_size = sz;
> > + ram_slots = slots;
> > + } else if ((!maxmem_str && slots_str) ||
> > + (maxmem_str && !slots_str)) {
> > + fprintf(stderr, "qemu: invalid -m option value: missing "
> > + "'%s' option\n", slots_str ? "maxmem" : "slots");
> > + exit(EXIT_FAILURE);
> > + }
> > break;
> > }
> > #ifdef CONFIG_TPM
> > @@ -4424,6 +4473,8 @@ int main(int argc, char **argv, char **envp)
> > current_machine->init_args = (QEMUMachineInitArgs) {
> > .machine = machine_class,
> > .ram_size = ram_size,
> > + .maxram_size = maxram_size,
> > + .ram_slots = ram_slots,
> > .boot_order = boot_order,
> > .kernel_filename = kernel_filename,
> > .kernel_cmdline = kernel_cmdline,
> >
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v3 2/4] sclp-s390: Add device to manage s390 memory hotplug
2014-05-07 18:05 [Qemu-devel] [PATCH v3 0/4] s390: Support for Hotplug of Standby Memory Matthew Rosato
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug Matthew Rosato
@ 2014-05-07 18:05 ` Matthew Rosato
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment Matthew Rosato
` (2 subsequent siblings)
4 siblings, 0 replies; 18+ messages in thread
From: Matthew Rosato @ 2014-05-07 18:05 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, borntraeger, aliguori, imammedo, cornelia.huck, pbonzini,
rth
Add sclpMemoryHotplugDev to contain associated data structures, etc.
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
hw/s390x/sclp.c | 32 ++++++++++++++++++++++++++++++++
include/hw/s390x/sclp.h | 19 +++++++++++++++++++
2 files changed, 51 insertions(+)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index d8ddf35..338dbdf 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -183,3 +183,35 @@ void s390_sclp_init(void)
OBJECT(dev), NULL);
qdev_init_nofail(dev);
}
+
+sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void)
+{
+ DeviceState *dev;
+ ObjectProperty *op = object_property_find(qdev_get_machine(),
+ TYPE_SCLP_MEMORY_HOTPLUG_DEV,
+ NULL);
+ if (!op) {
+ dev = qdev_create(NULL, TYPE_SCLP_MEMORY_HOTPLUG_DEV);
+ object_property_add_child(qdev_get_machine(),
+ TYPE_SCLP_MEMORY_HOTPLUG_DEV,
+ OBJECT(dev), NULL);
+ qdev_init_nofail(dev);
+ op = object_property_find(qdev_get_machine(),
+ TYPE_SCLP_MEMORY_HOTPLUG_DEV,
+ NULL);
+ }
+
+ return op->opaque;
+}
+
+static TypeInfo sclp_memory_hotplug_dev_info = {
+ .name = TYPE_SCLP_MEMORY_HOTPLUG_DEV,
+ .parent = TYPE_DEVICE,
+ .instance_size = sizeof(sclpMemoryHotplugDev),
+};
+
+static void register_types(void)
+{
+ type_register_static(&sclp_memory_hotplug_dev_info);
+}
+type_init(register_types);
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index 7ef1622..5f463bd 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -37,6 +37,7 @@
#define SCLP_STARTING_SUBINCREMENT_ID 0x10001
#define SCLP_INCREMENT_UNIT 0x10000
#define MAX_AVAIL_SLOTS 32
+#define MAX_STORAGE_INCREMENTS 1020
/* CPU hotplug SCLP codes */
#define SCLP_HAS_CPU_INFO 0x0C00000000000000ULL
@@ -156,6 +157,23 @@ typedef struct SCCB {
char data[SCCB_DATA_LEN];
} QEMU_PACKED SCCB;
+typedef struct sclpMemoryHotplugDev sclpMemoryHotplugDev;
+
+#define TYPE_SCLP_MEMORY_HOTPLUG_DEV "sclp-memory-hotplug-dev"
+#define SCLP_MEMORY_HOTPLUG_DEV(obj) \
+ OBJECT_CHECK(sclpMemoryHotplugDev, (obj), TYPE_SCLP_MEMORY_HOTPLUG_DEV)
+
+struct sclpMemoryHotplugDev {
+ DeviceState parent;
+ ram_addr_t standby_mem_size;
+ ram_addr_t padded_ram_size;
+ ram_addr_t pad_size;
+ ram_addr_t standby_subregion_size;
+ ram_addr_t rzm;
+ int increment_size;
+ char *standby_state_map;
+};
+
static inline int sccb_data_len(SCCB *sccb)
{
return be16_to_cpu(sccb->h.length) - sizeof(sccb->h);
@@ -163,6 +181,7 @@ static inline int sccb_data_len(SCCB *sccb)
void s390_sclp_init(void);
+sclpMemoryHotplugDev *get_sclp_memory_hotplug_dev(void);
void sclp_service_interrupt(uint32_t sccb);
void raise_irq_cpu_hotplug(void);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment
2014-05-07 18:05 [Qemu-devel] [PATCH v3 0/4] s390: Support for Hotplug of Standby Memory Matthew Rosato
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 1/4] vl.c: extend -m option to support options for memory hotplug Matthew Rosato
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 2/4] sclp-s390: Add device to manage s390 " Matthew Rosato
@ 2014-05-07 18:05 ` Matthew Rosato
2014-05-12 7:43 ` Christian Borntraeger
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 4/4] sclp-s390: Add memory hotplug SCLPs Matthew Rosato
2014-05-12 7:46 ` [Qemu-devel] [PATCH v3 0/4] s390: Support for Hotplug of Standby Memory Christian Borntraeger
4 siblings, 1 reply; 18+ messages in thread
From: Matthew Rosato @ 2014-05-07 18:05 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, borntraeger, aliguori, imammedo, cornelia.huck, pbonzini,
rth
When determining the memory increment size, use the maxmem size if
it was specified.
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
hw/s390x/s390-virtio-ccw.c | 44 ++++++++++++++++++++++++++++++++++++--------
target-s390x/cpu.h | 3 +++
2 files changed, 39 insertions(+), 8 deletions(-)
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 0d4f6ae..a8be0f7 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -17,6 +17,7 @@
#include "ioinst.h"
#include "css.h"
#include "virtio-ccw.h"
+#include "qemu/config-file.h"
void io_subsystem_reset(void)
{
@@ -84,17 +85,33 @@ static void ccw_init(QEMUMachineInitArgs *args)
ram_addr_t my_ram_size = args->ram_size;
MemoryRegion *sysmem = get_system_memory();
MemoryRegion *ram = g_new(MemoryRegion, 1);
- int shift = 0;
+ sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
uint8_t *storage_keys;
int ret;
VirtualCssBus *css_bus;
-
- /* s390x ram size detection needs a 16bit multiplier + an increment. So
- guests > 64GB can be specified in 2MB steps etc. */
- while ((my_ram_size >> (20 + shift)) > 65535) {
- shift++;
+ QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
+ ram_addr_t pad_size = 0;
+ ram_addr_t maxmem = qemu_opt_get_size(opts, "maxmem", 0);
+ ram_addr_t standby_mem_size = maxmem - my_ram_size;
+
+ /* The storage increment size is a multiple of 1M and is a power of 2.
+ * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
+ * The variable 'mhd->increment_size' is an exponent of 2 that can be
+ * used to calculate the size (in bytes) of an increment. */
+ mhd->increment_size = 20;
+ while ((my_ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
+ mhd->increment_size++;
+ }
+ while ((standby_mem_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
+ mhd->increment_size++;
}
- my_ram_size = my_ram_size >> (20 + shift) << (20 + shift);
+
+ /* The core and standby memory areas need to be aligned with
+ * the increment size */
+ standby_mem_size = standby_mem_size >> mhd->increment_size
+ << mhd->increment_size;
+ my_ram_size = my_ram_size >> mhd->increment_size
+ << mhd->increment_size;
/* let's propagate the changed ram size into the global variable. */
ram_size = my_ram_size;
@@ -109,11 +126,22 @@ static void ccw_init(QEMUMachineInitArgs *args)
/* register hypercalls */
virtio_ccw_register_hcalls();
- /* allocate RAM */
+ /* allocate RAM for core */
memory_region_init_ram(ram, NULL, "s390.ram", my_ram_size);
vmstate_register_ram_global(ram);
memory_region_add_subregion(sysmem, 0, ram);
+ /* If the size of ram is not on a MEM_SECTION_SIZE boundary,
+ calculate the pad size necessary to force this boundary. */
+ if (standby_mem_size) {
+ if (my_ram_size % MEM_SECTION_SIZE) {
+ pad_size = MEM_SECTION_SIZE - my_ram_size % MEM_SECTION_SIZE;
+ }
+ my_ram_size += standby_mem_size + pad_size;
+ mhd->pad_size = pad_size;
+ mhd->standby_mem_size = standby_mem_size;
+ }
+
/* allocate storage keys */
storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index aad277a..193eac3 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -1047,6 +1047,9 @@ static inline void cpu_inject_crw_mchk(S390CPU *cpu)
cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
}
+/* from s390-virtio-ccw */
+#define MEM_SECTION_SIZE 0x10000000UL
+
/* fpu_helper.c */
uint32_t set_cc_nz_f32(float32 v);
uint32_t set_cc_nz_f64(float64 v);
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment Matthew Rosato
@ 2014-05-12 7:43 ` Christian Borntraeger
2014-05-13 13:16 ` Matthew Rosato
0 siblings, 1 reply; 18+ messages in thread
From: Christian Borntraeger @ 2014-05-12 7:43 UTC (permalink / raw)
To: Matthew Rosato, qemu-devel
Cc: agraf, aliguori, imammedo, Cornelia Huck, pbonzini, rth
On 07/05/14 20:05, Matthew Rosato wrote:
> When determining the memory increment size, use the maxmem size if
> it was specified.
>
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> ---
> hw/s390x/s390-virtio-ccw.c | 44 ++++++++++++++++++++++++++++++++++++--------
> target-s390x/cpu.h | 3 +++
> 2 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 0d4f6ae..a8be0f7 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -17,6 +17,7 @@
> #include "ioinst.h"
> #include "css.h"
> #include "virtio-ccw.h"
> +#include "qemu/config-file.h"
>
> void io_subsystem_reset(void)
> {
> @@ -84,17 +85,33 @@ static void ccw_init(QEMUMachineInitArgs *args)
> ram_addr_t my_ram_size = args->ram_size;
> MemoryRegion *sysmem = get_system_memory();
> MemoryRegion *ram = g_new(MemoryRegion, 1);
> - int shift = 0;
> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
> uint8_t *storage_keys;
> int ret;
> VirtualCssBus *css_bus;
> -
> - /* s390x ram size detection needs a 16bit multiplier + an increment. So
> - guests > 64GB can be specified in 2MB steps etc. */
> - while ((my_ram_size >> (20 + shift)) > 65535) {
> - shift++;
> + QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
> + ram_addr_t pad_size = 0;
> + ram_addr_t maxmem = qemu_opt_get_size(opts, "maxmem", 0);
> + ram_addr_t standby_mem_size = maxmem - my_ram_size;
> +
> + /* The storage increment size is a multiple of 1M and is a power of 2.
> + * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
> + * The variable 'mhd->increment_size' is an exponent of 2 that can be
> + * used to calculate the size (in bytes) of an increment. */
> + mhd->increment_size = 20;
> + while ((my_ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
> + mhd->increment_size++;
> + }
> + while ((standby_mem_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
> + mhd->increment_size++;
> }
Looking back into the mail thread, Alex requested to make the code for standy/non-standby identical.
Now: The limit of 1020 (MAX_STORAGE_INCREMENTS) is only given if standby memory exists. Without standby memory, we could still used 64k as a divider.(zVM also does only impose this limit with standby memory).
What does that mean: With this patch the memory size granularity gets bigger. e.g. a guest can have
1019MB, 1020MB, 1022MB, 1024MB and so on (1021MB is no longer possible, but it was before).
We could now special case this or just leave it as is in this patch to make the code simpler. I think this is not a big problem, but we should add this to the comment and maybe also to the help text of the command line option (e.g.
diff --git a/qemu-options.hx b/qemu-options.hx
index 5edffa6..8c71283 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -217,7 +217,8 @@ DEF("m", HAS_ARG, QEMU_OPTION_m,
"-m [size=]megs\n"
" configure guest RAM\n"
" size: initial amount of guest memory (default: "
- stringify(DEFAULT_RAM_SIZE) "MiB)\n",
+ stringify(DEFAULT_RAM_SIZE) "MiB)\n"
+ "NOTE: Some architectures might enforce a specific granularity\n",
QEMU_ARCH_ALL)
STEXI
@item -m [size=]@var{megs}
@Alex,Conny: Any preference?
> - my_ram_size = my_ram_size >> (20 + shift) << (20 + shift);
> +
> + /* The core and standby memory areas need to be aligned with
> + * the increment size */
> + standby_mem_size = standby_mem_size >> mhd->increment_size
> + << mhd->increment_size;
> + my_ram_size = my_ram_size >> mhd->increment_size
> + << mhd->increment_size;
>
> /* let's propagate the changed ram size into the global variable. */
> ram_size = my_ram_size;
> @@ -109,11 +126,22 @@ static void ccw_init(QEMUMachineInitArgs *args)
> /* register hypercalls */
> virtio_ccw_register_hcalls();
>
> - /* allocate RAM */
> + /* allocate RAM for core */
> memory_region_init_ram(ram, NULL, "s390.ram", my_ram_size);memory_region_init_ram
> vmstate_register_ram_global(ram);
> memory_region_add_subregion(sysmem, 0, ram);
>
> + /* If the size of ram is not on a MEM_SECTION_SIZE boundary,
> + calculate the pad size necessary to force this boundary. */
> + if (standby_mem_size) {
> + if (my_ram_size % MEM_SECTION_SIZE) {
> + pad_size = MEM_SECTION_SIZE - my_ram_size % MEM_SECTION_SIZE;
> + }
> + my_ram_size += standby_mem_size + pad_size;
> + mhd->pad_size = pad_size;
> + mhd->standby_mem_size = standby_mem_size;
> + }
> +
> /* allocate storage keys */
> storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
>
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index aad277a..193eac3 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -1047,6 +1047,9 @@ static inline void cpu_inject_crw_mchk(S390CPU *cpu)
> cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
> }
>
> +/* from s390-virtio-ccw */
> +#define MEM_SECTION_SIZE 0x10000000UL
> +
> /* fpu_helper.c */
> uint32_t set_cc_nz_f32(float32 v);
> uint32_t set_cc_nz_f64(float64 v);
>
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment
2014-05-12 7:43 ` Christian Borntraeger
@ 2014-05-13 13:16 ` Matthew Rosato
2014-05-13 13:43 ` Alexander Graf
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Rosato @ 2014-05-13 13:16 UTC (permalink / raw)
To: Christian Borntraeger, qemu-devel
Cc: agraf, aliguori, pbonzini, Cornelia Huck, imammedo, rth
On 05/12/2014 03:43 AM, Christian Borntraeger wrote:
> On 07/05/14 20:05, Matthew Rosato wrote:
>> When determining the memory increment size, use the maxmem size if
>> it was specified.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>> ---
>> hw/s390x/s390-virtio-ccw.c | 44 ++++++++++++++++++++++++++++++++++++--------
>> target-s390x/cpu.h | 3 +++
>> 2 files changed, 39 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>> index 0d4f6ae..a8be0f7 100644
>> --- a/hw/s390x/s390-virtio-ccw.c
>> +++ b/hw/s390x/s390-virtio-ccw.c
>> @@ -17,6 +17,7 @@
>> #include "ioinst.h"
>> #include "css.h"
>> #include "virtio-ccw.h"
>> +#include "qemu/config-file.h"
>>
>> void io_subsystem_reset(void)
>> {
>> @@ -84,17 +85,33 @@ static void ccw_init(QEMUMachineInitArgs *args)
>> ram_addr_t my_ram_size = args->ram_size;
>> MemoryRegion *sysmem = get_system_memory();
>> MemoryRegion *ram = g_new(MemoryRegion, 1);
>> - int shift = 0;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>> uint8_t *storage_keys;
>> int ret;
>> VirtualCssBus *css_bus;
>> -
>> - /* s390x ram size detection needs a 16bit multiplier + an increment. So
>> - guests > 64GB can be specified in 2MB steps etc. */
>> - while ((my_ram_size >> (20 + shift)) > 65535) {
>> - shift++;
>> + QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
>> + ram_addr_t pad_size = 0;
>> + ram_addr_t maxmem = qemu_opt_get_size(opts, "maxmem", 0);
>> + ram_addr_t standby_mem_size = maxmem - my_ram_size;
>> +
>> + /* The storage increment size is a multiple of 1M and is a power of 2.
>> + * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
>> + * The variable 'mhd->increment_size' is an exponent of 2 that can be
>> + * used to calculate the size (in bytes) of an increment. */
>> + mhd->increment_size = 20;
>> + while ((my_ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
>> + mhd->increment_size++;
>> + }
>> + while ((standby_mem_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
>> + mhd->increment_size++;
>> }
>
> Looking back into the mail thread, Alex requested to make the code for standy/non-standby identical.
> Now: The limit of 1020 (MAX_STORAGE_INCREMENTS) is only given if standby memory exists. Without standby memory, we could still used 64k as a divider.(zVM also does only impose this limit with standby memory).
> What does that mean: With this patch the memory size granularity gets bigger. e.g. a guest can have
> 1019MB, 1020MB, 1022MB, 1024MB and so on (1021MB is no longer possible, but it was before).
Hmm, this is a good point. I didn't think about it when I made the
change per Alex. If nobody has a strong opinion here, I think I'd
rather go back to special casing this in the next version, to keep the
'normal case' (without standby memory) more robust.
>
> We could now special case this or just leave it as is in this patch to make the code simpler. I think this is not a big problem, but we should add this to the comment and maybe also to the help text of the command line option (e.g.
>
> diff --git a/qemu-options.hx b/qemu-options.hx
> index 5edffa6..8c71283 100644
> --- a/qemu-options.hx
> +++ b/qemu-options.hx
> @@ -217,7 +217,8 @@ DEF("m", HAS_ARG, QEMU_OPTION_m,
> "-m [size=]megs\n"
> " configure guest RAM\n"
> " size: initial amount of guest memory (default: "
> - stringify(DEFAULT_RAM_SIZE) "MiB)\n",
> + stringify(DEFAULT_RAM_SIZE) "MiB)\n"
> + "NOTE: Some architectures might enforce a specific granularity\n",
> QEMU_ARCH_ALL)
> STEXI
> @item -m [size=]@var{megs}
>
> @Alex,Conny: Any preference?
>
Sure, I'll add the doc hit & some extra commentary in the next version.
>> - my_ram_size = my_ram_size >> (20 + shift) << (20 + shift);
>> +
>> + /* The core and standby memory areas need to be aligned with
>> + * the increment size */
>> + standby_mem_size = standby_mem_size >> mhd->increment_size
>> + << mhd->increment_size;
>> + my_ram_size = my_ram_size >> mhd->increment_size
>> + << mhd->increment_size;
>>
>> /* let's propagate the changed ram size into the global variable. */
>> ram_size = my_ram_size;
>> @@ -109,11 +126,22 @@ static void ccw_init(QEMUMachineInitArgs *args)
>> /* register hypercalls */
>> virtio_ccw_register_hcalls();
>>
>> - /* allocate RAM */
>> + /* allocate RAM for core */
>> memory_region_init_ram(ram, NULL, "s390.ram", my_ram_size);memory_region_init_ram
>> vmstate_register_ram_global(ram);
>> memory_region_add_subregion(sysmem, 0, ram);
>>
>> + /* If the size of ram is not on a MEM_SECTION_SIZE boundary,
>> + calculate the pad size necessary to force this boundary. */
>> + if (standby_mem_size) {
>> + if (my_ram_size % MEM_SECTION_SIZE) {
>> + pad_size = MEM_SECTION_SIZE - my_ram_size % MEM_SECTION_SIZE;
>> + }
>> + my_ram_size += standby_mem_size + pad_size;
>> + mhd->pad_size = pad_size;
>> + mhd->standby_mem_size = standby_mem_size;
>> + }
>> +
>> /* allocate storage keys */
>> storage_keys = g_malloc0(my_ram_size / TARGET_PAGE_SIZE);
>>
>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>> index aad277a..193eac3 100644
>> --- a/target-s390x/cpu.h
>> +++ b/target-s390x/cpu.h
>> @@ -1047,6 +1047,9 @@ static inline void cpu_inject_crw_mchk(S390CPU *cpu)
>> cpu_interrupt(CPU(cpu), CPU_INTERRUPT_HARD);
>> }
>>
>> +/* from s390-virtio-ccw */
>> +#define MEM_SECTION_SIZE 0x10000000UL
>> +
>> /* fpu_helper.c */
>> uint32_t set_cc_nz_f32(float32 v);
>> uint32_t set_cc_nz_f64(float64 v);
>>
>
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment
2014-05-13 13:16 ` Matthew Rosato
@ 2014-05-13 13:43 ` Alexander Graf
2014-05-13 14:04 ` Matthew Rosato
0 siblings, 1 reply; 18+ messages in thread
From: Alexander Graf @ 2014-05-13 13:43 UTC (permalink / raw)
To: Matthew Rosato, Christian Borntraeger, qemu-devel
Cc: Cornelia Huck, imammedo, rth, aliguori, pbonzini
On 13.05.14 15:16, Matthew Rosato wrote:
> On 05/12/2014 03:43 AM, Christian Borntraeger wrote:
>> On 07/05/14 20:05, Matthew Rosato wrote:
>>> When determining the memory increment size, use the maxmem size if
>>> it was specified.
>>>
>>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>>> ---
>>> hw/s390x/s390-virtio-ccw.c | 44 ++++++++++++++++++++++++++++++++++++--------
>>> target-s390x/cpu.h | 3 +++
>>> 2 files changed, 39 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 0d4f6ae..a8be0f7 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -17,6 +17,7 @@
>>> #include "ioinst.h"
>>> #include "css.h"
>>> #include "virtio-ccw.h"
>>> +#include "qemu/config-file.h"
>>>
>>> void io_subsystem_reset(void)
>>> {
>>> @@ -84,17 +85,33 @@ static void ccw_init(QEMUMachineInitArgs *args)
>>> ram_addr_t my_ram_size = args->ram_size;
>>> MemoryRegion *sysmem = get_system_memory();
>>> MemoryRegion *ram = g_new(MemoryRegion, 1);
>>> - int shift = 0;
>>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>> uint8_t *storage_keys;
>>> int ret;
>>> VirtualCssBus *css_bus;
>>> -
>>> - /* s390x ram size detection needs a 16bit multiplier + an increment. So
>>> - guests > 64GB can be specified in 2MB steps etc. */
>>> - while ((my_ram_size >> (20 + shift)) > 65535) {
>>> - shift++;
>>> + QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
>>> + ram_addr_t pad_size = 0;
>>> + ram_addr_t maxmem = qemu_opt_get_size(opts, "maxmem", 0);
>>> + ram_addr_t standby_mem_size = maxmem - my_ram_size;
>>> +
>>> + /* The storage increment size is a multiple of 1M and is a power of 2.
>>> + * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
>>> + * The variable 'mhd->increment_size' is an exponent of 2 that can be
>>> + * used to calculate the size (in bytes) of an increment. */
>>> + mhd->increment_size = 20;
>>> + while ((my_ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
>>> + mhd->increment_size++;
>>> + }
>>> + while ((standby_mem_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
>>> + mhd->increment_size++;
>>> }
>> Looking back into the mail thread, Alex requested to make the code for standy/non-standby identical.
>> Now: The limit of 1020 (MAX_STORAGE_INCREMENTS) is only given if standby memory exists. Without standby memory, we could still used 64k as a divider.(zVM also does only impose this limit with standby memory).
>> What does that mean: With this patch the memory size granularity gets bigger. e.g. a guest can have
>> 1019MB, 1020MB, 1022MB, 1024MB and so on (1021MB is no longer possible, but it was before).
> Hmm, this is a good point. I didn't think about it when I made the
> change per Alex. If nobody has a strong opinion here, I think I'd
> rather go back to special casing this in the next version, to keep the
> 'normal case' (without standby memory) more robust.
Wouldn't it be more confusing if the guest configuration suddenly
changes when you add standby memory?
Alex
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment
2014-05-13 13:43 ` Alexander Graf
@ 2014-05-13 14:04 ` Matthew Rosato
0 siblings, 0 replies; 18+ messages in thread
From: Matthew Rosato @ 2014-05-13 14:04 UTC (permalink / raw)
To: Alexander Graf, Christian Borntraeger, qemu-devel
Cc: Cornelia Huck, imammedo, rth, aliguori, pbonzini
On 05/13/2014 09:43 AM, Alexander Graf wrote:
>
> On 13.05.14 15:16, Matthew Rosato wrote:
>> On 05/12/2014 03:43 AM, Christian Borntraeger wrote:
>>> On 07/05/14 20:05, Matthew Rosato wrote:
>>>> When determining the memory increment size, use the maxmem size if
>>>> it was specified.
>>>>
>>>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>>>> ---
>>>> hw/s390x/s390-virtio-ccw.c | 44
>>>> ++++++++++++++++++++++++++++++++++++--------
>>>> target-s390x/cpu.h | 3 +++
>>>> 2 files changed, 39 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>>> index 0d4f6ae..a8be0f7 100644
>>>> --- a/hw/s390x/s390-virtio-ccw.c
>>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>>> @@ -17,6 +17,7 @@
>>>> #include "ioinst.h"
>>>> #include "css.h"
>>>> #include "virtio-ccw.h"
>>>> +#include "qemu/config-file.h"
>>>>
>>>> void io_subsystem_reset(void)
>>>> {
>>>> @@ -84,17 +85,33 @@ static void ccw_init(QEMUMachineInitArgs *args)
>>>> ram_addr_t my_ram_size = args->ram_size;
>>>> MemoryRegion *sysmem = get_system_memory();
>>>> MemoryRegion *ram = g_new(MemoryRegion, 1);
>>>> - int shift = 0;
>>>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>>> uint8_t *storage_keys;
>>>> int ret;
>>>> VirtualCssBus *css_bus;
>>>> -
>>>> - /* s390x ram size detection needs a 16bit multiplier + an
>>>> increment. So
>>>> - guests > 64GB can be specified in 2MB steps etc. */
>>>> - while ((my_ram_size >> (20 + shift)) > 65535) {
>>>> - shift++;
>>>> + QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
>>>> + ram_addr_t pad_size = 0;
>>>> + ram_addr_t maxmem = qemu_opt_get_size(opts, "maxmem", 0);
>>>> + ram_addr_t standby_mem_size = maxmem - my_ram_size;
>>>> +
>>>> + /* The storage increment size is a multiple of 1M and is a
>>>> power of 2.
>>>> + * The number of storage increments must be
>>>> MAX_STORAGE_INCREMENTS or fewer.
>>>> + * The variable 'mhd->increment_size' is an exponent of 2 that
>>>> can be
>>>> + * used to calculate the size (in bytes) of an increment. */
>>>> + mhd->increment_size = 20;
>>>> + while ((my_ram_size >> mhd->increment_size) >
>>>> MAX_STORAGE_INCREMENTS) {
>>>> + mhd->increment_size++;
>>>> + }
>>>> + while ((standby_mem_size >> mhd->increment_size) >
>>>> MAX_STORAGE_INCREMENTS) {
>>>> + mhd->increment_size++;
>>>> }
>>> Looking back into the mail thread, Alex requested to make the code
>>> for standy/non-standby identical.
>>> Now: The limit of 1020 (MAX_STORAGE_INCREMENTS) is only given if
>>> standby memory exists. Without standby memory, we could still used
>>> 64k as a divider.(zVM also does only impose this limit with standby
>>> memory).
>>> What does that mean: With this patch the memory size granularity gets
>>> bigger. e.g. a guest can have
>>> 1019MB, 1020MB, 1022MB, 1024MB and so on (1021MB is no longer
>>> possible, but it was before).
>> Hmm, this is a good point. I didn't think about it when I made the
>> change per Alex. If nobody has a strong opinion here, I think I'd
>> rather go back to special casing this in the next version, to keep the
>> 'normal case' (without standby memory) more robust.
>
> Wouldn't it be more confusing if the guest configuration suddenly
> changes when you add standby memory?
>
In fairness, you are already changing the guest memory layout with the
introduction of standby memory in the first place, so what's a little
more change? :)
But, yes, I hear you -- The value in keeping the environment uniform
across configurations outweighs the benefits of allowing odd boundaries
in some cases (probably only test cases anyway). I can live with that.
Thanks for the feedback.
Matt
^ permalink raw reply [flat|nested] 18+ messages in thread
* [Qemu-devel] [PATCH v3 4/4] sclp-s390: Add memory hotplug SCLPs
2014-05-07 18:05 [Qemu-devel] [PATCH v3 0/4] s390: Support for Hotplug of Standby Memory Matthew Rosato
` (2 preceding siblings ...)
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 3/4] virtio-ccw: Include standby memory when calculating storage increment Matthew Rosato
@ 2014-05-07 18:05 ` Matthew Rosato
2014-05-12 7:35 ` Christian Borntraeger
2014-05-12 7:46 ` [Qemu-devel] [PATCH v3 0/4] s390: Support for Hotplug of Standby Memory Christian Borntraeger
4 siblings, 1 reply; 18+ messages in thread
From: Matthew Rosato @ 2014-05-07 18:05 UTC (permalink / raw)
To: qemu-devel
Cc: agraf, borntraeger, aliguori, imammedo, cornelia.huck, pbonzini,
rth
Add memory information to read SCP info and add handlers for
Read Storage Element Information, Attach Storage Element,
Assign Storage and Unassign Storage.
Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
---
hw/s390x/sclp.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++--
target-s390x/cpu.h | 15 ++++
target-s390x/kvm.c | 5 ++
3 files changed, 258 insertions(+), 7 deletions(-)
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 338dbdf..b9425ca 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -16,7 +16,8 @@
#include "sysemu/kvm.h"
#include "exec/memory.h"
#include "sysemu/sysemu.h"
-
+#include "exec/address-spaces.h"
+#include "qemu/config-file.h"
#include "hw/s390x/sclp.h"
#include "hw/s390x/event-facility.h"
@@ -33,10 +34,18 @@ static inline SCLPEventFacility *get_event_facility(void)
static void read_SCP_info(SCCB *sccb)
{
ReadInfo *read_info = (ReadInfo *) sccb;
+ sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
CPUState *cpu;
- int shift = 0;
int cpu_count = 0;
int i = 0;
+ int rnsize, rnmax;
+ QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
+ int slots = qemu_opt_get_number(opts, "slots", 0);
+ int max_avail_slots = s390_get_memslot_count(kvm_state);
+
+ if (slots > max_avail_slots) {
+ slots = max_avail_slots;
+ }
CPU_FOREACH(cpu) {
cpu_count++;
@@ -52,16 +61,222 @@ static void read_SCP_info(SCCB *sccb)
read_info->entries[i].type = 0;
}
- read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
+ /*
+ * The storage increment size is a multiple of 1M and is a power of 2.
+ * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
+ */
+ while ((ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
+ mhd->increment_size++;
+ }
+ while ((mhd->standby_mem_size >> mhd->increment_size) >
+ MAX_STORAGE_INCREMENTS) {
+ mhd->increment_size++;
+ }
+
+ mhd->standby_subregion_size = MEM_SECTION_SIZE;
+ /* Deduct the memory slot already used for core */
+ if (slots > 0) {
+ while ((mhd->standby_subregion_size * (slots - 1)
+ < mhd->standby_mem_size)) {
+ mhd->standby_subregion_size = mhd->standby_subregion_size << 1;
+ }
+ }
+ /*
+ * Initialize mapping of guest standby memory sections indicating which
+ * are and are not online. Assume all standby memory begins offline.
+ */
+ if (mhd->standby_state_map == 0) {
+ if (mhd->standby_mem_size % mhd->standby_subregion_size) {
+ mhd->standby_state_map = g_malloc0((mhd->standby_mem_size /
+ mhd->standby_subregion_size + 1) *
+ (mhd->standby_subregion_size /
+ MEM_SECTION_SIZE));
+ } else {
+ mhd->standby_state_map = g_malloc0(mhd->standby_mem_size /
+ MEM_SECTION_SIZE);
+ }
+ }
+
+ mhd->padded_ram_size = ram_size + mhd->pad_size;
+ mhd->rzm = 1 << mhd->increment_size;
+ rnsize = 1 << (mhd->increment_size - 20);
+ if (rnsize <= 128) {
+ read_info->rnsize = rnsize;
+ } else {
+ read_info->rnsize = 0;
+ read_info->rnsize2 = cpu_to_be32(rnsize);
+ }
- while ((ram_size >> (20 + shift)) > 65535) {
- shift++;
+ rnmax = ((ram_size + mhd->standby_mem_size + mhd->pad_size)
+ >> mhd->increment_size);
+ if (rnmax < 0x10000) {
+ read_info->rnmax = cpu_to_be16(rnmax);
+ } else {
+ read_info->rnmax = cpu_to_be16(0);
+ read_info->rnmax2 = cpu_to_be64(rnmax);
}
- read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
- read_info->rnsize = 1 << shift;
+
+ read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
+ SCLP_FC_ASSIGN_ATTACH_READ_STOR);
+
sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
}
+static void read_storage_element0_info(SCCB *sccb)
+{
+ int i, assigned;
+ int subincrement_id = SCLP_STARTING_SUBINCREMENT_ID;
+ ReadStorageElementInfo *storage_info = (ReadStorageElementInfo *) sccb;
+ sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
+
+ if ((ram_size >> mhd->increment_size) >= 0x10000) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
+ return;
+ }
+
+ /* Return information regarding core memory */
+ storage_info->max_id = cpu_to_be16(mhd->standby_mem_size ? 1 : 0);
+ assigned = ram_size >> mhd->increment_size;
+ storage_info->assigned = cpu_to_be16(assigned);
+
+ for (i = 0; i < assigned; i++) {
+ storage_info->entries[i] = cpu_to_be32(subincrement_id);
+ subincrement_id += SCLP_INCREMENT_UNIT;
+ }
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
+}
+
+static void read_storage_element1_info(SCCB *sccb)
+{
+ ReadStorageElementInfo *storage_info = (ReadStorageElementInfo *) sccb;
+ sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
+
+ if ((mhd->standby_mem_size >> mhd->increment_size) >= 0x10000) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
+ return;
+ }
+
+ /* Return information regarding standby memory */
+ storage_info->max_id = cpu_to_be16(mhd->standby_mem_size ? 1 : 0);
+ storage_info->assigned = cpu_to_be16(mhd->standby_mem_size >>
+ mhd->increment_size);
+ storage_info->standby = cpu_to_be16(mhd->standby_mem_size >>
+ mhd->increment_size);
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_STANDBY_READ_COMPLETION);
+}
+
+static void attach_storage_element(SCCB *sccb, uint16_t element)
+{
+ int i, assigned, subincrement_id;
+ AttachStorageElement *attach_info = (AttachStorageElement *) sccb;
+ sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
+
+ if (element != 1) {
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
+ return;
+ }
+
+ assigned = mhd->standby_mem_size >> mhd->increment_size;
+ attach_info->assigned = cpu_to_be16(assigned);
+ subincrement_id = ((ram_size >> mhd->increment_size) << 16)
+ + SCLP_STARTING_SUBINCREMENT_ID;
+ for (i = 0; i < assigned; i++) {
+ attach_info->entries[i] = cpu_to_be32(subincrement_id);
+ subincrement_id += SCLP_INCREMENT_UNIT;
+ }
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+}
+
+static void assign_storage(SCCB *sccb)
+{
+ MemoryRegion *mr = NULL;
+ int this_subregion_size;
+ AssignStorage *assign_info = (AssignStorage *) sccb;
+ sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
+ ram_addr_t assign_addr = (assign_info->rn - 1) * mhd->rzm;
+ MemoryRegion *sysmem = get_system_memory();
+
+ if ((assign_addr % MEM_SECTION_SIZE == 0) &&
+ (assign_addr >= mhd->padded_ram_size)) {
+ /* Re-use existing memory region if found */
+ mr = memory_region_find(sysmem, assign_addr, 1).mr;
+ if (!mr) {
+
+ MemoryRegion *standby_ram = g_new(MemoryRegion, 1);
+
+ /* offset to align to standby_subregion_size for allocation */
+ ram_addr_t offset = assign_addr -
+ (assign_addr - mhd->padded_ram_size)
+ % mhd->standby_subregion_size;
+
+ /* strlen("standby.ram") + 4 (Max of KVM_MEMORY_SLOTS) + NULL */
+ char id[16];
+ snprintf(id, 16, "standby.ram%d",
+ (int)((offset - mhd->padded_ram_size) /
+ mhd->standby_subregion_size) + 1);
+
+ /* Allocate a subregion of the calculated standby_subregion_size */
+ if (offset + mhd->standby_subregion_size >
+ mhd->padded_ram_size + mhd->standby_mem_size) {
+ this_subregion_size = mhd->padded_ram_size +
+ mhd->standby_mem_size - offset;
+ } else {
+ this_subregion_size = mhd->standby_subregion_size;
+ }
+
+ memory_region_init_ram(standby_ram, NULL, id, this_subregion_size);
+ vmstate_register_ram_global(standby_ram);
+ memory_region_add_subregion(sysmem, offset, standby_ram);
+ }
+ /* The specified subregion is no longer in standby */
+ mhd->standby_state_map[(assign_addr - mhd->padded_ram_size)
+ / MEM_SECTION_SIZE] = 1;
+ }
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+}
+
+static void unassign_storage(SCCB *sccb)
+{
+ MemoryRegion *mr = NULL;
+ AssignStorage *assign_info = (AssignStorage *) sccb;
+ sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
+ ram_addr_t unassign_addr = (assign_info->rn - 1) * mhd->rzm;
+ MemoryRegion *sysmem = get_system_memory();
+
+ /* if the addr is a multiple of 256 MB */
+ if ((unassign_addr % MEM_SECTION_SIZE == 0) &&
+ (unassign_addr >= mhd->padded_ram_size)) {
+ mhd->standby_state_map[(unassign_addr -
+ mhd->padded_ram_size) / MEM_SECTION_SIZE] = 0;
+
+ /* find the specified memory region and destroy it */
+ mr = memory_region_find(sysmem, unassign_addr, 1).mr;
+ if (mr) {
+ int i;
+ int is_removable = 1;
+ ram_addr_t map_offset = (unassign_addr - mhd->padded_ram_size -
+ (unassign_addr - mhd->padded_ram_size)
+ % mhd->standby_subregion_size);
+ /* Mark all affected subregions as 'standby' once again */
+ for (i = 0;
+ i < (mhd->standby_subregion_size / MEM_SECTION_SIZE);
+ i++) {
+
+ if (mhd->standby_state_map[i + map_offset / MEM_SECTION_SIZE]) {
+ is_removable = 0;
+ break;
+ }
+ }
+ if (is_removable) {
+ memory_region_del_subregion(sysmem, mr);
+ memory_region_destroy(mr);
+ g_free(mr);
+ }
+ }
+ }
+ sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
+}
+
/* Provide information about the CPU */
static void sclp_read_cpu_info(SCCB *sccb)
{
@@ -103,6 +318,22 @@ static void sclp_execute(SCCB *sccb, uint32_t code)
case SCLP_CMDW_READ_CPU_INFO:
sclp_read_cpu_info(sccb);
break;
+ case SCLP_READ_STORAGE_ELEMENT_INFO:
+ if (code & 0xff00) {
+ read_storage_element1_info(sccb);
+ } else {
+ read_storage_element0_info(sccb);
+ }
+ break;
+ case SCLP_ATTACH_STORAGE_ELEMENT:
+ attach_storage_element(sccb, (code & 0xff00) >> 8);
+ break;
+ case SCLP_ASSIGN_STORAGE:
+ assign_storage(sccb);
+ break;
+ case SCLP_UNASSIGN_STORAGE:
+ unassign_storage(sccb);
+ break;
default:
efc->command_handler(ef, sccb, code);
break;
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 193eac3..ec9879a 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -1049,6 +1049,7 @@ static inline void cpu_inject_crw_mchk(S390CPU *cpu)
/* from s390-virtio-ccw */
#define MEM_SECTION_SIZE 0x10000000UL
+#define MAX_AVAIL_SLOTS 32
/* fpu_helper.c */
uint32_t set_cc_nz_f32(float32 v);
@@ -1072,6 +1073,7 @@ void kvm_s390_enable_css_support(S390CPU *cpu);
int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
int vq, bool assign);
int kvm_s390_cpu_restart(S390CPU *cpu);
+int kvm_s390_get_memslot_count(KVMState *s);
#else
static inline void kvm_s390_io_interrupt(S390CPU *cpu,
uint16_t subchannel_id,
@@ -1096,6 +1098,10 @@ static inline int kvm_s390_cpu_restart(S390CPU *cpu)
{
return -ENOSYS;
}
+static inline int kvm_s390_get_memslot_count(KVMState *s)
+{
+ return MAX_AVAIL_SLOTS;
+}
#endif
static inline int s390_cpu_restart(S390CPU *cpu)
@@ -1106,6 +1112,15 @@ static inline int s390_cpu_restart(S390CPU *cpu)
return -ENOSYS;
}
+static inline int s390_get_memslot_count(KVMState *s)
+{
+ if (kvm_enabled()) {
+ return kvm_s390_get_memslot_count(s);
+ } else {
+ return MAX_AVAIL_SLOTS;
+ }
+}
+
static inline void s390_io_interrupt(S390CPU *cpu,
uint16_t subchannel_id,
uint16_t subchannel_nr,
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index b7b0edc..ea6123c 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -959,3 +959,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
}
return kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick);
}
+
+int kvm_s390_get_memslot_count(KVMState *s)
+{
+ return kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
+}
--
1.7.9.5
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 4/4] sclp-s390: Add memory hotplug SCLPs
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 4/4] sclp-s390: Add memory hotplug SCLPs Matthew Rosato
@ 2014-05-12 7:35 ` Christian Borntraeger
2014-05-13 18:02 ` Matthew Rosato
0 siblings, 1 reply; 18+ messages in thread
From: Christian Borntraeger @ 2014-05-12 7:35 UTC (permalink / raw)
To: Matthew Rosato, qemu-devel
Cc: agraf, aliguori, imammedo, cornelia.huck, pbonzini, rth
On 07/05/14 20:05, Matthew Rosato wrote:
> Add memory information to read SCP info and add handlers for
> Read Storage Element Information, Attach Storage Element,
> Assign Storage and Unassign Storage.
>
> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
> ---
> hw/s390x/sclp.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++--
> target-s390x/cpu.h | 15 ++++
> target-s390x/kvm.c | 5 ++
> 3 files changed, 258 insertions(+), 7 deletions(-)
>
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 338dbdf..b9425ca 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -16,7 +16,8 @@
> #include "sysemu/kvm.h"
> #include "exec/memory.h"
> #include "sysemu/sysemu.h"
> -
> +#include "exec/address-spaces.h"
> +#include "qemu/config-file.h"
> #include "hw/s390x/sclp.h"
> #include "hw/s390x/event-facility.h"
>
> @@ -33,10 +34,18 @@ static inline SCLPEventFacility *get_event_facility(void)
> static void read_SCP_info(SCCB *sccb)
> {
> ReadInfo *read_info = (ReadInfo *) sccb;
> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
This only works for the ccw machine. The legacy machine does not yet have this device and qemu complains about the system bus not being hotplug capable.
Cant you just initialize the device in the ccw machine and check here only for the existence?
> CPUState *cpu;
> - int shift = 0;
> int cpu_count = 0;
> int i = 0;
> + int rnsize, rnmax;
> + QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
> + int slots = qemu_opt_get_number(opts, "slots", 0);
> + int max_avail_slots = s390_get_memslot_count(kvm_state);
> +
> + if (slots > max_avail_slots) {
> + slots = max_avail_slots;
> + }
>
> CPU_FOREACH(cpu) {
> cpu_count++;
> @@ -52,16 +61,222 @@ static void read_SCP_info(SCCB *sccb)
> read_info->entries[i].type = 0;
> }
>
> - read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
> + /*
> + * The storage increment size is a multiple of 1M and is a power of 2.
> + * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
> + */
> + while ((ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
> + mhd->increment_size++;
> + }
> + while ((mhd->standby_mem_size >> mhd->increment_size) >
> + MAX_STORAGE_INCREMENTS) {
> + mhd->increment_size++;
> + }
> +
> + mhd->standby_subregion_size = MEM_SECTION_SIZE;
> + /* Deduct the memory slot already used for core */
> + if (slots > 0) {
> + while ((mhd->standby_subregion_size * (slots - 1)
> + < mhd->standby_mem_size)) {
> + mhd->standby_subregion_size = mhd->standby_subregion_size << 1;
> + }
> + }
> + /*
> + * Initialize mapping of guest standby memory sections indicating which
> + * are and are not online. Assume all standby memory begins offline.
> + */
> + if (mhd->standby_state_map == 0) {
> + if (mhd->standby_mem_size % mhd->standby_subregion_size) {
> + mhd->standby_state_map = g_malloc0((mhd->standby_mem_size /
> + mhd->standby_subregion_size + 1) *
> + (mhd->standby_subregion_size /
> + MEM_SECTION_SIZE));
> + } else {
> + mhd->standby_state_map = g_malloc0(mhd->standby_mem_size /
> + MEM_SECTION_SIZE);
> + }
> + }
> +
> + mhd->padded_ram_size = ram_size + mhd->pad_size;
> + mhd->rzm = 1 << mhd->increment_size;
> + rnsize = 1 << (mhd->increment_size - 20);
> + if (rnsize <= 128) {
> + read_info->rnsize = rnsize;
> + } else {
> + read_info->rnsize = 0;
> + read_info->rnsize2 = cpu_to_be32(rnsize);
> + }
>
> - while ((ram_size >> (20 + shift)) > 65535) {
> - shift++;
> + rnmax = ((ram_size + mhd->standby_mem_size + mhd->pad_size)
> + >> mhd->increment_size);
> + if (rnmax < 0x10000) {
> + read_info->rnmax = cpu_to_be16(rnmax);
> + } else {
> + read_info->rnmax = cpu_to_be16(0);
> + read_info->rnmax2 = cpu_to_be64(rnmax);
> }
> - read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
> - read_info->rnsize = 1 << shift;
> +
> + read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
> + SCLP_FC_ASSIGN_ATTACH_READ_STOR);
> +
> sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
> }
>
> +static void read_storage_element0_info(SCCB *sccb)
> +{
> + int i, assigned;
> + int subincrement_id = SCLP_STARTING_SUBINCREMENT_ID;
> + ReadStorageElementInfo *storage_info = (ReadStorageElementInfo *) sccb;
> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
> +
> + if ((ram_size >> mhd->increment_size) >= 0x10000) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
> + return;
> + }
> +
> + /* Return information regarding core memory */
> + storage_info->max_id = cpu_to_be16(mhd->standby_mem_size ? 1 : 0);
> + assigned = ram_size >> mhd->increment_size;
> + storage_info->assigned = cpu_to_be16(assigned);
> +
> + for (i = 0; i < assigned; i++) {
> + storage_info->entries[i] = cpu_to_be32(subincrement_id);
> + subincrement_id += SCLP_INCREMENT_UNIT;
> + }
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
> +}
> +
> +static void read_storage_element1_info(SCCB *sccb)
> +{
> + ReadStorageElementInfo *storage_info = (ReadStorageElementInfo *) sccb;
> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
Same here.
> +
> + if ((mhd->standby_mem_size >> mhd->increment_size) >= 0x10000) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
> + return;
> + }
> +
> + /* Return information regarding standby memory */
> + storage_info->max_id = cpu_to_be16(mhd->standby_mem_size ? 1 : 0);
> + storage_info->assigned = cpu_to_be16(mhd->standby_mem_size >>
> + mhd->increment_size);
> + storage_info->standby = cpu_to_be16(mhd->standby_mem_size >>
> + mhd->increment_size);
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_STANDBY_READ_COMPLETION);
> +}
> +
> +static void attach_storage_element(SCCB *sccb, uint16_t element)
> +{
> + int i, assigned, subincrement_id;
> + AttachStorageElement *attach_info = (AttachStorageElement *) sccb;
> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
> +
> + if (element != 1) {
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> + return;
> + }
> +
> + assigned = mhd->standby_mem_size >> mhd->increment_size;
> + attach_info->assigned = cpu_to_be16(assigned);
> + subincrement_id = ((ram_size >> mhd->increment_size) << 16)
> + + SCLP_STARTING_SUBINCREMENT_ID;
> + for (i = 0; i < assigned; i++) {
> + attach_info->entries[i] = cpu_to_be32(subincrement_id);
> + subincrement_id += SCLP_INCREMENT_UNIT;
> + }
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> +}
> +
> +static void assign_storage(SCCB *sccb)
> +{
> + MemoryRegion *mr = NULL;
> + int this_subregion_size;
> + AssignStorage *assign_info = (AssignStorage *) sccb;
> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
> + ram_addr_t assign_addr = (assign_info->rn - 1) * mhd->rzm;
> + MemoryRegion *sysmem = get_system_memory();
> +
> + if ((assign_addr % MEM_SECTION_SIZE == 0) &&
> + (assign_addr >= mhd->padded_ram_size)) {
> + /* Re-use existing memory region if found */
> + mr = memory_region_find(sysmem, assign_addr, 1).mr;
> + if (!mr) {
> +
> + MemoryRegion *standby_ram = g_new(MemoryRegion, 1);
> +
> + /* offset to align to standby_subregion_size for allocation */
> + ram_addr_t offset = assign_addr -
> + (assign_addr - mhd->padded_ram_size)
> + % mhd->standby_subregion_size;
> +
> + /* strlen("standby.ram") + 4 (Max of KVM_MEMORY_SLOTS) + NULL */
> + char id[16];
> + snprintf(id, 16, "standby.ram%d",
> + (int)((offset - mhd->padded_ram_size) /
> + mhd->standby_subregion_size) + 1);
> +
> + /* Allocate a subregion of the calculated standby_subregion_size */
> + if (offset + mhd->standby_subregion_size >
> + mhd->padded_ram_size + mhd->standby_mem_size) {
> + this_subregion_size = mhd->padded_ram_size +
> + mhd->standby_mem_size - offset;
> + } else {
> + this_subregion_size = mhd->standby_subregion_size;
> + }
> +
> + memory_region_init_ram(standby_ram, NULL, id, this_subregion_size);
> + vmstate_register_ram_global(standby_ram);
> + memory_region_add_subregion(sysmem, offset, standby_ram);
> + }
> + /* The specified subregion is no longer in standby */
> + mhd->standby_state_map[(assign_addr - mhd->padded_ram_size)
> + / MEM_SECTION_SIZE] = 1;
> + }
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> +}
> +
> +static void unassign_storage(SCCB *sccb)
> +{
> + MemoryRegion *mr = NULL;
> + AssignStorage *assign_info = (AssignStorage *) sccb;
> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
> + ram_addr_t unassign_addr = (assign_info->rn - 1) * mhd->rzm;
> + MemoryRegion *sysmem = get_system_memory();
> +
> + /* if the addr is a multiple of 256 MB */
> + if ((unassign_addr % MEM_SECTION_SIZE == 0) &&
> + (unassign_addr >= mhd->padded_ram_size)) {
> + mhd->standby_state_map[(unassign_addr -
> + mhd->padded_ram_size) / MEM_SECTION_SIZE] = 0;
> +
> + /* find the specified memory region and destroy it */
> + mr = memory_region_find(sysmem, unassign_addr, 1).mr;
> + if (mr) {
> + int i;
> + int is_removable = 1;
> + ram_addr_t map_offset = (unassign_addr - mhd->padded_ram_size -
> + (unassign_addr - mhd->padded_ram_size)
> + % mhd->standby_subregion_size);
> + /* Mark all affected subregions as 'standby' once again */
> + for (i = 0;
> + i < (mhd->standby_subregion_size / MEM_SECTION_SIZE);
> + i++) {
> +
> + if (mhd->standby_state_map[i + map_offset / MEM_SECTION_SIZE]) {
> + is_removable = 0;
> + break;
> + }
> + }
> + if (is_removable) {
> + memory_region_del_subregion(sysmem, mr);
> + memory_region_destroy(mr);
> + g_free(mr);
> + }
> + }
> + }
> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
> +}
> +
> /* Provide information about the CPU */
> static void sclp_read_cpu_info(SCCB *sccb)
> {
> @@ -103,6 +318,22 @@ static void sclp_execute(SCCB *sccb, uint32_t code)
> case SCLP_CMDW_READ_CPU_INFO:
> sclp_read_cpu_info(sccb);
> break;
> + case SCLP_READ_STORAGE_ELEMENT_INFO:
> + if (code & 0xff00) {
> + read_storage_element1_info(sccb);
> + } else {
> + read_storage_element0_info(sccb);
> + }
> + break;
> + case SCLP_ATTACH_STORAGE_ELEMENT:
> + attach_storage_element(sccb, (code & 0xff00) >> 8);
> + break;
> + case SCLP_ASSIGN_STORAGE:
> + assign_storage(sccb);
> + break;
> + case SCLP_UNASSIGN_STORAGE:
> + unassign_storage(sccb);
> + break;
> default:
> efc->command_handler(ef, sccb, code);
> break;
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 193eac3..ec9879a 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -1049,6 +1049,7 @@ static inline void cpu_inject_crw_mchk(S390CPU *cpu)
>
> /* from s390-virtio-ccw */
> #define MEM_SECTION_SIZE 0x10000000UL
> +#define MAX_AVAIL_SLOTS 32
>
> /* fpu_helper.c */
> uint32_t set_cc_nz_f32(float32 v);
> @@ -1072,6 +1073,7 @@ void kvm_s390_enable_css_support(S390CPU *cpu);
> int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
> int vq, bool assign);
> int kvm_s390_cpu_restart(S390CPU *cpu);
> +int kvm_s390_get_memslot_count(KVMState *s);
> #else
> static inline void kvm_s390_io_interrupt(S390CPU *cpu,
> uint16_t subchannel_id,
> @@ -1096,6 +1098,10 @@ static inline int kvm_s390_cpu_restart(S390CPU *cpu)
> {
> return -ENOSYS;
> }
> +static inline int kvm_s390_get_memslot_count(KVMState *s)
> +{
> + return MAX_AVAIL_SLOTS;
> +}
> #endif
>
> static inline int s390_cpu_restart(S390CPU *cpu)
> @@ -1106,6 +1112,15 @@ static inline int s390_cpu_restart(S390CPU *cpu)
> return -ENOSYS;
> }
>
> +static inline int s390_get_memslot_count(KVMState *s)
> +{
> + if (kvm_enabled()) {
> + return kvm_s390_get_memslot_count(s);
> + } else {
> + return MAX_AVAIL_SLOTS;
> + }
> +}
> +
> static inline void s390_io_interrupt(S390CPU *cpu,
> uint16_t subchannel_id,
> uint16_t subchannel_nr,
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index b7b0edc..ea6123c 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -959,3 +959,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
> }
> return kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick);
> }
> +
> +int kvm_s390_get_memslot_count(KVMState *s)
> +{
> + return kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
> +}
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 4/4] sclp-s390: Add memory hotplug SCLPs
2014-05-12 7:35 ` Christian Borntraeger
@ 2014-05-13 18:02 ` Matthew Rosato
2014-05-15 12:06 ` Christian Borntraeger
0 siblings, 1 reply; 18+ messages in thread
From: Matthew Rosato @ 2014-05-13 18:02 UTC (permalink / raw)
To: Christian Borntraeger, qemu-devel
Cc: agraf, aliguori, pbonzini, cornelia.huck, imammedo, rth
On 05/12/2014 03:35 AM, Christian Borntraeger wrote:
> On 07/05/14 20:05, Matthew Rosato wrote:
>> Add memory information to read SCP info and add handlers for
>> Read Storage Element Information, Attach Storage Element,
>> Assign Storage and Unassign Storage.
>>
>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>> ---
>> hw/s390x/sclp.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>> target-s390x/cpu.h | 15 ++++
>> target-s390x/kvm.c | 5 ++
>> 3 files changed, 258 insertions(+), 7 deletions(-)
>>
>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>> index 338dbdf..b9425ca 100644
>> --- a/hw/s390x/sclp.c
>> +++ b/hw/s390x/sclp.c
>> @@ -16,7 +16,8 @@
>> #include "sysemu/kvm.h"
>> #include "exec/memory.h"
>> #include "sysemu/sysemu.h"
>> -
>> +#include "exec/address-spaces.h"
>> +#include "qemu/config-file.h"
>> #include "hw/s390x/sclp.h"
>> #include "hw/s390x/event-facility.h"
>>
>> @@ -33,10 +34,18 @@ static inline SCLPEventFacility *get_event_facility(void)
>> static void read_SCP_info(SCCB *sccb)
>> {
>> ReadInfo *read_info = (ReadInfo *) sccb;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>
>
> This only works for the ccw machine. The legacy machine does not yet have this device and qemu complains about the system bus not being hotplug capable.
> Cant you just initialize the device in the ccw machine and check here only for the existence?
Agh, forgot about the legacy machine. Sure, I can split out creation
from the get_sclp_memory_hotplug_dev() code, and then rework the code here.
But then I got to thinking -- should I not be setting
SCLP_FC_ASSIGN_ATTACH_READ_STOR below when we are running in the legacy
machine? Because without the sclp_memory_hotplug_dev, we really aren't
supporting these new functions (assign_storage, unassign_storage,
assign_storage_element, read_storage_element*_info).
Then, I can probably add some assert(mhd) for the remaining
get_sclp_memory_hotplug_dev() calls, since we shouldn't get getting
there for the legacy machine.
What do you think? Alternatively, I'd have to add code to fudge returns
for each of these new fuctions.
>
>> CPUState *cpu;
>> - int shift = 0;
>> int cpu_count = 0;
>> int i = 0;
>> + int rnsize, rnmax;
>> + QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
>> + int slots = qemu_opt_get_number(opts, "slots", 0);
>> + int max_avail_slots = s390_get_memslot_count(kvm_state);
>> +
>> + if (slots > max_avail_slots) {
>> + slots = max_avail_slots;
>> + }
>>
>> CPU_FOREACH(cpu) {
>> cpu_count++;
>> @@ -52,16 +61,222 @@ static void read_SCP_info(SCCB *sccb)
>> read_info->entries[i].type = 0;
>> }
>>
>> - read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
>> + /*
>> + * The storage increment size is a multiple of 1M and is a power of 2.
>> + * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
>> + */
>> + while ((ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
>> + mhd->increment_size++;
>> + }
>> + while ((mhd->standby_mem_size >> mhd->increment_size) >
>> + MAX_STORAGE_INCREMENTS) {
>> + mhd->increment_size++;
>> + }
>> +
>> + mhd->standby_subregion_size = MEM_SECTION_SIZE;
>> + /* Deduct the memory slot already used for core */
>> + if (slots > 0) {
>> + while ((mhd->standby_subregion_size * (slots - 1)
>> + < mhd->standby_mem_size)) {
>> + mhd->standby_subregion_size = mhd->standby_subregion_size << 1;
>> + }
>> + }
>> + /*
>> + * Initialize mapping of guest standby memory sections indicating which
>> + * are and are not online. Assume all standby memory begins offline.
>> + */
>> + if (mhd->standby_state_map == 0) {
>> + if (mhd->standby_mem_size % mhd->standby_subregion_size) {
>> + mhd->standby_state_map = g_malloc0((mhd->standby_mem_size /
>> + mhd->standby_subregion_size + 1) *
>> + (mhd->standby_subregion_size /
>> + MEM_SECTION_SIZE));
>> + } else {
>> + mhd->standby_state_map = g_malloc0(mhd->standby_mem_size /
>> + MEM_SECTION_SIZE);
>> + }
>> + }
>> +
>> + mhd->padded_ram_size = ram_size + mhd->pad_size;
>> + mhd->rzm = 1 << mhd->increment_size;
>> + rnsize = 1 << (mhd->increment_size - 20);
>> + if (rnsize <= 128) {
>> + read_info->rnsize = rnsize;
>> + } else {
>> + read_info->rnsize = 0;
>> + read_info->rnsize2 = cpu_to_be32(rnsize);
>> + }
>>
>> - while ((ram_size >> (20 + shift)) > 65535) {
>> - shift++;
>> + rnmax = ((ram_size + mhd->standby_mem_size + mhd->pad_size)
>> + >> mhd->increment_size);
>> + if (rnmax < 0x10000) {
>> + read_info->rnmax = cpu_to_be16(rnmax);
>> + } else {
>> + read_info->rnmax = cpu_to_be16(0);
>> + read_info->rnmax2 = cpu_to_be64(rnmax);
>> }
>> - read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
>> - read_info->rnsize = 1 << shift;
>> +
>> + read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>> + SCLP_FC_ASSIGN_ATTACH_READ_STOR);
>> +
>> sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
>> }
>>
>> +static void read_storage_element0_info(SCCB *sccb)
>> +{
>> + int i, assigned;
>> + int subincrement_id = SCLP_STARTING_SUBINCREMENT_ID;
>> + ReadStorageElementInfo *storage_info = (ReadStorageElementInfo *) sccb;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>> +
>> + if ((ram_size >> mhd->increment_size) >= 0x10000) {
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
>> + return;
>> + }
>> +
>> + /* Return information regarding core memory */
>> + storage_info->max_id = cpu_to_be16(mhd->standby_mem_size ? 1 : 0);
>> + assigned = ram_size >> mhd->increment_size;
>> + storage_info->assigned = cpu_to_be16(assigned);
>> +
>> + for (i = 0; i < assigned; i++) {
>> + storage_info->entries[i] = cpu_to_be32(subincrement_id);
>> + subincrement_id += SCLP_INCREMENT_UNIT;
>> + }
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
>> +}
>> +
>> +static void read_storage_element1_info(SCCB *sccb)
>> +{
>> + ReadStorageElementInfo *storage_info = (ReadStorageElementInfo *) sccb;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>
> Same here.
>
>> +
>> + if ((mhd->standby_mem_size >> mhd->increment_size) >= 0x10000) {
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
>> + return;
>> + }
>> +
>> + /* Return information regarding standby memory */
>> + storage_info->max_id = cpu_to_be16(mhd->standby_mem_size ? 1 : 0);
>> + storage_info->assigned = cpu_to_be16(mhd->standby_mem_size >>
>> + mhd->increment_size);
>> + storage_info->standby = cpu_to_be16(mhd->standby_mem_size >>
>> + mhd->increment_size);
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_STANDBY_READ_COMPLETION);
>> +}
>> +
>> +static void attach_storage_element(SCCB *sccb, uint16_t element)
>> +{
>> + int i, assigned, subincrement_id;
>> + AttachStorageElement *attach_info = (AttachStorageElement *) sccb;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>> +
>> + if (element != 1) {
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
>> + return;
>> + }
>> +
>> + assigned = mhd->standby_mem_size >> mhd->increment_size;
>> + attach_info->assigned = cpu_to_be16(assigned);
>> + subincrement_id = ((ram_size >> mhd->increment_size) << 16)
>> + + SCLP_STARTING_SUBINCREMENT_ID;
>> + for (i = 0; i < assigned; i++) {
>> + attach_info->entries[i] = cpu_to_be32(subincrement_id);
>> + subincrement_id += SCLP_INCREMENT_UNIT;
>> + }
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>> +}
>> +
>> +static void assign_storage(SCCB *sccb)
>> +{
>> + MemoryRegion *mr = NULL;
>> + int this_subregion_size;
>> + AssignStorage *assign_info = (AssignStorage *) sccb;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>> + ram_addr_t assign_addr = (assign_info->rn - 1) * mhd->rzm;
>> + MemoryRegion *sysmem = get_system_memory();
>> +
>> + if ((assign_addr % MEM_SECTION_SIZE == 0) &&
>> + (assign_addr >= mhd->padded_ram_size)) {
>> + /* Re-use existing memory region if found */
>> + mr = memory_region_find(sysmem, assign_addr, 1).mr;
>> + if (!mr) {
>> +
>> + MemoryRegion *standby_ram = g_new(MemoryRegion, 1);
>> +
>> + /* offset to align to standby_subregion_size for allocation */
>> + ram_addr_t offset = assign_addr -
>> + (assign_addr - mhd->padded_ram_size)
>> + % mhd->standby_subregion_size;
>> +
>> + /* strlen("standby.ram") + 4 (Max of KVM_MEMORY_SLOTS) + NULL */
>> + char id[16];
>> + snprintf(id, 16, "standby.ram%d",
>> + (int)((offset - mhd->padded_ram_size) /
>> + mhd->standby_subregion_size) + 1);
>> +
>> + /* Allocate a subregion of the calculated standby_subregion_size */
>> + if (offset + mhd->standby_subregion_size >
>> + mhd->padded_ram_size + mhd->standby_mem_size) {
>> + this_subregion_size = mhd->padded_ram_size +
>> + mhd->standby_mem_size - offset;
>> + } else {
>> + this_subregion_size = mhd->standby_subregion_size;
>> + }
>> +
>> + memory_region_init_ram(standby_ram, NULL, id, this_subregion_size);
>> + vmstate_register_ram_global(standby_ram);
>> + memory_region_add_subregion(sysmem, offset, standby_ram);
>> + }
>> + /* The specified subregion is no longer in standby */
>> + mhd->standby_state_map[(assign_addr - mhd->padded_ram_size)
>> + / MEM_SECTION_SIZE] = 1;
>> + }
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>> +}
>> +
>> +static void unassign_storage(SCCB *sccb)
>> +{
>> + MemoryRegion *mr = NULL;
>> + AssignStorage *assign_info = (AssignStorage *) sccb;
>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>> + ram_addr_t unassign_addr = (assign_info->rn - 1) * mhd->rzm;
>> + MemoryRegion *sysmem = get_system_memory();
>> +
>> + /* if the addr is a multiple of 256 MB */
>> + if ((unassign_addr % MEM_SECTION_SIZE == 0) &&
>> + (unassign_addr >= mhd->padded_ram_size)) {
>> + mhd->standby_state_map[(unassign_addr -
>> + mhd->padded_ram_size) / MEM_SECTION_SIZE] = 0;
>> +
>> + /* find the specified memory region and destroy it */
>> + mr = memory_region_find(sysmem, unassign_addr, 1).mr;
>> + if (mr) {
>> + int i;
>> + int is_removable = 1;
>> + ram_addr_t map_offset = (unassign_addr - mhd->padded_ram_size -
>> + (unassign_addr - mhd->padded_ram_size)
>> + % mhd->standby_subregion_size);
>> + /* Mark all affected subregions as 'standby' once again */
>> + for (i = 0;
>> + i < (mhd->standby_subregion_size / MEM_SECTION_SIZE);
>> + i++) {
>> +
>> + if (mhd->standby_state_map[i + map_offset / MEM_SECTION_SIZE]) {
>> + is_removable = 0;
>> + break;
>> + }
>> + }
>> + if (is_removable) {
>> + memory_region_del_subregion(sysmem, mr);
>> + memory_region_destroy(mr);
>> + g_free(mr);
>> + }
>> + }
>> + }
>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>> +}
>> +
>> /* Provide information about the CPU */
>> static void sclp_read_cpu_info(SCCB *sccb)
>> {
>> @@ -103,6 +318,22 @@ static void sclp_execute(SCCB *sccb, uint32_t code)
>> case SCLP_CMDW_READ_CPU_INFO:
>> sclp_read_cpu_info(sccb);
>> break;
>> + case SCLP_READ_STORAGE_ELEMENT_INFO:
>> + if (code & 0xff00) {
>> + read_storage_element1_info(sccb);
>> + } else {
>> + read_storage_element0_info(sccb);
>> + }
>> + break;
>> + case SCLP_ATTACH_STORAGE_ELEMENT:
>> + attach_storage_element(sccb, (code & 0xff00) >> 8);
>> + break;
>> + case SCLP_ASSIGN_STORAGE:
>> + assign_storage(sccb);
>> + break;
>> + case SCLP_UNASSIGN_STORAGE:
>> + unassign_storage(sccb);
>> + break;
>> default:
>> efc->command_handler(ef, sccb, code);
>> break;
>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>> index 193eac3..ec9879a 100644
>> --- a/target-s390x/cpu.h
>> +++ b/target-s390x/cpu.h
>> @@ -1049,6 +1049,7 @@ static inline void cpu_inject_crw_mchk(S390CPU *cpu)
>>
>> /* from s390-virtio-ccw */
>> #define MEM_SECTION_SIZE 0x10000000UL
>> +#define MAX_AVAIL_SLOTS 32
>>
>> /* fpu_helper.c */
>> uint32_t set_cc_nz_f32(float32 v);
>> @@ -1072,6 +1073,7 @@ void kvm_s390_enable_css_support(S390CPU *cpu);
>> int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>> int vq, bool assign);
>> int kvm_s390_cpu_restart(S390CPU *cpu);
>> +int kvm_s390_get_memslot_count(KVMState *s);
>> #else
>> static inline void kvm_s390_io_interrupt(S390CPU *cpu,
>> uint16_t subchannel_id,
>> @@ -1096,6 +1098,10 @@ static inline int kvm_s390_cpu_restart(S390CPU *cpu)
>> {
>> return -ENOSYS;
>> }
>> +static inline int kvm_s390_get_memslot_count(KVMState *s)
>> +{
>> + return MAX_AVAIL_SLOTS;
>> +}
>> #endif
>>
>> static inline int s390_cpu_restart(S390CPU *cpu)
>> @@ -1106,6 +1112,15 @@ static inline int s390_cpu_restart(S390CPU *cpu)
>> return -ENOSYS;
>> }
>>
>> +static inline int s390_get_memslot_count(KVMState *s)
>> +{
>> + if (kvm_enabled()) {
>> + return kvm_s390_get_memslot_count(s);
>> + } else {
>> + return MAX_AVAIL_SLOTS;
>> + }
>> +}
>> +
>> static inline void s390_io_interrupt(S390CPU *cpu,
>> uint16_t subchannel_id,
>> uint16_t subchannel_nr,
>> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
>> index b7b0edc..ea6123c 100644
>> --- a/target-s390x/kvm.c
>> +++ b/target-s390x/kvm.c
>> @@ -959,3 +959,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>> }
>> return kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick);
>> }
>> +
>> +int kvm_s390_get_memslot_count(KVMState *s)
>> +{
>> + return kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
>> +}
>>
>
>
>
>
^ permalink raw reply [flat|nested] 18+ messages in thread* Re: [Qemu-devel] [PATCH v3 4/4] sclp-s390: Add memory hotplug SCLPs
2014-05-13 18:02 ` Matthew Rosato
@ 2014-05-15 12:06 ` Christian Borntraeger
0 siblings, 0 replies; 18+ messages in thread
From: Christian Borntraeger @ 2014-05-15 12:06 UTC (permalink / raw)
To: Matthew Rosato, qemu-devel
Cc: agraf, aliguori, pbonzini, cornelia.huck, imammedo, rth
On 13/05/14 20:02, Matthew Rosato wrote:
> On 05/12/2014 03:35 AM, Christian Borntraeger wrote:
>> On 07/05/14 20:05, Matthew Rosato wrote:
>>> Add memory information to read SCP info and add handlers for
>>> Read Storage Element Information, Attach Storage Element,
>>> Assign Storage and Unassign Storage.
>>>
>>> Signed-off-by: Matthew Rosato <mjrosato@linux.vnet.ibm.com>
>>> ---
>>> hw/s390x/sclp.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>>> target-s390x/cpu.h | 15 ++++
>>> target-s390x/kvm.c | 5 ++
>>> 3 files changed, 258 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
>>> index 338dbdf..b9425ca 100644
>>> --- a/hw/s390x/sclp.c
>>> +++ b/hw/s390x/sclp.c
>>> @@ -16,7 +16,8 @@
>>> #include "sysemu/kvm.h"
>>> #include "exec/memory.h"
>>> #include "sysemu/sysemu.h"
>>> -
>>> +#include "exec/address-spaces.h"
>>> +#include "qemu/config-file.h"
>>> #include "hw/s390x/sclp.h"
>>> #include "hw/s390x/event-facility.h"
>>>
>>> @@ -33,10 +34,18 @@ static inline SCLPEventFacility *get_event_facility(void)
>>> static void read_SCP_info(SCCB *sccb)
>>> {
>>> ReadInfo *read_info = (ReadInfo *) sccb;
>>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>
>>
>> This only works for the ccw machine. The legacy machine does not yet have this device and qemu complains about the system bus not being hotplug capable.
>> Cant you just initialize the device in the ccw machine and check here only for the existence?
>
> Agh, forgot about the legacy machine. Sure, I can split out creation
> from the get_sclp_memory_hotplug_dev() code, and then rework the code here.
>
> But then I got to thinking -- should I not be setting
> SCLP_FC_ASSIGN_ATTACH_READ_STOR below when we are running in the legacy
> machine? Because without the sclp_memory_hotplug_dev, we really aren't
> supporting these new functions (assign_storage, unassign_storage,
> assign_storage_element, read_storage_element*_info).
> Then, I can probably add some assert(mhd) for the remaining
> get_sclp_memory_hotplug_dev() calls, since we shouldn't get getting
> there for the legacy machine.
>
> What do you think? Alternatively, I'd have to add code to fudge returns
> for each of these new fuctions.
Given our test coverage for the legacy machine, I agree to not add memory hotplug
to this machine. Please make sure to enhance read_SCP_info to be able to handle the
non-existence of the hotplug device.
>
>>
>>> CPUState *cpu;
>>> - int shift = 0;
>>> int cpu_count = 0;
>>> int i = 0;
>>> + int rnsize, rnmax;
>>> + QemuOpts *opts = qemu_opts_find(qemu_find_opts("memory"), NULL);
>>> + int slots = qemu_opt_get_number(opts, "slots", 0);
>>> + int max_avail_slots = s390_get_memslot_count(kvm_state);
>>> +
>>> + if (slots > max_avail_slots) {
>>> + slots = max_avail_slots;
>>> + }
>>>
>>> CPU_FOREACH(cpu) {
>>> cpu_count++;
>>> @@ -52,16 +61,222 @@ static void read_SCP_info(SCCB *sccb)
>>> read_info->entries[i].type = 0;
>>> }
>>>
>>> - read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO);
>>> + /*
>>> + * The storage increment size is a multiple of 1M and is a power of 2.
>>> + * The number of storage increments must be MAX_STORAGE_INCREMENTS or fewer.
>>> + */
>>> + while ((ram_size >> mhd->increment_size) > MAX_STORAGE_INCREMENTS) {
>>> + mhd->increment_size++;
>>> + }
>>> + while ((mhd->standby_mem_size >> mhd->increment_size) >
>>> + MAX_STORAGE_INCREMENTS) {
>>> + mhd->increment_size++;
>>> + }
>>> +
>>> + mhd->standby_subregion_size = MEM_SECTION_SIZE;
>>> + /* Deduct the memory slot already used for core */
>>> + if (slots > 0) {
>>> + while ((mhd->standby_subregion_size * (slots - 1)
>>> + < mhd->standby_mem_size)) {
>>> + mhd->standby_subregion_size = mhd->standby_subregion_size << 1;
>>> + }
>>> + }
>>> + /*
>>> + * Initialize mapping of guest standby memory sections indicating which
>>> + * are and are not online. Assume all standby memory begins offline.
>>> + */
>>> + if (mhd->standby_state_map == 0) {
>>> + if (mhd->standby_mem_size % mhd->standby_subregion_size) {
>>> + mhd->standby_state_map = g_malloc0((mhd->standby_mem_size /
>>> + mhd->standby_subregion_size + 1) *
>>> + (mhd->standby_subregion_size /
>>> + MEM_SECTION_SIZE));
>>> + } else {
>>> + mhd->standby_state_map = g_malloc0(mhd->standby_mem_size /
>>> + MEM_SECTION_SIZE);
>>> + }
>>> + }
>>> +
>>> + mhd->padded_ram_size = ram_size + mhd->pad_size;
>>> + mhd->rzm = 1 << mhd->increment_size;
>>> + rnsize = 1 << (mhd->increment_size - 20);
>>> + if (rnsize <= 128) {
>>> + read_info->rnsize = rnsize;
>>> + } else {
>>> + read_info->rnsize = 0;
>>> + read_info->rnsize2 = cpu_to_be32(rnsize);
>>> + }
>>>
>>> - while ((ram_size >> (20 + shift)) > 65535) {
>>> - shift++;
>>> + rnmax = ((ram_size + mhd->standby_mem_size + mhd->pad_size)
>>> + >> mhd->increment_size);
>>> + if (rnmax < 0x10000) {
>>> + read_info->rnmax = cpu_to_be16(rnmax);
>>> + } else {
>>> + read_info->rnmax = cpu_to_be16(0);
>>> + read_info->rnmax2 = cpu_to_be64(rnmax);
>>> }
>>> - read_info->rnmax = cpu_to_be16(ram_size >> (20 + shift));
>>> - read_info->rnsize = 1 << shift;
>>> +
>>> + read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
>>> + SCLP_FC_ASSIGN_ATTACH_READ_STOR);
>>> +
>>> sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
>>> }
>>>
>>> +static void read_storage_element0_info(SCCB *sccb)
>>> +{
>>> + int i, assigned;
>>> + int subincrement_id = SCLP_STARTING_SUBINCREMENT_ID;
>>> + ReadStorageElementInfo *storage_info = (ReadStorageElementInfo *) sccb;
>>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>> +
>>> + if ((ram_size >> mhd->increment_size) >= 0x10000) {
>>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
>>> + return;
>>> + }
>>> +
>>> + /* Return information regarding core memory */
>>> + storage_info->max_id = cpu_to_be16(mhd->standby_mem_size ? 1 : 0);
>>> + assigned = ram_size >> mhd->increment_size;
>>> + storage_info->assigned = cpu_to_be16(assigned);
>>> +
>>> + for (i = 0; i < assigned; i++) {
>>> + storage_info->entries[i] = cpu_to_be32(subincrement_id);
>>> + subincrement_id += SCLP_INCREMENT_UNIT;
>>> + }
>>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
>>> +}
>>> +
>>> +static void read_storage_element1_info(SCCB *sccb)
>>> +{
>>> + ReadStorageElementInfo *storage_info = (ReadStorageElementInfo *) sccb;
>>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>
>> Same here.
>>
>>> +
>>> + if ((mhd->standby_mem_size >> mhd->increment_size) >= 0x10000) {
>>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_SCCB_BOUNDARY_VIOLATION);
>>> + return;
>>> + }
>>> +
>>> + /* Return information regarding standby memory */
>>> + storage_info->max_id = cpu_to_be16(mhd->standby_mem_size ? 1 : 0);
>>> + storage_info->assigned = cpu_to_be16(mhd->standby_mem_size >>
>>> + mhd->increment_size);
>>> + storage_info->standby = cpu_to_be16(mhd->standby_mem_size >>
>>> + mhd->increment_size);
>>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_STANDBY_READ_COMPLETION);
>>> +}
>>> +
>>> +static void attach_storage_element(SCCB *sccb, uint16_t element)
>>> +{
>>> + int i, assigned, subincrement_id;
>>> + AttachStorageElement *attach_info = (AttachStorageElement *) sccb;
>>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>> +
>>> + if (element != 1) {
>>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
>>> + return;
>>> + }
>>> +
>>> + assigned = mhd->standby_mem_size >> mhd->increment_size;
>>> + attach_info->assigned = cpu_to_be16(assigned);
>>> + subincrement_id = ((ram_size >> mhd->increment_size) << 16)
>>> + + SCLP_STARTING_SUBINCREMENT_ID;
>>> + for (i = 0; i < assigned; i++) {
>>> + attach_info->entries[i] = cpu_to_be32(subincrement_id);
>>> + subincrement_id += SCLP_INCREMENT_UNIT;
>>> + }
>>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>>> +}
>>> +
>>> +static void assign_storage(SCCB *sccb)
>>> +{
>>> + MemoryRegion *mr = NULL;
>>> + int this_subregion_size;
>>> + AssignStorage *assign_info = (AssignStorage *) sccb;
>>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>> + ram_addr_t assign_addr = (assign_info->rn - 1) * mhd->rzm;
>>> + MemoryRegion *sysmem = get_system_memory();
>>> +
>>> + if ((assign_addr % MEM_SECTION_SIZE == 0) &&
>>> + (assign_addr >= mhd->padded_ram_size)) {
>>> + /* Re-use existing memory region if found */
>>> + mr = memory_region_find(sysmem, assign_addr, 1).mr;
>>> + if (!mr) {
>>> +
>>> + MemoryRegion *standby_ram = g_new(MemoryRegion, 1);
>>> +
>>> + /* offset to align to standby_subregion_size for allocation */
>>> + ram_addr_t offset = assign_addr -
>>> + (assign_addr - mhd->padded_ram_size)
>>> + % mhd->standby_subregion_size;
>>> +
>>> + /* strlen("standby.ram") + 4 (Max of KVM_MEMORY_SLOTS) + NULL */
>>> + char id[16];
>>> + snprintf(id, 16, "standby.ram%d",
>>> + (int)((offset - mhd->padded_ram_size) /
>>> + mhd->standby_subregion_size) + 1);
>>> +
>>> + /* Allocate a subregion of the calculated standby_subregion_size */
>>> + if (offset + mhd->standby_subregion_size >
>>> + mhd->padded_ram_size + mhd->standby_mem_size) {
>>> + this_subregion_size = mhd->padded_ram_size +
>>> + mhd->standby_mem_size - offset;
>>> + } else {
>>> + this_subregion_size = mhd->standby_subregion_size;
>>> + }
>>> +
>>> + memory_region_init_ram(standby_ram, NULL, id, this_subregion_size);
>>> + vmstate_register_ram_global(standby_ram);
>>> + memory_region_add_subregion(sysmem, offset, standby_ram);
>>> + }
>>> + /* The specified subregion is no longer in standby */
>>> + mhd->standby_state_map[(assign_addr - mhd->padded_ram_size)
>>> + / MEM_SECTION_SIZE] = 1;
>>> + }
>>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>>> +}
>>> +
>>> +static void unassign_storage(SCCB *sccb)
>>> +{
>>> + MemoryRegion *mr = NULL;
>>> + AssignStorage *assign_info = (AssignStorage *) sccb;
>>> + sclpMemoryHotplugDev *mhd = get_sclp_memory_hotplug_dev();
>>> + ram_addr_t unassign_addr = (assign_info->rn - 1) * mhd->rzm;
>>> + MemoryRegion *sysmem = get_system_memory();
>>> +
>>> + /* if the addr is a multiple of 256 MB */
>>> + if ((unassign_addr % MEM_SECTION_SIZE == 0) &&
>>> + (unassign_addr >= mhd->padded_ram_size)) {
>>> + mhd->standby_state_map[(unassign_addr -
>>> + mhd->padded_ram_size) / MEM_SECTION_SIZE] = 0;
>>> +
>>> + /* find the specified memory region and destroy it */
>>> + mr = memory_region_find(sysmem, unassign_addr, 1).mr;
>>> + if (mr) {
>>> + int i;
>>> + int is_removable = 1;
>>> + ram_addr_t map_offset = (unassign_addr - mhd->padded_ram_size -
>>> + (unassign_addr - mhd->padded_ram_size)
>>> + % mhd->standby_subregion_size);
>>> + /* Mark all affected subregions as 'standby' once again */
>>> + for (i = 0;
>>> + i < (mhd->standby_subregion_size / MEM_SECTION_SIZE);
>>> + i++) {
>>> +
>>> + if (mhd->standby_state_map[i + map_offset / MEM_SECTION_SIZE]) {
>>> + is_removable = 0;
>>> + break;
>>> + }
>>> + }
>>> + if (is_removable) {
>>> + memory_region_del_subregion(sysmem, mr);
>>> + memory_region_destroy(mr);
>>> + g_free(mr);
>>> + }
>>> + }
>>> + }
>>> + sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_COMPLETION);
>>> +}
>>> +
>>> /* Provide information about the CPU */
>>> static void sclp_read_cpu_info(SCCB *sccb)
>>> {
>>> @@ -103,6 +318,22 @@ static void sclp_execute(SCCB *sccb, uint32_t code)
>>> case SCLP_CMDW_READ_CPU_INFO:
>>> sclp_read_cpu_info(sccb);
>>> break;
>>> + case SCLP_READ_STORAGE_ELEMENT_INFO:
>>> + if (code & 0xff00) {
>>> + read_storage_element1_info(sccb);
>>> + } else {
>>> + read_storage_element0_info(sccb);
>>> + }
>>> + break;
>>> + case SCLP_ATTACH_STORAGE_ELEMENT:
>>> + attach_storage_element(sccb, (code & 0xff00) >> 8);
>>> + break;
>>> + case SCLP_ASSIGN_STORAGE:
>>> + assign_storage(sccb);
>>> + break;
>>> + case SCLP_UNASSIGN_STORAGE:
>>> + unassign_storage(sccb);
>>> + break;
>>> default:
>>> efc->command_handler(ef, sccb, code);
>>> break;
>>> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
>>> index 193eac3..ec9879a 100644
>>> --- a/target-s390x/cpu.h
>>> +++ b/target-s390x/cpu.h
>>> @@ -1049,6 +1049,7 @@ static inline void cpu_inject_crw_mchk(S390CPU *cpu)
>>>
>>> /* from s390-virtio-ccw */
>>> #define MEM_SECTION_SIZE 0x10000000UL
>>> +#define MAX_AVAIL_SLOTS 32
>>>
>>> /* fpu_helper.c */
>>> uint32_t set_cc_nz_f32(float32 v);
>>> @@ -1072,6 +1073,7 @@ void kvm_s390_enable_css_support(S390CPU *cpu);
>>> int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>>> int vq, bool assign);
>>> int kvm_s390_cpu_restart(S390CPU *cpu);
>>> +int kvm_s390_get_memslot_count(KVMState *s);
>>> #else
>>> static inline void kvm_s390_io_interrupt(S390CPU *cpu,
>>> uint16_t subchannel_id,
>>> @@ -1096,6 +1098,10 @@ static inline int kvm_s390_cpu_restart(S390CPU *cpu)
>>> {
>>> return -ENOSYS;
>>> }
>>> +static inline int kvm_s390_get_memslot_count(KVMState *s)
>>> +{
>>> + return MAX_AVAIL_SLOTS;
>>> +}
>>> #endif
>>>
>>> static inline int s390_cpu_restart(S390CPU *cpu)
>>> @@ -1106,6 +1112,15 @@ static inline int s390_cpu_restart(S390CPU *cpu)
>>> return -ENOSYS;
>>> }
>>>
>>> +static inline int s390_get_memslot_count(KVMState *s)
>>> +{
>>> + if (kvm_enabled()) {
>>> + return kvm_s390_get_memslot_count(s);
>>> + } else {
>>> + return MAX_AVAIL_SLOTS;
>>> + }
>>> +}
>>> +
>>> static inline void s390_io_interrupt(S390CPU *cpu,
>>> uint16_t subchannel_id,
>>> uint16_t subchannel_nr,
>>> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
>>> index b7b0edc..ea6123c 100644
>>> --- a/target-s390x/kvm.c
>>> +++ b/target-s390x/kvm.c
>>> @@ -959,3 +959,8 @@ int kvm_s390_assign_subch_ioeventfd(EventNotifier *notifier, uint32_t sch,
>>> }
>>> return kvm_vm_ioctl(kvm_state, KVM_IOEVENTFD, &kick);
>>> }
>>> +
>>> +int kvm_s390_get_memslot_count(KVMState *s)
>>> +{
>>> + return kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
>>> +}
>>>
>>
>>
>>
>>
>
^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] s390: Support for Hotplug of Standby Memory
2014-05-07 18:05 [Qemu-devel] [PATCH v3 0/4] s390: Support for Hotplug of Standby Memory Matthew Rosato
` (3 preceding siblings ...)
2014-05-07 18:05 ` [Qemu-devel] [PATCH v3 4/4] sclp-s390: Add memory hotplug SCLPs Matthew Rosato
@ 2014-05-12 7:46 ` Christian Borntraeger
4 siblings, 0 replies; 18+ messages in thread
From: Christian Borntraeger @ 2014-05-12 7:46 UTC (permalink / raw)
To: Matthew Rosato, qemu-devel
Cc: agraf, aliguori, imammedo, Cornelia Huck, pbonzini, rth
On 07/05/14 20:05, Matthew Rosato wrote:
> This patchset adds support in s390 for a pool of standby memory,
> which can be set online/offline by the guest (ie, via chmem).
> New options, maxmem and slots, are added to the QEMU command line
> memory parameter to specify the total amount of memory available
> to the guest as well as the number of memory slots available.
> As part of this work, additional results are provided for the
> Read SCP Information SCLP, and new implentation is added for the
> Read Storage Element Information, Attach Storage Element,
> Assign Storage and Unassign Storage SCLPs, which enables the s390
> guest to manipulate the standby memory pool.
>
> This patchset is based on work originally done by Jeng-Fang (Nick)
> Wang.
>
> Changes for v3:
> * Remove some patches shared with ACPI memory hotplug that are now
> applied to master
> * Fix in sclp.h to avoid compiler error for s390x-linux-user mode
> * Fix in s390-virtio-ccw.c to avoid checking the value of
> mhd->increment_size before it's been set with a meaningful
> value.
> * Add MAX_STORAGE_INCREMENTS
>
> Changes for v2:
> * Removed the patch that introduced the standby-mem operand and
> instead included Igor Mammedov's patches that add the mem-opts
> 'maxmem' and 'slots', with a slight modification due to the removal
> of qemu_opts_create_nofail.
> * Patch 3 was inserted to add a new qom object that encapsulate variables
> used by s390 memory hotplug. Patches 4 and 5 adjusted to use this
> object.
> * Added additional code comments and other minor changes per Alexander
> Graf's comments
>
> Igor Mammedov (1):
> vl.c: extend -m option to support options for memory hotplug
>
> Matthew Rosato (3):
> sclp-s390: Add device to manage s390 memory hotplug
> virtio-ccw: Include standby memory when calculating storage increment
> sclp-s390: Add memory hotplug SCLPs
>
> hw/s390x/s390-virtio-ccw.c | 44 +++++--
> hw/s390x/sclp.c | 277 ++++++++++++++++++++++++++++++++++++++++++--
> include/hw/boards.h | 2 +
> include/hw/s390x/sclp.h | 19 +++
> qemu-options.hx | 9 +-
> target-s390x/cpu.h | 18 +++
> target-s390x/kvm.c | 5 +
> vl.c | 51 ++++++++
> 8 files changed, 407 insertions(+), 18 deletions(-)
>
I think the patch set is shaping up quite well. two things:
1. I had some small comments to the patches, please have a look
2. Igor, what are your plans on pushing the first patch (and implement hotplug for x86) Could you push patch1 independently from the other patches?
^ permalink raw reply [flat|nested] 18+ messages in thread