READ ID on a drive with no medium terminates normally and returns the made-up sector ID left over from the "Pretend we are spinning" emulation. The only error path is a data rate mismatch, and media_rate is assigned solely by pick_geometry(); it is never reset when the medium is removed. A guest that has just ejected a diskette is therefore told that one is still present.
READ and WRITE have a related problem: fd_seek() returns 2 both for "track/head out of range" and for "no medium", and fdctrl_start_transfer() reports that as ST0 = ABNTERM with ST1 = 0x00. Without ST1.MA the guest cannot tell an absent diskette from a transient error. Fail READ ID with ST0 = ABNTERM and ST1 = MA when the drive is empty, and set ST1.MA in the fd_seek() case 2 path when no medium is present. This does not make guests detect the removal -- real hardware never completes READ ID on an empty drive, because there are no index pulses, and OS/2 for one relies on that timeout -- but it stops the controller from claiming a diskette that is not there. tests/qtest/fdc-test.c starts QEMU with "-device floppy,id=floppy0" and no medium, so the drive is empty for the whole run, and test_read_id asserts a normal termination with a fabricated cylinder 8 / head 1. That contradicts its neighbours test_no_media_on_start and test_media_change, which state that DSKCHG signals an absent medium. Insert a medium before READ ID and eject it afterwards -- the rewritten test passes before and after this change -- and add test_read_id_no_media to cover the empty drive. Guests checked, reading and writing, with and without a medium: Linux 2.0.34 and 7.0, PC-DOS 7, IBM DOS 5.02, Windows for Workgroups 3.11 and OS/2 2.11. None changes behaviour. No version of the Linux floppy driver from 1.2.13 to master issues READ ID at all -- FD_READID is defined in the uapi header for FDRAWCMD users and the driver never sends it -- so Linux detects an empty drive by stepping the head and reading DSKCHG instead. Buglink: https://gitlab.com/qemu-project/qemu/-/issues/3971 Signed-off-by: Christian Quante <[email protected]> --- hw/block/fdc.c | 29 +++++++++++++++-- tests/qtest/fdc-test.c | 73 ++++++++++++++++++++++++++++++++++++++---- 2 files changed, 93 insertions(+), 9 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 2c1681b7d0..8895e39cb6 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -196,6 +196,12 @@ static void fd_init(FDrive *drv) #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1) +/* Is a diskette present in the drive? */ +static bool fd_media_present(FDrive *drv) +{ + return drv->blk != NULL && blk_is_inserted(drv->blk); +} + static int fd_sector_calc(uint8_t head, uint8_t track, uint8_t sect, uint8_t last_sect, uint8_t num_sides) { @@ -1476,8 +1482,15 @@ static void fdctrl_start_transfer(FDCtrl *fdctrl, int direction) NUM_SIDES(cur_drv))); switch (fd_seek(cur_drv, kh, kt, ks, fdctrl->config & FD_CONFIG_EIS)) { case 2: - /* sect too big */ - fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, 0x00, 0x00); + /* + * Track/head out of range, or no medium at all. Only the latter can + * be told apart by the guest, through ST1.MA: with no diskette in the + * drive there is no address mark to be found. Guests that + * distinguish an absent medium from an unreadable one rely on this. + */ + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, + fd_media_present(cur_drv) ? 0x00 : FD_SR1_MA, + 0x00); fdctrl->fifo[3] = kt; fdctrl->fifo[4] = kh; fdctrl->fifo[5] = ks; @@ -2303,6 +2316,18 @@ static void fdctrl_result_timer(void *opaque) FDCtrl *fdctrl = opaque; FDrive *cur_drv = get_cur_drv(fdctrl); + /* + * An empty drive has no address marks to read. Completing READ ID + * successfully, with the made-up sector ID left over from the "spinning" + * emulation below, tells the guest that a diskette is still present after + * it has been ejected. The only error path left was a data rate mismatch, + * and media_rate is never reset when the medium is removed. + */ + if (!fd_media_present(cur_drv)) { + FLOPPY_DPRINTF("read id on empty drive\n"); + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_MA, 0x00); + return; + } /* Pretend we are spinning. * This is needed for Coherent, which uses READ ID to check for * sector interleaving. diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c index 1b37a8a4d2..fa57a8b7cf 100644 --- a/tests/qtest/fdc-test.c +++ b/tests/qtest/fdc-test.c @@ -64,6 +64,12 @@ enum { DSKCHG = 0x80, }; +enum { + ST0_IC_MASK = 0xc0, /* interrupt code */ + ST0_IC_ABNTERM = 0x40, /* abnormal termination */ + + ST1_MA = 0x01, /* missing address mark */ +}; static char *test_image; @@ -270,6 +276,21 @@ static void test_cmos(void) g_assert(cmos == 0x40 || cmos == 0x50); } +static void media_insert(void) +{ + qtest_qmp_assert_success(global_qtest, + "{'execute':'blockdev-change-medium', 'arguments':{" + " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", + test_image); +} + +static void media_eject(void) +{ + qtest_qmp_assert_success(global_qtest, + "{'execute':'eject', 'arguments':{" + " 'id':'floppy0' }}"); +} + static void test_no_media_on_start(void) { uint8_t dir; @@ -301,10 +322,7 @@ static void test_media_insert(void) /* Insert media in drive. DSKCHK should not be reset until a step pulse * is sent. */ - qtest_qmp_assert_success(global_qtest, - "{'execute':'blockdev-change-medium', 'arguments':{" - " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", - test_image); + media_insert(); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); @@ -333,9 +351,7 @@ static void test_media_change(void) /* Eject the floppy and check that DSKCHG is set. Reading it out doesn't * reset the bit. */ - qtest_qmp_assert_success(global_qtest, - "{'execute':'eject', 'arguments':{" - " 'id':'floppy0' }}"); + media_eject(); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); @@ -414,6 +430,12 @@ static void test_read_id(void) uint8_t st0; uint8_t msr; + /* + * READ ID reads the address mark of the sector currently under the head, + * so it needs a medium. The preceding tests left the drive empty. + */ + media_insert(); + /* Seek to track 0 and check with READ ID */ send_seek(0); @@ -491,6 +513,42 @@ static void test_read_id(void) g_assert_cmpint(cyl, ==, 8); g_assert_cmpint(head, ==, 1); g_assert_cmpint(st0, ==, head << 2); + + /* Leave the drive as the following tests expect to find it. */ + media_eject(); +} + +/* + * An empty drive spins no diskette, so READ ID finds no address mark and must + * terminate abnormally. Reporting success (with a made-up sector ID) would + * tell the guest that a medium is still present after it has been ejected. + */ +static void test_read_id_no_media(void) +{ + uint8_t drive = 0; + uint8_t head = 0; + uint8_t st0, st1; + + floppy_send(CMD_READ_ID); + g_assert(!get_irq(FLOPPY_IRQ)); + floppy_send(head << 2 | drive); + + while (!get_irq(FLOPPY_IRQ)) { + clock_step(1000000000LL / 50); + } + + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); /* ST2 */ + floppy_recv(); /* cylinder */ + floppy_recv(); /* head */ + floppy_recv(); /* sector */ + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); /* sector size */ + g_assert(!get_irq(FLOPPY_IRQ)); + + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_MA, ==, ST1_MA); } static void test_read_no_dma_1(void) @@ -625,6 +683,7 @@ int main(int argc, char **argv) qtest_add_func("/fdc/sense_interrupt", test_sense_interrupt); qtest_add_func("/fdc/relative_seek", test_relative_seek); qtest_add_func("/fdc/read_id", test_read_id); + qtest_add_func("/fdc/read_id_no_media", test_read_id_no_media); qtest_add_func("/fdc/verify", test_verify); qtest_add_func("/fdc/media_insert", test_media_insert); qtest_add_func("/fdc/read_no_dma_1", test_read_no_dma_1); -- 2.53.0
