Re: [Qemu-devel] [PATCH 2/9] IDE: Add register hints to tracing
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-devel] [PATCH 2/9] IDE: Add register hints to tracing
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
[Qemu-devel] [PATCH 2/9] IDE: Add register hints to tracing
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(void *opaque, uint32_t addr) //hob = s->select & (1 << 7); hob = 0; switch(reg_