[Qemu-devel] [PATCH 0/2] tcg: Fix i686 booting sparc64 openbios

[Qemu-devel] [PATCH 0/2] tcg: Fix i686 booting sparc64 openbios

[Richard Henderson]

There's a minor typo here that affects dumping of 64-bit
registers on 32-bit hosts.  Kind of embarrasing that this
hasn't been seen previously.

The main change takes care of cases wherein there's overlap
between the indirect base register and the main global, which
can happen in conditions of very high register pressure.

The bug report is at

http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg04947.html


r~


Richard Henderson (2):
  tcg: Fix name for high-half register
  tcg: Fix allocation of indirect_base registers

 tcg/tcg.c | 70 +++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 48 insertions(+), 22 deletions(-)

-- 
2.5.5

[Qemu-devel] [PATCH 1/2] tcg: Fix name for high-half register

[Richard Henderson]

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 254427b..154ffe8 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -557,7 +557,7 @@ int tcg_global_mem_new_internal(TCGType type, TCGv_ptr base,
         ts2->mem_offset = offset + (1 - bigendian) * 4;
         pstrcpy(buf, sizeof(buf), name);
         pstrcat(buf, sizeof(buf), "_1");
-        ts->name = strdup(buf);
+        ts2->name = strdup(buf);
     } else {
         ts->base_type = type;
         ts->type = type;
-- 
2.5.5

[Qemu-devel] [PATCH 2/2] tcg: Fix allocation of indirect_base registers

[Richard Henderson]

When the number of available registers is low, we need to be
prepared for TS to overlap MEM_BASE.

This fixes the Sparc64 OpenBIOS boot on i686.

Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 tcg/tcg.c | 68 +++++++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 47 insertions(+), 21 deletions(-)

diff --git a/tcg/tcg.c b/tcg/tcg.c
index 154ffe8..6c26ee4 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -1743,32 +1743,71 @@ static TCGReg tcg_reg_alloc(TCGContext *s, TCGRegSet desired_regs,
     tcg_abort();
 }
 
+/* Mark a temporary as dead.  */
+static void temp_dead(TCGContext *s, TCGTemp *ts)
+{
+    if (ts->fixed_reg) {
+        return;
+    }
+    if (ts->val_type == TEMP_VAL_REG) {
+        s->reg_to_temp[ts->reg] = NULL;
+    }
+    ts->val_type = (temp_idx(s, ts) < s->nb_globals || ts->temp_local
+                    ? TEMP_VAL_MEM : TEMP_VAL_DEAD);
+}
+
 /* Make sure the temporary is in a register.  If needed, allocate the register
    from DESIRED while avoiding ALLOCATED.  */
 static void temp_load(TCGContext *s, TCGTemp *ts, TCGRegSet desired_regs,
                       TCGRegSet allocated_regs)
 {
-    TCGReg reg;
+    TCGReg reg, base_reg;
+    TCGTemp *base;
 
     switch (ts->val_type) {
     case TEMP_VAL_REG:
         return;
+
     case TEMP_VAL_CONST:
         reg = tcg_reg_alloc(s, desired_regs, allocated_regs, ts->indirect_base);
         tcg_out_movi(s, ts->type, reg, ts->val);
         ts->mem_coherent = 0;
         break;
+
     case TEMP_VAL_MEM:
-        reg = tcg_reg_alloc(s, desired_regs, allocated_regs, ts->indirect_base);
-        if (ts->indirect_reg) {
-            tcg_regset_set_reg(allocated_regs, reg);
-            temp_load(s, ts->mem_base,
-                      tcg_target_available_regs[TCG_TYPE_PTR],
-                      allocated_regs);
+        base = ts->mem_base;
+        base_reg = base->reg;
+        if (ts->indirect_reg && base->val_type != TEMP_VAL_REG) {
+            TCGRegSet reg_avail = desired_regs & ~allocated_regs;
+            TCGRegSet base_avail = (tcg_target_available_regs[TCG_TYPE_PTR]
+                                    & ~allocated_regs);
+            if (is_power_of_2(reg_avail)) {
+                /* There is only one register available for TS.  If there are
+                   other available registers for BASE, make sure we pick one
+                   of them.  Otherwise BASE and TS will share a reg.  */
+                TCGRegSet avail = base_avail & ~reg_avail;
+                if (avail) {
+                    base_avail = avail;
+                }
+                temp_load(s, base, base_avail, allocated_regs);
+                base_reg = base->reg;
+            } else {
+                temp_load(s, base, base_avail, allocated_regs);
+                base_reg = base->reg;
+                tcg_regset_reset_reg(allocated_regs, base_reg);
+            }
         }
-        tcg_out_ld(s, ts->type, reg, ts->mem_base->reg, ts->mem_offset);
+
+        reg = tcg_reg_alloc(s, desired_regs, allocated_regs, ts->indirect_base);
+        tcg_out_ld(s, ts->type, reg, base_reg, ts->mem_offset);
         ts->mem_coherent = 1;
+
+        /* If the registers overlap, zap the info from BASE.  */
+        if (reg == base_reg) {
+            temp_dead(s, base);
+        }
         break;
+
     case TEMP_VAL_DEAD:
     default:
         tcg_abort();
@@ -1778,19 +1817,6 @@ static void temp_load(TCGContext *s, TCGTemp *ts, TCGRegSet desired_regs,
     s->reg_to_temp[reg] = ts;
 }
 
-/* mark a temporary as dead. */
-static inline void temp_dead(TCGContext *s, TCGTemp *ts)
-{
-    if (ts->fixed_reg) {
-        return;
-    }
-    if (ts->val_type == TEMP_VAL_REG) {
-        s->reg_to_temp[ts->reg] = NULL;
-    }
-    ts->val_type = (temp_idx(s, ts) < s->nb_globals || ts->temp_local
-                    ? TEMP_VAL_MEM : TEMP_VAL_DEAD);
-}
-
 /* sync a temporary to memory. 'allocated_regs' is used in case a
    temporary registers needs to be allocated to store a constant. */
 static void temp_sync(TCGContext *s, TCGTemp *ts, TCGRegSet allocated_regs)
-- 
2.5.5

Re: [Qemu-devel] [PATCH 1/2] tcg: Fix name for high-half register

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

On Fri, Jun 17, 2016 at 10:03:26PM -0700, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>
Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  tcg/tcg.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tcg/tcg.c b/tcg/tcg.c
> index 254427b..154ffe8 100644
> --- a/tcg/tcg.c
> +++ b/tcg/tcg.c
> @@ -557,7 +557,7 @@ int tcg_global_mem_new_internal(TCGType type, TCGv_ptr base,
>          ts2->mem_offset = offset + (1 - bigendian) * 4;
>          pstrcpy(buf, sizeof(buf), name);
>          pstrcat(buf, sizeof(buf), "_1");
> -        ts->name = strdup(buf);
> +        ts2->name = strdup(buf);
>      } else {
>          ts->base_type = type;
>          ts->type = type;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [Qemu-devel] [PATCH 0/2] tcg: Fix i686 booting sparc64 openbios

[Thomas Huth]

On 18.06.2016 07:03, Richard Henderson wrote:
> There's a minor typo here that affects dumping of 64-bit
> registers on 32-bit hosts.  Kind of embarrasing that this
> hasn't been seen previously.
> 
> The main change takes care of cases wherein there's overlap
> between the indirect base register and the main global, which
> can happen in conditions of very high register pressure.
> 
> The bug report is at
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg04947.html

With these two patches applied, the segfault is gone indeed, and
OpenBIOS starts fine again with the 32-bit qemu-system-sparc64 binary, so:

Tested-by: Thomas Huth <thuth@redhat.com>

Re: [Qemu-devel] [PATCH 0/2] tcg: Fix i686 booting sparc64 openbios

[Mark Cave-Ayland]

On 18/06/16 06:03, Richard Henderson wrote:

> There's a minor typo here that affects dumping of 64-bit
> registers on 32-bit hosts.  Kind of embarrasing that this
> hasn't been seen previously.
> 
> The main change takes care of cases wherein there's overlap
> between the indirect base register and the main global, which
> can happen in conditions of very high register pressure.
> 
> The bug report is at
> 
> http://lists.nongnu.org/archive/html/qemu-devel/2016-06/msg04947.html
> 
> 
> r~

Hi Richard,

I've just given this a test in my i386 VM under qemu-system-sparc64, and
while the assert doesn't trigger immediately whilst booting into
OpenBIOS, it still occurs further along in the boot process:


$ ./qemu-system-sparc64 -cdrom debian-7.7.0-sparc-netinst.iso -nographic
OpenBIOS for Sparc64
Configuration device id QEMU version 1 machine id 0
kernel cmdline
CPUs: 1 x SUNW,UltraSPARC-IIi
UUID: 00000000-0000-0000-0000-000000000000
Welcome to OpenBIOS v1.1 built on Apr 18 2016 08:20
  Type 'help' for detailed information
Trying disk:a...
No valid state has been set by load or init-program

0 > boot cdrom Not a Linux kernel image
Not a bootable ELF image
Loading a.out image...
Loaded 7680 bytes
entry point is 0x4000

Jumping to entry point 0000000000004000 for type 0000000000000005...
switching to new context: entry point 0x4000 stack 0x00000000ffe84a09
SILO Version 1.4.14
EXT2 superblock magic is wrong
EXT2 superblock magic is wrong
\


                  Welcome to Debian GNU/Linux wheezy!

This is a Debian installation CDROM, built on 20141018-19:00.
Keep it once you have installed your system, as you can boot from it
to repair the system on your hard disk if that ever becomes necessary.

WARNING: You should completely back up all of your hard disks before
  proceeding. The installation procedure can completely and irreversibly
  erase them! If you haven't made backups yet, remove the rescue CD from
  the drive and press L1-A to get back to the OpenBoot prompt.

Debian GNU/Linux comes with ABSOLUTELY NO WARRANTY, to the extent permitted
by applicable law.

[ ENTER - Boot install ]   [ Type "expert" - Boot into expert mode ]
                           [ Type "rescue" - Boot into rescue mode ]
boot:
Allocated 64 Megs of memory at 0x40000000 for kernel
EXT2 superblock magic is wrong
Loaded kernel version 3.2.63
EXT2 superblock magic is wrong
Loading initial ramdisk (5042437 bytes at 0x4400000 phys, 0x40C00000
virt)...
/
[    0.000000] PROMLIB: Sun IEEE Boot Prom 'OBP 3.10.24 1999/01/01 01:01'
[    0.000000] PROMLIB: Root node compatible: sun4u
[    0.000000] Initializing cgroup subsys cpuset
[    0.000000] Initializing cgroup subsys cpu
[    0.000000] Linux version 3.2.0-4-sparc64
(debian-kernel@lists.debian.org) (gcc version 4.6.3 (Debian 4.6.3-14) )
#1 Debian 3.2.63-2
[    0.000000] bootconsole [earlyprom0] enabled
[    0.000000] ARCH: SUN4U
[    0.000000] Ethernet address: 52:54:00:12:34:56
[    0.000000] Kernel: Using 2 locked TLB entries for main kernel image.
[    0.000000] Remapping the kernel... done.
[    0.000000] OF stdout device is: /pci@1fe,0/ebus@3/su
[    0.000000] PROM: Built device tree with 33893 bytes of memory.
[    0.000000] Top of RAM: 0x7e80000, Total RAM: 0x7e80000
[    0.000000] Memory hole size: 0MB
[    0.000000] Zone PFN ranges:
[    0.000000]   Normal   0x00000000 -> 0x00003f40
[    0.000000] Movable zone start PFN for each node
[    0.000000] early_node_map[1] active PFN ranges
[    0.000000]     0: 0x00000000 -> 0x00003f40
[    0.000000] Booting Linux...
[    0.000000] CPU CAPS: [flush,stbar,swap,muldiv,v9,mul32,div32,v8plus]
[    0.000000] CPU CAPS: [vis]
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.
Total pages: 16065
[    0.000000] Kernel command line:
[    0.000000] PID hash table entries: 512 (order: -1, 4096 bytes)
[    0.000000] Dentry cache hash table entries: 16384 (order: 4, 131072
bytes)
[    0.000000] Inode-cache hash table entries: 8192 (order: 3, 65536 bytes)
[    0.000000] Memory: 110272k available (3440k kernel code, 1440k data,
192k init) [fffff80000000000,0000000007e80000]
[    0.000000] NR_IRQS:255
[    0.000000] clocksource: mult[a000000] shift[24]
[    0.000000] clockevent: mult[1999999a] shift[32]
[    0.000000] Console: colour dummy device 80x25
[    0.000000] console [tty0] enabled, bootconsole disabled
/home/build/src/qemu/tcg/tcg.c:1743: tcg fatal error
Aborted


ATB,

Mark.