fdc: fix/rewrite seek, media_changed and interrupt handling
v3 cover fdc: fix/rewrite seek, media_changed and interrupt handling
The fd_seek will return 'track is invalid' if there is no media in drive.
Implied seek should have the same behavior as normal seek. We will use the fd_seek function instead of a direct modification of head, track and sector values. The result value is used only while read/write commands were issued.
Fixed sense interrupt status command.
Pavel Hrdina (4):
fdc: fix implied seek while there is no medina in drive
fdc-test: introduced qtest read_without_media
fdc: rewrite seek and DSKCHG bit handling
fdc: fix interrupt handling
hw/fdc.c | 99 ++++++++++++++++++++++++++++-------------------------- tests/fdc-test.c | 63 ++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 48 deletions(-)
Signature · 1 line
1.7.7.6
PATCH v3 1/4 fdc: fix implied seek while there is no medina in drive
The Windows uses 'READ' command at the start of an instalation without checking the 'dir' register. We have to abort the transfer with an abnormal termination if there is no media in the drive.
--- hw/fdc.c | 3 +++ 1 files changed, 3 insertions(+), 0 deletions(-)
hw/fdc.c +3 -0
@@ -159,6 +159,9 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, drv->sect = sect; } +if (drv->bs ==NULL || !bdrv_is_inserted (drv->bs)) + ret =2 ; +return ret; }
Signature · 1 line
1.7.7.6
PATCH v3 2/4 fdc-test: introduced qtest read_without_media
In reply to Pavel HrdinaIf you try to read from a floppy drive without a media, you should get an abnormal termination error.
--- tests/fdc-test.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 63 insertions(+), 0 deletions(-)
tests/fdc-test.c +63 -0
@@ -49,6 +49,7 @@ enum {enum {CMD_SENSE_INT =0x08 ,CMD_SEEK =0x0f , +CMD_READ =0xe6 , };enum {
@@ -99,6 +100,59 @@ static void ack_irq(void)g_assert (!get_irq (FLOPPY_IRQ )); } +static uint8_t send_read_command (void ) +{ +static uint8_t drive =0 ; +static uint8_t head =0 ; +static uint8_t cyl =0 ; +static uint8_t sect_addr =1 ; +static uint8_t sect_size =2 ; +static uint8_t eot =1 ; +static uint8_t gap =0x1b ; +static uint8_t gpl =0xff ; + +uint8_t msr =0 ; +uint8_t st0; + +uint8_t ret =0 ; + +floppy_send (CMD_READ ); +floppy_send (head <<2 | drive); +g_assert (!get_irq (FLOPPY_IRQ )); +floppy_send (cyl); +floppy_send (head); +floppy_send (sect_addr); +floppy_send (sect_size); +floppy_send (eot); +floppy_send (gap); +floppy_send (gpl); + +uint8_t i =0 ; +uint8_t n =2 ; +for (; i < n; i++) { + msr =inb (FLOPPY_BASE + reg_msr); +if (msr ==0xd0 ) +break ; +sleep (1 ); + } + +if (i >= n) +return 1 ; + + st0 =floppy_recv (); +if (st0 !=0x40 ) + ret =1 ; + +floppy_recv (); +floppy_recv (); +floppy_recv (); +floppy_recv (); +floppy_recv (); +floppy_recv (); + +return ret; +} +static void send_step_pulse (void ) {int drive =0 ;
@@ -146,6 +200,14 @@ static void test_no_media_on_start(void)assert_bit_set (dir,DSKCHG ); } +static void test_read_without_media (void ) +{ +uint8_t ret; + + ret =send_read_command (); +g_assert (ret ==0 ); +} +static void test_media_change (void ) {uint8_t dir;
@@ -214,6 +276,7 @@ int main(int argc, char **argv)qtest_irq_intercept_in (global_qtest,"ioapic" );qtest_add_func ("/fdc/cmos" , test_cmos);qtest_add_func ("/fdc/no_media_on_start" , test_no_media_on_start); +qtest_add_func ("/fdc/read_without_media" , test_read_without_media);qtest_add_func ("/fdc/media_change" , test_media_change); ret =g_test_run ();
Signature · 1 line
1.7.7.6
PATCH v3 3/4 fdc: rewrite seek and DSKCHG bit handling
In reply to Pavel HrdinaThis bit is cleared on every successful seek to a different track (cylinder). The seek is also called on revalidate or on read/write/format commands which also clear the DSKCHG bit.
--- hw/fdc.c | 66 ++++++++++++++++++++++++++++++------------------------------- 1 files changed, 32 insertions(+), 34 deletions(-)
hw/fdc.c +32 -34
@@ -153,8 +153,11 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, }#endif drv->head = head; -if (drv->track != track) +if (drv->track != track) { ret =1 ; +if (drv->bs !=NULL &&bdrv_is_inserted (drv->bs)) + drv->media_changed =0 ; + } drv->track = track; drv->sect = sect; }
@@ -169,9 +172,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,static void fd_recalibrate (FDrive *drv) {FLOPPY_DPRINTF ("recalibrate\n" ); - drv->head =0 ; - drv->track =0 ; - drv->sect =1 ; +fd_seek (drv,0 ,0 ,1 ,1 ); }/* Revalidate a disk drive after a disk change */
@@ -710,14 +711,6 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0)qemu_set_irq (fdctrl->irq,1 ); fdctrl->sra |=FD_SRA_INTPEND ; } -if (status0 &FD_SR0_SEEK ) { -FDrive *cur_drv; -/* A seek clears the disk change line (if a disk is inserted) */ - cur_drv =get_cur_drv (fdctrl); -if (cur_drv->bs !=NULL &&bdrv_is_inserted (cur_drv->bs)) { - cur_drv->media_changed =0 ; - } - } fdctrl->reset_sensei =0 ; fdctrl->status0 = status0;
@@ -1003,30 +996,39 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv) fd_sector(cur_drv));/* XXX: cur_drv->sect >= cur_drv->last_sect should be an error in fact */ -if (cur_drv->sect >= cur_drv->last_sect || - cur_drv->sect == fdctrl->eot) { - cur_drv->sect =1 ; +uint8_t new_head = cur_drv->head; +uint8_t new_track = cur_drv->track; +uint8_t new_sect = cur_drv->sect; + +int ret =1 ; + +if (new_sect >= cur_drv->last_sect || + new_sect == fdctrl->eot) { + new_sect =1 ;if (FD_MULTI_TRACK (fdctrl->data_state)) { -if (cur_drv->head ==0 && +if (new_head ==0 && (cur_drv->flags &FDISK_DBL_SIDES ) !=0 ) { - cur_drv->head =1 ; + new_head =1 ; }else { - cur_drv->head =0 ; - cur_drv->track++; + new_head =0 ; + new_track++;if ((cur_drv->flags &FDISK_DBL_SIDES ) ==0 ) -return 0 ; + ret =0 ; } }else { - cur_drv->track++; -return 0 ; + new_track++; + ret =0 ; } -FLOPPY_DPRINTF ("seek to next track (%d %02x %02x => %d)\n" , +if (ret ==1 ) { +FLOPPY_DPRINTF ("seek to next track (%d %02x %02x => %d)\n" , cur_drv->head, cur_drv->track, cur_drv->sect,fd_sector (cur_drv)); + } }else { - cur_drv->sect++; + new_sect++; } -return 1 ; +fd_seek (cur_drv, new_head, new_track, new_sect,1 ); +return ret; }/* Callback for transfer end (stop or abort) */
@@ -1622,11 +1624,7 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction)/* The seek command just sends step pulses to the drive and doesn't care if * there is a medium inserted of if it's banging the head against the drive. */ -if (fdctrl->fifo[2 ] > cur_drv->max_track) { - cur_drv->track = cur_drv->max_track; - }else { - cur_drv->track = fdctrl->fifo[2 ]; - } +fd_seek (cur_drv, cur_drv->head, fdctrl->fifo[2 ], cur_drv->sect,1 );/* Raise Interrupt */ fdctrl_raise_irq (fdctrl,FD_SR0_SEEK ); }
@@ -1691,9 +1689,9 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)SET_CUR_DRV (fdctrl, fdctrl->fifo[1 ] &FD_DOR_SELMASK ); cur_drv =get_cur_drv (fdctrl);if (fdctrl->fifo[2 ] + cur_drv->track >= cur_drv->max_track) { - cur_drv->track = cur_drv->max_track -1 ; +fd_seek (cur_drv, cur_drv->head, cur_drv->max_track -1 , cur_drv->sect,1 ); }else { - cur_drv->track += fdctrl->fifo[2 ]; +fd_seek (cur_drv, cur_drv->head, fdctrl->fifo[2 ], cur_drv->sect,1 ); }fdctrl_reset_fifo (fdctrl);/* Raise Interrupt */
@@ -1707,9 +1705,9 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)SET_CUR_DRV (fdctrl, fdctrl->fifo[1 ] &FD_DOR_SELMASK ); cur_drv =get_cur_drv (fdctrl);if (fdctrl->fifo[2 ] > cur_drv->track) { - cur_drv->track =0 ; +fd_seek (cur_drv, cur_drv->head,0 , cur_drv->sect,1 ); }else { - cur_drv->track -= fdctrl->fifo[2 ]; +fd_seek (cur_drv, cur_drv->head, fdctrl->fifo[2 ], cur_drv->sect,1 ); }fdctrl_reset_fifo (fdctrl);/* Raise Interrupt */
Signature · 1 line
1.7.7.6
Re: [Qemu-devel] [PATCH v3 3/4] fdc: rewrite seek and DSKCHG bit handling
Il 12/06/2012 13:19, Pavel Hrdina ha scritto:
Quoted 14 lines
This bit is cleared on every successful seek to a different track (cylinder). The seek is also called on revalidate or on read/write/format commands which also clear the DSKCHG bit.
Signed-off-by Pavel Hrdina <[email protected]>--- hw/fdc.c | 66 ++++++++++++++++++++++++++++++------------------------------- 1 files changed, 32 insertions(+), 34 deletions(-)hw/fdc.c +0
-0
hw/fdc.c static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, · line 159
Quoted 8 lines
hw/fdc.c +3
-1@@ -153,8 +153,11 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, }#endif drv->head = head; -if (drv->track != track) +if (drv->track != track) { ret =1 ; +if (drv->bs !=NULL &&bdrv_is_inserted (drv->bs)) + drv->media_changed =0 ;
Coding style...
Otherwise looks good.
Paolo
Quoted 121 lines
hw/fdc.c +29
-33+ } drv->track = track; drv->sect = sect; }@@ -169,9 +172,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect,static void fd_recalibrate (FDrive *drv) {FLOPPY_DPRINTF ("recalibrate\n" ); - drv->head =0 ; - drv->track =0 ; - drv->sect =1 ; +fd_seek (drv,0 ,0 ,1 ,1 ); }/* Revalidate a disk drive after a disk change */ @@ -710,14 +711,6 @@ static void fdctrl_raise_irq(FDCtrl *fdctrl, uint8_t status0)qemu_set_irq (fdctrl->irq,1 ); fdctrl->sra |=FD_SRA_INTPEND ; } -if (status0 &FD_SR0_SEEK ) { -FDrive *cur_drv; -/* A seek clears the disk change line (if a disk is inserted) */ - cur_drv =get_cur_drv (fdctrl); -if (cur_drv->bs !=NULL &&bdrv_is_inserted (cur_drv->bs)) { - cur_drv->media_changed =0 ; - } - } fdctrl->reset_sensei =0 ; fdctrl->status0 = status0;@@ -1003,30 +996,39 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv) fd_sector(cur_drv));/* XXX: cur_drv->sect >= cur_drv->last_sect should be an error in fact */ -if (cur_drv->sect >= cur_drv->last_sect || - cur_drv->sect == fdctrl->eot) { - cur_drv->sect =1 ; +uint8_t new_head = cur_drv->head; +uint8_t new_track = cur_drv->track; +uint8_t new_sect = cur_drv->sect; + +int ret =1 ; + +if (new_sect >= cur_drv->last_sect || + new_sect == fdctrl->eot) { + new_sect =1 ;if (FD_MULTI_TRACK (fdctrl->data_state)) { -if (cur_drv->head ==0 && +if (new_head ==0 && (cur_drv->flags &FDISK_DBL_SIDES ) !=0 ) { - cur_drv->head =1 ; + new_head =1 ; }else { - cur_drv->head =0 ; - cur_drv->track++; + new_head =0 ; + new_track++;if ((cur_drv->flags &FDISK_DBL_SIDES ) ==0 ) -return 0 ; + ret =0 ; } }else { - cur_drv->track++; -return 0 ; + new_track++; + ret =0 ; } -FLOPPY_DPRINTF ("seek to next track (%d %02x %02x => %d)\n" , +if (ret ==1 ) { +FLOPPY_DPRINTF ("seek to next track (%d %02x %02x => %d)\n" , cur_drv->head, cur_drv->track, cur_drv->sect,fd_sector (cur_drv)); + } }else { - cur_drv->sect++; + new_sect++; } -return 1 ; +fd_seek (cur_drv, new_head, new_track, new_sect,1 ); +return ret; }/* Callback for transfer end (stop or abort) */ @@ -1622,11 +1624,7 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction)/* The seek command just sends step pulses to the drive and doesn't care if * there is a medium inserted of if it's banging the head against the drive. */ -if (fdctrl->fifo[2 ] > cur_drv->max_track) { - cur_drv->track = cur_drv->max_track; - }else { - cur_drv->track = fdctrl->fifo[2 ]; - } +fd_seek (cur_drv, cur_drv->head, fdctrl->fifo[2 ], cur_drv->sect,1 );/* Raise Interrupt */ fdctrl_raise_irq (fdctrl,FD_SR0_SEEK ); }@@ -1691,9 +1689,9 @@ static void fdctrl_handle_relative_seek_out(FDCtrl *fdctrl, int direction)SET_CUR_DRV (fdctrl, fdctrl->fifo[1 ] &FD_DOR_SELMASK ); cur_drv =get_cur_drv (fdctrl);if (fdctrl->fifo[2 ] + cur_drv->track >= cur_drv->max_track) { - cur_drv->track = cur_drv->max_track -1 ; +fd_seek (cur_drv, cur_drv->head, cur_drv->max_track -1 , cur_drv->sect,1 ); }else { - cur_drv->track += fdctrl->fifo[2 ]; +fd_seek (cur_drv, cur_drv->head, fdctrl->fifo[2 ], cur_drv->sect,1 ); }fdctrl_reset_fifo (fdctrl);/* Raise Interrupt */ @@ -1707,9 +1705,9 @@ static void fdctrl_handle_relative_seek_in(FDCtrl *fdctrl, int direction)SET_CUR_DRV (fdctrl, fdctrl->fifo[1 ] &FD_DOR_SELMASK ); cur_drv =get_cur_drv (fdctrl);if (fdctrl->fifo[2 ] > cur_drv->track) { - cur_drv->track =0 ; +fd_seek (cur_drv, cur_drv->head,0 , cur_drv->sect,1 ); }else { - cur_drv->track -= fdctrl->fifo[2 ]; +fd_seek (cur_drv, cur_drv->head, fdctrl->fifo[2 ], cur_drv->sect,1 ); }fdctrl_reset_fifo (fdctrl);/* Raise Interrupt */
PATCH v3 4/4 fdc: fix interrupt handling
In reply to Pavel HrdinaIf you call the SENSE INTERRUPT STATUS commad while there is no interrupt waiting you get as reasult unknown command.
Fixed status0 register handling for read/write/format commands.
--- hw/fdc.c | 30 ++++++++++++++++-------------- 1 files changed, 16 insertions(+), 14 deletions(-)
hw/fdc.c +16 -14
@@ -305,6 +305,7 @@ enum { };enum { +FD_SR0_HEAD =0x04 ,FD_SR0_EQPMT =0x10 ,FD_SR0_SEEK =0x20 ,FD_SR0_ABNTERM =0x40 ,
@@ -970,14 +971,14 @@ static void fdctrl_reset_fifo(FDCtrl *fdctrl) }/* Set FIFO status for the host to read */ -static void fdctrl_set_fifo (FDCtrl *fdctrl,int fifo_len,int do_irq) +static void fdctrl_set_fifo (FDCtrl *fdctrl,int fifo_len,uint8_t status0) { fdctrl->data_dir =FD_DIR_READ ; fdctrl->data_len = fifo_len; fdctrl->data_pos =0 ; fdctrl->msr |=FD_MSR_CMDBUSY |FD_MSR_RQM |FD_MSR_DIO ; -if (do_irq) -fdctrl_raise_irq (fdctrl,0x00 ); +if (status0) +fdctrl_raise_irq (fdctrl, status0); }/* Set an error: unimplemented/unknown command */
@@ -1038,10 +1039,11 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0,FDrive *cur_drv; cur_drv =get_cur_drv (fdctrl); + fdctrl->status0 = status0 |FD_SR0_SEEK | (cur_drv->head <<2 ) |GET_CUR_DRV (fdctrl); +FLOPPY_DPRINTF ("transfer status: %02x %02x %02x (%02x)\n" , - status0, status1, status2, - status0 | (cur_drv->head <<2 ) |GET_CUR_DRV (fdctrl)); - fdctrl->fifo[0 ] = status0 | (cur_drv->head <<2 ) |GET_CUR_DRV (fdctrl); + status0, status1, status2, fdctrl->status0); + fdctrl->fifo[0 ] = fdctrl->status0; fdctrl->fifo[1 ] = status1; fdctrl->fifo[2 ] = status2; fdctrl->fifo[3 ] = cur_drv->track;
@@ -1054,7 +1056,7 @@ static void fdctrl_stop_transfer(FDCtrl *fdctrl, uint8_t status0, } fdctrl->msr |=FD_MSR_RQM |FD_MSR_DIO ; fdctrl->msr &= ~FD_MSR_NONDMA ; -fdctrl_set_fifo (fdctrl,7 ,1 ); +fdctrl_set_fifo (fdctrl,7 , fdctrl->status0); }/* Prepare a data transfer (either DMA or FIFO) */
@@ -1168,7 +1170,7 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)if (direction !=FD_DIR_WRITE ) fdctrl->msr |=FD_MSR_DIO ;/* IO based transfer: calculate len */ -fdctrl_raise_irq (fdctrl,0x00 ); +fdctrl_raise_irq (fdctrl,FD_SR0_SEEK );return ; }
@@ -1596,16 +1598,16 @@ static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction) {FDrive *cur_drv =get_cur_drv (fdctrl); -if (fdctrl->reset_sensei >0 ) { +if (fdctrl->reset_sensei >0 ) { fdctrl->fifo[0 ] =FD_SR0_RDYCHG +FD_RESET_SENSEI_COUNT - fdctrl->reset_sensei; fdctrl->reset_sensei--; + }else if (!(fdctrl->sra &FD_SRA_INTPEND )) { + fdctrl->fifo[0 ] =FD_SR0_INVCMD ; +fdctrl_set_fifo (fdctrl,1 ,0 ); +return ; }else { -/* XXX: status0 handling is broken for read/write -commands, so we do this hack. It should be suppressed -ASAP */ - fdctrl->fifo[0 ] = -FD_SR0_SEEK | (cur_drv->head <<2 ) |GET_CUR_DRV (fdctrl); + fdctrl->fifo[0 ] = (fdctrl->status0 & ~FD_SR0_HEAD ) |GET_CUR_DRV (fdctrl); } fdctrl->fifo[1 ] = cur_drv->track;
Signature · 1 line
1.7.7.6
Re: [Qemu-devel] [PATCH v3 4/4] fdc: fix interrupt handling
Il 12/06/2012 13:19, Pavel Hrdina ha scritto:
hw/fdc.c static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction) · line 1610
Quoted 7 lines
} else {hw/fdc.c +1
-5-/* XXX: status0 handling is broken for read/write -commands, so we do this hack. It should be suppressed -ASAP */ - fdctrl->fifo[0 ] = -FD_SR0_SEEK | (cur_drv->head <<2 ) |GET_CUR_DRV (fdctrl); + fdctrl->fifo[0 ] = (fdctrl->status0 & ~FD_SR0_HEAD ) |GET_CUR_DRV (fdctrl);
Should bit 0 be cleared also, before ORing GET_CUR_DRV(fdctrl)?
Paolo
Re: [Qemu-devel] [PATCH v3 4/4] fdc: fix interrupt handling
Quoted 11 lines · Paolo Bonzini
Il 12/06/2012 13:19, Pavel Hrdina ha scritto:
hw/fdc.c static void fdctrl_handle_sense_interrupt_status(FDCtrl *fdctrl, int direction)
Quoted 7 lines
} else {hw/fdc.c +1
-5-/* XXX: status0 handling is broken for read/write -commands, so we do this hack. It should be suppressed -ASAP */ - fdctrl->fifo[0 ] = -FD_SR0_SEEK | (cur_drv->head<<2 ) |GET_CUR_DRV (fdctrl); + fdctrl->fifo[0 ] = (fdctrl->status0& ~FD_SR0_HEAD ) |GET_CUR_DRV (fdctrl);Should bit 0 be cleared also, before ORing
GET_CUR_DRV(fdctrl)?Paolo
According to the code bits 0 and 1 don't have to be cleared, but it would be probably better clear them too. Good point.
Pavel