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

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

Li Guang wrote:
> 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.
>

Ok, I found the problem finally,
load_image_targphys depends on rom_reset to take effect,
if it's located at reset handler, rom_reset is called before it,
it surely failed to do the right job.

so, I have to write my own code to load blob into RAM.

Thanks!