Re: [Qemu-devel] [PATCH] sun4m: Add FCode ROM for TCX framebuffer

[Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>]

On 20/08/13 23:41, Peter Maydell wrote:

> On 20 August 2013 23:25, Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk>  wrote:
>> Upstream OpenBIOS now implements SBus probing in order to determine the
>> contents of a physical bus slot, which is required to allow OpenBIOS to
>> identify the framebuffer without help from the fw_cfg interface.
>>
>> SBus probing works by detecting the presence of an FCode program
>> (effectively tokenised Forth) at the base address of each slot, and if
>> present executes it so that it creates its own device node in the
>> OpenBIOS device tree.
>>
>> The FCode ROM is generated as part of the OpenBIOS build and should
>> generally be updated at the same time.
>
>>   hw/display/tcx.c     |    7 +++++++
>>   hw/sparc/sun4m.c     |   32 +++++++++++++++++++++++++-------
>>   pc-bios/QEMU,tcx.bin |  Bin 0 ->  1242 bytes
>
> Shouldn't this blob come in the same patch as an update to some
> git module, so that we keep track of the sources used to build
> the blob?

Unfortunately the OpenBIOS repository is still based in SVN :(  There is 
a git-svn mirror on git.qemu.org, but currently it needs to be manually 
updated and so is generally not particularly helpful. For the 1.6 
release I got Anthony to manually update the repository on git.qemu.org 
so that the git submodule reference was updated as part of the pull request.

The main reason for not updating the git submodule in this particular 
commit is because this is actually a precursor to another larger sun4m 
framebuffer patch, and once both these patches are hopefully accepted 
then my plan is to send a single pull request to update all 3 of the 
OpenBIOS images at the same time rather than to split architectures 
across different OpenBIOS versions.

As the existing OpenBIOS image won't bother to check for the new FCode 
ROMs then in the short term it will use the existing hardwired device 
nodes and so remain bisectable.

>> --- a/hw/sparc/sun4m.c
>> +++ b/hw/sparc/sun4m.c
>
>> +    fcode_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, TCX_ROM_FILE);
>> +    if (fcode_filename) {
>> +        ret = load_image_targphys(fcode_filename, addr, FCODE_MAX_ROM_SIZE);
>> +    }
>
> This looks like the wrong place for this -- surely the tcx device
> should load its own fcode blob, not defer to the board code
> to do it?

Good point - that does make more sense. I'll rework the patch with the 
ROM load moved into the device itself.

> (For that matter, presumably if this is an SBus device then
> the offsets of the ROM, DAC, etc etc are all fixed relative to
> the base address of the SBus slot, and the tcx device itself
> should be creating a container with all its component parts
> at the right offset. But that's not an issue for this patch.)

Indeed - I suspect a lot of the sparc-softmmu code is still based upon 
legacy APIs :/


ATB,

Mark.