Re: [Qemu-block] [Qemu-devel] [PATCH 2/9] IDE: Add register hints to tracing

2017-08-25 Thread Philippe Mathieu-Daudé

On 08/08/2017 03:32 PM, John Snow wrote:

Name the registers for tracing purposes.

Signed-off-by: John Snow 
---
  hw/ide/core.c   | 88 +
  hw/ide/trace-events |  4 +--
  2 files changed, 70 insertions(+), 22 deletions(-)

diff --git a/hw/ide/core.c b/hw/ide/core.c
index 53fa084..6235bdf 100644
--- a/hw/ide/core.c
+++ b/hw/ide/core.c
@@ -1177,13 +1177,37 @@ static void ide_clear_hob(IDEBus *bus)
  bus->ifs[1].select &= ~(1 << 7);
  }
  
+/* IOport [W]rite [R]egisters */

+enum ATA_IOPORT_WR {
+ATA_IOPORT_WR_DATA = 0,
+ATA_IOPORT_WR_FEATURES = 1,
+ATA_IOPORT_WR_SECTOR_COUNT = 2,
+ATA_IOPORT_WR_SECTOR_NUMBER = 3,
+ATA_IOPORT_WR_CYLINDER_LOW = 4,
+ATA_IOPORT_WR_CYLINDER_HIGH = 5,
+ATA_IOPORT_WR_DEVICE_HEAD = 6,
+ATA_IOPORT_WR_COMMAND = 7,
+ATA_IOPORT_WR_NUM_REGISTERS,
+};
+
+const char *ATA_IOPORT_WR_lookup[ATA_IOPORT_WR_NUM_REGISTERS] = {
+[ATA_IOPORT_WR_DATA] = "Data",
+[ATA_IOPORT_WR_FEATURES] = "Features",
+[ATA_IOPORT_WR_SECTOR_COUNT] = "Sector Count",
+[ATA_IOPORT_WR_SECTOR_NUMBER] = "Sector Number",
+[ATA_IOPORT_WR_CYLINDER_LOW] = "Cylinder Low",
+[ATA_IOPORT_WR_CYLINDER_HIGH] = "Cylinder High",
+[ATA_IOPORT_WR_DEVICE_HEAD] = "Device/Head",
+[ATA_IOPORT_WR_COMMAND] = "Command"
+};
+
  void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val)
  {
  IDEBus *bus = opaque;
  IDEState *s = idebus_active_if(bus);
  int reg_num = addr & 7;
  
-trace_ide_ioport_write(addr, val, bus, s);

+trace_ide_ioport_write(addr, ATA_IOPORT_WR_lookup[reg_num], val, bus, s);
  
  /* ignore writes to command block while busy with previous command */

  if (reg_num != 7 && (s->status & (BUSY_STAT|DRQ_STAT))) {
@@ -1193,43 +1217,43 @@ void ide_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
  switch(reg_num) {
  case 0:
  break;
-case 1:
-   ide_clear_hob(bus);
+case ATA_IOPORT_WR_FEATURES:
+ide_clear_hob(bus);
  /* NOTE: data is written to the two drives */
-   bus->ifs[0].hob_feature = bus->ifs[0].feature;
-   bus->ifs[1].hob_feature = bus->ifs[1].feature;
+bus->ifs[0].hob_feature = bus->ifs[0].feature;
+bus->ifs[1].hob_feature = bus->ifs[1].feature;
  bus->ifs[0].feature = val;
  bus->ifs[1].feature = val;
  break;
-case 2:
+case ATA_IOPORT_WR_SECTOR_COUNT:
ide_clear_hob(bus);
bus->ifs[0].hob_nsector = bus->ifs[0].nsector;
bus->ifs[1].hob_nsector = bus->ifs[1].nsector;
  bus->ifs[0].nsector = val;
  bus->ifs[1].nsector = val;
  break;
-case 3:
+case ATA_IOPORT_WR_SECTOR_NUMBER:
ide_clear_hob(bus);
bus->ifs[0].hob_sector = bus->ifs[0].sector;
bus->ifs[1].hob_sector = bus->ifs[1].sector;
  bus->ifs[0].sector = val;
  bus->ifs[1].sector = val;
  break;
-case 4:
+case ATA_IOPORT_WR_CYLINDER_LOW:
ide_clear_hob(bus);
bus->ifs[0].hob_lcyl = bus->ifs[0].lcyl;
bus->ifs[1].hob_lcyl = bus->ifs[1].lcyl;
  bus->ifs[0].lcyl = val;
  bus->ifs[1].lcyl = val;
  break;
-case 5:
+case ATA_IOPORT_WR_CYLINDER_HIGH:
ide_clear_hob(bus);
bus->ifs[0].hob_hcyl = bus->ifs[0].hcyl;
bus->ifs[1].hob_hcyl = bus->ifs[1].hcyl;
  bus->ifs[0].hcyl = val;
  bus->ifs[1].hcyl = val;
  break;
-case 6:
+case ATA_IOPORT_WR_DEVICE_HEAD:
/* FIXME: HOB readback uses bit 7 */
  bus->ifs[0].select = (val & ~0x10) | 0xa0;
  bus->ifs[1].select = (val | 0x10) | 0xa0;
@@ -1237,7 +1261,7 @@ void ide_ioport_write(void *opaque, uint32_t addr, 
uint32_t val)
  bus->unit = (val >> 4) & 1;
  break;
  default:
-case 7:
+case ATA_IOPORT_WR_COMMAND:
  /* command */
  ide_exec_cmd(bus, val);
  break;
@@ -2044,6 +2068,30 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val)
  }
  }
  
