Re: [Qemu-devel] [PATCH] pc: Don't listen on debug ports by default

[Andreas Färber]

Am 11.09.2012 16:26, schrieb Jan Kiszka:
> On 2012-09-11 16:19, Avi Kivity wrote:
>> On 09/11/2012 05:11 PM, Jan Kiszka wrote:
>>> On 2012-09-11 14:53, Avi Kivity wrote:
>>>> On 09/11/2012 02:57 PM, Jan Kiszka wrote:
>>>>
>>>>> Only listen on debug ports when we also handle them. They are better
>>>>> handled by debugcon these days which is runtime configurable.
>>>>>
>>>>> Signed-off-by: Jan Kiszka <address@hidden>
>>>>> ---
>>>>>  hw/pc.c |    6 ++++--
>>>>>  1 files changed, 4 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/hw/pc.c b/hw/pc.c
>>>>> index 112739a..134d5f7 100644
>>>>> --- a/hw/pc.c
>>>>> +++ b/hw/pc.c
>>>>> @@ -539,12 +539,12 @@ static void bochs_bios_write(void *opaque, uint32_t 
>>>>> addr, uint32_t val)
>>>>>      case 0x401:
>>>>>          /* used to be panic, now unused */
>>>>>          break;
>>>>> +#ifdef DEBUG_BIOS
>>>>>      case 0x402:
>>>>>      case 0x403:
>>>>> -#ifdef DEBUG_BIOS
>>>>>          fprintf(stderr, "%c", val);
>>>>> -#endif
>>>>>          break;
>>>>> +#endif
>>>>>      case 0x8900:
>>>>>          /* same as Bochs power off */
>>>>>          if (val == shutdown_str[shutdown_index]) {
>>>>> @@ -598,8 +598,10 @@ static void *bochs_bios_init(void)
>>>>>  
>>>>>      register_ioport_write(0x400, 1, 2, bochs_bios_write, NULL);
>>>>>      register_ioport_write(0x401, 1, 2, bochs_bios_write, NULL);
>>>>> +#ifdef DEBUG_BIOS
>>>>>      register_ioport_write(0x402, 1, 1, bochs_bios_write, NULL);
>>>>>      register_ioport_write(0x403, 1, 1, bochs_bios_write, NULL);
>>>>> +#endif
>>>>>      register_ioport_write(0x8900, 1, 1, bochs_bios_write, NULL);
>>>>>  
>>>>>      register_ioport_write(0x501, 1, 1, bochs_bios_write, NULL);
>>>>>
>>>>
>>>>
>>>> Why not drop DEBUG_BIOS completely?  If you want to debug the bios, use
>>>> debugcon.
>>>
>>> Probably true. There is more practically dead stuff below this that just
>>> prints if DEBUG_BIOS is enabled.
>>
>> Actually it is autotest that is at fault here.  It is installing a
>> debugcon with non-standard iobase atop a builtin device.
>> Pre-memory-API, we did not detect that.
> 
> Well, the cruft in pc.c was disabled in practice, just leaving useless
> /dev/null-like ioports behind. I still think we should drop all of them,
> they have no meaning.
> 
>>
>> We can subclass isa-debugcon as bochs-debugcon, change the default
>> ioport to 0x402, and use that instead of the code in pc.c.  How does
>> that sound?
>>
>> Autotest will need to be changed to use the new device type.
>>
> 
> That is another story: providing a debugcon that defaults to 0x402, the
> port I always forgot when I want to debug the BIOS. We could provide
> bochs-debugcon in addition to the existing interface, avoiding autotest
> breakage and still making the usage smoother.

Hervé and Anthony had discussed that for qemu-test and I thought their
conclusion was to use debug-con with the iobase parameter? CC'ing.

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg