Subject: Re: [RFC] [PATCH] Write to HVC terminal from purgatory code

From: Michael Ellerman <[email-masked]>
Date: Thu, 17 Sep 2009 21:54:40 +1000

On Mon, 2009-09-07 at 10:44 +0530, M. Mohan Kumar wrote:
> Write to HVC terminal from purgatory code
> 
> Current x86/x86-64 kexec-tools print the message "I'm in purgatory" to
> serial console/VGA while executing the purgatory code.  Implement this
> feature for POWERPC pseries platform by using the H_PUT_TERM_CHAR
> hypervisor call by printng to hvc console.

Hi Mohan, just a few comments ...

> diff --git a/kexec/arch/ppc64/fs2dt.c b/kexec/arch/ppc64/fs2dt.c
> index b01ff86..bd9d36c 100644
> --- a/kexec/arch/ppc64/fs2dt.c
> +++ b/kexec/arch/ppc64/fs2dt.c
> @@ -434,6 +434,9 @@ static void putnode(void)
>  	if (!strcmp(basename,"/chosen/")) {
>  		size_t cmd_len = 0;
>  		char *param = NULL;
> +		char filename[MAXPATH];
> +		char buff[64];
> +		int fd;
>  
>  		cmd_len = strlen(local_cmdline);
>  		if (cmd_len != 0) {
> @@ -446,7 +449,6 @@ static void putnode(void)
>  
>  		/* ... if not, grab root= from the old command line */
>  		if (!param) {
> -			char filename[MAXPATH];
>  			FILE *fp;
>  			char *last_cmdline = NULL;
>  			char *old_param;
> @@ -483,8 +485,51 @@ static void putnode(void)
>  		dt += (cmd_len + 3)/4;
>  
>  		fprintf(stderr, "Modified cmdline:%s\n", local_cmdline);
> +
> +		/*
> +		 * Determine the platform type/stdout type, so that purgatory
> +		 * code can print 'I'm in purgatory' message. Currently only
> +		 * pseries/hvcterminal is supported.
> +		 */
> +		strcpy(filename, pathname);
> +		strcat(filename, "linux,stdout-path");

strncat() ?

> +		fd = open(filename, O_RDONLY);
> +		if (fd == -1) {
> +			printf("Unable to find linux,stdout-path, printing"
> +					" from purgatory is diabled\n");
> +			goto no_debug;
> +		}
> +		if (fstat(fd, &statbuf)) {
> +			printf("Unable to stat linux,stdout-path, printing"
> +					" from purgatory is diabled\n");
> +			close(fd);
> +			goto no_debug;
> +		}
> +		read(fd, buff, statbuf.st_size);

Be nice to check it's going to fit in buff here.

> +		close(fd);
> +		strcpy(filename, "/proc/device-tree/");
> +		strcat(filename, buff);
> +		strcat(filename, "/compatible");

strncpy / strncat.

> +		fd = open(filename, O_RDONLY);
> +		if (fd == -1) {
> +			printf("Unable to find linux,stdout-path/compatible, "
> +				" printing from purgatory is diabled\n");

You may as well print the actual path here, seeing as you have it.

> +			goto no_debug;
> +		}
> +		if (fstat(fd, &statbuf)) {
> +			printf("Unable to stat linux,stdout-path/compatible, "
> +				" printing from purgatory is diabled\n");
> +			close(fd);
> +			goto no_debug;
> +		}
> +		read(fd, buff, statbuf.st_size);
> +		if (!strcmp(buff, "hvterm1") ||
> +					!strcmp(buff, "hvterm-protocol"))
> +			my_debug = 1;

I have a personal hatred for variables called my_foo, but that's just
personal taste I guess :)

> +		close(fd);
>  	}
>  
> +no_debug:
>  	for (i=0; i < numlist; i++) {
>  		dp = namelist[i];
>  		strcpy(dn, dp->d_name);

> diff --git a/purgatory/arch/ppc64/console-ppc64.c b/purgatory/arch/ppc64/console-ppc64.c
> index d6da7b3..78a233b 100644
> --- a/purgatory/arch/ppc64/console-ppc64.c
> +++ b/purgatory/arch/ppc64/console-ppc64.c
> @@ -20,8 +20,22 @@
>   */
>  
>  #include <purgatory.h>
> +#include "hvCall.h"
> +
> +extern int debug;
>  
>  void putchar(int c)
>  {
> +	char buff[16];
> +	unsigned long *lbuf = (unsigned long *)buff;
> +
> +	if (!debug) /* running on non pseries */
> +		return;
> +
> +	if (c == '\n')
> +		putchar('\r');
> +
> +	buff[0] = c;
> +	plpar_hcall_norets(H_PUT_TERM_CHAR, 0, 1, lbuf[0], lbuf[1]);
>  	return;
>  }
> diff --git a/purgatory/arch/ppc64/hvCall.S b/purgatory/arch/ppc64/hvCall.S
> new file mode 100644
> index 0000000..e401f81
> --- /dev/null
> +++ b/purgatory/arch/ppc64/hvCall.S
> @@ -0,0 +1,28 @@
> +/*
> + * This file contains the generic function to perform a call to the
> + * pSeries LPAR hypervisor.
> + *
> + * Created by M. Mohan Kumar ([email-masked])
> + * Copyright (C) IBM Corporation
> + * Taken from linux/arch/powerpc/platforms/pseries/hvCall.S
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License
> + * as published by the Free Software Foundation; either version
> + * 2 of the License, or (at your option) any later version.
> + */
> +
> +#define HVSC	.long 0x44000022
> +.text
> +	.machine ppc64
> +.globl .plpar_hcall_norets
> +.plpar_hcall_norets:
> +	or	6,6,6			# medium low priority
> +        mfcr	0
> +        stw	0,8(1)
> +
> +        HVSC 				/* invoke the hypervisor */
> +
> +        lwz	0,8(1)
> +        mtcrf	0xff,0
> +        blr                             /* return r3 = status */

It's much easier to read if you use r0, r1 etc.

cheers