Author: mcayland Date: Mon Aug 19 09:40:12 2013 New Revision: 1209 URL: http://tracker.coreboot.org/trac/openbios/changeset/1209
Log: SPARC32: fix romvec stdin/stdout field initialisation
Previously the romvec stdin/stdout ihandles were configured so that they were set to the current stdin and stdout paths at initialisation time. Unfortunately as stdin/stdout can be changed with the input and output words, they can end up pointing to invalid ihandles causing a crash when trying to output to the console.
Fix this by pointing the romvec structure to the address of the stdin/stdout variables so that they are always in sync. Similarly we also resolve the text path stdin/stdout variables at boot time to ensure that they will never be stale.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
Modified: trunk/openbios-devel/arch/sparc32/boot.c trunk/openbios-devel/arch/sparc32/romvec.c
Modified: trunk/openbios-devel/arch/sparc32/boot.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/boot.c Mon Aug 19 09:40:10 2013 (r1208) +++ trunk/openbios-devel/arch/sparc32/boot.c Mon Aug 19 09:40:12 2013 (r1209) @@ -23,7 +23,7 @@ void go(void) { ucell address, type, size; - int image_retval = 0, proplen, target, device; + int image_retval = 0, intprop, proplen, target, device; phandle_t chosen; char *prop, *id, *name; static char bootpathbuf[128], bootargsbuf[128], buf[128]; @@ -40,8 +40,19 @@ needs to be set up to pass certain parameters using a C struct. Hence this section extracts the relevant boot information and places it in obp_arg. */ - /* Get the name of the selected boot device, along with the device and unit number */ + /* Get the stdin and stdout paths */ chosen = find_dev("/chosen"); + intprop = get_int_property(chosen, "stdin", &proplen); + PUSH(intprop); + fword("get-instance-path"); + ((struct linux_romvec *)romvec)->pv_stdin = pop_fstr_copy(); + + intprop = get_int_property(chosen, "stdout", &proplen); + PUSH(intprop); + fword("get-instance-path"); + ((struct linux_romvec *)romvec)->pv_stdout = pop_fstr_copy(); + + /* Get the name of the selected boot device, along with the device and unit number */ prop = get_property(chosen, "bootpath", &proplen); strncpy(bootpathbuf, prop, proplen); prop = get_property(chosen, "bootargs", &proplen);
Modified: trunk/openbios-devel/arch/sparc32/romvec.c ============================================================================== --- trunk/openbios-devel/arch/sparc32/romvec.c Mon Aug 19 09:40:10 2013 (r1208) +++ trunk/openbios-devel/arch/sparc32/romvec.c Mon Aug 19 09:40:12 2013 (r1209) @@ -25,7 +25,6 @@ #endif
char obp_stdin, obp_stdout; -static int obp_fd_stdin, obp_fd_stdout; const char *obp_stdin_path, *obp_stdout_path;
struct linux_arguments_v0 obp_arg; @@ -498,15 +497,12 @@ romvec0.pv_v2bootargs.bootpath = &bootpath;
romvec0.pv_v2bootargs.bootargs = &obp_arg.argv[1]; - romvec0.pv_v2bootargs.fd_stdin = &obp_fd_stdin; - romvec0.pv_v2bootargs.fd_stdout = &obp_fd_stdout;
- push_str(obp_stdin_path); - fword("open-dev"); - obp_fd_stdin = POP(); - push_str(obp_stdout_path); - fword("open-dev"); - obp_fd_stdout = POP(); + /* Point fd_stdin/fd_stdout to the Forth stdin/stdout variables */ + fword("stdin"); + romvec0.pv_v2bootargs.fd_stdin = cell2pointer(POP()); + fword("stdout"); + romvec0.pv_v2bootargs.fd_stdout = cell2pointer(POP());
romvec0.v3_memalloc = obp_memalloc_handler;
On Mon, Aug 19, 2013 at 09:40:13AM +0200, repository service wrote:
Author: mcayland Date: Mon Aug 19 09:40:12 2013 New Revision: 1209 URL: http://tracker.coreboot.org/trac/openbios/changeset/1209
Log: SPARC32: fix romvec stdin/stdout field initialisation
Previously the romvec stdin/stdout ihandles were configured so that they were set to the current stdin and stdout paths at initialisation time. Unfortunately as stdin/stdout can be changed with the input and output words, they can end up pointing to invalid ihandles causing a crash when trying to output to the console.
Fix this by pointing the romvec structure to the address of the stdin/stdout variables so that they are always in sync. Similarly we also resolve the text path stdin/stdout variables at boot time to ensure that they will never be stale.
Signed-off-by: Mark Cave-Ayland mark.cave-ayland@ilande.co.uk
Modified: trunk/openbios-devel/arch/sparc32/boot.c trunk/openbios-devel/arch/sparc32/romvec.c
Since this patch has been applied, I am unable to boot a Debian SPARC32 guest using SILO as a bootloader. OpenBios stops at "Trying disk".
One can find such a guest there: http://people.debian.org/~aurel32/qemu/sparc/debian_etch_sparc_small.qcow2
On 08/12/13 10:36, Aurelien Jarno wrote:
On Mon, Aug 19, 2013 at 09:40:13AM +0200, repository service wrote:
Author: mcayland Date: Mon Aug 19 09:40:12 2013 New Revision: 1209 URL: http://tracker.coreboot.org/trac/openbios/changeset/1209
Log: SPARC32: fix romvec stdin/stdout field initialisation
Previously the romvec stdin/stdout ihandles were configured so that they were set to the current stdin and stdout paths at initialisation time. Unfortunately as stdin/stdout can be changed with the input and output words, they can end up pointing to invalid ihandles causing a crash when trying to output to the console.
Fix this by pointing the romvec structure to the address of the stdin/stdout variables so that they are always in sync. Similarly we also resolve the text path stdin/stdout variables at boot time to ensure that they will never be stale.
Signed-off-by: Mark Cave-Aylandmark.cave-ayland@ilande.co.uk
Modified: trunk/openbios-devel/arch/sparc32/boot.c trunk/openbios-devel/arch/sparc32/romvec.c
Since this patch has been applied, I am unable to boot a Debian SPARC32 guest using SILO as a bootloader. OpenBios stops at "Trying disk".
One can find such a guest there: http://people.debian.org/~aurel32/qemu/sparc/debian_etch_sparc_small.qcow2
Hi Aurelian,
Thanks for the bug report. I've just downloaded your image above and tested it against a fresh build of OpenBIOS r1235 here and it works fine for me (QEMU 1.7, gcc 4.7.1, sparc-elf-gcc cross-compile on Debian amd64). Is there anything else about your build environment that may be different somehow?
ATB,
Mark.
Hi,
On Sun, Dec 08, 2013 at 11:18:11AM +0000, Mark Cave-Ayland wrote:
On 08/12/13 10:36, Aurelien Jarno wrote:
On Mon, Aug 19, 2013 at 09:40:13AM +0200, repository service wrote:
Author: mcayland Date: Mon Aug 19 09:40:12 2013 New Revision: 1209 URL: http://tracker.coreboot.org/trac/openbios/changeset/1209
Log: SPARC32: fix romvec stdin/stdout field initialisation
Previously the romvec stdin/stdout ihandles were configured so that they were set to the current stdin and stdout paths at initialisation time. Unfortunately as stdin/stdout can be changed with the input and output words, they can end up pointing to invalid ihandles causing a crash when trying to output to the console.
Fix this by pointing the romvec structure to the address of the stdin/stdout variables so that they are always in sync. Similarly we also resolve the text path stdin/stdout variables at boot time to ensure that they will never be stale.
Signed-off-by: Mark Cave-Aylandmark.cave-ayland@ilande.co.uk
Modified: trunk/openbios-devel/arch/sparc32/boot.c trunk/openbios-devel/arch/sparc32/romvec.c
Since this patch has been applied, I am unable to boot a Debian SPARC32 guest using SILO as a bootloader. OpenBios stops at "Trying disk".
One can find such a guest there: http://people.debian.org/~aurel32/qemu/sparc/debian_etch_sparc_small.qcow2
Hi Aurelian,
Thanks for the bug report. I've just downloaded your image above and tested it against a fresh build of OpenBIOS r1235 here and it works fine for me (QEMU 1.7, gcc 4.7.1, sparc-elf-gcc cross-compile on Debian amd64). Is there anything else about your build environment that may be different somehow?
I have tried on almost the same build environment than you, and I also on a native gcc 4.8 compiler on sparc. For me this also doesn't work with the OpenBIOS version provided in QEMU 1.7.0, using the following command:
qemu-system-sparc -hda /tmp/debian_etch_sparc_small.qcow2
Best regards, Aurelien
On 08/12/13 12:25, Aurelien Jarno wrote:
Hi Aurelian,
Thanks for the bug report. I've just downloaded your image above and tested it against a fresh build of OpenBIOS r1235 here and it works fine for me (QEMU 1.7, gcc 4.7.1, sparc-elf-gcc cross-compile on Debian amd64). Is there anything else about your build environment that may be different somehow?
I have tried on almost the same build environment than you, and I also on a native gcc 4.8 compiler on sparc. For me this also doesn't work with the OpenBIOS version provided in QEMU 1.7.0, using the following command:
qemu-system-sparc -hda /tmp/debian_etch_sparc_small.qcow2
Best regards, Aurelien
Hi Aurelien,
Ahhh I see now - there was a change a while back to the way in which boot ordering information was passed from QEMU to OpenBIOS (with respect to default devices) in order to determine default boot devices. The net result was that things didn't work without an explicit boot device specified added to the QEMU command line.
At the moment, you should be able to boot by adding "-boot c" to your QEMU command line. I'm not sure why that particular commit triggers the bug in your case though. Do you think that booting QEMU in this way without a specified boot device is a bug? If so, any insight as to what causes this would be gratefully appreciated.
ATB,
Mark.
On Sun, Dec 08, 2013 at 12:37:22PM +0000, Mark Cave-Ayland wrote:
On 08/12/13 12:25, Aurelien Jarno wrote:
Hi Aurelian,
Thanks for the bug report. I've just downloaded your image above and tested it against a fresh build of OpenBIOS r1235 here and it works fine for me (QEMU 1.7, gcc 4.7.1, sparc-elf-gcc cross-compile on Debian amd64). Is there anything else about your build environment that may be different somehow?
I have tried on almost the same build environment than you, and I also on a native gcc 4.8 compiler on sparc. For me this also doesn't work with the OpenBIOS version provided in QEMU 1.7.0, using the following command:
qemu-system-sparc -hda /tmp/debian_etch_sparc_small.qcow2
Best regards, Aurelien
Hi Aurelien,
Ahhh I see now - there was a change a while back to the way in which boot ordering information was passed from QEMU to OpenBIOS (with respect to default devices) in order to determine default boot devices. The net result was that things didn't work without an explicit boot device specified added to the QEMU command line.
At the moment, you should be able to boot by adding "-boot c" to your QEMU command line. I'm not sure why that particular commit triggers the bug in your case though. Do you think that booting QEMU in this way without a specified boot device is a bug? If so, any insight as to what causes this would be gratefully appreciated.
Unfortunately it doesn't seems to work. I tried the following command, with both the version in QEMU 1.7.0 and revision 1236, and it doesn't work, hanging after "Trying disk...":
qemu-system-sparc -hda /tmp/debian_etch_sparc_small.qcow2 -boot c
Best regards, Aurelien
On 08/12/13 13:29, Aurelien Jarno wrote:
Unfortunately it doesn't seems to work. I tried the following command, with both the version in QEMU 1.7.0 and revision 1236, and it doesn't work, hanging after "Trying disk...":
qemu-system-sparc -hda /tmp/debian_etch_sparc_small.qcow2 -boot c
Best regards, Aurelien
Hmmm okay that should work. I've been quite ill recently so my build tree is probably a few weeks out of date; I'll rebase onto master, do a complete clean build and try again.
ATB,
Mark.
-
Aurelien Jarno -
Mark Cave-Ayland -
repository service