Initialise empty NVRAM with default values. This also enables IDE UDMA
mode in AmigaOS that is faster but has to be enabled in environment
due to problems with real hardware but that does not affect emulation
so we can use faster defaults here.

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
index 849c9fc6e0..5c5585d39a 100644
--- a/hw/ppc/amigaone.c
+++ b/hw/ppc/amigaone.c
@@ -52,6 +52,28 @@ static const char dummy_fw[] = {
 #define NVRAM_ADDR 0xfd0e0000
 #define NVRAM_SIZE (4 * KiB)
 
+static char default_env[] =
+    "baudrate=115200\0"
+    "stdout=vga\0"
+    "stdin=ps2kbd\0"
+    "bootcmd=boota; menu; run menuboot_cmd\0"
+    "boot1=ide\0"
+    "boot2=cdrom\0"
+    "boota_timeout=3\0"
+    "ide_doreset=on\0"
+    "pci_irqa=9\0"
+    "pci_irqa_select=level\0"
+    "pci_irqb=10\0"
+    "pci_irqb_select=level\0"
+    "pci_irqc=11\0"
+    "pci_irqc_select=level\0"
+    "pci_irqd=7\0"
+    "pci_irqd_select=level\0"
+    "a1ide_irq=1111\0"
+    "a1ide_xfer=FFFF\0";
+#define CRC32_DEFAULT_ENV 0xb5548481
+#define CRC32_ALL_ZEROS   0x603b0489
+
 #define TYPE_A1_NVRAM "a1-nvram"
 OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
 
@@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, Error **errp)
 {
     A1NVRAMState *s = A1_NVRAM(dev);
     void *p;
-    uint32_t *c;
+    uint32_t crc, *c;
 
     memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, "nvram",
                                   NVRAM_SIZE, &error_fatal);
@@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, Error **errp)
             return;
         }
     }
+    crc = crc32(0, p + 4, NVRAM_SIZE - 4);
+    if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set default */
+        *c = cpu_to_be32(CRC32_DEFAULT_ENV);
+        /* Also copies terminating \0 as env is terminated by \0\0 */
+        memcpy(p + 4, default_env, sizeof(default_env));
+        if (s->blk) {
+            blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p, 0);
+        }
+        return;
+    }
     if (*c == 0) {
         *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
         if (s->blk) {
             blk_pwrite(s->blk, 0, 4, p, 0);
         }
     }
+    if (be32_to_cpu(*c) != crc) {
+        warn_report("NVRAM checksum mismatch");
+    }
 }
 
 static const Property nvram_properties[] = {
-- 
2.30.9

Hi Zoltan,

Minor review comments in case you respin (not blocking).

On 27/2/25 17:39, BALATON Zoltan wrote:
> Initialise empty NVRAM with default values. This also enables IDE UDMA
> mode in AmigaOS that is faster but has to be enabled in environment
> due to problems with real hardware but that does not affect emulation
> so we can use faster defaults here.
> 
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
>   hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
>   1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
> index 849c9fc6e0..5c5585d39a 100644
> --- a/hw/ppc/amigaone.c
> +++ b/hw/ppc/amigaone.c
> @@ -52,6 +52,28 @@ static const char dummy_fw[] = {
>   #define NVRAM_ADDR 0xfd0e0000
>   #define NVRAM_SIZE (4 * KiB)
>   
> +static char default_env[] =

'const'

> +    "baudrate=115200\0"
> +    "stdout=vga\0"
> +    "stdin=ps2kbd\0"
> +    "bootcmd=boota; menu; run menuboot_cmd\0"
> +    "boot1=ide\0"
> +    "boot2=cdrom\0"
> +    "boota_timeout=3\0"
> +    "ide_doreset=on\0"
> +    "pci_irqa=9\0"
> +    "pci_irqa_select=level\0"
> +    "pci_irqb=10\0"
> +    "pci_irqb_select=level\0"
> +    "pci_irqc=11\0"
> +    "pci_irqc_select=level\0"
> +    "pci_irqd=7\0"
> +    "pci_irqd_select=level\0"
> +    "a1ide_irq=1111\0"
> +    "a1ide_xfer=FFFF\0";
> +#define CRC32_DEFAULT_ENV 0xb5548481
> +#define CRC32_ALL_ZEROS   0x603b0489

> +
>   #define TYPE_A1_NVRAM "a1-nvram"
>   OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
>   
> @@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, Error **errp)
>   {
>       A1NVRAMState *s = A1_NVRAM(dev);
>       void *p;
> -    uint32_t *c;
> +    uint32_t crc, *c;
>   
>       memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, "nvram",
>                                     NVRAM_SIZE, &error_fatal);
> @@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, Error **errp)
>               return;
>           }
>       }
> +    crc = crc32(0, p + 4, NVRAM_SIZE - 4);
> +    if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set default */
> +        *c = cpu_to_be32(CRC32_DEFAULT_ENV);

