All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] sparc-softmmu uninitialized memory read?
@ 2012-05-05 15:37 Andreas Färber
  2012-05-06 11:32 ` Blue Swirl
  0 siblings, 1 reply; 7+ messages in thread
From: Andreas Färber @ 2012-05-05 15:37 UTC (permalink / raw)
  To: Blue Swirl; +Cc: Mark Cave-Ayland, Alexander Graf, qemu-devel

Hello Blue,

Testing a potential AREG0 fix for ppc host by malc I got an error
running `./sparc-softmmu/sparc-softmmu` (same with CD/kernel):

qemu: fatal: Trap 0x07 while interrupts disabled, Error state
pc: 00005e0c  npc: 00005e10
General Registers:
%g0-7: 00000000 00000001 babababa 00000000 00000020 07ffff08 07ffe000
babababa

Current Register Window:
%o0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
%l0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000
%i0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
00000000

Floating Point Registers:
%f00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
%f08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
%f16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
%f24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
psr: 048000c0 (icc: N--- SPE: SP-) wim: 00000001
fsr: 00000000 y: 00000020
Abgebrochen

The 0xbabababa in %g2 and %g7 is a signature I've seen in uninitialized
memory on openSUSE 12.1 Betas. So I ran valgrind, and the following
caught my eye on both ppc and x86_64:

==18801== Command: ./sparc-softmmu/qemu-system-sparc
==18801==
==18801== Thread 2:
==18801== Conditional jump or move depends on uninitialised value(s)
==18801==    at 0x25C5AF: compute_all_logic (cc_helper.c:37)
==18801==    by 0x25C648: helper_compute_psr (cc_helper.c:470)
==18801==    by 0x8CD0981: ???
==18801==  Uninitialised value was created by a heap allocation
==18801==    at 0x4C27CE8: memalign (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==18801==    by 0x4C27D97: posix_memalign (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==18801==    by 0x1F2101: qemu_memalign (oslib-posix.c:93)
==18801==    by 0x1F21A9: qemu_vmalloc (oslib-posix.c:126)
==18801==    by 0x2665F6: qemu_ram_alloc_from_ptr (exec.c:2647)
==18801==    by 0x286D76: memory_region_init_ram (memory.c:954)
==18801==    by 0x297FFD: ram_init1 (sun4m.c:757)
==18801==    by 0x204DAE: qdev_init (qdev.c:151)
==18801==    by 0x204EEC: qdev_init_nofail (qdev.c:258)
==18801==    by 0x298845: ram_init.constprop.7 (sun4m.c:783)
==18801==    by 0x298980: sun4m_hw_init (sun4m.c:862)
==18801==    by 0x2994A2: ss5_init (sun4m.c:1289)

This is at 8f473dd104f0937ce98523fa6f9de0bd845aebbe, and cc_helper.c:37
is int32_t dst argument of get_NZ_icc(), which is always called with
CC_DST, i.e. env->cc_dst.

This seems to indicate that a read from uninitialized memory occurred,
from which cc_dst is being initialized?

Any idea where that could originate from or how to further debug?
It doesn't seem to be caused by the
7d21dcc84b8c07918124a9c0708694d2fb013f65 OpenBIOS r1056 update.

Regards,

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [Qemu-devel] sparc-softmmu uninitialized memory read?
  2012-05-05 15:37 [Qemu-devel] sparc-softmmu uninitialized memory read? Andreas Färber
@ 2012-05-06 11:32 ` Blue Swirl
  2012-05-06 14:02   ` Andreas Färber
  0 siblings, 1 reply; 7+ messages in thread
From: Blue Swirl @ 2012-05-06 11:32 UTC (permalink / raw)
  To: Andreas Färber; +Cc: Mark Cave-Ayland, Alexander Graf, qemu-devel

On Sat, May 5, 2012 at 3:37 PM, Andreas Färber <afaerber@suse.de> wrote:
> Hello Blue,
>
> Testing a potential AREG0 fix for ppc host by malc I got an error
> running `./sparc-softmmu/sparc-softmmu` (same with CD/kernel):
>
> qemu: fatal: Trap 0x07 while interrupts disabled, Error state
> pc: 00005e0c  npc: 00005e10
> General Registers:
> %g0-7: 00000000 00000001 babababa 00000000 00000020 07ffff08 07ffe000
> babababa
>
> Current Register Window:
> %o0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> %l0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
> %i0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000
>
> Floating Point Registers:
> %f00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> %f08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> %f16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> %f24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
> psr: 048000c0 (icc: N--- SPE: SP-) wim: 00000001
> fsr: 00000000 y: 00000020
> Abgebrochen
>
> The 0xbabababa in %g2 and %g7 is a signature I've seen in uninitialized
> memory on openSUSE 12.1 Betas. So I ran valgrind, and the following
> caught my eye on both ppc and x86_64:
>
> ==18801== Command: ./sparc-softmmu/qemu-system-sparc
> ==18801==
> ==18801== Thread 2:
> ==18801== Conditional jump or move depends on uninitialised value(s)
> ==18801==    at 0x25C5AF: compute_all_logic (cc_helper.c:37)
> ==18801==    by 0x25C648: helper_compute_psr (cc_helper.c:470)
> ==18801==    by 0x8CD0981: ???
> ==18801==  Uninitialised value was created by a heap allocation
> ==18801==    at 0x4C27CE8: memalign (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==18801==    by 0x4C27D97: posix_memalign (in
> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==18801==    by 0x1F2101: qemu_memalign (oslib-posix.c:93)
> ==18801==    by 0x1F21A9: qemu_vmalloc (oslib-posix.c:126)
> ==18801==    by 0x2665F6: qemu_ram_alloc_from_ptr (exec.c:2647)
> ==18801==    by 0x286D76: memory_region_init_ram (memory.c:954)
> ==18801==    by 0x297FFD: ram_init1 (sun4m.c:757)
> ==18801==    by 0x204DAE: qdev_init (qdev.c:151)
> ==18801==    by 0x204EEC: qdev_init_nofail (qdev.c:258)
> ==18801==    by 0x298845: ram_init.constprop.7 (sun4m.c:783)
> ==18801==    by 0x298980: sun4m_hw_init (sun4m.c:862)
> ==18801==    by 0x2994A2: ss5_init (sun4m.c:1289)
>
> This is at 8f473dd104f0937ce98523fa6f9de0bd845aebbe, and cc_helper.c:37
> is int32_t dst argument of get_NZ_icc(), which is always called with
> CC_DST, i.e. env->cc_dst.
>
> This seems to indicate that a read from uninitialized memory occurred,
> from which cc_dst is being initialized?

This should happen in target-sparc/cpu.c:45
    memset(env, 0, offsetof(CPUSPARCState, breakpoints));

cc_dst is between structure start and CPU_COMMON.

89aaf60dedbe0e6415acfe816e02b538e5c54e68 fixed a bug relating to reset recently.

>
> Any idea where that could originate from or how to further debug?
> It doesn't seem to be caused by the
> 7d21dcc84b8c07918124a9c0708694d2fb013f65 OpenBIOS r1056 update.
>
> Regards,
>
> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [Qemu-devel] sparc-softmmu uninitialized memory read?
  2012-05-06 11:32 ` Blue Swirl
