| Message ID | 1312376904-16115-8-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Aug 3, 2011 at 1:07 PM, Markus Armbruster <armbru@redhat.com> wrote: > No functional change. > > It would be nice to have handler functions in the table, like commit > e1a064f9 did for ATAPI. Left for another day. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > hw/ide/core.c | 104 +++++++++++++++++++++++++++++++++++++++++++-------------- > 1 files changed, 79 insertions(+), 25 deletions(-) > > diff --git a/hw/ide/core.c b/hw/ide/core.c > index 1c4dc2f..a25c175 100644 > --- a/hw/ide/core.c > +++ b/hw/ide/core.c > @@ -876,6 +876,77 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val) > } > } > > +#define HD_OK (1u << IDE_HD) > +#define CD_OK (1u << IDE_CD) > +#define CFA_OK (1u << IDE_CFATA) > +#define HD_CFA_OK (HD_OK | CFA_OK) > +#define ALL_OK (HD_OK | CD_OK | CFA_OK) > + > +/* See ACS-2 T13/2015-D Table B.2 Command codes */ > +uint8_t ide_cmd_table[0x100] = { Missing 'static'. > + /* NOP not implemented, mandatory for CD */ > + [CFA_REQ_EXT_ERROR_CODE] = CFA_OK, > + [WIN_DSM] = ALL_OK, > + [WIN_DEVICE_RESET] = CD_OK, > + [WIN_RECAL] = ALL_OK, > + [WIN_READ] = ALL_OK, > + [WIN_READ_ONCE] = ALL_OK, > + [WIN_READ_EXT] = ALL_OK, > + [WIN_READDMA_EXT] = ALL_OK, > + [WIN_READ_NATIVE_MAX_EXT] = ALL_OK, > + [WIN_MULTREAD_EXT] = ALL_OK, > + [WIN_WRITE] = ALL_OK, > + [WIN_WRITE_ONCE] = ALL_OK, > + [WIN_WRITE_EXT] = ALL_OK, > + [WIN_WRITEDMA_EXT] = ALL_OK, > + [CFA_WRITE_SECT_WO_ERASE] = ALL_OK, > + [WIN_MULTWRITE_EXT] = ALL_OK, > + [WIN_WRITE_VERIFY] = ALL_OK, > + [WIN_VERIFY] = ALL_OK, > + [WIN_VERIFY_ONCE] = ALL_OK, > + [WIN_VERIFY_EXT] = ALL_OK, > + [WIN_SEEK] = HD_CFA_OK, > + [CFA_TRANSLATE_SECTOR] = CFA_OK, > + [WIN_DIAGNOSE] = ALL_OK, > + [WIN_SPECIFY] = ALL_OK, > + [WIN_STANDBYNOW2] = ALL_OK, > + [WIN_IDLEIMMEDIATE2] = ALL_OK, > + [WIN_STANDBY2] = ALL_OK, > + [WIN_SETIDLE2] = ALL_OK, > + [WIN_CHECKPOWERMODE2] = ALL_OK, > + [WIN_SLEEPNOW2] = ALL_OK, > + [WIN_PACKETCMD] = CD_OK, > + [WIN_PIDENTIFY] = CD_OK, > + [WIN_SMART] = HD_CFA_OK, > + [CFA_ACCESS_METADATA_STORAGE] = CFA_OK, > + [CFA_ERASE_SECTORS] = CFA_OK, > + [WIN_MULTREAD] = ALL_OK, > + [WIN_MULTWRITE] = ALL_OK, > + [WIN_SETMULT] = ALL_OK, > + [WIN_READDMA] = ALL_OK, > + [WIN_READDMA_ONCE] = ALL_OK, > + [WIN_WRITEDMA] = ALL_OK, > + [WIN_WRITEDMA_ONCE] = ALL_OK, > + [CFA_WRITE_MULTI_WO_ERASE] = ALL_OK, > + [WIN_STANDBYNOW1] = ALL_OK, > + [WIN_IDLEIMMEDIATE] = ALL_OK, > + [WIN_STANDBY] = ALL_OK, > + [WIN_SETIDLE1] = ALL_OK, > + [WIN_CHECKPOWERMODE1] = ALL_OK, > + [WIN_SLEEPNOW1] = ALL_OK, > + [WIN_FLUSH_CACHE] = ALL_OK, > + [WIN_FLUSH_CACHE_EXT] = ALL_OK, > + [WIN_IDENTIFY] = ALL_OK, > + [WIN_SETFEATURES] = ALL_OK, > + [IBM_SENSE_CONDITION] = CFA_OK, > + [CFA_WEAR_LEVEL] = CFA_OK, > + [WIN_READ_NATIVE_MAX] = ALL_OK, > +}; > + > +static bool ide_cmd_permitted(IDEState *s, uint32_t cmd) > +{ > + return cmd <= 0xff && (ide_cmd_table[cmd] & (1u << s->drive_kind)); > +} > > void ide_exec_cmd(IDEBus *bus, uint32_t val) > { > @@ -895,6 +966,10 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET) > return; > > + if (!ide_cmd_permitted(s, val)) { > + goto abort_cmd; > + } > + > switch(val) { > case WIN_DSM: > switch (s->feature) { > @@ -1115,21 +1190,15 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > ide_set_irq(s->bus); > break; > case WIN_SEEK: > - if(s->drive_kind == IDE_CD) > - goto abort_cmd; > /* XXX: Check that seek is within bounds */ > s->status = READY_STAT | SEEK_STAT; > ide_set_irq(s->bus); > break; > /* ATAPI commands */ > case WIN_PIDENTIFY: > - if (s->drive_kind == IDE_CD) { > - ide_atapi_identify(s); > - s->status = READY_STAT | SEEK_STAT; > - ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop); > - } else { > - ide_abort_command(s); > - } > + ide_atapi_identify(s); > + s->status = READY_STAT | SEEK_STAT; > + ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop); > ide_set_irq(s->bus); > break; > case WIN_DIAGNOSE: > @@ -1146,15 +1215,11 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > ide_set_irq(s->bus); > break; > case WIN_DEVICE_RESET: > - if (s->drive_kind != IDE_CD) > - goto abort_cmd; > ide_set_signature(s); > s->status = 0x00; /* NOTE: READY is _not_ set */ > s->error = 0x01; > break; > case WIN_PACKETCMD: > - if (s->drive_kind != IDE_CD) > - goto abort_cmd; > /* overlapping commands not supported */ > if (s->feature & 0x02) > goto abort_cmd; > @@ -1166,16 +1231,12 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > break; > /* CF-ATA commands */ > case CFA_REQ_EXT_ERROR_CODE: > - if (s->drive_kind != IDE_CFATA) > - goto abort_cmd; > s->error = 0x09; /* miscellaneous error */ > s->status = READY_STAT | SEEK_STAT; > ide_set_irq(s->bus); > break; > case CFA_ERASE_SECTORS: > case CFA_WEAR_LEVEL: > - if (s->drive_kind != IDE_CFATA) > - goto abort_cmd; > if (val == CFA_WEAR_LEVEL) > s->nsector = 0; > if (val == CFA_ERASE_SECTORS) > @@ -1185,8 +1246,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > ide_set_irq(s->bus); > break; > case CFA_TRANSLATE_SECTOR: > - if (s->drive_kind != IDE_CFATA) > - goto abort_cmd; > s->error = 0x00; > s->status = READY_STAT | SEEK_STAT; > memset(s->io_buffer, 0, 0x200); > @@ -1205,8 +1264,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > ide_set_irq(s->bus); > break; > case CFA_ACCESS_METADATA_STORAGE: > - if (s->drive_kind != IDE_CFATA) > - goto abort_cmd; > switch (s->feature) { > case 0x02: /* Inquiry Metadata Storage */ > ide_cfata_metadata_inquiry(s); > @@ -1225,8 +1282,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > ide_set_irq(s->bus); > break; > case IBM_SENSE_CONDITION: > - if (s->drive_kind != IDE_CFATA) > - goto abort_cmd; > switch (s->feature) { > case 0x01: /* sense temperature in device */ > s->nsector = 0x50; /* +20 C */ > @@ -1239,8 +1294,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > break; > > case WIN_SMART: > - if (s->drive_kind == IDE_CD) > - goto abort_cmd; > if (s->hcyl != 0xc2 || s->lcyl != 0x4f) > goto abort_cmd; > if (!s->smart_enabled && s->feature != SMART_ENABLE) > @@ -1395,6 +1448,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) > } > break; > default: > + /* should not be reachable */ > abort_cmd: > ide_abort_command(s); > ide_set_irq(s->bus); > -- > 1.7.6 > > >
Blue Swirl <blauwirbel@gmail.com> writes: > On Wed, Aug 3, 2011 at 1:07 PM, Markus Armbruster <armbru@redhat.com> wrote: >> No functional change. >> >> It would be nice to have handler functions in the table, like commit >> e1a064f9 did for ATAPI. Left for another day. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> hw/ide/core.c | 104 +++++++++++++++++++++++++++++++++++++++++++-------------- >> 1 files changed, 79 insertions(+), 25 deletions(-) >> >> diff --git a/hw/ide/core.c b/hw/ide/core.c >> index 1c4dc2f..a25c175 100644 >> --- a/hw/ide/core.c >> +++ b/hw/ide/core.c >> @@ -876,6 +876,77 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val) >> } >> } >> >> +#define HD_OK (1u << IDE_HD) >> +#define CD_OK (1u << IDE_CD) >> +#define CFA_OK (1u << IDE_CFATA) >> +#define HD_CFA_OK (HD_OK | CFA_OK) >> +#define ALL_OK (HD_OK | CD_OK | CFA_OK) >> + >> +/* See ACS-2 T13/2015-D Table B.2 Command codes */ >> +uint8_t ide_cmd_table[0x100] = { > > Missing 'static'. Thanks, will fix. [...]
Am 03.08.2011 18:53, schrieb Blue Swirl: > On Wed, Aug 3, 2011 at 1:07 PM, Markus Armbruster <armbru@redhat.com> wrote: >> No functional change. >> >> It would be nice to have handler functions in the table, like commit >> e1a064f9 did for ATAPI. Left for another day. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> hw/ide/core.c | 104 +++++++++++++++++++++++++++++++++++++++++++-------------- >> 1 files changed, 79 insertions(+), 25 deletions(-) >> >> diff --git a/hw/ide/core.c b/hw/ide/core.c >> index 1c4dc2f..a25c175 100644 >> --- a/hw/ide/core.c >> +++ b/hw/ide/core.c >> @@ -876,6 +876,77 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val) >> } >> } >> >> +#define HD_OK (1u << IDE_HD) >> +#define CD_OK (1u << IDE_CD) >> +#define CFA_OK (1u << IDE_CFATA) >> +#define HD_CFA_OK (HD_OK | CFA_OK) >> +#define ALL_OK (HD_OK | CD_OK | CFA_OK) >> + >> +/* See ACS-2 T13/2015-D Table B.2 Command codes */ >> +uint8_t ide_cmd_table[0x100] = { > > Missing 'static'. And const, while we're at it. >> + /* NOP not implemented, mandatory for CD */ >> + [CFA_REQ_EXT_ERROR_CODE] = CFA_OK, >> + [WIN_DSM] = ALL_OK, >> + [WIN_DEVICE_RESET] = CD_OK, >> + [WIN_RECAL] = ALL_OK, >> + [WIN_READ] = ALL_OK, >> + [WIN_READ_ONCE] = ALL_OK, >> + [WIN_READ_EXT] = ALL_OK, >> + [WIN_READDMA_EXT] = ALL_OK, >> + [WIN_READ_NATIVE_MAX_EXT] = ALL_OK, >> + [WIN_MULTREAD_EXT] = ALL_OK, >> + [WIN_WRITE] = ALL_OK, >> + [WIN_WRITE_ONCE] = ALL_OK, >> + [WIN_WRITE_EXT] = ALL_OK, >> + [WIN_WRITEDMA_EXT] = ALL_OK, >> + [CFA_WRITE_SECT_WO_ERASE] = ALL_OK, >> + [WIN_MULTWRITE_EXT] = ALL_OK, >> + [WIN_WRITE_VERIFY] = ALL_OK, >> + [WIN_VERIFY] = ALL_OK, >> + [WIN_VERIFY_ONCE] = ALL_OK, >> + [WIN_VERIFY_EXT] = ALL_OK, >> + [WIN_SEEK] = HD_CFA_OK, >> + [CFA_TRANSLATE_SECTOR] = CFA_OK, >> + [WIN_DIAGNOSE] = ALL_OK, >> + [WIN_SPECIFY] = ALL_OK, >> + [WIN_STANDBYNOW2] = ALL_OK, >> + [WIN_IDLEIMMEDIATE2] = ALL_OK, >> + [WIN_STANDBY2] = ALL_OK, >> + [WIN_SETIDLE2] = ALL_OK, >> + [WIN_CHECKPOWERMODE2] = ALL_OK, >> + [WIN_SLEEPNOW2] = ALL_OK, >> + [WIN_PACKETCMD] = CD_OK, >> + [WIN_PIDENTIFY] = CD_OK, >> + [WIN_SMART] = HD_CFA_OK, >> + [CFA_ACCESS_METADATA_STORAGE] = CFA_OK, >> + [CFA_ERASE_SECTORS] = CFA_OK, >> + [WIN_MULTREAD] = ALL_OK, >> + [WIN_MULTWRITE] = ALL_OK, >> + [WIN_SETMULT] = ALL_OK, >> + [WIN_READDMA] = ALL_OK, >> + [WIN_READDMA_ONCE] = ALL_OK, >> + [WIN_WRITEDMA] = ALL_OK, >> + [WIN_WRITEDMA_ONCE] = ALL_OK, >> + [CFA_WRITE_MULTI_WO_ERASE] = ALL_OK, >> + [WIN_STANDBYNOW1] = ALL_OK, >> + [WIN_IDLEIMMEDIATE] = ALL_OK, >> + [WIN_STANDBY] = ALL_OK, >> + [WIN_SETIDLE1] = ALL_OK, >> + [WIN_CHECKPOWERMODE1] = ALL_OK, >> + [WIN_SLEEPNOW1] = ALL_OK, >> + [WIN_FLUSH_CACHE] = ALL_OK, >> + [WIN_FLUSH_CACHE_EXT] = ALL_OK, >> + [WIN_IDENTIFY] = ALL_OK, >> + [WIN_SETFEATURES] = ALL_OK, >> + [IBM_SENSE_CONDITION] = CFA_OK, >> + [CFA_WEAR_LEVEL] = CFA_OK, >> + [WIN_READ_NATIVE_MAX] = ALL_OK, >> +}; >> + >> +static bool ide_cmd_permitted(IDEState *s, uint32_t cmd) >> +{ >> + return cmd <= 0xff && (ide_cmd_table[cmd] & (1u << s->drive_kind)); >> +} Doesn't cmd < ARRAY_SIZE(ide_cmd_table) better describe what you want? Kevin
Kevin Wolf <kwolf@redhat.com> writes: > Am 03.08.2011 18:53, schrieb Blue Swirl: >> On Wed, Aug 3, 2011 at 1:07 PM, Markus Armbruster <armbru@redhat.com> wrote: >>> No functional change. >>> >>> It would be nice to have handler functions in the table, like commit >>> e1a064f9 did for ATAPI. Left for another day. >>> >>> Signed-off-by: Markus Armbruster <armbru@redhat.com> >>> --- >>> hw/ide/core.c | 104 +++++++++++++++++++++++++++++++++++++++++++-------------- >>> 1 files changed, 79 insertions(+), 25 deletions(-) >>> >>> diff --git a/hw/ide/core.c b/hw/ide/core.c >>> index 1c4dc2f..a25c175 100644 >>> --- a/hw/ide/core.c >>> +++ b/hw/ide/core.c >>> @@ -876,6 +876,77 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val) >>> } >>> } >>> >>> +#define HD_OK (1u << IDE_HD) >>> +#define CD_OK (1u << IDE_CD) >>> +#define CFA_OK (1u << IDE_CFATA) >>> +#define HD_CFA_OK (HD_OK | CFA_OK) >>> +#define ALL_OK (HD_OK | CD_OK | CFA_OK) >>> + >>> +/* See ACS-2 T13/2015-D Table B.2 Command codes */ >>> +uint8_t ide_cmd_table[0x100] = { >> >> Missing 'static'. > > And const, while we're at it. Yes. >>> + /* NOP not implemented, mandatory for CD */ >>> + [CFA_REQ_EXT_ERROR_CODE] = CFA_OK, >>> + [WIN_DSM] = ALL_OK, >>> + [WIN_DEVICE_RESET] = CD_OK, >>> + [WIN_RECAL] = ALL_OK, >>> + [WIN_READ] = ALL_OK, >>> + [WIN_READ_ONCE] = ALL_OK, >>> + [WIN_READ_EXT] = ALL_OK, >>> + [WIN_READDMA_EXT] = ALL_OK, >>> + [WIN_READ_NATIVE_MAX_EXT] = ALL_OK, >>> + [WIN_MULTREAD_EXT] = ALL_OK, >>> + [WIN_WRITE] = ALL_OK, >>> + [WIN_WRITE_ONCE] = ALL_OK, >>> + [WIN_WRITE_EXT] = ALL_OK, >>> + [WIN_WRITEDMA_EXT] = ALL_OK, >>> + [CFA_WRITE_SECT_WO_ERASE] = ALL_OK, >>> + [WIN_MULTWRITE_EXT] = ALL_OK, >>> + [WIN_WRITE_VERIFY] = ALL_OK, >>> + [WIN_VERIFY] = ALL_OK, >>> + [WIN_VERIFY_ONCE] = ALL_OK, >>> + [WIN_VERIFY_EXT] = ALL_OK, >>> + [WIN_SEEK] = HD_CFA_OK, >>> + [CFA_TRANSLATE_SECTOR] = CFA_OK, >>> + [WIN_DIAGNOSE] = ALL_OK, >>> + [WIN_SPECIFY] = ALL_OK, >>> + [WIN_STANDBYNOW2] = ALL_OK, >>> + [WIN_IDLEIMMEDIATE2] = ALL_OK, >>> + [WIN_STANDBY2] = ALL_OK, >>> + [WIN_SETIDLE2] = ALL_OK, >>> + [WIN_CHECKPOWERMODE2] = ALL_OK, >>> + [WIN_SLEEPNOW2] = ALL_OK, >>> + [WIN_PACKETCMD] = CD_OK, >>> + [WIN_PIDENTIFY] = CD_OK, >>> + [WIN_SMART] = HD_CFA_OK, >>> + [CFA_ACCESS_METADATA_STORAGE] = CFA_OK, >>> + [CFA_ERASE_SECTORS] = CFA_OK, >>> + [WIN_MULTREAD] = ALL_OK, >>> + [WIN_MULTWRITE] = ALL_OK, >>> + [WIN_SETMULT] = ALL_OK, >>> + [WIN_READDMA] = ALL_OK, >>> + [WIN_READDMA_ONCE] = ALL_OK, >>> + [WIN_WRITEDMA] = ALL_OK, >>> + [WIN_WRITEDMA_ONCE] = ALL_OK, >>> + [CFA_WRITE_MULTI_WO_ERASE] = ALL_OK, >>> + [WIN_STANDBYNOW1] = ALL_OK, >>> + [WIN_IDLEIMMEDIATE] = ALL_OK, >>> + [WIN_STANDBY] = ALL_OK, >>> + [WIN_SETIDLE1] = ALL_OK, >>> + [WIN_CHECKPOWERMODE1] = ALL_OK, >>> + [WIN_SLEEPNOW1] = ALL_OK, >>> + [WIN_FLUSH_CACHE] = ALL_OK, >>> + [WIN_FLUSH_CACHE_EXT] = ALL_OK, >>> + [WIN_IDENTIFY] = ALL_OK, >>> + [WIN_SETFEATURES] = ALL_OK, >>> + [IBM_SENSE_CONDITION] = CFA_OK, >>> + [CFA_WEAR_LEVEL] = CFA_OK, >>> + [WIN_READ_NATIVE_MAX] = ALL_OK, >>> +}; >>> + >>> +static bool ide_cmd_permitted(IDEState *s, uint32_t cmd) >>> +{ >>> + return cmd <= 0xff && (ide_cmd_table[cmd] & (1u << s->drive_kind)); >>> +} > > Doesn't cmd < ARRAY_SIZE(ide_cmd_table) better describe what you want? Since I touch this patch anyway, I'll do this your way.
diff --git a/hw/ide/core.c b/hw/ide/core.c index 1c4dc2f..a25c175 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -876,6 +876,77 @@ void ide_ioport_write(void *opaque, uint32_t addr, uint32_t val) } } +#define HD_OK (1u << IDE_HD) +#define CD_OK (1u << IDE_CD) +#define CFA_OK (1u << IDE_CFATA) +#define HD_CFA_OK (HD_OK | CFA_OK) +#define ALL_OK (HD_OK | CD_OK | CFA_OK) + +/* See ACS-2 T13/2015-D Table B.2 Command codes */ +uint8_t ide_cmd_table[0x100] = { + /* NOP not implemented, mandatory for CD */ + [CFA_REQ_EXT_ERROR_CODE] = CFA_OK, + [WIN_DSM] = ALL_OK, + [WIN_DEVICE_RESET] = CD_OK, + [WIN_RECAL] = ALL_OK, + [WIN_READ] = ALL_OK, + [WIN_READ_ONCE] = ALL_OK, + [WIN_READ_EXT] = ALL_OK, + [WIN_READDMA_EXT] = ALL_OK, + [WIN_READ_NATIVE_MAX_EXT] = ALL_OK, + [WIN_MULTREAD_EXT] = ALL_OK, + [WIN_WRITE] = ALL_OK, + [WIN_WRITE_ONCE] = ALL_OK, + [WIN_WRITE_EXT] = ALL_OK, + [WIN_WRITEDMA_EXT] = ALL_OK, + [CFA_WRITE_SECT_WO_ERASE] = ALL_OK, + [WIN_MULTWRITE_EXT] = ALL_OK, + [WIN_WRITE_VERIFY] = ALL_OK, + [WIN_VERIFY] = ALL_OK, + [WIN_VERIFY_ONCE] = ALL_OK, + [WIN_VERIFY_EXT] = ALL_OK, + [WIN_SEEK] = HD_CFA_OK, + [CFA_TRANSLATE_SECTOR] = CFA_OK, + [WIN_DIAGNOSE] = ALL_OK, + [WIN_SPECIFY] = ALL_OK, + [WIN_STANDBYNOW2] = ALL_OK, + [WIN_IDLEIMMEDIATE2] = ALL_OK, + [WIN_STANDBY2] = ALL_OK, + [WIN_SETIDLE2] = ALL_OK, + [WIN_CHECKPOWERMODE2] = ALL_OK, + [WIN_SLEEPNOW2] = ALL_OK, + [WIN_PACKETCMD] = CD_OK, + [WIN_PIDENTIFY] = CD_OK, + [WIN_SMART] = HD_CFA_OK, + [CFA_ACCESS_METADATA_STORAGE] = CFA_OK, + [CFA_ERASE_SECTORS] = CFA_OK, + [WIN_MULTREAD] = ALL_OK, + [WIN_MULTWRITE] = ALL_OK, + [WIN_SETMULT] = ALL_OK, + [WIN_READDMA] = ALL_OK, + [WIN_READDMA_ONCE] = ALL_OK, + [WIN_WRITEDMA] = ALL_OK, + [WIN_WRITEDMA_ONCE] = ALL_OK, + [CFA_WRITE_MULTI_WO_ERASE] = ALL_OK, + [WIN_STANDBYNOW1] = ALL_OK, + [WIN_IDLEIMMEDIATE] = ALL_OK, + [WIN_STANDBY] = ALL_OK, + [WIN_SETIDLE1] = ALL_OK, + [WIN_CHECKPOWERMODE1] = ALL_OK, + [WIN_SLEEPNOW1] = ALL_OK, + [WIN_FLUSH_CACHE] = ALL_OK, + [WIN_FLUSH_CACHE_EXT] = ALL_OK, + [WIN_IDENTIFY] = ALL_OK, + [WIN_SETFEATURES] = ALL_OK, + [IBM_SENSE_CONDITION] = CFA_OK, + [CFA_WEAR_LEVEL] = CFA_OK, + [WIN_READ_NATIVE_MAX] = ALL_OK, +}; + +static bool ide_cmd_permitted(IDEState *s, uint32_t cmd) +{ + return cmd <= 0xff && (ide_cmd_table[cmd] & (1u << s->drive_kind)); +} void ide_exec_cmd(IDEBus *bus, uint32_t val) { @@ -895,6 +966,10 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) if ((s->status & (BUSY_STAT|DRQ_STAT)) && val != WIN_DEVICE_RESET) return; + if (!ide_cmd_permitted(s, val)) { + goto abort_cmd; + } + switch(val) { case WIN_DSM: switch (s->feature) { @@ -1115,21 +1190,15 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) ide_set_irq(s->bus); break; case WIN_SEEK: - if(s->drive_kind == IDE_CD) - goto abort_cmd; /* XXX: Check that seek is within bounds */ s->status = READY_STAT | SEEK_STAT; ide_set_irq(s->bus); break; /* ATAPI commands */ case WIN_PIDENTIFY: - if (s->drive_kind == IDE_CD) { - ide_atapi_identify(s); - s->status = READY_STAT | SEEK_STAT; - ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop); - } else { - ide_abort_command(s); - } + ide_atapi_identify(s); + s->status = READY_STAT | SEEK_STAT; + ide_transfer_start(s, s->io_buffer, 512, ide_transfer_stop); ide_set_irq(s->bus); break; case WIN_DIAGNOSE: @@ -1146,15 +1215,11 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) ide_set_irq(s->bus); break; case WIN_DEVICE_RESET: - if (s->drive_kind != IDE_CD) - goto abort_cmd; ide_set_signature(s); s->status = 0x00; /* NOTE: READY is _not_ set */ s->error = 0x01; break; case WIN_PACKETCMD: - if (s->drive_kind != IDE_CD) - goto abort_cmd; /* overlapping commands not supported */ if (s->feature & 0x02) goto abort_cmd; @@ -1166,16 +1231,12 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) break; /* CF-ATA commands */ case CFA_REQ_EXT_ERROR_CODE: - if (s->drive_kind != IDE_CFATA) - goto abort_cmd; s->error = 0x09; /* miscellaneous error */ s->status = READY_STAT | SEEK_STAT; ide_set_irq(s->bus); break; case CFA_ERASE_SECTORS: case CFA_WEAR_LEVEL: - if (s->drive_kind != IDE_CFATA) - goto abort_cmd; if (val == CFA_WEAR_LEVEL) s->nsector = 0; if (val == CFA_ERASE_SECTORS) @@ -1185,8 +1246,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) ide_set_irq(s->bus); break; case CFA_TRANSLATE_SECTOR: - if (s->drive_kind != IDE_CFATA) - goto abort_cmd; s->error = 0x00; s->status = READY_STAT | SEEK_STAT; memset(s->io_buffer, 0, 0x200); @@ -1205,8 +1264,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) ide_set_irq(s->bus); break; case CFA_ACCESS_METADATA_STORAGE: - if (s->drive_kind != IDE_CFATA) - goto abort_cmd; switch (s->feature) { case 0x02: /* Inquiry Metadata Storage */ ide_cfata_metadata_inquiry(s); @@ -1225,8 +1282,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) ide_set_irq(s->bus); break; case IBM_SENSE_CONDITION: - if (s->drive_kind != IDE_CFATA) - goto abort_cmd; switch (s->feature) { case 0x01: /* sense temperature in device */ s->nsector = 0x50; /* +20 C */ @@ -1239,8 +1294,6 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) break; case WIN_SMART: - if (s->drive_kind == IDE_CD) - goto abort_cmd; if (s->hcyl != 0xc2 || s->lcyl != 0x4f) goto abort_cmd; if (!s->smart_enabled && s->feature != SMART_ENABLE) @@ -1395,6 +1448,7 @@ void ide_exec_cmd(IDEBus *bus, uint32_t val) } break; default: + /* should not be reachable */ abort_cmd: ide_abort_command(s); ide_set_irq(s->bus);
No functional change. It would be nice to have handler functions in the table, like commit e1a064f9 did for ATAPI. Left for another day. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- hw/ide/core.c | 104 +++++++++++++++++++++++++++++++++++++++++++-------------- 1 files changed, 79 insertions(+), 25 deletions(-)