Prefer the ld/st API over cpu_to/from:

            stl_be_p(c, CRC32_DEFAULT_ENV);

> +        /* Also copies terminating \0 as env is terminated by \0\0 */
> +        memcpy(p + 4, default_env, sizeof(default_env));
> +        if (s->blk) {
> +            blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p, 0);
> +        }
> +        return;
> +    }
>       if (*c == 0) {
>           *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
>           if (s->blk) {
>               blk_pwrite(s->blk, 0, 4, p, 0);
>           }
>       }
> +    if (be32_to_cpu(*c) != crc) {

        if (ldl_be_p(c) != crc) {

> +        warn_report("NVRAM checksum mismatch");
> +    }
>   }

On Fri, 7 Mar 2025, Philippe Mathieu-Daudé wrote:
> Hi Zoltan,
>
> Minor review comments in case you respin (not blocking).
>
> On 27/2/25 17:39, BALATON Zoltan wrote:
>> Initialise empty NVRAM with default values. This also enables IDE UDMA
>> mode in AmigaOS that is faster but has to be enabled in environment
>> due to problems with real hardware but that does not affect emulation
>> so we can use faster defaults here.
>> 
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>   hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
>>   1 file changed, 36 insertions(+), 1 deletion(-)
>> 
>> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
>> index 849c9fc6e0..5c5585d39a 100644
>> --- a/hw/ppc/amigaone.c
>> +++ b/hw/ppc/amigaone.c
>> @@ -52,6 +52,28 @@ static const char dummy_fw[] = {
>>   #define NVRAM_ADDR 0xfd0e0000
>>   #define NVRAM_SIZE (4 * KiB)
>>   +static char default_env[] =
>
> 'const'

OK. Could be fixed up on merge by Nick or I can send a new version if 
needed.

>> +    "baudrate=115200\0"
>> +    "stdout=vga\0"
>> +    "stdin=ps2kbd\0"
>> +    "bootcmd=boota; menu; run menuboot_cmd\0"
>> +    "boot1=ide\0"
>> +    "boot2=cdrom\0"
>> +    "boota_timeout=3\0"
>> +    "ide_doreset=on\0"
>> +    "pci_irqa=9\0"
>> +    "pci_irqa_select=level\0"
>> +    "pci_irqb=10\0"
>> +    "pci_irqb_select=level\0"
>> +    "pci_irqc=11\0"
>> +    "pci_irqc_select=level\0"
>> +    "pci_irqd=7\0"
>> +    "pci_irqd_select=level\0"
>> +    "a1ide_irq=1111\0"
>> +    "a1ide_xfer=FFFF\0";
>> +#define CRC32_DEFAULT_ENV 0xb5548481
>> +#define CRC32_ALL_ZEROS   0x603b0489
>
>> +
>>   #define TYPE_A1_NVRAM "a1-nvram"
>>   OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
>>   @@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, Error 
>> **errp)
>>   {
>>       A1NVRAMState *s = A1_NVRAM(dev);
>>       void *p;
>> -    uint32_t *c;
>> +    uint32_t crc, *c;
>>         memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, "nvram",
>>                                     NVRAM_SIZE, &error_fatal);
>> @@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, Error 
>> **errp)
>>               return;
>>           }
>>       }
>> +    crc = crc32(0, p + 4, NVRAM_SIZE - 4);
>> +    if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set default 
>> */
>> +        *c = cpu_to_be32(CRC32_DEFAULT_ENV);
>
> Prefer the ld/st API over cpu_to/from:
>
>           stl_be_p(c, CRC32_DEFAULT_ENV);
>
>> +        /* Also copies terminating \0 as env is terminated by \0\0 */
>> +        memcpy(p + 4, default_env, sizeof(default_env));
>> +        if (s->blk) {
>> +            blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p, 
>> 0);
>> +        }
>> +        return;
>> +    }
>>       if (*c == 0) {
>>           *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
>>           if (s->blk) {
>>               blk_pwrite(s->blk, 0, 4, p, 0);
>>           }
>>       }
>> +    if (be32_to_cpu(*c) != crc) {
>
>       if (ldl_be_p(c) != crc) {

Why? Here we want to convert a value from host CPU endianness to a 
specific endianness and vice versa in code running on the host. (We are 
not accessing guest memory, we operate on the memory region pointer. The 
guest is not even running yet.)

Also:

static inline int ldl_be_p(const void *ptr)
{
     return be_bswap(ldl_he_p(ptr), 32);
}

static inline int ldl_he_p(const void *ptr)
{
     int32_t r;
     __builtin_memcpy(&r, ptr, sizeof(r));
     return r;
}

#define be_bswap(v, size) glue(__builtin_bswap, size)(v)

so this is

int32_t r;
__builtin_memcpy(&r, c, sizeof(r));
__builtin_bswap32(r);

versus

static inline type endian ## size ## _to_cpu(type v)
{
     return glue(endian, _bswap)(v, size);
}

which is just

__builtin_bswap32(*c);

The second one makes more sense to me and don't see why I'd want to do it 
in a more cumbersome way when we end up with the same result but simpler.

Regards,
BALATON Zoltan

>> +        warn_report("NVRAM checksum mismatch");
>> +    }
>>   }

