Skip to contentSkip to conversation

s390x: fix memory detection for guests > 64GB

Qemu-devel In review Christian Borntraeger 2 participants 9 messages May 10, 2011 – May 17, 2011

lore
Share & tools
lore

Christian Borntraeger

author
#0 lore raw

Alex,

the s390 memory detection has a 16bit field that specifies the amount of increments. This patch adopts the memory size to always fit into that scheme. This also fixes virtio detection for these guests, since the descriptor page is located after the main memory.

Signed-off-by Christian Borntraeger <[email protected]>
target-s390x/op_helper.c +5 -2
@@ -2361,6 +2361,7 @@ static void ext_interrupt(CPUState *env, int type, uint32_t param,
 int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
 {
     int r = 0;
+    int shift = 0;
 
 #ifdef DEBUG_HELPER
     printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code);

@@ -2375,8 +2376,10 @@ int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
     switch(code) {
         case SCLP_CMDW_READ_SCP_INFO:
         case SCLP_CMDW_READ_SCP_INFO_FORCED:
-            stw_phys(sccb + SCP_MEM_CODE, ram_size >> 20);
-            stb_phys(sccb + SCP_INCREMENT, 1);
+            while ((ram_size >> (20 + shift)) > 65535)
+               shift++;
+            stw_phys(sccb + SCP_MEM_CODE, ram_size >> (20 + shift));
+            stb_phys(sccb + SCP_INCREMENT, 1 << shift);
             stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
 
             if (kvm_enabled()) {
vl.c +10 -0
index 68c3b53..7b4adff 100644
@@ -2962,6 +2962,16 @@ int main(int argc, char **argv, char **envp)
     if (ram_size == 0)
         ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;
 
+    /* s390x ram size detection needs a 16bit multiplier + an increment. So
+       guests > 64GB can be specified in 2MB steps etc */
+    if (strstr(machine->name, "s390")) { 
+        int shift = 0;
+
+        while ((ram_size >> (20 + shift)) > 65535)
+               shift++;
+        ram_size = ram_size >> (20 + shift) << (20 + shift);
+    }
+
     /* init the dynamic translator */
     cpu_exec_init_all(tb_size * 1024 * 1024);
#1 lore raw
Quoted 20 lines · Christian Borntraeger

Alex,

the s390 memory detection has a 16bit field that specifies the amount of increments. This patch adopts the memory size to always fit into that scheme. This also fixes virtio detection for these guests, since the descriptor page is located after the main memory.

Signed-off-by Christian Borntraeger <[email protected]>
target-s390x/op_helper.c +1 -0
@@ -2361,6 +2361,7 @@ static void ext_interrupt(CPUState *env, int type, uint32_t param,
 int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
 {
    int r = 0;
+    int shift = 0;
 
 #ifdef DEBUG_HELPER
    printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code);

target-s390x/op_helper.c int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code) · line 2380

Quoted 7 lines
target-s390x/op_helper.c +2 -2
@@ -2375,8 +2376,10 @@ int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
    switch(code) {
        case SCLP_CMDW_READ_SCP_INFO:
        case SCLP_CMDW_READ_SCP_INFO_FORCED:
-            stw_phys(sccb + SCP_MEM_CODE, ram_size >> 20);
-            stb_phys(sccb + SCP_INCREMENT, 1);
+            while ((ram_size >> (20 + shift)) > 65535)
+               shift++;

Please run scripts/checkpatch.pl on your patch.

Alex

author
#2 lore raw

PATCH v2 1/1 s390x: fix memory detection for guests > 64GB

the s390 memory detection has a 16bit field that specifies the amount of increments. This patch adopts the memory size to always fit into that scheme. This also fixes virtio detection for these guests, since the descriptor page is located after the main memory.

Signed-off-by Christian Borntraeger <[email protected]>
---
 target-s390x/op_helper.c |    8 ++++++--
 vl.c                     |   11 +++++++++++
 2 files changed, 17 insertions(+), 2 deletions(-)
target-s390x/op_helper.c +6 -2
Index: b/target-s390x/op_helper.c
===================================================================
@@ -2361,6 +2361,7 @@ static void ext_interrupt(CPUState *env,
 int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
 {
     int r = 0;
+    int shift = 0;
 
 #ifdef DEBUG_HELPER
     printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code);

@@ -2375,8 +2376,11 @@ int sclp_service_call(CPUState *env, uin
     switch(code) {
         case SCLP_CMDW_READ_SCP_INFO:
         case SCLP_CMDW_READ_SCP_INFO_FORCED:
-            stw_phys(sccb + SCP_MEM_CODE, ram_size >> 20);
-            stb_phys(sccb + SCP_INCREMENT, 1);
+            while ((ram_size >> (20 + shift)) > 65535) {
+                shift++;
+            }
+            stw_phys(sccb + SCP_MEM_CODE, ram_size >> (20 + shift));
+            stb_phys(sccb + SCP_INCREMENT, 1 << shift);
             stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
 
             if (kvm_enabled()) {
vl.c +11 -0
Index: b/vl.c
===================================================================
@@ -2962,6 +2962,17 @@ int main(int argc, char **argv, char **e
     if (ram_size == 0)
         ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;
 
+    /* s390x ram size detection needs a 16bit multiplier + an increment. So
+       guests > 64GB can be specified in 2MB steps etc */
+    if (strstr(machine->name, "s390")) {
+        int shift = 0;
+
+        while ((ram_size >> (20 + shift)) > 65535) {
+            shift++;
+        }
+        ram_size = ram_size >> (20 + shift) << (20 + shift);
+    }
+
     /* init the dynamic translator */
     cpu_exec_init_all(tb_size * 1024 * 1024);
#3 lore raw
Quoted 45 lines · Christian Borntraeger

the s390 memory detection has a 16bit field that specifies the amount of increments. This patch adopts the memory size to always fit into that scheme. This also fixes virtio detection for these guests, since the descriptor page is located after the main memory.

Signed-off-by Christian Borntraeger <[email protected]>

---
target-s390x/op_helper.c | 8 ++++++--
vl.c | 11 +++++++++++

2 files changed, 17 insertions(+), 2 deletions(-)

Index: b/target-s390x/op_helper.c
===================================================================

target-s390x/op_helper.c +6 -2
@@ -2361,6 +2361,7 @@ static void ext_interrupt(CPUState *env,
 int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
 {
    int r = 0;
+    int shift = 0;
 
 #ifdef DEBUG_HELPER
    printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code);

@@ -2375,8 +2376,11 @@ int sclp_service_call(CPUState *env, uin
    switch(code) {
        case SCLP_CMDW_READ_SCP_INFO:
        case SCLP_CMDW_READ_SCP_INFO_FORCED:
-            stw_phys(sccb + SCP_MEM_CODE, ram_size >> 20);
-            stb_phys(sccb + SCP_INCREMENT, 1);
+            while ((ram_size >> (20 + shift)) > 65535) {
+                shift++;
+            }
+            stw_phys(sccb + SCP_MEM_CODE, ram_size >> (20 + shift));
+            stb_phys(sccb + SCP_INCREMENT, 1 << shift);
            stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
 
            if (kvm_enabled()) {

Index: b/vl.c
===================================================================

vl.c +0 -0
@@ -2962,6 +2962,17 @@ int main(int argc, char **argv, char **e
    if (ram_size == 0)
        ram_size = DEFAULT_RAM_SIZE * 1024 * 1024;

vl.c int main(int argc, char **argv, char **e · line 2973

Quoted 10 lines
vl.c +9 -0
 
+    /* s390x ram size detection needs a 16bit multiplier + an increment. So
+       guests > 64GB can be specified in 2MB steps etc */
+    if (strstr(machine->name, "s390")) {
+        int shift = 0;
+
+        while ((ram_size >> (20 + shift)) > 65535) {
+            shift++;
+        }
+        ram_size = ram_size >> (20 + shift) << (20 + shift);

This one really belongs to hw/s390-virtio.c. Just move the same code to s390_init there and it should work out. The less hacks we can have in generic code, the better :).

Alex

author
#4 lore raw
Quoted 11 lines · Alexander Graf

vl.c int main(int argc, char **argv, char **e · line 2973

Quoted 9 lines
vl.c +9 -0
+    /* s390x ram size detection needs a 16bit multiplier + an increment. So
+       guests > 64GB can be specified in 2MB steps etc */
+    if (strstr(machine->name, "s390")) {
+        int shift = 0;
+
+        while ((ram_size >> (20 + shift)) > 65535) {
+            shift++;
+        }
+        ram_size = ram_size >> (20 + shift) << (20 + shift);

This one really belongs to hw/s390-virtio.c. Just move the same code to s390_init there and it should work out. The less hacks we can have in generic code, the better :).

Unfortunately this does not work. The slcp op helper uses the global variable ram_size fro vl.c. I could change the global ram_size in s390_init, but this is also pretty ugly.

Christian

#5 lore raw
Quoted 11 lines · Christian Borntraeger
Quoted 10 lines · Alexander Graf
Quoted 9 lines
vl.c +9 -0
+    /* s390x ram size detection needs a 16bit multiplier + an increment. So
+       guests > 64GB can be specified in 2MB steps etc */
+    if (strstr(machine->name, "s390")) {
+        int shift = 0;
+
+        while ((ram_size >> (20 + shift)) > 65535) {
+            shift++;
+        }
+        ram_size = ram_size >> (20 + shift) << (20 + shift);
Quoted 5 lines
Quoted 1 line

This one really belongs to hw/s390-virtio.c. Just move the same code to s390_init there and it should work out. The less hacks we can have in generic code, the better :).

Unfortunately this does not work. The slcp op helper uses the global variable ram_size fro vl.c. I could change the global ram_size in s390_init, but this is also pretty ugly.

I'd definitely prefer that over the change on vl.c :)

Alex

author
#6 lore raw
Quoted 1 line

I'd definitely prefer that over the change on vl.c :)

the s390 memory detection has a 16bit field that specifies the amount of increments. This patch adopts the memory size to always fit into that scheme. This also fixes virtio detection for these guests, since the descriptor page is located after the main memory.

Signed-off-by Christian Borntraeger <[email protected]>
---
 hw/s390-virtio.c         |   20 +++++++++++++++-----
 target-s390x/op_helper.c |    8 ++++++--
 2 files changed, 21 insertions(+), 7 deletions(-)
hw/s390-virtio.c +15 -5
Index: b/hw/s390-virtio.c
===================================================================
@@ -133,7 +133,7 @@ int s390_virtio_hypercall(CPUState *env,
 }
 
 /* PC hardware initialisation */
-static void s390_init(ram_addr_t ram_size,
+static void s390_init(ram_addr_t my_ram_size,
                       const char *boot_device,
                       const char *kernel_filename,
                       const char *kernel_cmdline,

@@ -145,19 +145,29 @@ static void s390_init(ram_addr_t ram_siz
     ram_addr_t kernel_size = 0;
     ram_addr_t initrd_offset;
     ram_addr_t initrd_size = 0;
+    int shift = 0;
     uint8_t *storage_keys;
     int i;
 
+    /* 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++;
+    }
+    my_ram_size = my_ram_size >> (20 + shift) << (20 + shift);
+
+    /* lets propagate the changed ram size into the global variable. */
+    ram_size = my_ram_size;
 
     /* get a BUS */
-    s390_bus = s390_virtio_bus_init(&ram_size);
+    s390_bus = s390_virtio_bus_init(&my_ram_size);
 
     /* allocate RAM */
-    ram_addr = qemu_ram_alloc(NULL, "s390.ram", ram_size);
-    cpu_register_physical_memory(0, ram_size, ram_addr);
+    ram_addr = qemu_ram_alloc(NULL, "s390.ram", my_ram_size);
+    cpu_register_physical_memory(0, my_ram_size, ram_addr);
 
     /* allocate storage keys */
-    storage_keys = qemu_mallocz(ram_size / TARGET_PAGE_SIZE);
+    storage_keys = qemu_mallocz(my_ram_size / TARGET_PAGE_SIZE);
 
     /* init CPUs */
     if (cpu_model == NULL) {
target-s390x/op_helper.c +6 -2
Index: b/target-s390x/op_helper.c
===================================================================
@@ -2361,6 +2361,7 @@ static void ext_interrupt(CPUState *env,
 int sclp_service_call(CPUState *env, uint32_t sccb, uint64_t code)
 {
     int r = 0;
+    int shift = 0;
 
 #ifdef DEBUG_HELPER
     printf("sclp(0x%x, 0x%" PRIx64 ")\n", sccb, code);

@@ -2375,8 +2376,11 @@ int sclp_service_call(CPUState *env, uin
     switch(code) {
         case SCLP_CMDW_READ_SCP_INFO:
         case SCLP_CMDW_READ_SCP_INFO_FORCED:
-            stw_phys(sccb + SCP_MEM_CODE, ram_size >> 20);
-            stb_phys(sccb + SCP_INCREMENT, 1);
+            while ((ram_size >> (20 + shift)) > 65535) {
+                shift++;
+            }
+            stw_phys(sccb + SCP_MEM_CODE, ram_size >> (20 + shift));
+            stb_phys(sccb + SCP_INCREMENT, 1 << shift);
             stw_phys(sccb + SCP_RESPONSE_CODE, 0x10);
 
             if (kvm_enabled()) {
#7 lore raw
Quoted 6 lines · Christian Borntraeger
Quoted 1 line

I'd definitely prefer that over the change on vl.c :)

the s390 memory detection has a 16bit field that specifies the amount of increments. This patch adopts the memory size to always fit into that scheme. This also fixes virtio detection for these guests, since the descriptor page is located after the main memory.

How did you test the patch?

agraf@s390t27:/home/agraf/git/qemu> ./s390x-softmmu/qemu-system-s390x -snapshot -nographic /media/studio/images/SUSE/s390/sles11.raw -kernel /boot/image -m 3600 -enable-kvm

[works]

agraf@s390t27:/home/agraf/git/qemu> ./s390x-softmmu/qemu-system-s390x -snapshot -nographic /media/studio/images/SUSE/s390/sles11.raw -kernel /boot/image -m 3700 -enable-kvm
qemu_madvise: Invalid argument
Need MADV_DONTFORK in absence of synchronous KVM MMU

agraf@s390t27:/home/agraf/git/qemu> ./s390x-softmmu/qemu-system-s390x -snapshot -nographic /media/studio/images/SUSE/s390/sles11.raw -kernel /boot/image -m 3700 Segmentation fault

Alex

#8 lore raw
Quoted 14 lines · Alexander Graf
Quoted 6 lines · Christian Borntraeger
Quoted 1 line

I'd definitely prefer that over the change on vl.c :)

the s390 memory detection has a 16bit field that specifies the amount of increments. This patch adopts the memory size to always fit into that scheme. This also fixes virtio detection for these guests, since the descriptor page is located after the main memory.

How did you test the patch?

agraf@s390t27:/home/agraf/git/qemu> ./s390x-softmmu/qemu-system-s390x -snapshot -nographic /media/studio/images/SUSE/s390/sles11.raw -kernel /boot/image -m 3600 -enable-kvm

Quoted 7 lines

[works]

agraf@s390t27:/home/agraf/git/qemu> ./s390x-softmmu/qemu-system-s390x -snapshot -nographic /media/studio/images/SUSE/s390/sles11.raw -kernel /boot/image -m 3700 -enable-kvm
qemu_madvise: Invalid argument
Need MADV_DONTFORK in absence of synchronous KVM MMU

agraf@s390t27:/home/agraf/git/qemu> ./s390x-softmmu/qemu-system-s390x -snapshot -nographic /media/studio/images/SUSE/s390/sles11.raw -kernel /boot/image -m 3700 Segmentation fault

Ah, my fault - I had too little swap space :). Now I can even run an emulated VM with >64GB of RAM - yay!

Alex