Patchwork pc: Drop practically unused BOCHS BIOS debug ports

login
register
mail settings
Submitter Jan Kiszka
Date Sept. 11, 2012, 2:38 p.m.
Message ID <504F4CE1.7020405@siemens.com>
Download mbox | patch
Permalink /patch/183125/
State New
Headers show

Comments

Jan Kiszka - Sept. 11, 2012, 2:38 p.m.
We have debugcon these days to listen on those ports that receive debug
messages. Also drop the others that have no effect anymore.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/pc.c |   27 ---------------------------
 1 files changed, 0 insertions(+), 27 deletions(-)
Andreas Färber - Sept. 11, 2012, 2:48 p.m.
Am 11.09.2012 16:38, schrieb Jan Kiszka:
> We have debugcon these days to listen on those ports that receive debug
> messages. Also drop the others that have no effect anymore.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

This looks pretty identical to Hervé's "pc: remove DEBUG_BIOS define and
QEMU exit I/O ports", doesn't it? The discussion there concluded that
compatibility needs to be provided for older machines.

http://patchwork.ozlabs.org/patch/169542/
http://patchwork.ozlabs.org/patch/169543/

Andreas

> ---
>  hw/pc.c |   27 ---------------------------
>  1 files changed, 0 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/pc.c b/hw/pc.c
> index 112739a..7e7e0e2 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -53,9 +53,6 @@
>  #include "bitmap.h"
>  #include "vga-pci.h"
>  
> -/* output Bochs bios info messages */
> -//#define DEBUG_BIOS
> -
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
>  
> @@ -534,17 +531,6 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
>      static int shutdown_index = 0;
>  
>      switch(addr) {
> -        /* Bochs BIOS messages */
> -    case 0x400:
> -    case 0x401:
> -        /* used to be panic, now unused */
> -        break;
> -    case 0x402:
> -    case 0x403:
> -#ifdef DEBUG_BIOS
> -        fprintf(stderr, "%c", val);
> -#endif
> -        break;
>      case 0x8900:
>          /* same as Bochs power off */
>          if (val == shutdown_str[shutdown_index]) {
> @@ -558,16 +544,9 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
>          }
>          break;
>  
> -        /* LGPL'ed VGA BIOS messages */
>      case 0x501:
>      case 0x502:
>          exit((val << 1) | 1);
> -    case 0x500:
> -    case 0x503:
> -#ifdef DEBUG_BIOS
> -        fprintf(stderr, "%c", val);
> -#endif
> -        break;
>      }
>  }
>  
> @@ -596,17 +575,11 @@ static void *bochs_bios_init(void)
>      uint64_t *numa_fw_cfg;
>      int i, j;
>  
> -    register_ioport_write(0x400, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x401, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x402, 1, 1, bochs_bios_write, NULL);
> -    register_ioport_write(0x403, 1, 1, bochs_bios_write, NULL);
>      register_ioport_write(0x8900, 1, 1, bochs_bios_write, NULL);
>  
>      register_ioport_write(0x501, 1, 1, bochs_bios_write, NULL);
>      register_ioport_write(0x501, 1, 2, bochs_bios_write, NULL);
>      register_ioport_write(0x502, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
> -    register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
>  
>      fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>  
>
Jan Kiszka - Sept. 11, 2012, 3:04 p.m.
On 2012-09-11 16:48, Andreas Färber wrote:
> Am 11.09.2012 16:38, schrieb Jan Kiszka:
>> We have debugcon these days to listen on those ports that receive debug
>> messages. Also drop the others that have no effect anymore.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> 
> This looks pretty identical to Hervé's "pc: remove DEBUG_BIOS define and
> QEMU exit I/O ports", doesn't it? The discussion there concluded that
> compatibility needs to be provided for older machines.

I'm not removing any behaviour that can be achieved _without_
additionally patching QEMU. I'm just removing practically dead debug
code, leaving in the bits that do have effects.

Jan
Anthony Liguori - Sept. 11, 2012, 3:06 p.m.
Jan Kiszka <jan.kiszka@siemens.com> writes:

> We have debugcon these days to listen on those ports that receive debug
> messages. Also drop the others that have no effect anymore.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Reviewed-by: Anthony Liguori <aliguori@us.ibm.com>

I'll pick this up after it goes through testing.

Regards,

Anthony Liguori

> ---
>  hw/pc.c |   27 ---------------------------
>  1 files changed, 0 insertions(+), 27 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 112739a..7e7e0e2 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -53,9 +53,6 @@
>  #include "bitmap.h"
>  #include "vga-pci.h"
>  
> -/* output Bochs bios info messages */
> -//#define DEBUG_BIOS
> -
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
>  
> @@ -534,17 +531,6 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
>      static int shutdown_index = 0;
>  
>      switch(addr) {
> -        /* Bochs BIOS messages */
> -    case 0x400:
> -    case 0x401:
> -        /* used to be panic, now unused */
> -        break;
> -    case 0x402:
> -    case 0x403:
> -#ifdef DEBUG_BIOS
> -        fprintf(stderr, "%c", val);
> -#endif
> -        break;
>      case 0x8900:
>          /* same as Bochs power off */
>          if (val == shutdown_str[shutdown_index]) {
> @@ -558,16 +544,9 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
>          }
>          break;
>  
> -        /* LGPL'ed VGA BIOS messages */
>      case 0x501:
>      case 0x502:
>          exit((val << 1) | 1);
> -    case 0x500:
> -    case 0x503:
> -#ifdef DEBUG_BIOS
> -        fprintf(stderr, "%c", val);
> -#endif
> -        break;
>      }
>  }
>  
> @@ -596,17 +575,11 @@ static void *bochs_bios_init(void)
>      uint64_t *numa_fw_cfg;
>      int i, j;
>  
> -    register_ioport_write(0x400, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x401, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x402, 1, 1, bochs_bios_write, NULL);
> -    register_ioport_write(0x403, 1, 1, bochs_bios_write, NULL);
>      register_ioport_write(0x8900, 1, 1, bochs_bios_write, NULL);
>  
>      register_ioport_write(0x501, 1, 1, bochs_bios_write, NULL);
>      register_ioport_write(0x501, 1, 2, bochs_bios_write, NULL);
>      register_ioport_write(0x502, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
> -    register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
>  
>      fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>  
> -- 
> 1.7.3.4
Andreas Färber - Sept. 11, 2012, 3:11 p.m.
Am 11.09.2012 17:04, schrieb Jan Kiszka:
> On 2012-09-11 16:48, Andreas Färber wrote:
>> Am 11.09.2012 16:38, schrieb Jan Kiszka:
>>> We have debugcon these days to listen on those ports that receive debug
>>> messages. Also drop the others that have no effect anymore.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>
>> This looks pretty identical to Hervé's "pc: remove DEBUG_BIOS define and
>> QEMU exit I/O ports", doesn't it? The discussion there concluded that
>> compatibility needs to be provided for older machines.
> 
> I'm not removing any behaviour that can be achieved _without_
> additionally patching QEMU. I'm just removing practically dead debug
> code, leaving in the bits that do have effects.

Didn't want to argue about that, just pointing you to the discussion
thread that you have snipped above. It was Anthony's request. ;)

Andreas
Jan Kiszka - Sept. 11, 2012, 3:27 p.m.
On 2012-09-11 17:11, Andreas Färber wrote:
> Am 11.09.2012 17:04, schrieb Jan Kiszka:
>> On 2012-09-11 16:48, Andreas Färber wrote:
>>> Am 11.09.2012 16:38, schrieb Jan Kiszka:
>>>> We have debugcon these days to listen on those ports that receive debug
>>>> messages. Also drop the others that have no effect anymore.
>>>>
>>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>>
>>> This looks pretty identical to Hervé's "pc: remove DEBUG_BIOS define and
>>> QEMU exit I/O ports", doesn't it? The discussion there concluded that
>>> compatibility needs to be provided for older machines.
>>
>> I'm not removing any behaviour that can be achieved _without_
>> additionally patching QEMU. I'm just removing practically dead debug
>> code, leaving in the bits that do have effects.
> 
> Didn't want to argue about that, just pointing you to the discussion
> thread that you have snipped above. It was Anthony's request. ;)

The 2nd patch cited did more than what my patch does by making those
ports optional (and apparently default off) that do have effect.

Jan
Anthony Liguori - Sept. 17, 2012, 6:25 p.m.
Jan Kiszka <jan.kiszka@siemens.com> writes:

> We have debugcon these days to listen on those ports that receive debug
> messages. Also drop the others that have no effect anymore.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>

Applied. Thanks.

Regards,

Anthony Liguori

> ---
>  hw/pc.c |   27 ---------------------------
>  1 files changed, 0 insertions(+), 27 deletions(-)
>
> diff --git a/hw/pc.c b/hw/pc.c
> index 112739a..7e7e0e2 100644
> --- a/hw/pc.c
> +++ b/hw/pc.c
> @@ -53,9 +53,6 @@
>  #include "bitmap.h"
>  #include "vga-pci.h"
>  
> -/* output Bochs bios info messages */
> -//#define DEBUG_BIOS
> -
>  /* debug PC/ISA interrupts */
>  //#define DEBUG_IRQ
>  
> @@ -534,17 +531,6 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
>      static int shutdown_index = 0;
>  
>      switch(addr) {
> -        /* Bochs BIOS messages */
> -    case 0x400:
> -    case 0x401:
> -        /* used to be panic, now unused */
> -        break;
> -    case 0x402:
> -    case 0x403:
> -#ifdef DEBUG_BIOS
> -        fprintf(stderr, "%c", val);
> -#endif
> -        break;
>      case 0x8900:
>          /* same as Bochs power off */
>          if (val == shutdown_str[shutdown_index]) {
> @@ -558,16 +544,9 @@ static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
>          }
>          break;
>  
> -        /* LGPL'ed VGA BIOS messages */
>      case 0x501:
>      case 0x502:
>          exit((val << 1) | 1);
> -    case 0x500:
> -    case 0x503:
> -#ifdef DEBUG_BIOS
> -        fprintf(stderr, "%c", val);
> -#endif
> -        break;
>      }
>  }
>  
> @@ -596,17 +575,11 @@ static void *bochs_bios_init(void)
>      uint64_t *numa_fw_cfg;
>      int i, j;
>  
> -    register_ioport_write(0x400, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x401, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x402, 1, 1, bochs_bios_write, NULL);
> -    register_ioport_write(0x403, 1, 1, bochs_bios_write, NULL);
>      register_ioport_write(0x8900, 1, 1, bochs_bios_write, NULL);
>  
>      register_ioport_write(0x501, 1, 1, bochs_bios_write, NULL);
>      register_ioport_write(0x501, 1, 2, bochs_bios_write, NULL);
>      register_ioport_write(0x502, 1, 2, bochs_bios_write, NULL);
> -    register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
> -    register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
>  
>      fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);
>  
> -- 
> 1.7.3.4