+/* IOport [R]ead [R]egisters */

+enum ATA_IOPORT_RR {
+ATA_IOPORT_RR_DATA = 0,
+ATA_IOPORT_RR_ERROR = 1,
+ATA_IOPORT_RR_SECTOR_COUNT = 2,
+ATA_IOPORT_RR_SECTOR_NUMBER = 3,
+ATA_IOPORT_RR_CYLINDER_LOW = 4,
+ATA_IOPORT_RR_CYLINDER_HIGH = 5,
+ATA_IOPORT_RR_DEVICE_HEAD = 6,
+ATA_IOPORT_RR_STATUS = 7,
+ATA_IOPORT_RR_NUM_REGISTERS,
+};
+
+const char *ATA_IOPORT_RR_lookup[ATA_IOPORT_RR_NUM_REGISTERS] = {
+[ATA_IOPORT_RR_DATA] = "Data",
+[ATA_IOPORT_RR_ERROR] = "Error",
+[ATA_IOPORT_RR_SECTOR_COUNT] = "Sector Count",
+[ATA_IOPORT_RR_SECTOR_NUMBER] = "Sector Number",
+[ATA_IOPORT_RR_CYLINDER_LOW] = "Cylinder Low",
+[ATA_IOPORT_RR_CYLINDER_HIGH] = "Cylinder High",
+[ATA_IOPORT_RR_DEVICE_HEAD] = "Device/Head",
+[ATA_IOPORT_RR_STATUS] = "Status"
+};
+
  uint32_t ide_ioport_read(void *opaque, uint32_t addr)
  {
  IDEBus *bus = opaque;
@@ -2056,10 +2104,10 @@ uint32_t ide_ioport_read

Re: [Qemu-block] [Qemu-devel] [PATCH 2/9] IDE: Add register hints to tracing

2017-08-08 Thread Eric Blake
On 08/08/2017 01:32 PM, John Snow wrote:
> Name the registers for tracing purposes.
> 
> Signed-off-by: John Snow 
> ---
>  hw/ide/core.c   | 88 
> +
>  hw/ide/trace-events |  4 +--
>  2 files changed, 70 insertions(+), 22 deletions(-)

> -case 1:
> - ide_clear_hob(bus);
> +case ATA_IOPORT_WR_FEATURES:
> +ide_clear_hob(bus);

Cool, fixing tab damage in the process.

I did a similar improvement to NBD traces, and it's definitely worth it
to have traces include names in addition to numbers.

Reviewed-by: Eric Blake 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.   +1-919-301-3266
Virtualization:  qemu.org | libvirt.org



signature.asc
Description: OpenPGP digital signature