Misc fixes for floppy emulation
v2 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 v1->v2:
- updated commit messages
- added missing 'break' and braces
- moved patch 8 before patch 6
Hervé Poussineau (10):
fdc: take side count into account
fdc: set busy bit when starting a command
fdc: most control commands do not generate interrupts
fdc: emulate stepping 0
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 | 117 ++++++++++++++++++++++++++++++++++++++++++++++---------------- hw/pc.c | 3 +- 4 files changed, 137 insertions(+), 67 deletions(-)
Signature · 1 line
1.7.7.3
PATCH v2 1/10 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
PATCH v2 2/10 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
Re: [Qemu-devel] [PATCH v2 02/10] fdc: set busy bit when starting a command
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 3 lines
This bit must be active while a command is currently executed.
Signed-off-by Hervé Poussineau <[email protected]>
I believe this could use some more cleanup. The flag is set and reset multiple times in places that aren't quite obvious. Not saying it should be done in this patch, but maybe in a follow-up series.
Kevin
PATCH v2 3/10 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 v2 4/10 fdc: emulate stepping 0
In reply to Hervé PoussineauStepping 1 (S82078B) is not fully i82078 compatible, so better stick to initial revision
--- hw/fdc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
hw/fdc.c +1 -1
@@ -1385,7 +1385,7 @@ static void fdctrl_handle_version(FDCtrl *fdctrl, int direction)static void fdctrl_handle_partid (FDCtrl *fdctrl,int direction) { - fdctrl->fifo[0 ] =0x41 ;/* Stepping 1 */ + fdctrl->fifo[0 ] =0x01 ;/* Stepping 0 */ fdctrl_set_fifo (fdctrl,1 ,0 ); }
Signature · 1 line
1.7.7.3
Re: [Qemu-devel] [PATCH v2 04/10] fdc: emulate stepping 0
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 12 lines
Stepping 1 (S82078B) is not fully i82078 compatible, so better stick to initial revision
Signed-off-by Hervé Poussineau <[email protected]>--- hw/fdc.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)hw/fdc.c +0
-0
hw/fdc.c static void fdctrl_handle_version(FDCtrl *fdctrl, int direction) · line 1391
Quoted 8 lines
hw/fdc.c +1
-1@@ -1385,7 +1385,7 @@ static void fdctrl_handle_version(FDCtrl *fdctrl, int direction)static void fdctrl_handle_partid (FDCtrl *fdctrl,int direction) { - fdctrl->fifo[0 ] =0x41 ;/* Stepping 1 */ + fdctrl->fifo[0 ] =0x01 ;/* Stepping 0 */ fdctrl_set_fifo (fdctrl,1 ,0 ); }
Hm, this is the kind of change that I'm hesitant to make because I don't understand the implications. Can you give some more details what is fixed by this and why you think it's harmless for currently working OSes?
My spec says this:
6 3 14 PART ID COMMAND
This command can be used to identify the floppy disk controller as an enhanced controller The first stepping of the 82078 (all 44 pin versions) will yield 0x41 in the result phase of this command Any future enhancements on these parts will be denoted by the 5 LSBs (0x01 to 0x1F)
It doesn't even define what 0x01 would mean.
Kevin
Re: [Qemu-devel] [PATCH v2 04/10] fdc: emulate stepping 0
Hi,
Quoted 7 lines · Kevin Wolf
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 2 lines
Stepping 1 (S82078B) is not fully i82078 compatible, so better stick to initial revision
Quoted 1 line
Hm, this is the kind of change that I'm hesitant to make because I don't understand the implications. Can you give some more details what is fixed by this and why you think it's harmless for currently working OSes?
I was thinking at comments like
https://git.kernel.org/?p=linux/kernel/git/stable/linux-stable.git;a=blob;f=drivers/char/ftape/lowlevel/fdc-io.c;h=65c9d2ec60bdd7221a30a776fba377c37b89c078;hb=refs/heads/linux-2.6.18.y#l1119
Anyway, I didn't see any real problem yet with current code.
I'll resend this serie without this patch.
Regards
Hervé
PATCH v2 5/10 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 v2 6/10 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
PATCH v2 7/10 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
Re: [Qemu-devel] [PATCH v2 07/10] block: add a transfer rate for floppy types
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 9 lines
Floppies must be read at a specific transfer rate, depending of its own format. Update floppy description table to include required transfer rate.
Signed-off-by Hervé Poussineau <[email protected]>--- block.c | 74 ++++++++++++++++++++++++++++++++----------------------------- block.h | 10 +++++++- hw/fdc.c | 3 +- hw/pc.c | 3 +-
Meh. Having any floppy-specific logic in the block layer is wrong. We need to finally get this moved into fdc.c.
Kevin
Re: [Qemu-devel] [PATCH v2 07/10] block: add a transfer rate for floppy types
Quoted 11 lines · Kevin Wolf
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 9 lines
Floppies must be read at a specific transfer rate, depending of its own format. Update floppy description table to include required transfer rate.
Signed-off-by Hervé Poussineau <[email protected]>---
block.c | 74 ++++++++++++++++++++++++++++++++-----------------------------block.h| 10 +++++++-
hw/fdc.c | 3 +-
hw/pc.c | 3 +-
Quoted 2 lines
Meh. Having any floppy-specific logic in the block layer is wrong. We need to finally get this moved into
fdc.c.
Well, actually this code was moved recently from fdc.c to block.c (5bbdbb4676d17e782ae83055bac58e0751b25e4b). The other geometry guessing functions (ATA CHS) are also there. If we supported native floppy (or ATA) pass trough, the geometry would have to be read from the host device, so I think it's logical to keep that in block level instead of all devices. Maybe we could also split block.c into block-fdc.c, block-ata.c etc.
Re: [Qemu-devel] [PATCH v2 07/10] block: add a transfer rate for floppy types
Am 21.01.2012 20:02, schrieb Blue Swirl:
Quoted 15 lines · Kevin Wolf
Quoted 11 lines
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 9 lines
Floppies must be read at a specific transfer rate, depending of its own format. Update floppy description table to include required transfer rate.
Signed-off-by Hervé Poussineau <[email protected]>--- block.c | 74 ++++++++++++++++++++++++++++++++----------------------------- block.h | 10 +++++++- hw/fdc.c | 3 +- hw/pc.c | 3 +-Quoted 2 lines
Meh. Having any floppy-specific logic in the block layer is wrong. We need to finally get this moved into
fdc.c.
Quoted 7 lines
Well, actually this code was moved recently from
fdc.ctoblock.c(5bbdbb4676d17e782ae83055bac58e0751b25e4b). The other geometry guessing functions (ATA CHS) are also there. If we supported native floppy (or ATA) pass trough, the geometry would have to be read from the host device, so I think it's logical to keep that in block level instead of all devices. Maybe we could also splitblock.cintoblock-fdc.c,block-ata.cetc.
The geometry is guest state, so it shouldn't be in the block layer, which deals with host state. Maybe we could need some hw/block.c that deals with guest state concepts that are shared between multiple device. Images or passthrough backends could provide defaults.
I'll admit that not having an obvious place for media (we only have it for images and guest devices) doesn't make the design decisions easier.
Kevin
Re: [Qemu-devel] [PATCH v2 07/10] block: add a transfer rate for floppy types
Quoted 24 lines · Kevin Wolf
Am 21.01.2012 20:02, schrieb Blue Swirl:
Quoted 15 lines · Kevin Wolf
Quoted 11 lines
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 9 lines
Floppies must be read at a specific transfer rate, depending of its own format. Update floppy description table to include required transfer rate.
Signed-off-by Hervé Poussineau <[email protected]>---
block.c | 74 ++++++++++++++++++++++++++++++++-----------------------------block.h| 10 +++++++-
hw/fdc.c | 3 +-
hw/pc.c | 3 +-Quoted 2 lines
Meh. Having any floppy-specific logic in the block layer is wrong. We need to finally get this moved into
fdc.c.Quoted 7 lines
Well, actually this code was moved recently from
fdc.ctoblock.c(5bbdbb4676d17e782ae83055bac58e0751b25e4b). The other geometry guessing functions (ATA CHS) are also there. If we supported native floppy (or ATA) pass trough, the geometry would have to be read from the host device, so I think it's logical to keep that in block level instead of all devices. Maybe we could also splitblock.cintoblock-fdc.c,block-ata.cetc.
Quoted 4 lines
The geometry is guest state, so it shouldn't be in the block layer, which deals with host state. Maybe we could need some
hw/block.cthat deals with guest state concepts that are shared between multiple device. Images or passthrough backends could provide defaults.
Yes, that's even better.
Quoted 2 lines
I'll admit that not having an obvious place for media (we only have it for images and guest devices) doesn't make the design decisions easier.
Right. Maybe the layering is not correct.
Quoted 1 line
Kevin
Re: [Qemu-devel] [PATCH v2 07/10] block: add a transfer rate for floppy types
In reply to Kevin WolfQuoted 24 lines · Kevin Wolf
Am 21.01.2012 20:02, schrieb Blue Swirl:
Quoted 15 lines · Kevin Wolf
Quoted 11 lines
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 9 lines
Floppies must be read at a specific transfer rate, depending of its own format. Update floppy description table to include required transfer rate.
Signed-off-by Hervé Poussineau <[email protected]>--- block.c | 74 ++++++++++++++++++++++++++++++++----------------------------- block.h | 10 +++++++- hw/fdc.c | 3 +- hw/pc.c | 3 +-Quoted 2 lines
Meh. Having any floppy-specific logic in the block layer is wrong. We need to finally get this moved into
fdc.c.Quoted 7 lines
Well, actually this code was moved recently from
fdc.ctoblock.c(5bbdbb4676d17e782ae83055bac58e0751b25e4b). The other geometry guessing functions (ATA CHS) are also there. If we supported native floppy (or ATA) pass trough, the geometry would have to be read from the host device, so I think it's logical to keep that in block level instead of all devices. Maybe we could also splitblock.cintoblock-fdc.c,block-ata.cetc.
Quoted 2 lines
The geometry is guest state, so it shouldn't be in the block layer, which deals with host state.
Fully agree.
I tried to tackle geometry in my quest to untangle block device host and guest state, but didn't get far.
Quoted 6 lines
Maybe we could need some
hw/block.cthat
deals with guest state concepts that are shared between multiple device. Images or passthrough backends could provide defaults.I'll admit that not having an obvious place for media (we only have it for images and guest devices) doesn't make the design decisions easier.
Yup.
PATCH v2 8/10 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
PATCH v2 9/10 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 | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)
hw/fdc.c +6 -7
@@ -1599,13 +1599,12 @@ 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); -if (fdctrl->fifo[2 ] > cur_drv->max_track) { -fdctrl_raise_irq (fdctrl,FD_SR0_ABNTERM |FD_SR0_SEEK ); - }else { - cur_drv->track = fdctrl->fifo[2 ]; -/* Raise Interrupt */ -fdctrl_raise_irq (fdctrl,FD_SR0_SEEK ); - } +/* 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. +*/ + cur_drv->track = fdctrl->fifo[2 ]; +/* 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
Re: [Qemu-devel] [PATCH v2 09/10] fdc: fix seek command, which shouldn't check tracks
Am 15.01.2012 08:51, schrieb Hervé Poussineau:
Quoted 17 lines
The 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.
Signed-off-by Hervé Poussineau <[email protected]>--- hw/fdc.c | 13 ++++++------- 1 files changed, 6 insertions(+), 7 deletions(-)hw/fdc.c +0
-1@@ -1599,13 +1599,12 @@ 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); -if (fdctrl->fifo[2 ] > cur_drv->max_track) {
hw/fdc.c static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction) · line 1605
Quoted 10 lines
hw/fdc.c +4
-6-fdctrl_raise_irq (fdctrl,FD_SR0_ABNTERM |FD_SR0_SEEK ); - }else { - cur_drv->track = fdctrl->fifo[2 ]; -/* Raise Interrupt */ -fdctrl_raise_irq (fdctrl,FD_SR0_SEEK ); - } +/* 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. +*/ + cur_drv->track = fdctrl->fifo[2 ];
Do you really want to update this with an invalid value? This value is used in other places without being checked again against max_track.
hw/fdc.c static void fdctrl_handle_seek(FDCtrl *fdctrl, int direction) · line 1610
Quoted 5 lines
hw/fdc.c +2
-0+/* Raise Interrupt */ +fdctrl_raise_irq (fdctrl,FD_SR0_SEEK ); }static void fdctrl_handle_perpendicular_mode (FDCtrl *fdctrl,int direction)
Kevin
PATCH v2 10/10 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