Re: [Qemu-devel] [PATCH] hw/misc/blob-loader: add a generic blob loader

[Li Guang <lig.fnst@cn.fujitsu.com>]

Peter Crosthwaite wrote:
> On Mon, Jan 6, 2014 at 2:22 PM, Li Guang<lig.fnst@cn.fujitsu.com>  wrote:
>    
>> Peter Crosthwaite wrote:
>>      
>>> On Mon, Jan 6, 2014 at 1:55 PM, Li Guang<lig.fnst@cn.fujitsu.com>   wrote:
>>>
>>>        
>>>> Li Guang wrote:
>>>>
>>>>          
>>>>> Paolo Bonzini wrote:
>>>>>
>>>>>            
>>>>>> Il 02/01/2014 11:51, Peter Crosthwaite ha scritto:
>>>>>>
>>>>>>              
>>>>>>>>> No, please use "realize" and avoid init.  This way you can use an
>>>>>>>>> Error*
>>>>>>>>> to report the error.
>>>>>>>>>
>>>>>>>>> Also, the actual load_image_targphys call probably should be done in
>>>>>>>>> a
>>>>>>>>> reset handler, not at realize time.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>                    
>>>>>>> Ok I think that settles it. The actual blobbing needs to happen at
>>>>>>> reset time. Perhaps the correct approach is to do as much as possible
>>>>>>> (file-path / address sanitsation etc) at realize time, then only the
>>>>>>> actual blob load happens at reset. Going on what Paolo said, I think
>>>>>>> for this device ::init is actually a nop.
>>>>>>>
>>>>>>>                
>>>>>> Yeah, also because init is in fact a legacy interface to realize.
>>>>>>
>>>>>> Paolo
>>>>>>
>>>>>>
>>>>>>              
>>>>> Ok, thanks!
>>>>>
>>>>>
>>>>>
>>>>>            
>>>> Sorry, seems load blob at reset handler can't do the right job,
>>>> while the same action can play very well at init or realize.
>>>>
>>>>          
>>> What's the exact problem with the reset idea?
>>>
>>>
>>>
>>>        
>> code snippet:
>> static void blob_loader_reset(DeviceState *dev)
>> {
>>      BlobLoaderState *s = BLOB_LOADER(dev);
>>
>>      if (load_image_targphys(s->file, s->hwaddr, MAX_BLOB_SIZE)<  0) {
>>          error_report("can't load %s\n", s->file);
>>          exit(1);
>>      }
>> }
>>
>> if it is device reset handler, no result,
>> kernel doesn't find and parse blob,
>> if it is called at device realize phase,
>> it works.
>>
>>      
> Need to figure out why I think. There's no fundamental problem here
> AFAIK. It must be a bug somewhere.
>
>    
>> Thanks!
>>
>>
>>      
>>>> any suggestion to figure out this problem?
>>>>          
> Run QEMU in GDB and break on your new reset function to see if it is
> ever called. If not have a look into QOM/qdev to see how resets work
> and how that plays with -device args. If yes, have a look into
> load_image_targphys and see why that's not working.
>
>
>    

reset handler definitely be called, and
load_image_targphys has no problem,
I guess it may be impacted by other RAM related codes,
just can't figure out it quickly.

Thanks!

>>>> Thanks!
>>>>
>>>>
>>>>
>>>>
>>>>          
>>>
>>>
>>>        
>>
>>
>>      
>
>