Re: [Qemu-devel] [PATCH 4/6] qemu-log: Rename the public-facing cpu_set_log function to qemu_set_log

[Peter Maydell]

On 11 February 2013 19:28, Andreas Färber <address@hidden> wrote:
> Am 11.02.2013 17:41, schrieb Peter Maydell:
>> diff --git a/hw/ppc.c b/hw/ppc.c
>> index c52e22f..6053bd5 100644
>> --- a/hw/ppc.c
>> +++ b/hw/ppc.c
>> @@ -1189,7 +1189,7 @@ void PPC_debug_write (void *opaque, uint32_t addr, 
>> uint32_t val)
>>          break;
>>      case 2:
>>          printf("Set loglevel to %04" PRIx32 "\n", val);
>> -        cpu_set_log(val | 0x100);
>> +        qemu_set_log(val | 0x100);
>>          break;
>>      }
>>  }
>
> I haven't the foggiest what this magic number is about, did you find out?

Alex said PReP was yours and I should ask you :-)

0x100 is CPU_LOG_TB_CPU. This raw 0x100 was introduced in commit
fd0bbb12 by Fabrice in 2004 with the helpful commit message
"cmdline init fix". At that time 0x100 meant the same thing
it does today, so you could just replace it with the symbolic
constant.

I have no idea if there's any documentation of the OpenFirmware/QEMU
contract for what the semantics of the magic io port are :-)
The OpenHackWare end of this connection does this:

#ifdef DEBUG_BIOS
    emit_buffer[emit_pos] = '\0';
    if (strcmp(emit_buffer, "Call Kernel!") == 0) {
        /* Set qemu in debug mode:
         * log in_asm,op,int,ioport,cpu
         */
        uint16_t loglevel = 0x02 | 0x10 | 0x80;
        //        outw(0xFF02, loglevel);
        outb(0x0F02, loglevel);
    }
    emit_pos = 0;
#endif

and frankly encoding QEMU's internal constant values into the BIOS
is insane (for a start, the specified constant doesn't do what the
comment says, because it's missing 'op', which is 0x4).

So, er, yeah. I think if I were you I'd change the code to one of:
 (a) use the symbolic constant rather than 0x100
 (b) drop the "| 0x100", ie change it to "just pass the value
through", on the basis that anybody messing with DEBUG_BIOS is
capable of setting the flags they actually want on their end
 (c) change it to "anything non-zero means we set the log
level to some 'debug info we think you might like' setting;
zero means set log level to zero" (keeps QEMU internal
constant values internal)
 (d) change it to just ignore attempts to set the loglevel
via the debug port (don't let the guest mess with user command
line specified logging level)
 (e) rip the entire case out as a gross hack and assume nobody was
weird enough to be running a DEBUG_BIOS openhackware
(as d, plus remove odd code from QEMU)

depending on how ugly you think it is and how spiky you're feeling :-)

-- PMM