On 7/3/25 15:46, BALATON Zoltan wrote:
> On Fri, 7 Mar 2025, Philippe Mathieu-Daudé wrote:
>> Hi Zoltan,
>>
>> Minor review comments in case you respin (not blocking).
>>
>> On 27/2/25 17:39, BALATON Zoltan wrote:
>>> Initialise empty NVRAM with default values. This also enables IDE UDMA
>>> mode in AmigaOS that is faster but has to be enabled in environment
>>> due to problems with real hardware but that does not affect emulation
>>> so we can use faster defaults here.
>>>
>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>> ---
>>>   hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
>>>   1 file changed, 36 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
>>> index 849c9fc6e0..5c5585d39a 100644
>>> --- a/hw/ppc/amigaone.c
>>> +++ b/hw/ppc/amigaone.c
>>> @@ -52,6 +52,28 @@ static const char dummy_fw[] = {
>>>   #define NVRAM_ADDR 0xfd0e0000
>>>   #define NVRAM_SIZE (4 * KiB)
>>>   +static char default_env[] =
>>
>> 'const'
> 
> OK. Could be fixed up on merge by Nick or I can send a new version if 
> needed.
> 
>>> +    "baudrate=115200\0"
>>> +    "stdout=vga\0"
>>> +    "stdin=ps2kbd\0"
>>> +    "bootcmd=boota; menu; run menuboot_cmd\0"
>>> +    "boot1=ide\0"
>>> +    "boot2=cdrom\0"
>>> +    "boota_timeout=3\0"
>>> +    "ide_doreset=on\0"
>>> +    "pci_irqa=9\0"
>>> +    "pci_irqa_select=level\0"
>>> +    "pci_irqb=10\0"
>>> +    "pci_irqb_select=level\0"
>>> +    "pci_irqc=11\0"
>>> +    "pci_irqc_select=level\0"
>>> +    "pci_irqd=7\0"
>>> +    "pci_irqd_select=level\0"
>>> +    "a1ide_irq=1111\0"
>>> +    "a1ide_xfer=FFFF\0";
>>> +#define CRC32_DEFAULT_ENV 0xb5548481
>>> +#define CRC32_ALL_ZEROS   0x603b0489
>>
>>> +
>>>   #define TYPE_A1_NVRAM "a1-nvram"
>>>   OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
>>>   @@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, 
>>> Error **errp)
>>>   {
>>>       A1NVRAMState *s = A1_NVRAM(dev);
>>>       void *p;
>>> -    uint32_t *c;
>>> +    uint32_t crc, *c;
>>>         memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, 
>>> "nvram",
>>>                                     NVRAM_SIZE, &error_fatal);
>>> @@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, 
>>> Error **errp)
>>>               return;
>>>           }
>>>       }
>>> +    crc = crc32(0, p + 4, NVRAM_SIZE - 4);
>>> +    if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set 
>>> default */
>>> +        *c = cpu_to_be32(CRC32_DEFAULT_ENV);
>>
>> Prefer the ld/st API over cpu_to/from:
>>
>>           stl_be_p(c, CRC32_DEFAULT_ENV);
>>
>>> +        /* Also copies terminating \0 as env is terminated by \0\0 */
>>> +        memcpy(p + 4, default_env, sizeof(default_env));
>>> +        if (s->blk) {
>>> +            blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), 
>>> p, 0);
>>> +        }
>>> +        return;
>>> +    }
>>>       if (*c == 0) {
>>>           *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
>>>           if (s->blk) {
>>>               blk_pwrite(s->blk, 0, 4, p, 0);
>>>           }
>>>       }
>>> +    if (be32_to_cpu(*c) != crc) {
>>
>>       if (ldl_be_p(c) != crc) {
> 
> Why? Here we want to convert a value from host CPU endianness to a 
> specific endianness and vice versa in code running on the host. (We are 
> not accessing guest memory, we operate on the memory region pointer. The 
> guest is not even running yet.)
> 
> Also:
> 
> static inline int ldl_be_p(const void *ptr)
> {
>      return be_bswap(ldl_he_p(ptr), 32);
> }
> 
> static inline int ldl_he_p(const void *ptr)
> {
>      int32_t r;
>      __builtin_memcpy(&r, ptr, sizeof(r));
>      return r;
> }
> 
> #define be_bswap(v, size) glue(__builtin_bswap, size)(v)
> 
> so this is
> 
> int32_t r;
> __builtin_memcpy(&r, c, sizeof(r));

This call makes the address alignment access safe.

Sometimes we use similar API doing unaligned access and static
analyzers complain [*]. Rather than maintaining 2 differents APIs
with some corner cases in one, we could always use the reliable
one.

[*] see for example commit 5814c084679 ("hw/net/virtio-net.c: Don't 
assume IP length field is aligned")

> __builtin_bswap32(r);
> 
> versus
> 
> static inline type endian ## size ## _to_cpu(type v)
> {
>      return glue(endian, _bswap)(v, size);
> }
> 
> which is just
> 
> __builtin_bswap32(*c);
> 
> The second one makes more sense to me and don't see why I'd want to do 
> it in a more cumbersome way when we end up with the same result but 
> simpler.
> 
> Regards,
> BALATON Zoltan
> 
>>> +        warn_report("NVRAM checksum mismatch");
>>> +    }
>>>   }

On Fri, 7 Mar 2025, Philippe Mathieu-Daudé wrote:
> On 7/3/25 15:46, BALATON Zoltan wrote:
>> On Fri, 7 Mar 2025, Philippe Mathieu-Daudé wrote:
>>> Hi Zoltan,
>>> 
>>> Minor review comments in case you respin (not blocking).
>>> 
>>> On 27/2/25 17:39, BALATON Zoltan wrote:
>>>> Initialise empty NVRAM with default values. This also enables IDE UDMA
>>>> mode in AmigaOS that is faster but has to be enabled in environment
>>>> due to problems with real hardware but that does not affect emulation
>>>> so we can use faster defaults here.
>>>> 
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>> ---
>>>>   hw/ppc/amigaone.c | 37 ++++++++++++++++++++++++++++++++++++-
>>>>   1 file changed, 36 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/hw/ppc/amigaone.c b/hw/ppc/amigaone.c
>>>> index 849c9fc6e0..5c5585d39a 100644
>>>> --- a/hw/ppc/amigaone.c
>>>> +++ b/hw/ppc/amigaone.c
>>>> @@ -52,6 +52,28 @@ static const char dummy_fw[] = {
>>>>   #define NVRAM_ADDR 0xfd0e0000
>>>>   #define NVRAM_SIZE (4 * KiB)
>>>>   +static char default_env[] =
>>> 
>>> 'const'
>> 
>> OK. Could be fixed up on merge by Nick or I can send a new version if 
>> needed.
>> 
>>>> +    "baudrate=115200\0"
>>>> +    "stdout=vga\0"
>>>> +    "stdin=ps2kbd\0"
>>>> +    "bootcmd=boota; menu; run menuboot_cmd\0"
>>>> +    "boot1=ide\0"
>>>> +    "boot2=cdrom\0"
>>>> +    "boota_timeout=3\0"
>>>> +    "ide_doreset=on\0"
>>>> +    "pci_irqa=9\0"
>>>> +    "pci_irqa_select=level\0"
>>>> +    "pci_irqb=10\0"
>>>> +    "pci_irqb_select=level\0"
>>>> +    "pci_irqc=11\0"
>>>> +    "pci_irqc_select=level\0"
>>>> +    "pci_irqd=7\0"
>>>> +    "pci_irqd_select=level\0"
>>>> +    "a1ide_irq=1111\0"
>>>> +    "a1ide_xfer=FFFF\0";
>>>> +#define CRC32_DEFAULT_ENV 0xb5548481
>>>> +#define CRC32_ALL_ZEROS   0x603b0489
>>> 
>>>> +
>>>>   #define TYPE_A1_NVRAM "a1-nvram"
>>>>   OBJECT_DECLARE_SIMPLE_TYPE(A1NVRAMState, A1_NVRAM)
>>>>   @@ -94,7 +116,7 @@ static void nvram_realize(DeviceState *dev, Error 
>>>> **errp)
>>>>   {
>>>>       A1NVRAMState *s = A1_NVRAM(dev);
>>>>       void *p;
>>>> -    uint32_t *c;
>>>> +    uint32_t crc, *c;
>>>>         memory_region_init_rom_device(&s->mr, NULL, &nvram_ops, s, 
>>>> "nvram",
>>>>                                     NVRAM_SIZE, &error_fatal);
>>>> @@ -113,12 +135,25 @@ static void nvram_realize(DeviceState *dev, Error 
>>>> **errp)
>>>>               return;
>>>>           }
>>>>       }
>>>> +    crc = crc32(0, p + 4, NVRAM_SIZE - 4);
>>>> +    if (crc == CRC32_ALL_ZEROS) { /* If env is uninitialized set default 
>>>> */
>>>> +        *c = cpu_to_be32(CRC32_DEFAULT_ENV);
>>> 
>>> Prefer the ld/st API over cpu_to/from:
>>> 
>>>           stl_be_p(c, CRC32_DEFAULT_ENV);
>>> 
>>>> +        /* Also copies terminating \0 as env is terminated by \0\0 */
>>>> +        memcpy(p + 4, default_env, sizeof(default_env));
>>>> +        if (s->blk) {
>>>> +            blk_pwrite(s->blk, 0, sizeof(crc) + sizeof(default_env), p, 
>>>> 0);
>>>> +        }
>>>> +        return;
>>>> +    }
>>>>       if (*c == 0) {
>>>>           *c = cpu_to_be32(crc32(0, p + 4, NVRAM_SIZE - 4));
>>>>           if (s->blk) {
>>>>               blk_pwrite(s->blk, 0, 4, p, 0);
>>>>           }
>>>>       }
>>>> +    if (be32_to_cpu(*c) != crc) {
>>> 
>>>       if (ldl_be_p(c) != crc) {
>> 
>> Why? Here we want to convert a value from host CPU endianness to a specific 
>> endianness and vice versa in code running on the host. (We are not 
>> accessing guest memory, we operate on the memory region pointer. The guest 
>> is not even running yet.)
>> 
>> Also:
>> 
>> static inline int ldl_be_p(const void *ptr)
>> {
>>      return be_bswap(ldl_he_p(ptr), 32);
>> }
>> 
>> static inline int ldl_he_p(const void *ptr)
>> {
>>      int32_t r;
>>      __builtin_memcpy(&r, ptr, sizeof(r));
>>      return r;
>> }
>> 
>> #define be_bswap(v, size) glue(__builtin_bswap, size)(v)
>> 
>> so this is
>> 
>> int32_t r;
>> __builtin_memcpy(&r, c, sizeof(r));
>
> This call makes the address alignment access safe.
>
> Sometimes we use similar API doing unaligned access and static
> analyzers complain [*]. Rather than maintaining 2 differents APIs
> with some corner cases in one, we could always use the reliable
> one.
>
> [*] see for example commit 5814c084679 ("hw/net/virtio-net.c: Don't assume IP 
> length field is aligned")

That concern does not apply here as it's unlikely to have a memory region 
allocated unaligned so I'd keep this until it's removed from everywhere. 
We have a lot of uses of cpu_to_, _to_cpu now so it's not likely it would 
go away soon. I see no advantage in introducing unneeded complexity here 
to prevent something that cannot happen.

Regards,
BALATON Zoltan

>> __builtin_bswap32(r);
>> 
>> versus
>> 
>> static inline type endian ## size ## _to_cpu(type v)
>> {
>>      return glue(endian, _bswap)(v, size);
>> }
>> 
>> which is just
>> 
>> __builtin_bswap32(*c);
>> 
>> The second one makes more sense to me and don't see why I'd want to do it 
>> in a more cumbersome way when we end up with the same result but simpler.
>> 
>> Regards,
>> BALATON Zoltan
>> 
>>>> +        warn_report("NVRAM checksum mismatch");
>>>> +    }
>>>>   }
>
>