@ 2012-05-06 14:02   ` Andreas Färber
  2012-05-06 16:44     ` Blue Swirl
  0 siblings, 1 reply; 7+ messages in thread
From: Andreas Färber @ 2012-05-06 14:02 UTC (permalink / raw)
  To: Blue Swirl; +Cc: Mark Cave-Ayland, Alexander Graf, qemu-devel

Am 06.05.2012 13:32, schrieb Blue Swirl:
> On Sat, May 5, 2012 at 3:37 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Hello Blue,
>>
>> Testing a potential AREG0 fix for ppc host by malc I got an error
>> running `./sparc-softmmu/sparc-softmmu` (same with CD/kernel):
>>
>> qemu: fatal: Trap 0x07 while interrupts disabled, Error state
>> pc: 00005e0c  npc: 00005e10
>> General Registers:
>> %g0-7: 00000000 00000001 babababa 00000000 00000020 07ffff08 07ffe000
>> babababa
>>
>> Current Register Window:
>> %o0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> %l0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>> %i0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>> 00000000
>>
>> Floating Point Registers:
>> %f00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> %f08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> %f16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> %f24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>> psr: 048000c0 (icc: N--- SPE: SP-) wim: 00000001
>> fsr: 00000000 y: 00000020
>> Abgebrochen
>>
>> The 0xbabababa in %g2 and %g7 is a signature I've seen in uninitialized
>> memory on openSUSE 12.1 Betas. So I ran valgrind, and the following
>> caught my eye on both ppc and x86_64:
>>
>> ==18801== Command: ./sparc-softmmu/qemu-system-sparc
>> ==18801==
>> ==18801== Thread 2:
>> ==18801== Conditional jump or move depends on uninitialised value(s)
>> ==18801==    at 0x25C5AF: compute_all_logic (cc_helper.c:37)
>> ==18801==    by 0x25C648: helper_compute_psr (cc_helper.c:470)
>> ==18801==    by 0x8CD0981: ???
>> ==18801==  Uninitialised value was created by a heap allocation
>> ==18801==    at 0x4C27CE8: memalign (in
>> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==18801==    by 0x4C27D97: posix_memalign (in
>> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>> ==18801==    by 0x1F2101: qemu_memalign (oslib-posix.c:93)
>> ==18801==    by 0x1F21A9: qemu_vmalloc (oslib-posix.c:126)
>> ==18801==    by 0x2665F6: qemu_ram_alloc_from_ptr (exec.c:2647)
>> ==18801==    by 0x286D76: memory_region_init_ram (memory.c:954)
>> ==18801==    by 0x297FFD: ram_init1 (sun4m.c:757)
>> ==18801==    by 0x204DAE: qdev_init (qdev.c:151)
>> ==18801==    by 0x204EEC: qdev_init_nofail (qdev.c:258)
>> ==18801==    by 0x298845: ram_init.constprop.7 (sun4m.c:783)
>> ==18801==    by 0x298980: sun4m_hw_init (sun4m.c:862)
>> ==18801==    by 0x2994A2: ss5_init (sun4m.c:1289)
>>
>> This is at 8f473dd104f0937ce98523fa6f9de0bd845aebbe, and cc_helper.c:37
>> is int32_t dst argument of get_NZ_icc(), which is always called with
>> CC_DST, i.e. env->cc_dst.
>>
>> This seems to indicate that a read from uninitialized memory occurred,
>> from which cc_dst is being initialized?
> 
> This should happen in target-sparc/cpu.c:45
>     memset(env, 0, offsetof(CPUSPARCState, breakpoints));
> 
> cc_dst is between structure start and CPU_COMMON.
> 
> 89aaf60dedbe0e6415acfe816e02b538e5c54e68 fixed a bug relating to reset recently.

The still-current master commit above includes that fix though, and
that's no explanation for the uninitialized memory stemming from sun4m
RAM as opposed to QOM object_new(). Somewhere a read is happening,
possibly in OpenBIOS, from uninitialized memory that is then stored into
the CPUSPARCState after that has been zero-initialized, IIUC.

My issue here is that sparc64 boots HelenOS fine up until it's trying to
load the kernel (identical to x86_64 host) but sparc32 exits really
early on ppc. It might well be that there's a bug hidden in malc's TCG
patch that's causing the fatal error state, but the uninitialized memory
report is on both TCG hosts, so unlikely TCG-related.

/-F

>> Any idea where that could originate from or how to further debug?
>> It doesn't seem to be caused by the
>> 7d21dcc84b8c07918124a9c0708694d2fb013f65 OpenBIOS r1056 update.
>>
>> Regards,
>>
>> Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [Qemu-devel] sparc-softmmu uninitialized memory read?
  2012-05-06 14:02   ` Andreas Färber
@ 2012-05-06 16:44     ` Blue Swirl
  2012-05-06 19:22       ` Andreas Färber
  0 siblings, 1 reply; 7+ messages in thread
From: Blue Swirl @ 2012-05-06 16:44 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Mark Cave-Ayland, The OpenBIOS Mailinglist, Alexander Graf, qemu-devel

On Sun, May 6, 2012 at 2:02 PM, Andreas Färber <afaerber@suse.de> wrote:
> Am 06.05.2012 13:32, schrieb Blue Swirl:
>> On Sat, May 5, 2012 at 3:37 PM, Andreas Färber <afaerber@suse.de> wrote:
>>> Hello Blue,
>>>
>>> Testing a potential AREG0 fix for ppc host by malc I got an error
>>> running `./sparc-softmmu/sparc-softmmu` (same with CD/kernel):
>>>
>>> qemu: fatal: Trap 0x07 while interrupts disabled, Error state
>>> pc: 00005e0c  npc: 00005e10
>>> General Registers:
>>> %g0-7: 00000000 00000001 babababa 00000000 00000020 07ffff08 07ffe000
>>> babababa
>>>
>>> Current Register Window:
>>> %o0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>> 00000000
>>> %l0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>> 00000000
>>> %i0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>> 00000000
>>>
>>> Floating Point Registers:
>>> %f00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> %f08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> %f16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> %f24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>> psr: 048000c0 (icc: N--- SPE: SP-) wim: 00000001
>>> fsr: 00000000 y: 00000020
>>> Abgebrochen
>>>
>>> The 0xbabababa in %g2 and %g7 is a signature I've seen in uninitialized
>>> memory on openSUSE 12.1 Betas. So I ran valgrind, and the following
>>> caught my eye on both ppc and x86_64:
>>>
>>> ==18801== Command: ./sparc-softmmu/qemu-system-sparc
>>> ==18801==
>>> ==18801== Thread 2:
>>> ==18801== Conditional jump or move depends on uninitialised value(s)
>>> ==18801==    at 0x25C5AF: compute_all_logic (cc_helper.c:37)
>>> ==18801==    by 0x25C648: helper_compute_psr (cc_helper.c:470)
>>> ==18801==    by 0x8CD0981: ???
>>> ==18801==  Uninitialised value was created by a heap allocation
>>> ==18801==    at 0x4C27CE8: memalign (in
>>> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>> ==18801==    by 0x4C27D97: posix_memalign (in
>>> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>> ==18801==    by 0x1F2101: qemu_memalign (oslib-posix.c:93)
>>> ==18801==    by 0x1F21A9: qemu_vmalloc (oslib-posix.c:126)
>>> ==18801==    by 0x2665F6: qemu_ram_alloc_from_ptr (exec.c:2647)
>>> ==18801==    by 0x286D76: memory_region_init_ram (memory.c:954)
>>> ==18801==    by 0x297FFD: ram_init1 (sun4m.c:757)
>>> ==18801==    by 0x204DAE: qdev_init (qdev.c:151)
>>> ==18801==    by 0x204EEC: qdev_init_nofail (qdev.c:258)
>>> ==18801==    by 0x298845: ram_init.constprop.7 (sun4m.c:783)
>>> ==18801==    by 0x298980: sun4m_hw_init (sun4m.c:862)
>>> ==18801==    by 0x2994A2: ss5_init (sun4m.c:1289)
>>>
>>> This is at 8f473dd104f0937ce98523fa6f9de0bd845aebbe, and cc_helper.c:37
>>> is int32_t dst argument of get_NZ_icc(), which is always called with
>>> CC_DST, i.e. env->cc_dst.
>>>
>>> This seems to indicate that a read from uninitialized memory occurred,
>>> from which cc_dst is being initialized?
>>
>> This should happen in target-sparc/cpu.c:45
>>     memset(env, 0, offsetof(CPUSPARCState, breakpoints));
>>
>> cc_dst is between structure start and CPU_COMMON.
>>
>> 89aaf60dedbe0e6415acfe816e02b538e5c54e68 fixed a bug relating to reset recently.
>
> The still-current master commit above includes that fix though, and
> that's no explanation for the uninitialized memory stemming from sun4m
> RAM as opposed to QOM object_new(). Somewhere a read is happening,
> possibly in OpenBIOS, from uninitialized memory that is then stored into
> the CPUSPARCState after that has been zero-initialized, IIUC.

Ok, I see it now. OpenBIOS assumes that the Sparc32 SMP table is valid
when the valid field is nonzero, indicating secondary processor setup
so OpenBIOS jumps to the location indicated with the SMP table. With
0xbabababa in memory, this fragile logic fails and there is the early
crash.
https://tracker.coreboot.org/trac/openbios/browser/trunk/openbios-devel/arch/sparc32/entry.S#L132
https://tracker.coreboot.org/trac/openbios/browser/trunk/openbios-devel/arch/sparc32/entry.S#L157

I think the current logic would also not survive a reset just when a
secondary processor is brought online.

The fix is to make the SMP table logic robust, for example with a
checksum. We could also read CPU ID from MXCC and skip the check for
boot CPU, though MXCC should not exist for all models.

>
> My issue here is that sparc64 boots HelenOS fine up until it's trying to
> load the kernel (identical to x86_64 host) but sparc32 exits really
> early on ppc. It might well be that there's a bug hidden in malc's TCG
> patch that's causing the fatal error state, but the uninitialized memory
> report is on both TCG hosts, so unlikely TCG-related.
>
> /-F
>
>>> Any idea where that could originate from or how to further debug?
>>> It doesn't seem to be caused by the
>>> 7d21dcc84b8c07918124a9c0708694d2fb013f65 OpenBIOS r1056 update.
>>>
>>> Regards,
>>>
>>> Andreas
>
> --
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
> GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 7+ messages in thread
* Re: [Qemu-devel] sparc-softmmu uninitialized memory read?
  2012-05-06 16:44     ` Blue Swirl
@ 2012-05-06 19:22       ` Andreas Färber
  2012-05-06 19:27         ` malc
  0 siblings, 1 reply; 7+ messages in thread
From: Andreas Färber @ 2012-05-06 19:22 UTC (permalink / raw)
  To: Blue Swirl, malc
  Cc: The OpenBIOS Mailinglist, Mark Cave-Ayland, Alexander Graf, qemu-devel

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

Am 06.05.2012 18:44, schrieb Blue Swirl:
> On Sun, May 6, 2012 at 2:02 PM, Andreas Färber <afaerber@suse.de> wrote:
>> Am 06.05.2012 13:32, schrieb Blue Swirl:
>>> On Sat, May 5, 2012 at 3:37 PM, Andreas Färber <afaerber@suse.de> wrote:
>>>> Hello Blue,
>>>>
>>>> Testing a potential AREG0 fix for ppc host by malc I got an error
>>>> running `./sparc-softmmu/sparc-softmmu` (same with CD/kernel):
>>>>
>>>> qemu: fatal: Trap 0x07 while interrupts disabled, Error state
>>>> pc: 00005e0c  npc: 00005e10
>>>> General Registers:
>>>> %g0-7: 00000000 00000001 babababa 00000000 00000020 07ffff08 07ffe000
>>>> babababa
>>>>
>>>> Current Register Window:
>>>> %o0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> %l0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>> %i0-7: 00000000 00000000 00000000 00000000 00000000 00000000 00000000
>>>> 00000000
>>>>
>>>> Floating Point Registers:
>>>> %f00: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>> %f08: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>> %f16: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>> %f24: 0000000000000000 0000000000000000 0000000000000000 0000000000000000
>>>> psr: 048000c0 (icc: N--- SPE: SP-) wim: 00000001
>>>> fsr: 00000000 y: 00000020
>>>> Abgebrochen
>>>>
>>>> The 0xbabababa in %g2 and %g7 is a signature I've seen in uninitialized
>>>> memory on openSUSE 12.1 Betas. So I ran valgrind, and the following
>>>> caught my eye on both ppc and x86_64:
>>>>
>>>> ==18801== Command: ./sparc-softmmu/qemu-system-sparc
>>>> ==18801==
>>>> ==18801== Thread 2:
>>>> ==18801== Conditional jump or move depends on uninitialised value(s)
>>>> ==18801==    at 0x25C5AF: compute_all_logic (cc_helper.c:37)
>>>> ==18801==    by 0x25C648: helper_compute_psr (cc_helper.c:470)
>>>> ==18801==    by 0x8CD0981: ???
>>>> ==18801==  Uninitialised value was created by a heap allocation
>>>> ==18801==    at 0x4C27CE8: memalign (in
>>>> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>> ==18801==    by 0x4C27D97: posix_memalign (in
>>>> /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>>>> ==18801==    by 0x1F2101: qemu_memalign (oslib-posix.c:93)
>>>> ==18801==    by 0x1F21A9: qemu_vmalloc (oslib-posix.c:126)
>>>> ==18801==    by 0x2665F6: qemu_ram_alloc_from_ptr (exec.c:2647)
>>>> ==18801==    by 0x286D76: memory_region_init_ram (memory.c:954)
>>>> ==18801==    by 0x297FFD: ram_init1 (sun4m.c:757)
>>>> ==18801==    by 0x204DAE: qdev_init (qdev.c:151)
>>>> ==18801==    by 0x204EEC: qdev_init_nofail (qdev.c:258)
>>>> ==18801==    by 0x298845: ram_init.constprop.7 (sun4m.c:783)
>>>> ==18801==    by 0x298980: sun4m_hw_init (sun4m.c:862)
>>>> ==18801==    by 0x2994A2: ss5_init (sun4m.c:1289)
>>>>
>>>> This is at 8f473dd104f0937ce98523fa6f9de0bd845aebbe, and cc_helper.c:37
>>>> is int32_t dst argument of get_NZ_icc(), which is always called with
>>>> CC_DST, i.e. env->cc_dst.
>>>>
>>>> This seems to indicate that a read from uninitialized memory occurred,
>>>> from which cc_dst is being initialized?
>>>
>>> This should happen in target-sparc/cpu.c:45
>>>     memset(env, 0, offsetof(CPUSPARCState, breakpoints));
>>>
>>> cc_dst is between structure start and CPU_COMMON.
>>>
>>> 89aaf60dedbe0e6415acfe816e02b538e5c54e68 fixed a bug relating to reset recently.
>>
>> The still-current master commit above includes that fix though, and
>> that's no explanation for the uninitialized memory stemming from sun4m
>> RAM as opposed to QOM object_new(). Somewhere a read is happening,
>> possibly in OpenBIOS, from uninitialized memory that is then stored into
>> the CPUSPARCState after that has been zero-initialized, IIUC.
> 
> Ok, I see it now. OpenBIOS assumes that the Sparc32 SMP table is valid
> when the valid field is nonzero, indicating secondary processor setup
> so OpenBIOS jumps to the location indicated with the SMP table. With
> 0xbabababa in memory, this fragile logic fails and there is the early
> crash.
> https://tracker.coreboot.org/trac/openbios/browser/trunk/openbios-devel/arch/sparc32/entry.S#L132
> https://tracker.coreboot.org/trac/openbios/browser/trunk/openbios-devel/arch/sparc32/entry.S#L157

Great! I have tested the following workaround:

diff --git a/hw/sun4m.c b/hw/sun4m.c
index 34088ad..55d5bdc 100644
--- a/hw/sun4m.c
+++ b/hw/sun4m.c
@@ -755,6 +755,7 @@ static int ram_init1(SysBusDevice *dev)
     RamDevice *d = FROM_SYSBUS(RamDevice, dev);

     memory_region_init_ram(&d->ram, "sun4m.ram", d->size);
+    memset(memory_region_get_ram_ptr(&d->ram), 0, d->size);
     vmstate_register_ram_global(&d->ram);
     sysbus_init_mmio(dev, &d->ram);
     return 0;

This makes sparc32 work on ppc with malc's attached patch (and doesn't
break on x86_64).

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[-- Attachment #2: malc_tcgppc_areg0.diff --]
[-- Type: text/x-patch, Size: 2527 bytes --]

diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index dc40716..4efbefa 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -509,7 +509,7 @@ static void tcg_out_call (TCGContext *s, tcg_target_long arg, int const_arg)
 #include "../../softmmu_defs.h"
 
 #ifdef CONFIG_TCG_PASS_AREG0
-#error CONFIG_TCG_PASS_AREG0 is not supported
+/* #error CONFIG_TCG_PASS_AREG0 is not supported */
 /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
    int mmu_idx) */
 static const void * const qemu_ld_helpers[4] = {
@@ -614,6 +614,17 @@ static void tcg_out_qemu_ld (TCGContext *s, const TCGArg *args, int opc)
 #endif
 
     /* slow path */
+#ifdef CONFIG_TCG_PASS_AREG0
+    tcg_out_mov (s, TCG_TYPE_I32, 3, TCG_AREG0);
+#if TARGET_LONG_BITS == 32
+    tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
+    tcg_out_movi (s, TCG_TYPE_I32, 5, mem_index);
+#else
+    tcg_out_mov (s, TCG_TYPE_I32, 5, addr_reg2);
+    tcg_out_mov (s, TCG_TYPE_I32, 6, addr_reg);
+    tcg_out_movi (s, TCG_TYPE_I32, 7, mem_index);
+#endif
+#else
 #if TARGET_LONG_BITS == 32
     tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg);
     tcg_out_movi (s, TCG_TYPE_I32, 4, mem_index);
@@ -622,6 +633,7 @@ static void tcg_out_qemu_ld (TCGContext *s, const TCGArg *args, int opc)
     tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
     tcg_out_movi (s, TCG_TYPE_I32, 5, mem_index);
 #endif
+#endif
 
     tcg_out_call (s, (tcg_target_long) qemu_ld_helpers[s_bits], 1);
     switch (opc) {
@@ -810,6 +822,17 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
 #endif
 
     /* slow path */
+#ifdef CONFIG_TCG_PASS_AREG0
+    tcg_out_mov (s, TCG_TYPE_I32, 3, TCG_AREG0);
+#if TARGET_LONG_BITS == 32
+    tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
+    ir = 5;
+#else
+    tcg_out_mov (s, TCG_TYPE_I32, 5, addr_reg2);
+    tcg_out_mov (s, TCG_TYPE_I32, 6, addr_reg);
+    ir = 7;
+#endif
+#else
 #if TARGET_LONG_BITS == 32
     tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg);
     ir = 4;
@@ -822,6 +845,7 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
     ir = 4;
 #endif
 #endif
+#endif
 
     switch (opc) {
     case 0:
@@ -844,7 +868,7 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
         tcg_out_mov (s, TCG_TYPE_I32, ir, data_reg);
         break;
     case 3:
-#ifdef TCG_TARGET_CALL_ALIGN_ARGS
+#if defined TCG_TARGET_CALL_ALIGN_ARGS && !defined CONFIG_TCG_PASS_AREG0
         ir = 5;
 #endif
         tcg_out_mov (s, TCG_TYPE_I32, ir++, data_reg2);

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [Qemu-devel] sparc-softmmu uninitialized memory read?
  2012-05-06 19:22       ` Andreas Färber
@ 2012-05-06 19:27         ` malc
  2012-05-07  0:02           ` Andreas Färber
  0 siblings, 1 reply; 7+ messages in thread
From: malc @ 2012-05-06 19:27 UTC (permalink / raw)
  To: Andreas Färber
  Cc: Blue Swirl, The OpenBIOS Mailinglist, Mark Cave-Ayland,
	Alexander Graf, qemu-devel

On Sun, 6 May 2012, Andreas F?rber wrote:

> Am 06.05.2012 18:44, schrieb Blue Swirl:
> > On Sun, May 6, 2012 at 2:02 PM, Andreas F?rber <afaerber@suse.de> wrote:
> >> Am 06.05.2012 13:32, schrieb Blue Swirl:
> >>> On Sat, May 5, 2012 at 3:37 PM, Andreas F?rber <afaerber@suse.de> wrote:
> >>>> Hello Blue,

[..snip..]

> Great! I have tested the following workaround:
> 
> diff --git a/hw/sun4m.c b/hw/sun4m.c
> index 34088ad..55d5bdc 100644
> --- a/hw/sun4m.c
> +++ b/hw/sun4m.c
> @@ -755,6 +755,7 @@ static int ram_init1(SysBusDevice *dev)
>      RamDevice *d = FROM_SYSBUS(RamDevice, dev);
> 
>      memory_region_init_ram(&d->ram, "sun4m.ram", d->size);
> +    memset(memory_region_get_ram_ptr(&d->ram), 0, d->size);
>      vmstate_register_ram_global(&d->ram);
>      sysbus_init_mmio(dev, &d->ram);
>      return 0;
> 
> This makes sparc32 work on ppc with malc's attached patch (and doesn't
> break on x86_64).
> 

The attached patch is broken for non SysV calling conventions, would be
nice if you could test things on Darwin (and, if your power5 box still has
AIX, on AIX)

diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
index dc40716..311af18 100644
--- a/tcg/ppc/tcg-target.c
+++ b/tcg/ppc/tcg-target.c
@@ -509,7 +509,7 @@ static void tcg_out_call (TCGContext *s, tcg_target_long arg, int const_arg)
 #include "../../softmmu_defs.h"
 
 #ifdef CONFIG_TCG_PASS_AREG0
-#error CONFIG_TCG_PASS_AREG0 is not supported
+/* #error CONFIG_TCG_PASS_AREG0 is not supported */
 /* helper signature: helper_ld_mmu(CPUState *env, target_ulong addr,
    int mmu_idx) */
 static const void * const qemu_ld_helpers[4] = {
@@ -614,6 +614,24 @@ static void tcg_out_qemu_ld (TCGContext *s, const TCGArg *args, int opc)
 #endif
 
     /* slow path */
+#ifdef CONFIG_TCG_PASS_AREG0
+    tcg_out_mov (s, TCG_TYPE_I32, 3, TCG_AREG0);
+#if TARGET_LONG_BITS == 32
+    tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
+    tcg_out_movi (s, TCG_TYPE_I32, 5, mem_index);
+#else
+    {
+#ifdef TCG_TARGET_CALL_ALIGN_ARGS
+        int ir = 5;
+#else
+        int ir = 4;
+#endif
+        tcg_out_mov (s, TCG_TYPE_I32, ir, addr_reg2);
+        tcg_out_mov (s, TCG_TYPE_I32, ir + 1, addr_reg);
+        tcg_out_movi (s, TCG_TYPE_I32, ir + 2, mem_index);
+    }
+#endif
+#else
 #if TARGET_LONG_BITS == 32
     tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg);
     tcg_out_movi (s, TCG_TYPE_I32, 4, mem_index);
@@ -622,6 +640,7 @@ static void tcg_out_qemu_ld (TCGContext *s, const TCGArg *args, int opc)
     tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
     tcg_out_movi (s, TCG_TYPE_I32, 5, mem_index);
 #endif
+#endif
 
     tcg_out_call (s, (tcg_target_long) qemu_ld_helpers[s_bits], 1);
     switch (opc) {
@@ -810,6 +829,17 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
 #endif
 
     /* slow path */
+#ifdef CONFIG_TCG_PASS_AREG0
+    tcg_out_mov (s, TCG_TYPE_I32, 3, TCG_AREG0);
+#if TARGET_LONG_BITS == 32
+    tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
+    ir = 5;
+#else
+    tcg_out_mov (s, TCG_TYPE_I32, 5, addr_reg2);
+    tcg_out_mov (s, TCG_TYPE_I32, 6, addr_reg);
+    ir = 7;
+#endif
+#else
 #if TARGET_LONG_BITS == 32
     tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg);
     ir = 4;
@@ -822,6 +852,7 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
     ir = 4;
 #endif
 #endif
+#endif
 
     switch (opc) {
     case 0:
@@ -844,7 +875,7 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
         tcg_out_mov (s, TCG_TYPE_I32, ir, data_reg);
         break;
     case 3:
-#ifdef TCG_TARGET_CALL_ALIGN_ARGS
+#if defined TCG_TARGET_CALL_ALIGN_ARGS && !defined CONFIG_TCG_PASS_AREG0
         ir = 5;
 #endif
         tcg_out_mov (s, TCG_TYPE_I32, ir++, data_reg2);

-- 
mailto:av1474@comtv.ru

^ permalink raw reply related	[flat|nested] 7+ messages in thread
* Re: [Qemu-devel] sparc-softmmu uninitialized memory read?
  2012-05-06 19:27         ` malc
@ 2012-05-07  0:02           ` Andreas Färber
  0 siblings, 0 replies; 7+ messages in thread
From: Andreas Färber @ 2012-05-07  0:02 UTC (permalink / raw)
  To: malc; +Cc: Blue Swirl, qemu-ppc, Alexander Graf, qemu-devel

Am 06.05.2012 21:27, schrieb malc:
> The attached patch is broken for non SysV calling conventions, would be
> nice if you could test things on Darwin (and, if your power5 box still has
> AIX, on AIX)

I replaced AIX 5.x with openSUSE, sorry. :)

> diff --git a/tcg/ppc/tcg-target.c b/tcg/ppc/tcg-target.c
> index dc40716..311af18 100644
> --- a/tcg/ppc/tcg-target.c
> +++ b/tcg/ppc/tcg-target.c
[...]
> @@ -810,6 +829,17 @@ static void tcg_out_qemu_st (TCGContext *s, const TCGArg *args, int opc)
>  #endif
>  
>      /* slow path */
> +#ifdef CONFIG_TCG_PASS_AREG0
> +    tcg_out_mov (s, TCG_TYPE_I32, 3, TCG_AREG0);
> +#if TARGET_LONG_BITS == 32
> +    tcg_out_mov (s, TCG_TYPE_I32, 4, addr_reg);
> +    ir = 5;
> +#else
> +    tcg_out_mov (s, TCG_TYPE_I32, 5, addr_reg2);
> +    tcg_out_mov (s, TCG_TYPE_I32, 6, addr_reg);

Here we should be using r4 + r5 for non-aligned targets. Alternative
patch sent that hopefully avoids such issues and the code duplication.

If you prefer two separate code paths for some reason, please at least
consider using a fool-proof alignment macro such as proposed.

/-F

> +    ir = 7;
> +#endif
> +#else
>  #if TARGET_LONG_BITS == 32
>      tcg_out_mov (s, TCG_TYPE_I32, 3, addr_reg);
>      ir = 4;
[snip]

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-05-07  0:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-05-05 15:37 [Qemu-devel] sparc-softmmu uninitialized memory read? Andreas Färber
2012-05-06 11:32 ` Blue Swirl
2012-05-06 14:02   ` Andreas Färber
2012-05-06 16:44     ` Blue Swirl
2012-05-06 19:22       ` Andreas Färber
2012-05-06 19:27         ` malc
2012-05-07  0:02           ` Andreas Färber

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.