Re: [Xen-devel] [PATCH V5] xen: arm: platforms: Adding reset support for xgene arm64 platform.

[Pranavkumar Sawargaonkar <pranavkumar@xxxxxxxxxx>]

Hi Ian,

On 27 January 2014 16:02, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
> On Mon, 2014-01-27 at 14:12 +0530, Pranavkumar Sawargaonkar wrote:
>> +    /* Retrieve base address and size */
>> +    res = dt_device_get_address(dev, 0, &reset_addr, &reset_size);
>> +    if ( res )
>> +    {
>> +        printk("XGENE: Unable to retrieve the base address for reset\n");
>> +        return 0;
>> +    }
>> +
>> +    /* Get reset mask */
>> +    res = dt_property_read_u32(dev, "mask", &reset_mask);
>> +    if ( !res )
>> +    {
>
> At this point reset_addr and reset_size are set and xgene_storm_reset
> will try to use them with reset_mask -- which I suppose is 0 at this
> point (due to .bss initialisation + dt_prop_read not touching it on
> failure).
I thought that if reset_addr or reset_size is 0 then ioremap will not
return me a success hence code will return it without writing the reg.
But i think mask can cause issue if uninitialized, so i will add a
global flag  (valid_reset_vals) and will set it at the end of the
xgene_storm_init, and in fuction xgene_storm_reset i will first check
if this falg is set or not before proceeding further in reset.
Is this fine ?

>
> Is that safe / ok?
>
> In fact, on failure of the dt_device_get_address xgene_storm_reset will
> try and use the values too and they may be uninitialised or may be
> DT_BAD_ADDR depending on the location of the failure.
>
> Could this be potentially harmful? Obviously it is not expected to
> successfully reset under these circumstances, but what else could it do?
> (e.g. turn off the fan and melt the system?)
>
> I'd suggest to set a flag right at the end which xgene_storm_reset can
> check. Either an explicit boolean or initialise reset_addr to ~(u64)0
> when it is declared and gather the address into a local variable before
> setting the global only after init has succeeded.

Ok will do that.
>
> (I'd also accept your assurances that writing to random memory locations
> is safe, on the off chance that this is true ;-))
>
> Ian.
>

Thanks,
Pranav

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel