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

[Jan Kiszka]

On 2012-09-11 13:48, Jan Kiszka wrote:
> On 2012-09-11 13:27, Julien Grall wrote:
>> On 09/11/2012 10:25 AM, Avi Kivity wrote:
>>> On 09/11/2012 12:15 PM, Avi Kivity wrote:
>>>    
>>>> On 09/04/2012 06:13 PM, Julien Grall wrote:
>>>>      
>>>>> This is the nineth version of patch series about ioport registration.
>>>>>
>>>>> Some part of QEMU still use register_ioport* functions to register ioport.
>>>>> These functions doesn't allow to use Memory Listener on it.
>>>>>        
>>>> Thanks, applied all (w/ updated patch 4), will push shortly.
>>>>
>>>>
>>>>      
>>> Aborts with the command line
>>>
>>>    qemu-system-x86_64 -device isa-debugcon,iobase=0x402,chardev=stdio 
>>> -chardev stdio,id=stdio
>>>
>>>
>>>    
>>
>> I have bisected and found the problem with bochs_bios_init in hw/pc.c.
>> Bosch already register the iport 0x402. I'm not sure that it's a bug.
>> In fact register_ioport_read/write check if the current ioport is used
>> with the opaque variable.
>> Before my patch, bochs_bios_init registered it's ioport with opaque
>> NULL, so if someone (like debugcon) wants to use the ioport there is
>> no problem. But now, I used portio_list_init to register bochs ioport,
>> so the opaque is not NULL.
>> I don't really know how to resolve this problem. Perhaps we could
>> just improve the debug message.
> 
> My suggestion:
> 
> pc: Don't listen on debug ports by default
> 
> 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>
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 112739a..70abf0e 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -539,9 +539,9 @@ 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;
> 

OK, ket's try properly again:

----8<-----

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);
-- 
1.7.3.4