Misc fixes for floppy emulation
v3 cover Misc fixes for floppy emulation
Here are misc fixes done by VirtualBox team.
With these patches, floppy emulation is now good enough to run Xenix.
Changes v2->v3:
- removed patch changing controller stepping to 0
- fixed out of bound access after bad seek command
Changes v1->v2:
- updated commit messages
- added missing 'break' and braces
Hervé Poussineau (9):
fdc: take side count into account
fdc: set busy bit when starting a command
fdc: most control commands do not generate interrupts
fdc: handle read-only floppies (abort early on write commands)
fdc: add CCR (Configuration Control Register) write register
block: add a transfer rate for floppy types
fdc: check if media rate is correct before doing any transfer
fdc: fix seek command, which shouldn't check tracks
fdc: DIR (Digital Input Register) should return status of current
drive...
block.c | 74 ++++++++++++++++++++++------------------- block.h | 10 +++++- hw/fdc.c | 111 ++++++++++++++++++++++++++++++++++++++++++++++++-------------- hw/pc.c | 3 +- 4 files changed, 136 insertions(+), 62 deletions(-)
Signature · 1 line
1.7.7.3
PATCH v3 1/9 fdc: take side count into account
Floppies can be simple or double-sided. However, current code was only taking the common case into account (ie 2 sides).
--- hw/fdc.c | 17 +++++++++++------ 1 files changed, 11 insertions(+), 6 deletions(-)
hw/fdc.c +11 -6
@@ -95,16 +95,19 @@ static void fd_init(FDrive *drv) drv->max_track =0 ; } +#define NUM_SIDES (drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1) +static int fd_sector_calc (uint8_t head,uint8_t track,uint8_t sect, -uint8_t last_sect) +uint8_t last_sect,uint8_t num_sides) { -return (((track *2 ) + head) * last_sect) + sect -1 ; +return (((track * num_sides) + head) * last_sect) + sect -1 ; }/* Returns current position, in sectors, for given drive */ static int fd_sector (FDrive *drv) { -return fd_sector_calc (drv->head, drv->track, drv->sect, drv->last_sect); +return fd_sector_calc (drv->head, drv->track, drv->sect, drv->last_sect, +NUM_SIDES (drv)); }/* Seek to a new position:
@@ -135,7 +138,7 @@ static int fd_seek(FDrive *drv, uint8_t head, uint8_t track, uint8_t sect, drv->max_track, drv->last_sect);return 3 ; } - sector =fd_sector_calc (head, track, sect, drv->last_sect); + sector =fd_sector_calc (head, track, sect, drv->last_sect,NUM_SIDES (drv)); ret =0 ;if (sector !=fd_sector (drv)) {#if 0
@@ -1019,7 +1022,8 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction) ks = fdctrl->fifo[4 ];FLOPPY_DPRINTF ("Start transfer at %d %d %02x %02x (%d)\n" ,GET_CUR_DRV (fdctrl), kh, kt, ks, -fd_sector_calc (kh, kt, ks, cur_drv->last_sect)); +fd_sector_calc (kh, kt, ks, cur_drv->last_sect, +NUM_SIDES (cur_drv)));switch (fd_seek (cur_drv, kh, kt, ks, fdctrl->config &FD_CONFIG_EIS )) {case 2 :/* sect too big */
@@ -1289,7 +1293,8 @@ static void fdctrl_format_sector(FDCtrl *fdctrl) ks = fdctrl->fifo[8 ];FLOPPY_DPRINTF ("format sector at %d %d %02x %02x (%d)\n" ,GET_CUR_DRV (fdctrl), kh, kt, ks, -fd_sector_calc (kh, kt, ks, cur_drv->last_sect)); +fd_sector_calc (kh, kt, ks, cur_drv->last_sect, +NUM_SIDES (cur_drv)));switch (fd_seek (cur_drv, kh, kt, ks, fdctrl->config &FD_CONFIG_EIS )) {case 2 :/* sect too big */
Signature · 1 line
1.7.7.3
Re: [Qemu-devel] [PATCH v3 1/9] fdc: take side count into account
Quoted 2 lines · Hervé Poussineau
Floppies can be simple or double-sided. However, current code was only taking the common case into account (ie 2 sides).
Impact? Are single sided floppies broken before the patch? How?
Re: [Qemu-devel] [PATCH v3 1/9] fdc: take side count into account
Quoted 6 lines · Markus Armbruster
Quoted 2 lines · Hervé Poussineau
Floppies can be simple or double-sided. However, current code was only taking the common case into account (ie 2 sides).
Impact?
This repairs single-sided floppies
Quoted 1 line
Are single sided floppies broken before the patch? How?
Yes. For head > 0, wrong sector number is calculated, so data is read/written at wrong place on underlying block device.
Fortunately, mostly nobody is still using single-sided floppies (only some 360 kB floppies are single-sided), so bug has been neglected since a long time.
Hervé
Re: [Qemu-devel] [PATCH v3 1/9] fdc: take side count into account
Quoted 10 lines · Hervé Poussineau
Quoted 6 lines · Markus Armbruster
Quoted 2 lines · Hervé Poussineau
Floppies can be simple or double-sided. However, current code was only taking the common case into account (ie 2 sides).
Impact?
This repairs single-sided floppies
Quoted 8 lines
Quoted 1 line
Are single sided floppies broken before the patch? How?
Yes. For head > 0, wrong sector number is calculated, so data is read/written at wrong place on underlying block device.
Fortunately, mostly nobody is still using single-sided floppies (only some 360 kB floppies are single-sided), so bug has been neglected since a long time.
Nice info, thanks. Best put it in the commit message.
Re: [Qemu-devel] [PATCH v3 1/9] fdc: take side count into account
Hi,
Quoted 9 lines · Markus Armbruster
Quoted 7 lines
Quoted 1 line
Are single sided floppies broken before the patch? How?
Yes. For head > 0, wrong sector number is calculated, so data is read/written at wrong place on underlying block device.
Fortunately, mostly nobody is still using single-sided floppies (only some 360 kB floppies are single-sided), so bug has been neglected since a long time.
Nice info, thanks. Best put it in the commit message.
Will do when respinning the serie
Hervé
PATCH v3 2/9 fdc: set busy bit when starting a command
In reply to Hervé PoussineauThis bit must be active while a command is currently executed.
--- hw/fdc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
hw/fdc.c +1 -1
@@ -1446,7 +1446,6 @@ static void fdctrl_handle_readid(FDCtrl *fdctrl, int direction) {FDrive *cur_drv =get_cur_drv (fdctrl); -/* XXX: should set main status register to busy */ cur_drv->head = (fdctrl->fifo[1 ] >>2 ) &1 ;qemu_mod_timer (fdctrl->result_timer,qemu_get_clock_ns (vm_clock) + (get_ticks_per_sec () /50 ));
@@ -1734,6 +1733,7 @@ static void fdctrl_write_data(FDCtrl *fdctrl, uint32_t value) pos = command_to_handler[value &0xff ];FLOPPY_DPRINTF ("%s command\n" , handlers[pos].name); fdctrl->data_len = handlers[pos].parameters +1 ; + fdctrl->msr |=FD_MSR_CMDBUSY ; }FLOPPY_DPRINTF ("%s: %02x\n" , __func__, value);
Signature · 1 line
1.7.7.3
PATCH v3 3/9 fdc: most control commands do not generate interrupts
In reply to Hervé PoussineauIn fact, only three control commands generate an interrupt:read_id, recalibrate and seek
--- hw/fdc.c | 12 ++++++------ 1 files changed, 6 insertions(+), 6 deletions(-)
hw/fdc.c +6 -6
@@ -1348,7 +1348,7 @@ static void fdctrl_handle_lock(FDCtrl *fdctrl, int direction) { fdctrl->lock = (fdctrl->fifo[0 ] &0x80 ) ?1 :0 ; fdctrl->fifo[0 ] = fdctrl->lock <<4 ; -fdctrl_set_fifo (fdctrl,1 , fdctrl->lock); +fdctrl_set_fifo (fdctrl,1 ,0 ); }static void fdctrl_handle_dumpreg (FDCtrl *fdctrl,int direction)
@@ -1380,7 +1380,7 @@ static void fdctrl_handle_version(FDCtrl *fdctrl, int direction) {/* Controller's version */ fdctrl->fifo[0 ] = fdctrl->version; -fdctrl_set_fifo (fdctrl,1 ,1 ); +fdctrl_set_fifo (fdctrl,1 ,0 ); }static void fdctrl_handle_partid (FDCtrl *fdctrl,int direction)
@@ -1439,7 +1439,7 @@ static void fdctrl_handle_save(FDCtrl *fdctrl, int direction) fdctrl->fifo[12 ] = fdctrl->pwrd; fdctrl->fifo[13 ] =0 ; fdctrl->fifo[14 ] =0 ; -fdctrl_set_fifo (fdctrl,15 ,1 ); +fdctrl_set_fifo (fdctrl,15 ,0 ); }static void fdctrl_handle_readid (FDCtrl *fdctrl,int direction)
@@ -1580,7 +1580,7 @@ static void fdctrl_handle_powerdown_mode(FDCtrl *fdctrl, int direction) { fdctrl->pwrd = fdctrl->fifo[1 ]; fdctrl->fifo[0 ] = fdctrl->fifo[1 ]; -fdctrl_set_fifo (fdctrl,1 ,1 ); +fdctrl_set_fifo (fdctrl,1 ,0 ); }static void fdctrl_handle_option (FDCtrl *fdctrl,int direction)
@@ -1599,7 +1599,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct fdctrl->fifo[0 ] = fdctrl->fifo[1 ]; fdctrl->fifo[2 ] =0 ; fdctrl->fifo[3 ] =0 ; -fdctrl_set_fifo (fdctrl,4 ,1 ); +fdctrl_set_fifo (fdctrl,4 ,0 ); }else {fdctrl_reset_fifo (fdctrl); }
@@ -1607,7 +1607,7 @@ static void fdctrl_handle_drive_specification_command(FDCtrl *fdctrl, int direct/* ERROR */ fdctrl->fifo[0 ] =0x80 | (cur_drv->head <<2 ) |GET_CUR_DRV (fdctrl); -fdctrl_set_fifo (fdctrl,1 ,1 ); +fdctrl_set_fifo (fdctrl,1 ,0 ); } }
Signature · 1 line
1.7.7.3
PATCH v3 4/9 fdc: handle read-only floppies (abort early on write commands)
In reply to Hervé PoussineauA real floppy doesn't attempt to write to read-only media either.
--- hw/fdc.c | 11 +++++++++++ 1 files changed, 11 insertions(+), 0 deletions(-)
hw/fdc.c +11 -0
@@ -300,6 +300,7 @@ enum { };enum { +FD_SR1_NW =0x02 ,/* Not writable */ FD_SR1_EC =0x80 ,/* End of cylinder */ };
@@ -1179,6 +1180,16 @@ static int fdctrl_transfer_handler (void *opaque, int nchan,break ;case FD_DIR_WRITE :/* WRITE commands */ +if (cur_drv->ro) { +/* Handle readonly medium early, no need to do DMA, touch the +* LED or attempt any writes. A real floppy doesn't attempt +* to write to readonly media either. */ +fdctrl_stop_transfer (fdctrl, +FD_SR0_ABNTERM |FD_SR0_SEEK ,FD_SR1_NW , +0x00 ); + goto transfer_error; + } +DMA_read_memory (nchan, fdctrl->fifo + rel_pos, fdctrl->data_pos, len); if (bdrv_write(cur_drv->bs, fd_sector(cur_drv),
Signature · 1 line
1.7.7.3
PATCH v3 5/9 fdc: add CCR (Configuration Control Register) write register
In reply to Hervé PoussineauDIR and CCR registers share the same address ; DIR is read-only while CCR is write-only
--- hw/fdc.c | 22 ++++++++++++++++++++++ 1 files changed, 22 insertions(+), 0 deletions(-)
hw/fdc.c +22 -0
@@ -224,6 +224,7 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value);static uint32_t fdctrl_read_data (FDCtrl *fdctrl);static void fdctrl_write_data (FDCtrl *fdctrl,uint32_t value);static uint32_t fdctrl_read_dir (FDCtrl *fdctrl); +static void fdctrl_write_ccr (FDCtrl *fdctrl,uint32_t value);enum {FD_DIR_WRITE =0 ,
@@ -248,6 +249,7 @@ enum {FD_REG_DSR =0x04 ,FD_REG_FIFO =0x05 ,FD_REG_DIR =0x07 , + FD_REG_CCR = 0x07, }; enum {
@@ -491,6 +493,9 @@ static void fdctrl_write (void *opaque, uint32_t reg, uint32_t value)case FD_REG_FIFO :fdctrl_write_data (fdctrl, value);break ; +case FD_REG_CCR : +fdctrl_write_ccr (fdctrl, value); +break ;default :break ; }
@@ -881,6 +886,23 @@ static void fdctrl_write_rate(FDCtrl *fdctrl, uint32_t value) fdctrl->dsr = value; } +/* Configuration control register: 0x07 (write) */ +static void fdctrl_write_ccr (FDCtrl *fdctrl,uint32_t value) +{ +/* Reset mode */ +if (!(fdctrl->dor & FD_DOR_nRESET)) { +FLOPPY_DPRINTF ("Floppy controller in RESET state !\n" ); +return ; + } +FLOPPY_DPRINTF ("configuration control register set to 0x%02x\n" , value); + +/* Only the rate selection bits used in AT mode, and we +* store those in the DSR. +*/ + fdctrl->dsr = (fdctrl->dsr & ~FD_DSR_DRATEMASK ) | + (value &FD_DSR_DRATEMASK ); +} +static int fdctrl_media_changed (FDrive *drv) {int ret;
Signature · 1 line
1.7.7.3
Re: [Qemu-devel] [PATCH v3 5/9] fdc: add CCR (Configuration Control Register) write register
Quoted 2 lines · Hervé Poussineau
DIR and CCR registers share the same address ; DIR is read-only while CCR is write-only
Looks like guest writes to CCR are silently ignored before this patch. Is that correct? Impact?
Re: [Qemu-devel] [PATCH v3 5/9] fdc: add CCR (Configuration Control Register) write register
Quoted 7 lines · Markus Armbruster
Quoted 2 lines · Hervé Poussineau
DIR and CCR registers share the same address ; DIR is read-only while CCR is write-only
Looks like guest writes to CCR are silently ignored before this patch. Is that correct?
Yes.
Quoted 1 line
Impact?
CCR is only used to program media rate, which is not checked without this patchset.
So, none for this patch.
Re: [Qemu-devel] [PATCH v3 5/9] fdc: add CCR (Configuration Control Register) write register
Quoted 9 lines · Hervé Poussineau
Quoted 7 lines · Markus Armbruster
Quoted 2 lines · Hervé Poussineau
DIR and CCR registers share the same address ; DIR is read-only while CCR is write-only
Looks like guest writes to CCR are silently ignored before this patch. Is that correct?
Quoted 8 lines
Yes.
Quoted 1 line
Impact?
CCR is only used to program media rate, which is not checked without this patchset.
So, none for this patch.
Aha, it's just a prerequisite for the rest of the series. Makes sense.
Explain in commit message?
Re: [Qemu-devel] [PATCH v3 5/9] fdc: add CCR (Configuration Control Register) write register
Quoted 12 lines · Markus Armbruster
Quoted 10 lines · Hervé Poussineau
Quoted 6 lines · Markus Armbruster
Quoted 2 lines · Hervé Poussineau
DIR and CCR registers share the same address ; DIR is read-only while CCR is write-only
Looks like guest writes to CCR are silently ignored before this patch. Is that correct?
Yes.
Quoted 1 line
Impact?
Quoted 9 lines
Quoted 4 lines
CCR is only used to program media rate, which is not checked without this patchset.
So, none for this patch.
Aha, it's just a prerequisite for the rest of the series. Makes sense.
Explain in commit message?
Will do when respinning the serie
Hervé
PATCH v3 6/9 block: add a transfer rate for floppy types
In reply to Hervé PoussineauFloppies must be read at a specific transfer rate, depending of its own format. Update floppy description table to include required transfer rate.
--- block.c | 74 ++++++++++++++++++++++++++++++++----------------------------- block.h | 10 +++++++- hw/fdc.c | 3 +- hw/pc.c | 3 +- 4 files changed, 52 insertions(+), 38 deletions(-)
block.c +39 -35
@@ -1884,58 +1884,60 @@ typedef struct FDFormat {uint8_t last_sect;uint8_t max_track;uint8_t max_head; +FDriveRate rate; } FDFormat;static const FDFormat fd_formats[] = {/* First entry is default format */ /* 1.44 MB 3"1/2 floppy disks */ - {FDRIVE_DRV_144 ,18 ,80 ,1 , }, - {FDRIVE_DRV_144 ,20 ,80 ,1 , }, - {FDRIVE_DRV_144 ,21 ,80 ,1 , }, - {FDRIVE_DRV_144 ,21 ,82 ,1 , }, - {FDRIVE_DRV_144 ,21 ,83 ,1 , }, - {FDRIVE_DRV_144 ,22 ,80 ,1 , }, - {FDRIVE_DRV_144 ,23 ,80 ,1 , }, - {FDRIVE_DRV_144 ,24 ,80 ,1 , }, + {FDRIVE_DRV_144 ,18 ,80 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_144 ,20 ,80 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_144 ,21 ,80 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_144 ,21 ,82 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_144 ,21 ,83 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_144 ,22 ,80 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_144 ,23 ,80 ,1 ,FDRIVE_RATE_500K }, + {FDRIVE_DRV_144 ,24 ,80 ,1 ,FDRIVE_RATE_500K , },/* 2.88 MB 3"1/2 floppy disks */ - {FDRIVE_DRV_288 ,36 ,80 ,1 , }, - {FDRIVE_DRV_288 ,39 ,80 ,1 , }, - {FDRIVE_DRV_288 ,40 ,80 ,1 , }, - {FDRIVE_DRV_288 ,44 ,80 ,1 , }, - {FDRIVE_DRV_288 ,48 ,80 ,1 , }, + {FDRIVE_DRV_288 ,36 ,80 ,1 ,FDRIVE_RATE_1M , }, + {FDRIVE_DRV_288 ,39 ,80 ,1 ,FDRIVE_RATE_1M , }, + {FDRIVE_DRV_288 ,40 ,80 ,1 ,FDRIVE_RATE_1M , }, + {FDRIVE_DRV_288 ,44 ,80 ,1 ,FDRIVE_RATE_1M , }, + {FDRIVE_DRV_288 ,48 ,80 ,1 ,FDRIVE_RATE_1M , },/* 720 kB 3"1/2 floppy disks */ - {FDRIVE_DRV_144 ,9 ,80 ,1 , }, - {FDRIVE_DRV_144 ,10 ,80 ,1 , }, - {FDRIVE_DRV_144 ,10 ,82 ,1 , }, - {FDRIVE_DRV_144 ,10 ,83 ,1 , }, - {FDRIVE_DRV_144 ,13 ,80 ,1 , }, - {FDRIVE_DRV_144 ,14 ,80 ,1 , }, + {FDRIVE_DRV_144 ,9 ,80 ,1 ,FDRIVE_RATE_250K , }, + {FDRIVE_DRV_144 ,10 ,80 ,1 ,FDRIVE_RATE_250K , }, + {FDRIVE_DRV_144 ,10 ,82 ,1 ,FDRIVE_RATE_250K , }, + {FDRIVE_DRV_144 ,10 ,83 ,1 ,FDRIVE_RATE_250K , }, + {FDRIVE_DRV_144 ,13 ,80 ,1 ,FDRIVE_RATE_250K , }, + {FDRIVE_DRV_144 ,14 ,80 ,1 ,FDRIVE_RATE_250K , },/* 1.2 MB 5"1/4 floppy disks */ - {FDRIVE_DRV_120 ,15 ,80 ,1 , }, - {FDRIVE_DRV_120 ,18 ,80 ,1 , }, - {FDRIVE_DRV_120 ,18 ,82 ,1 , }, - {FDRIVE_DRV_120 ,18 ,83 ,1 , }, - {FDRIVE_DRV_120 ,20 ,80 ,1 , }, + {FDRIVE_DRV_120 ,15 ,80 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_120 ,18 ,80 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_120 ,18 ,82 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_120 ,18 ,83 ,1 ,FDRIVE_RATE_500K , }, + {FDRIVE_DRV_120 ,20 ,80 ,1 ,FDRIVE_RATE_500K , },/* 720 kB 5"1/4 floppy disks */ - {FDRIVE_DRV_120 ,9 ,80 ,1 , }, - {FDRIVE_DRV_120 ,11 ,80 ,1 , }, + {FDRIVE_DRV_120 ,9 ,80 ,1 ,FDRIVE_RATE_250K , }, + {FDRIVE_DRV_120 ,11 ,80 ,1 ,FDRIVE_RATE_250K , },/* 360 kB 5"1/4 floppy disks */ - {FDRIVE_DRV_120 ,9 ,40 ,1 , }, - {FDRIVE_DRV_120 ,9 ,40 ,0 , }, - {FDRIVE_DRV_120 ,10 ,41 ,1 , }, - {FDRIVE_DRV_120 ,10 ,42 ,1 , }, + {FDRIVE_DRV_120 ,9 ,40 ,1 ,FDRIVE_RATE_300K , }, + {FDRIVE_DRV_120 ,9 ,40 ,0 ,FDRIVE_RATE_300K , }, + {FDRIVE_DRV_120 ,10 ,41 ,1 ,FDRIVE_RATE_300K , }, + {FDRIVE_DRV_120 ,10 ,42 ,1 ,FDRIVE_RATE_300K , },/* 320 kB 5"1/4 floppy disks */ - {FDRIVE_DRV_120 ,8 ,40 ,1 , }, - {FDRIVE_DRV_120 ,8 ,40 ,0 , }, + {FDRIVE_DRV_120 ,8 ,40 ,1 ,FDRIVE_RATE_250K , }, + {FDRIVE_DRV_120 ,8 ,40 ,0 ,FDRIVE_RATE_250K , },/* 360 kB must match 5"1/4 better than 3"1/2... */ - {FDRIVE_DRV_144 ,9 ,80 ,0 , }, + {FDRIVE_DRV_144 ,9 ,80 ,0 ,FDRIVE_RATE_250K , },/* end */ - {FDRIVE_DRV_NONE ,-1 ,-1 ,0 , }, + {FDRIVE_DRV_NONE ,-1 ,-1 ,0 ,0 , }, };void bdrv_get_floppy_geometry_hint (BlockDriverState *bs,int *nb_heads,int *max_track,int *last_sect, -FDriveType drive_in,FDriveType *drive) +FDriveType drive_in,FDriveType *drive, +FDriveRate *rate) {const FDFormat *parse;uint64_t nb_sectors, size;
@@ -1944,6 +1946,7 @@ void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads,bdrv_get_geometry_hint (bs, nb_heads, max_track, last_sect);if (*nb_heads !=0 && *max_track !=0 && *last_sect !=0 ) {/* User defined disk */ + *rate =FDRIVE_RATE_500K ; }else {bdrv_get_geometry (bs, &nb_sectors); match =-1 ;
@@ -1978,6 +1981,7 @@ void bdrv_get_floppy_geometry_hint(BlockDriverState *bs, int *nb_heads, *max_track = parse->max_track; *last_sect = parse->last_sect; *drive = parse->drive; + *rate = parse->rate; } }
block.h +9 -1
@@ -240,9 +240,17 @@ typedef enum FDriveType {FDRIVE_DRV_NONE =0x03 ,/* No drive connected */ } FDriveType; +typedef enum FDriveRate { +FDRIVE_RATE_500K =0x00 ,/* 500 Kbps */ +FDRIVE_RATE_300K =0x01 ,/* 300 Kbps */ +FDRIVE_RATE_250K =0x02 ,/* 250 Kbps */ +FDRIVE_RATE_1M =0x03 ,/* 1 Mbps */ +}FDriveRate ; +void bdrv_get_floppy_geometry_hint (BlockDriverState *bs,int *nb_heads,int *max_track,int *last_sect, -FDriveType drive_in,FDriveType *drive); +FDriveType drive_in,FDriveType *drive, +FDriveRate *rate);int bdrv_get_translation_hint (BlockDriverState *bs);void bdrv_set_on_error (BlockDriverState *bs,BlockErrorAction on_read_error,BlockErrorAction on_write_error);
hw/fdc.c +2 -1
@@ -172,12 +172,13 @@ static void fd_revalidate(FDrive *drv) {int nb_heads, max_track, last_sect, ro;FDriveType drive; +FDriveRate rate;FLOPPY_DPRINTF ("revalidate\n" );if (drv->bs !=NULL &&bdrv_is_inserted (drv->bs)) { ro =bdrv_is_read_only (drv->bs);bdrv_get_floppy_geometry_hint (drv->bs, &nb_heads, &max_track, - &last_sect, drv->drive, &drive); + &last_sect, drv->drive, &drive, &rate);if (nb_heads !=0 && max_track !=0 && last_sect !=0 ) {FLOPPY_DPRINTF ("User defined disk (%d %d %d)" , nb_heads -1 , max_track, last_sect);
hw/pc.c +2 -1
@@ -336,6 +336,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size, {int val, nb, nb_heads, max_track, last_sect, i;FDriveType fd_type[2 ] = {FDRIVE_DRV_NONE ,FDRIVE_DRV_NONE }; +FDriveRate rate;BlockDriverState *fd[MAX_FD ];static pc_cmos_init_late_arg arg;
@@ -384,7 +385,7 @@ void pc_cmos_init(ram_addr_t ram_size, ram_addr_t above_4g_mem_size,if (fd[i] &&bdrv_is_inserted (fd[i])) {bdrv_get_floppy_geometry_hint (fd[i], &nb_heads, &max_track, &last_sect,FDRIVE_DRV_NONE , - &fd_type[i]); + &fd_type[i], &rate); } } }
Signature · 1 line
1.7.7.3
PATCH v3 7/9 fdc: check if media rate is correct before doing any transfer
In reply to Hervé PoussineauThe programmed rate has to be the same as the required rate for the floppy format ; if that's not the case, the transfer should abort.
Revalidate floppy after migration, so media_rate field doesn't have to be saved/restored.
--- hw/fdc.c | 25 ++++++++++++++++++++++++- 1 files changed, 24 insertions(+), 1 deletions(-)
hw/fdc.c +24 -1
@@ -83,6 +83,7 @@ typedef struct FDrive {uint16_t bps;/* Bytes per sector */ uint8_t ro;/* Is read-only */ uint8_t media_changed;/* Is media changed */ +uint8_t media_rate;/* Data rate of medium */ } FDrive;static void fd_init (FDrive *drv)
@@ -195,6 +196,7 @@ static void fd_revalidate(FDrive *drv) drv->last_sect = last_sect; drv->ro = ro; drv->drive = drive; + drv->media_rate = rate; }else {FLOPPY_DPRINTF ("No disk in drive\n" ); drv->last_sect =0 ;
@@ -303,6 +305,7 @@ enum { };enum { +FD_SR1_MA =0x01 ,/* Missing address mark */ FD_SR1_NW =0x02 ,/* Not writable */ FD_SR1_EC =0x80 ,/* End of cylinder */ };
@@ -582,6 +585,7 @@ static int fdc_post_load(void *opaque, int version_id)SET_CUR_DRV (s, s->dor_vmstate &FD_DOR_SELMASK ); s->dor = s->dor_vmstate & ~FD_DOR_SELMASK ; +fd_revalidate (get_cur_drv (s));return 0 ; }
@@ -1077,6 +1081,18 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction)break ; } +/* Check the data rate. If the programmed data rate does not match +* the currently inserted medium, the operation has to fail. */ +if ((fdctrl->dsr &FD_DSR_DRATEMASK ) != cur_drv->media_rate) { +FLOPPY_DPRINTF ("data rate mismatch (fdc=%d, media=%d)\n" , + fdctrl->dsr &FD_DSR_DRATEMASK , cur_drv->media_rate); +fdctrl_stop_transfer (fdctrl,FD_SR0_ABNTERM ,FD_SR1_MA ,0x00 ); + fdctrl->fifo[3 ] = kt; + fdctrl->fifo[4 ] = kh; + fdctrl->fifo[5 ] = ks; +return ; + } +/* Set the FIFO state */ fdctrl->data_dir = direction; fdctrl->data_pos =0 ;
@@ -1799,7 +1815,14 @@ static void fdctrl_result_timer(void *opaque)if (cur_drv->last_sect !=0 ) { cur_drv->sect = (cur_drv->sect % cur_drv->last_sect) +1 ; } -fdctrl_stop_transfer (fdctrl,0x00 ,0x00 ,0x00 ); +/* READ_ID can't automatically succeed! */ +if ((fdctrl->dsr &FD_DSR_DRATEMASK ) != cur_drv->media_rate) { +FLOPPY_DPRINTF ("read id rate mismatch (fdc=%d, media=%d)\n" , + fdctrl->dsr &FD_DSR_DRATEMASK , cur_drv->media_rate); +fdctrl_stop_transfer (fdctrl,FD_SR0_ABNTERM ,FD_SR1_MA ,0x00 ); + }else { +fdctrl_stop_transfer (fdctrl,0x00 ,0x00 ,0x00 ); + } }static void fdctrl_change_cb (void *opaque,bool load)
Signature · 1 line
1.7.7.3
Re: [Qemu-devel] [PATCH v3 7/9] fdc: check if media rate is correct before doing any transfer
Quoted 5 lines · Hervé Poussineau
The programmed rate has to be the same as the required rate for the floppy format ; if that's not the case, the transfer should abort.
Revalidate floppy after migration, so
media_ratefield doesn't have to be saved/restored.
I'm not at all sure that's a good idea.
The rate is guest state. It's value depends on the media. We guess the media to use for a particular image. Same thing as for the geometry (heads, tracks, sectors per track). We do migrate the geometry, see vmstate_fdrive.
Why is it a good idea to treat rate differently?
Doesn't your fd_revalidate() in fdc_post_load() clobber the migrated geometry?
PATCH v3 8/9 fdc: fix seek command, which shouldn't check tracks
In reply to Hervé PoussineauThe seek command just sends step pulses to the drive and doesn't care if there is a medium inserted of if it is banging the head against the drive.
--- hw/fdc.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)
hw/fdc.c +6 -3
@@ -1599,13 +1599,16 @@ static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction)SET_CUR_DRV (fdctrl, fdctrl->fifo[1 ] &FD_DOR_SELMASK ); cur_drv =get_cur_drv (fdctrl);fdctrl_reset_fifo (fdctrl); +/* 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) { -fdctrl_raise_irq (fdctrl,FD_SR0_ABNTERM |FD_SR0_SEEK ); + cur_drv->track = cur_drv->max_track; }else { cur_drv->track = fdctrl->fifo[2 ]; -/* Raise Interrupt */ -fdctrl_raise_irq (fdctrl,FD_SR0_SEEK ); } +/* Raise Interrupt */ +fdctrl_raise_irq (fdctrl,FD_SR0_SEEK ); }static void fdctrl_handle_perpendicular_mode (FDCtrl *fdctrl,int direction)
Signature · 1 line
1.7.7.3
PATCH v3 9/9 fdc: DIR (Digital Input Register) should return status of current drive...
In reply to Hervé Poussineau--- hw/fdc.c | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-)
hw/fdc.c +3 -7
@@ -215,6 +215,7 @@ static void fdctrl_reset_fifo(FDCtrl *fdctrl);static int fdctrl_transfer_handler (void *opaque,int nchan,int dma_pos,int dma_len);static void fdctrl_raise_irq (FDCtrl *fdctrl,uint8_t status0); +static FDrive *get_cur_drv (FDCtrl *fdctrl);static uint32_t fdctrl_read_statusA (FDCtrl *fdctrl);static uint32_t fdctrl_read_statusB (FDCtrl *fdctrl);
@@ -934,14 +935,9 @@ static uint32_t fdctrl_read_dir(FDCtrl *fdctrl) {uint32_t retval =0 ; -if (fdctrl_media_changed (drv0 (fdctrl)) - ||fdctrl_media_changed (drv1 (fdctrl)) -#if MAX_FD == 4 - ||fdctrl_media_changed (drv2 (fdctrl)) - ||fdctrl_media_changed (drv3 (fdctrl)) -#endif - ) +if (fdctrl_media_changed (get_cur_drv (fdctrl))) { retval |=FD_DIR_DSKCHG ; + }if (retval !=0 ) {FLOPPY_DPRINTF ("Floppy digital input register: 0x%02x\n" , retval); }
Signature · 1 line
1.7.7.3
Re: [Qemu-devel] [PATCH v3 9/9] fdc: DIR (Digital Input Register) should return status of current drive...
Impact? What exactly is broken and fixed?
Looks sane, except for the migration of media rate. See my reply to 7/9.
Hi,
Quoted 3 lines · Markus Armbruster
Looks sane, except for the migration of media rate. See my reply to 7/9.
I'm not very fluent with requirements for store/save/migration compatibility things, so can someone knowing them can look at the migration of media rate problem? Paolo?
Markus reply is available at:
https://lists.gnu.org/archive/html/qemu-devel/2012-01/msg03601.html
Thanks in advance
Regards,
Hervé