[v2] external/gard: Print an error if run on an FSP system

Submitted by Cyril Bur on Aug. 3, 2017, 7:08 a.m.

Details

Message ID 20170803070852.6746-1-cyril.bur@au1.ibm.com
State New
Headers show

Commit Message

Cyril Bur Aug. 3, 2017, 7:08 a.m.
Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
---
 external/gard/gard.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

Comments

Vasant Hegde Aug. 7, 2017, 4:51 a.m.
On 08/03/2017 12:38 PM, Cyril Bur wrote:
> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
> ---
>  external/gard/gard.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/external/gard/gard.c b/external/gard/gard.c
> index 4b26a3b8..55e2a924 100644
> --- a/external/gard/gard.c
> +++ b/external/gard/gard.c
> @@ -43,7 +43,9 @@
>
>  #define CLEARED_RECORD_ID 0xFFFFFFFF
>
> -#define FDT_ACTIVE_FLASH_PATH "/proc/device-tree/chosen/ibm,system-flash"
> +#define FDT_PATH "/proc/device-tree"
> +#define FDT_FSP_NODE FDT_PATH"/fsps"

How about checking "ibm,system-flash" property and if that doesn't exist then 
throw an error?

-Vasant
Cyril Bur Aug. 7, 2017, 5:18 a.m.
On Mon, 2017-08-07 at 10:21 +0530, Vasant Hegde wrote:
> On 08/03/2017 12:38 PM, Cyril Bur wrote:
> > Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
> > ---
> >  external/gard/gard.c | 15 ++++++++++++++-
> >  1 file changed, 14 insertions(+), 1 deletion(-)
> > 
> > diff --git a/external/gard/gard.c b/external/gard/gard.c
> > index 4b26a3b8..55e2a924 100644
> > --- a/external/gard/gard.c
> > +++ b/external/gard/gard.c
> > @@ -43,7 +43,9 @@
> > 
> >  #define CLEARED_RECORD_ID 0xFFFFFFFF
> > 
> > -#define FDT_ACTIVE_FLASH_PATH "/proc/device-tree/chosen/ibm,system-flash"
> > +#define FDT_PATH "/proc/device-tree"
> > +#define FDT_FSP_NODE FDT_PATH"/fsps"
> 
> How about checking "ibm,system-flash" property and if that doesn't exist then 
> throw an error?
> 

Hi Vasant,

This already happens and not having "ibm,system-flash" doesn't
necessarily mean the system is FSP. The current message for this is in
external/common/arch_flash_powerpc.c:

 	fd = open(fdt_flash_path, O_RDONLY);
	if (fd == -1) {
		fprintf(stderr, "Couldn't open '%s' FDT attribute to determine
which flash device to use\n", fdt_flash_path);
		fprintf(stderr, "Is your skiboot new enough to expose the flash
through the device tree
\n");                                                                  
            
		hint_root();
		return -1;
	}


That is taken from get_dev_mtd() and it is called with fdt_flash_path
being the path to "ibm,system-flash".

Admittedly skiboots which don't expose "ibm,system-flash" are going to
be old these days...

Cyril

> -Vasant
>
Vasant Hegde Aug. 7, 2017, 2:31 p.m.
On 08/07/2017 10:48 AM, Cyril Bur wrote:
> On Mon, 2017-08-07 at 10:21 +0530, Vasant Hegde wrote:
>> On 08/03/2017 12:38 PM, Cyril Bur wrote:
>>> Signed-off-by: Cyril Bur <cyril.bur@au1.ibm.com>
>>> ---
>>>  external/gard/gard.c | 15 ++++++++++++++-
>>>  1 file changed, 14 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/external/gard/gard.c b/external/gard/gard.c
>>> index 4b26a3b8..55e2a924 100644
>>> --- a/external/gard/gard.c
>>> +++ b/external/gard/gard.c
>>> @@ -43,7 +43,9 @@
>>>
>>>  #define CLEARED_RECORD_ID 0xFFFFFFFF
>>>
>>> -#define FDT_ACTIVE_FLASH_PATH "/proc/device-tree/chosen/ibm,system-flash"
>>> +#define FDT_PATH "/proc/device-tree"
>>> +#define FDT_FSP_NODE FDT_PATH"/fsps"
>>
>> How about checking "ibm,system-flash" property and if that doesn't exist then
>> throw an error?
>>
>
> Hi Vasant,

Hi Cyril,

>
> This already happens and not having "ibm,system-flash" doesn't
> necessarily mean the system is FSP. The current message for this is in
> external/common/arch_flash_powerpc.c:

Thanks for the clarification. I missed this.

-Vasant

Patch hide | download patch | download mbox

diff --git a/external/gard/gard.c b/external/gard/gard.c
index 4b26a3b8..55e2a924 100644
--- a/external/gard/gard.c
+++ b/external/gard/gard.c
@@ -43,7 +43,9 @@ 
 
 #define CLEARED_RECORD_ID 0xFFFFFFFF
 
-#define FDT_ACTIVE_FLASH_PATH "/proc/device-tree/chosen/ibm,system-flash"
+#define FDT_PATH "/proc/device-tree"
+#define FDT_FSP_NODE FDT_PATH"/fsps"
+#define FDT_ACTIVE_FLASH_PATH FDT_PATH"/chosen/ibm,system-flash"
 #define SYSFS_MTD_PATH "/sys/class/mtd/"
 #define FLASH_GARD_PART "GUARD"
 
@@ -563,6 +565,11 @@  static void usage(const char *progname)
 	}
 }
 
+static bool is_fsp(void)
+{
+	return access(FDT_FSP_NODE, F_OK) == 0;
+}
+
 static struct option global_options[] = {
 	{ "file", required_argument, 0, 'f' },
 	{ "part", no_argument, 0, 'p' },
@@ -586,6 +593,12 @@  int main(int argc, char **argv)
 	ctx = &_ctx;
 	memset(ctx, 0, sizeof(*ctx));
 
+	if (is_fsp()) {
+		fprintf(stderr, "This is the OpenPower gard tool which does "
+				"not support FSP systems\n");
+		return EXIT_FAILURE;
+	}
+
 	/* process global options */
 	for (;;) {
 		int c;