Patch

diff --git a/hw/pc.c b/hw/pc.c
index 112739a..7e7e0e2 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -53,9 +53,6 @@ 
 #include "bitmap.h"
 #include "vga-pci.h"
 
-/* output Bochs bios info messages */
-//#define DEBUG_BIOS
-
 /* debug PC/ISA interrupts */
 //#define DEBUG_IRQ
 
@@ -534,17 +531,6 @@  static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
     static int shutdown_index = 0;
 
     switch(addr) {
-        /* Bochs BIOS messages */
-    case 0x400:
-    case 0x401:
-        /* used to be panic, now unused */
-        break;
-    case 0x402:
-    case 0x403:
-#ifdef DEBUG_BIOS
-        fprintf(stderr, "%c", val);
-#endif
-        break;
     case 0x8900:
         /* same as Bochs power off */
         if (val == shutdown_str[shutdown_index]) {
@@ -558,16 +544,9 @@  static void bochs_bios_write(void *opaque, uint32_t addr, uint32_t val)
         }
         break;
 
-        /* LGPL'ed VGA BIOS messages */
     case 0x501:
     case 0x502:
         exit((val << 1) | 1);
-    case 0x500:
-    case 0x503:
-#ifdef DEBUG_BIOS
-        fprintf(stderr, "%c", val);
-#endif
-        break;
     }
 }
 
@@ -596,17 +575,11 @@  static void *bochs_bios_init(void)
     uint64_t *numa_fw_cfg;
     int i, j;
 
-    register_ioport_write(0x400, 1, 2, bochs_bios_write, NULL);
-    register_ioport_write(0x401, 1, 2, bochs_bios_write, NULL);
-    register_ioport_write(0x402, 1, 1, bochs_bios_write, NULL);
-    register_ioport_write(0x403, 1, 1, bochs_bios_write, NULL);
     register_ioport_write(0x8900, 1, 1, bochs_bios_write, NULL);
 
     register_ioport_write(0x501, 1, 1, bochs_bios_write, NULL);
     register_ioport_write(0x501, 1, 2, bochs_bios_write, NULL);
     register_ioport_write(0x502, 1, 2, bochs_bios_write, NULL);
-    register_ioport_write(0x500, 1, 1, bochs_bios_write, NULL);
-    register_ioport_write(0x503, 1, 1, bochs_bios_write, NULL);
 
     fw_cfg = fw_cfg_init(BIOS_CFG_IOPORT, BIOS_CFG_IOPORT + 1, 0, 0);