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 <mjros...@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);
>>
>
>
>
>
