* [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support @ 2015-12-07 23:34 John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry John Snow ` (9 more replies) 0 siblings, 10 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel Yes, it's been broken for ten years. No, it's not a CVE. The problem is that QEMU doesn't have a configuration option for the type of floppy drive you want. It determines that based on the type of diskette inserted at boot time. If you don't insert one, it always chooses a 1.44MB type. If you want to insert a 2.88MB floppy after boot, you simply cannot. "Wow, who cares?" Good question -- Unfortunately, the virtio-win floppy disk images that Red Hat/fedora ship require a 2.88MB drive, so if you forgot to insert them at boot, you'd have to change your VM configuration and try again. For a one-shot operation, that's kind of obnoxious -- it'd be nice to allow one to just insert the diskette on-demand. "OK, What are you changing in this decades-old device?" (1) Add a new property to allow users to specify what kind of drive they want without relying on magical guessing behavior. Choices are: 120, 144, 288, auto, and none. 120, 144 and 288 refer to 1.20MB, 1.44MB, and 2.88MB drives. auto refers to the auto-detect behavior QEMU currently has. none ... hides the drive. You probably don't want to use this. (2) Add the concept of physical diskette size to QEMU, classifying 120-style diskettes as fundamentally different from 144 and 288 ones. (3) Revamp the automatic guessing heuristic to understand that 2.88MB style drives can accept 1.44MB diskettes. (4) Change the automatic fallback type for the automatic guessing heuristic from 1.44MB to 2.88MB as it is a more diverse drive. (5) A lot of code cleanup in general. "Won't this break everything, you madman?" No: I tested this in MS-DOS 6.22, Fedora 23 and Windows 8.1. All seemed perfectly happy with 2.88MB drives as the default for 1.44 or 2.88MB floppy diskette images. If any guests are discovered to be unable to cope with this default, they are free to choose a 1.44MB drive type at boot, or insert an appropriate diskette. By and large, this appears to improve the diskette compatibility for most guests. ________________________________________________________________________________ For convenience, this branch is available at: https://github.com/jnsnow/qemu.git branch fdc-default https://github.com/jnsnow/qemu/tree/fdc-default This version is tagged fdc-default-v2: https://github.com/jnsnow/qemu/releases/tag/fdc-default-v2 John Snow (10): fdc: move pick_geometry fdc: refactor pick_geometry fdc: add disk field fdc: add default drive type option fdc: do not call revalidate on eject fdc: implement new drive type property fdc: add physical disk sizes fdc: rework pick_geometry qtest/fdc: Support for 2.88MB drives fdc: change auto fallback drive to 288 hw/block/fdc.c | 317 +++++++++++++++++++++++++++++-------------- hw/core/qdev-properties.c | 11 ++ hw/i386/pc.c | 17 +-- include/hw/block/fdc.h | 9 +- include/hw/qdev-properties.h | 1 + qapi/block.json | 16 +++ tests/fdc-test.c | 2 +- 7 files changed, 255 insertions(+), 118 deletions(-) -- 2.4.3 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-15 21:51 ` Hervé Poussineau 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 02/10] fdc: refactor pick_geometry John Snow ` (8 subsequent siblings) 9 siblings, 1 reply; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel Code motion: I want to refactor this function to work with FDrive directly, so shuffle it below that definition. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 90 +++++++++++++++++++++++++++++----------------------------- 1 file changed, 45 insertions(+), 45 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 4292ece..246b631 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -114,51 +114,6 @@ static const FDFormat fd_formats[] = { { FDRIVE_DRV_NONE, -1, -1, 0, 0, }, }; -static void pick_geometry(BlockBackend *blk, int *nb_heads, - int *max_track, int *last_sect, - FDriveType drive_in, FDriveType *drive, - FDriveRate *rate) -{ - const FDFormat *parse; - uint64_t nb_sectors, size; - int i, first_match, match; - - blk_get_geometry(blk, &nb_sectors); - match = -1; - first_match = -1; - for (i = 0; ; i++) { - parse = &fd_formats[i]; - if (parse->drive == FDRIVE_DRV_NONE) { - break; - } - if (drive_in == parse->drive || - drive_in == FDRIVE_DRV_NONE) { - size = (parse->max_head + 1) * parse->max_track * - parse->last_sect; - if (nb_sectors == size) { - match = i; - break; - } - if (first_match == -1) { - first_match = i; - } - } - } - if (match == -1) { - if (first_match == -1) { - match = 1; - } else { - match = first_match; - } - parse = &fd_formats[match]; - } - *nb_heads = parse->max_head + 1; - *max_track = parse->max_track; - *last_sect = parse->last_sect; - *drive = parse->drive; - *rate = parse->rate; -} - #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv) #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive)) @@ -286,6 +241,51 @@ static void fd_recalibrate(FDrive *drv) fd_seek(drv, 0, 0, 1, 1); } +static void pick_geometry(BlockBackend *blk, int *nb_heads, + int *max_track, int *last_sect, + FDriveType drive_in, FDriveType *drive, + FDriveRate *rate) +{ + const FDFormat *parse; + uint64_t nb_sectors, size; + int i, first_match, match; + + blk_get_geometry(blk, &nb_sectors); + match = -1; + first_match = -1; + for (i = 0; ; i++) { + parse = &fd_formats[i]; + if (parse->drive == FDRIVE_DRV_NONE) { + break; + } + if (drive_in == parse->drive || + drive_in == FDRIVE_DRV_NONE) { + size = (parse->max_head + 1) * parse->max_track * + parse->last_sect; + if (nb_sectors == size) { + match = i; + break; + } + if (first_match == -1) { + first_match = i; + } + } + } + if (match == -1) { + if (first_match == -1) { + match = 1; + } else { + match = first_match; + } + parse = &fd_formats[match]; + } + *nb_heads = parse->max_head + 1; + *max_track = parse->max_track; + *last_sect = parse->last_sect; + *drive = parse->drive; + *rate = parse->rate; +} + /* Revalidate a disk drive after a disk change */ static void fd_revalidate(FDrive *drv) { -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry John Snow @ 2015-12-15 21:51 ` Hervé Poussineau 2015-12-15 21:56 ` John Snow 2015-12-16 21:14 ` John Snow 0 siblings, 2 replies; 17+ messages in thread From: Hervé Poussineau @ 2015-12-15 21:51 UTC (permalink / raw) To: John Snow, qemu-block; +Cc: kwolf, armbru, qemu-devel Le 08/12/2015 00:34, John Snow a écrit : > Code motion: I want to refactor this function to work with FDrive > directly, so shuffle it below that definition. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > hw/block/fdc.c | 90 +++++++++++++++++++++++++++++----------------------------- > 1 file changed, 45 insertions(+), 45 deletions(-) > > diff --git a/hw/block/fdc.c b/hw/block/fdc.c > index 4292ece..246b631 100644 > --- a/hw/block/fdc.c > +++ b/hw/block/fdc.c > @@ -114,51 +114,6 @@ static const FDFormat fd_formats[] = { > { FDRIVE_DRV_NONE, -1, -1, 0, 0, }, > }; > > -static void pick_geometry(BlockBackend *blk, int *nb_heads, > - int *max_track, int *last_sect, > - FDriveType drive_in, FDriveType *drive, > - FDriveRate *rate) > -{ > - const FDFormat *parse; > - uint64_t nb_sectors, size; > - int i, first_match, match; > - > - blk_get_geometry(blk, &nb_sectors); > - match = -1; > - first_match = -1; > - for (i = 0; ; i++) { > - parse = &fd_formats[i]; > - if (parse->drive == FDRIVE_DRV_NONE) { > - break; > - } > - if (drive_in == parse->drive || > - drive_in == FDRIVE_DRV_NONE) { > - size = (parse->max_head + 1) * parse->max_track * > - parse->last_sect; > - if (nb_sectors == size) { > - match = i; > - break; > - } > - if (first_match == -1) { > - first_match = i; > - } > - } > - } > - if (match == -1) { > - if (first_match == -1) { > - match = 1; > - } else { > - match = first_match; > - } > - parse = &fd_formats[match]; > - } > - *nb_heads = parse->max_head + 1; > - *max_track = parse->max_track; > - *last_sect = parse->last_sect; > - *drive = parse->drive; > - *rate = parse->rate; > -} > - > #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv) > #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive)) > > @@ -286,6 +241,51 @@ static void fd_recalibrate(FDrive *drv) > fd_seek(drv, 0, 0, 1, 1); > } > > +static void pick_geometry(BlockBackend *blk, int *nb_heads, > + int *max_track, int *last_sect, > + FDriveType drive_in, FDriveType *drive, > + FDriveRate *rate) > +{ > + const FDFormat *parse; > + uint64_t nb_sectors, size; > + int i, first_match, match; > + > + blk_get_geometry(blk, &nb_sectors); > + match = -1; > + first_match = -1; > + for (i = 0; ; i++) { > + parse = &fd_formats[i]; > + if (parse->drive == FDRIVE_DRV_NONE) { > + break; > + } > + if (drive_in == parse->drive || > + drive_in == FDRIVE_DRV_NONE) { > + size = (parse->max_head + 1) * parse->max_track * > + parse->last_sect; > + if (nb_sectors == size) { > + match = i; > + break; > + } > + if (first_match == -1) { > + first_match = i; > + } > + } > + } > + if (match == -1) { > + if (first_match == -1) { > + match = 1; > + } else { > + match = first_match; > + } > + parse = &fd_formats[match]; > + } > + *nb_heads = parse->max_head + 1; > + *max_track = parse->max_track; > + *last_sect = parse->last_sect; > + *drive = parse->drive; > + *rate = parse->rate; > +} > + > /* Revalidate a disk drive after a disk change */ > static void fd_revalidate(FDrive *drv) > { I think it would be better to move the FDiskFlags enum, FDrive, FDCtrl, FDCtrlISABus structures to include/hw/block/fdc.h That way, we can embed the floppy controller into another chip. I'll need it soon for some PIIX4 improvements. Regards, Hervé ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry 2015-12-15 21:51 ` Hervé Poussineau @ 2015-12-15 21:56 ` John Snow 2015-12-16 21:14 ` John Snow 1 sibling, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-15 21:56 UTC (permalink / raw) To: Hervé Poussineau, qemu-block; +Cc: kwolf, armbru, qemu-devel On 12/15/2015 04:51 PM, Hervé Poussineau wrote: > Le 08/12/2015 00:34, John Snow a écrit : >> Code motion: I want to refactor this function to work with FDrive >> directly, so shuffle it below that definition. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> hw/block/fdc.c | 90 >> +++++++++++++++++++++++++++++----------------------------- >> 1 file changed, 45 insertions(+), 45 deletions(-) >> >> diff --git a/hw/block/fdc.c b/hw/block/fdc.c >> index 4292ece..246b631 100644 >> --- a/hw/block/fdc.c >> +++ b/hw/block/fdc.c >> @@ -114,51 +114,6 @@ static const FDFormat fd_formats[] = { >> { FDRIVE_DRV_NONE, -1, -1, 0, 0, }, >> }; >> >> -static void pick_geometry(BlockBackend *blk, int *nb_heads, >> - int *max_track, int *last_sect, >> - FDriveType drive_in, FDriveType *drive, >> - FDriveRate *rate) >> -{ >> - const FDFormat *parse; >> - uint64_t nb_sectors, size; >> - int i, first_match, match; >> - >> - blk_get_geometry(blk, &nb_sectors); >> - match = -1; >> - first_match = -1; >> - for (i = 0; ; i++) { >> - parse = &fd_formats[i]; >> - if (parse->drive == FDRIVE_DRV_NONE) { >> - break; >> - } >> - if (drive_in == parse->drive || >> - drive_in == FDRIVE_DRV_NONE) { >> - size = (parse->max_head + 1) * parse->max_track * >> - parse->last_sect; >> - if (nb_sectors == size) { >> - match = i; >> - break; >> - } >> - if (first_match == -1) { >> - first_match = i; >> - } >> - } >> - } >> - if (match == -1) { >> - if (first_match == -1) { >> - match = 1; >> - } else { >> - match = first_match; >> - } >> - parse = &fd_formats[match]; >> - } >> - *nb_heads = parse->max_head + 1; >> - *max_track = parse->max_track; >> - *last_sect = parse->last_sect; >> - *drive = parse->drive; >> - *rate = parse->rate; >> -} >> - >> #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv) >> #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive)) >> >> @@ -286,6 +241,51 @@ static void fd_recalibrate(FDrive *drv) >> fd_seek(drv, 0, 0, 1, 1); >> } >> >> +static void pick_geometry(BlockBackend *blk, int *nb_heads, >> + int *max_track, int *last_sect, >> + FDriveType drive_in, FDriveType *drive, >> + FDriveRate *rate) >> +{ >> + const FDFormat *parse; >> + uint64_t nb_sectors, size; >> + int i, first_match, match; >> + >> + blk_get_geometry(blk, &nb_sectors); >> + match = -1; >> + first_match = -1; >> + for (i = 0; ; i++) { >> + parse = &fd_formats[i]; >> + if (parse->drive == FDRIVE_DRV_NONE) { >> + break; >> + } >> + if (drive_in == parse->drive || >> + drive_in == FDRIVE_DRV_NONE) { >> + size = (parse->max_head + 1) * parse->max_track * >> + parse->last_sect; >> + if (nb_sectors == size) { >> + match = i; >> + break; >> + } >> + if (first_match == -1) { >> + first_match = i; >> + } >> + } >> + } >> + if (match == -1) { >> + if (first_match == -1) { >> + match = 1; >> + } else { >> + match = first_match; >> + } >> + parse = &fd_formats[match]; >> + } >> + *nb_heads = parse->max_head + 1; >> + *max_track = parse->max_track; >> + *last_sect = parse->last_sect; >> + *drive = parse->drive; >> + *rate = parse->rate; >> +} >> + >> /* Revalidate a disk drive after a disk change */ >> static void fd_revalidate(FDrive *drv) >> { > > I think it would be better to move the FDiskFlags enum, FDrive, FDCtrl, > FDCtrlISABus structures to include/hw/block/fdc.h > That way, we can embed the floppy controller into another chip. I'll > need it soon for some PIIX4 improvements. > > Regards, > > Hervé > Deal. (Why are you embedding a floppy controller? :) --js ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry 2015-12-15 21:51 ` Hervé Poussineau 2015-12-15 21:56 ` John Snow @ 2015-12-16 21:14 ` John Snow 1 sibling, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-16 21:14 UTC (permalink / raw) To: Hervé Poussineau, qemu-block; +Cc: kwolf, armbru, qemu-devel On 12/15/2015 04:51 PM, Hervé Poussineau wrote: > Le 08/12/2015 00:34, John Snow a écrit : >> Code motion: I want to refactor this function to work with FDrive >> directly, so shuffle it below that definition. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- >> hw/block/fdc.c | 90 >> +++++++++++++++++++++++++++++----------------------------- >> 1 file changed, 45 insertions(+), 45 deletions(-) >> >> diff --git a/hw/block/fdc.c b/hw/block/fdc.c >> index 4292ece..246b631 100644 >> --- a/hw/block/fdc.c >> +++ b/hw/block/fdc.c >> @@ -114,51 +114,6 @@ static const FDFormat fd_formats[] = { >> { FDRIVE_DRV_NONE, -1, -1, 0, 0, }, >> }; >> >> -static void pick_geometry(BlockBackend *blk, int *nb_heads, >> - int *max_track, int *last_sect, >> - FDriveType drive_in, FDriveType *drive, >> - FDriveRate *rate) >> -{ >> - const FDFormat *parse; >> - uint64_t nb_sectors, size; >> - int i, first_match, match; >> - >> - blk_get_geometry(blk, &nb_sectors); >> - match = -1; >> - first_match = -1; >> - for (i = 0; ; i++) { >> - parse = &fd_formats[i]; >> - if (parse->drive == FDRIVE_DRV_NONE) { >> - break; >> - } >> - if (drive_in == parse->drive || >> - drive_in == FDRIVE_DRV_NONE) { >> - size = (parse->max_head + 1) * parse->max_track * >> - parse->last_sect; >> - if (nb_sectors == size) { >> - match = i; >> - break; >> - } >> - if (first_match == -1) { >> - first_match = i; >> - } >> - } >> - } >> - if (match == -1) { >> - if (first_match == -1) { >> - match = 1; >> - } else { >> - match = first_match; >> - } >> - parse = &fd_formats[match]; >> - } >> - *nb_heads = parse->max_head + 1; >> - *max_track = parse->max_track; >> - *last_sect = parse->last_sect; >> - *drive = parse->drive; >> - *rate = parse->rate; >> -} >> - >> #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv) >> #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive)) >> >> @@ -286,6 +241,51 @@ static void fd_recalibrate(FDrive *drv) >> fd_seek(drv, 0, 0, 1, 1); >> } >> >> +static void pick_geometry(BlockBackend *blk, int *nb_heads, >> + int *max_track, int *last_sect, >> + FDriveType drive_in, FDriveType *drive, >> + FDriveRate *rate) >> +{ >> + const FDFormat *parse; >> + uint64_t nb_sectors, size; >> + int i, first_match, match; >> + >> + blk_get_geometry(blk, &nb_sectors); >> + match = -1; >> + first_match = -1; >> + for (i = 0; ; i++) { >> + parse = &fd_formats[i]; >> + if (parse->drive == FDRIVE_DRV_NONE) { >> + break; >> + } >> + if (drive_in == parse->drive || >> + drive_in == FDRIVE_DRV_NONE) { >> + size = (parse->max_head + 1) * parse->max_track * >> + parse->last_sect; >> + if (nb_sectors == size) { >> + match = i; >> + break; >> + } >> + if (first_match == -1) { >> + first_match = i; >> + } >> + } >> + } >> + if (match == -1) { >> + if (first_match == -1) { >> + match = 1; >> + } else { >> + match = first_match; >> + } >> + parse = &fd_formats[match]; >> + } >> + *nb_heads = parse->max_head + 1; >> + *max_track = parse->max_track; >> + *last_sect = parse->last_sect; >> + *drive = parse->drive; >> + *rate = parse->rate; >> +} >> + >> /* Revalidate a disk drive after a disk change */ >> static void fd_revalidate(FDrive *drv) >> { > > I think it would be better to move the FDiskFlags enum, FDrive, FDCtrl, > FDCtrlISABus structures to include/hw/block/fdc.h > That way, we can embed the floppy controller into another chip. I'll > need it soon for some PIIX4 improvements. > > Regards, > > Hervé > Actually, I'm going to decline on this -- I don't know precisely what you need moved, and it's trivial for you to cut the headers out when you need them. It'll be more work re-shuffling this series than it's worth for me to then not get it completely right anyway. Just ping me with a patch to move what you need and I will get that done for you then, thanks. --js ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 02/10] fdc: refactor pick_geometry 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 03/10] fdc: add disk field John Snow ` (7 subsequent siblings) 9 siblings, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel Modify this function to operate directly on FDrive objects instead of unpacking and passing all of those parameters manually. Helps reduce complexity in each caller, and reduces the number of args. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 54 +++++++++++++++++++++++------------------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 246b631..09bb63d 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -241,11 +241,9 @@ static void fd_recalibrate(FDrive *drv) fd_seek(drv, 0, 0, 1, 1); } -static void pick_geometry(BlockBackend *blk, int *nb_heads, - int *max_track, int *last_sect, - FDriveType drive_in, FDriveType *drive, - FDriveRate *rate) +static void pick_geometry(FDrive *drv) { + BlockBackend *blk = drv->blk; const FDFormat *parse; uint64_t nb_sectors, size; int i, first_match, match; @@ -258,8 +256,8 @@ static void pick_geometry(BlockBackend *blk, int *nb_heads, if (parse->drive == FDRIVE_DRV_NONE) { break; } - if (drive_in == parse->drive || - drive_in == FDRIVE_DRV_NONE) { + if (drv->drive == parse->drive || + drv->drive == FDRIVE_DRV_NONE) { size = (parse->max_head + 1) * parse->max_track * parse->last_sect; if (nb_sectors == size) { @@ -279,41 +277,35 @@ static void pick_geometry(BlockBackend *blk, int *nb_heads, } parse = &fd_formats[match]; } - *nb_heads = parse->max_head + 1; - *max_track = parse->max_track; - *last_sect = parse->last_sect; - *drive = parse->drive; - *rate = parse->rate; + + if (parse->max_head == 0) { + drv->flags &= ~FDISK_DBL_SIDES; + } else { + drv->flags |= FDISK_DBL_SIDES; + } + drv->max_track = parse->max_track; + drv->last_sect = parse->last_sect; + drv->drive = parse->drive; + drv->media_rate = parse->rate; + + if (drv->media_inserted) { + FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", + parse->max_head + 1, + drv->max_track, drv->last_sect, + drv->ro ? "ro" : "rw"); + } } /* Revalidate a disk drive after a disk change */ static void fd_revalidate(FDrive *drv) { - int nb_heads, max_track, last_sect, ro; - FDriveType drive; - FDriveRate rate; - FLOPPY_DPRINTF("revalidate\n"); if (drv->blk != NULL) { - ro = blk_is_read_only(drv->blk); - pick_geometry(drv->blk, &nb_heads, &max_track, - &last_sect, drv->drive, &drive, &rate); + drv->ro = blk_is_read_only(drv->blk); + pick_geometry(drv); if (!drv->media_inserted) { FLOPPY_DPRINTF("No disk in drive\n"); - } else { - FLOPPY_DPRINTF("Floppy disk (%d h %d t %d s) %s\n", nb_heads, - max_track, last_sect, ro ? "ro" : "rw"); } - if (nb_heads == 1) { - drv->flags &= ~FDISK_DBL_SIDES; - } else { - drv->flags |= FDISK_DBL_SIDES; - } - drv->max_track = max_track; - drv->last_sect = last_sect; - drv->ro = ro; - drv->drive = drive; - drv->media_rate = rate; } else { FLOPPY_DPRINTF("No drive connected\n"); drv->last_sect = 0; -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 03/10] fdc: add disk field 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 02/10] fdc: refactor pick_geometry John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option John Snow ` (6 subsequent siblings) 9 siblings, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel This allows us to distinguish between the current disk type and the current drive type. The drive is what's reported to CMOS, the disk is whatever the pick_geometry function suspects has been inserted. The drive field maintains the exact same meaning as it did previously, however pick_geometry/fd_revalidate will be refactored to *never* update the drive field, considering it frozen in-place during an earlier initialization call. Before this patch, pick_geometry/fd_revalidate could only change the drive field when it was FDRIVE_DRV_NONE, which indicated that we had not yet reported our drive type to CMOS. After we "pick one," even though pick_geometry/fd_revalidate re-set drv->drive, it should always be the same as the value going into these calls, so it is effectively already static. As of this patch, disk and drive will always be the same, but that may not be true by the end of this series. Disk does not need to be migrated because it is not user-visible state nor is it currently used for any calculations. It is purely informative, and will be rebuilt automatically via fd_revalidate on the new host. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 09bb63d..13fef23 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -133,7 +133,8 @@ typedef struct FDrive { FDCtrl *fdctrl; BlockBackend *blk; /* Drive status */ - FDriveType drive; + FDriveType drive; /* CMOS drive type */ + FDriveType disk; /* Current disk type */ uint8_t perpendicular; /* 2.88 MB access mode */ /* Position */ uint8_t head; @@ -157,6 +158,7 @@ static void fd_init(FDrive *drv) drv->drive = FDRIVE_DRV_NONE; drv->perpendicular = 0; /* Disk */ + drv->disk = FDRIVE_DRV_NONE; drv->last_sect = 0; drv->max_track = 0; } @@ -286,6 +288,7 @@ static void pick_geometry(FDrive *drv) drv->max_track = parse->max_track; drv->last_sect = parse->last_sect; drv->drive = parse->drive; + drv->disk = drv->media_inserted ? parse->drive : FDRIVE_DRV_NONE; drv->media_rate = parse->rate; if (drv->media_inserted) { -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow ` (2 preceding siblings ...) 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 03/10] fdc: add disk field John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-07 23:56 ` Eric Blake 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 05/10] fdc: do not call revalidate on eject John Snow ` (5 subsequent siblings) 9 siblings, 1 reply; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel This patch adds a new explicit Floppy Drive Type option. The existing behavior in QEMU is to automatically guess a drive type based on the media inserted, or if a diskette is not present, arbitrarily assign one. This behavior can be described as "auto." This patch adds explicit behaviors: 120, 144, 288, auto, and none. The new "auto" behavior is intended to mimick current behavior, while the other types pick one explicitly. In a future patch, the goal is to change the FDC's default drive type from auto (falling back to 1.44MB) to auto (falling back to 2.88MB). In order to allow users to obtain the old behaviors, though, a mechanism for specifying the exact type of drive we want is needed. This patch adds the properties, but it is not acted on yet in favor of making those changes a little more explicitly clear in standalone patches later in this patch set. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 108 ++++++++++++++++++++++++++----------------- hw/core/qdev-properties.c | 11 +++++ hw/i386/pc.c | 17 +++---- include/hw/block/fdc.h | 9 +--- include/hw/qdev-properties.h | 1 + qapi/block.json | 16 +++++++ 6 files changed, 103 insertions(+), 59 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 13fef23..498eb9c 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -60,58 +60,66 @@ typedef enum FDriveRate { } FDriveRate; typedef struct FDFormat { - FDriveType drive; + FloppyDriveType drive; uint8_t last_sect; uint8_t max_track; uint8_t max_head; FDriveRate rate; } FDFormat; +/** + * FDRIVE_DEFAULT: The default drive type if none specified. + * FDRIVE_AUTO_FALLBACK: The default drive type to assume if + * no media is inserted. + */ +#define FDRIVE_DEFAULT FLOPPY_DRIVE_TYPE_AUTO +#define FDRIVE_AUTO_FALLBACK FLOPPY_DRIVE_TYPE_144 + 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_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, }, + { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_144, 22, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_144, 23, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_144, 24, 80, 1, FDRIVE_RATE_500K, }, /* 2.88 MB 3"1/2 floppy disks */ - { 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, }, + { FLOPPY_DRIVE_TYPE_288, 36, 80, 1, FDRIVE_RATE_1M, }, + { FLOPPY_DRIVE_TYPE_288, 39, 80, 1, FDRIVE_RATE_1M, }, + { FLOPPY_DRIVE_TYPE_288, 40, 80, 1, FDRIVE_RATE_1M, }, + { FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, }, + { FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, }, /* 720 kB 3"1/2 floppy disks */ - { 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, }, + { FLOPPY_DRIVE_TYPE_144, 9, 80, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_144, 10, 80, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_144, 10, 82, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_144, 10, 83, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_144, 13, 80, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_144, 14, 80, 1, FDRIVE_RATE_250K, }, /* 1.2 MB 5"1/4 floppy disks */ - { 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, }, + { FLOPPY_DRIVE_TYPE_120, 15, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_120, 18, 82, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_120, 18, 83, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, }, /* 720 kB 5"1/4 floppy disks */ - { FDRIVE_DRV_120, 9, 80, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_120, 11, 80, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_120, 9, 80, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_120, 11, 80, 1, FDRIVE_RATE_250K, }, /* 360 kB 5"1/4 floppy disks */ - { 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, }, + { FLOPPY_DRIVE_TYPE_120, 9, 40, 1, FDRIVE_RATE_300K, }, + { FLOPPY_DRIVE_TYPE_120, 9, 40, 0, FDRIVE_RATE_300K, }, + { FLOPPY_DRIVE_TYPE_120, 10, 41, 1, FDRIVE_RATE_300K, }, + { FLOPPY_DRIVE_TYPE_120, 10, 42, 1, FDRIVE_RATE_300K, }, /* 320 kB 5"1/4 floppy disks */ - { FDRIVE_DRV_120, 8, 40, 1, FDRIVE_RATE_250K, }, - { FDRIVE_DRV_120, 8, 40, 0, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_120, 8, 40, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_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_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_144, 9, 80, 0, FDRIVE_RATE_250K, }, /* end */ - { FDRIVE_DRV_NONE, -1, -1, 0, 0, }, + { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, }, }; #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv) @@ -133,8 +141,9 @@ typedef struct FDrive { FDCtrl *fdctrl; BlockBackend *blk; /* Drive status */ - FDriveType drive; /* CMOS drive type */ - FDriveType disk; /* Current disk type */ + FloppyDriveType drive; /* CMOS drive type */ + FloppyDriveType disk; /* Current disk type */ + uint8_t perpendicular; /* 2.88 MB access mode */ /* Position */ uint8_t head; @@ -155,10 +164,10 @@ typedef struct FDrive { static void fd_init(FDrive *drv) { /* Drive */ - drv->drive = FDRIVE_DRV_NONE; + drv->drive = FLOPPY_DRIVE_TYPE_NONE; drv->perpendicular = 0; /* Disk */ - drv->disk = FDRIVE_DRV_NONE; + drv->disk = FLOPPY_DRIVE_TYPE_NONE; drv->last_sect = 0; drv->max_track = 0; } @@ -255,11 +264,11 @@ static void pick_geometry(FDrive *drv) first_match = -1; for (i = 0; ; i++) { parse = &fd_formats[i]; - if (parse->drive == FDRIVE_DRV_NONE) { + if (parse->drive == FLOPPY_DRIVE_TYPE_NONE) { break; } if (drv->drive == parse->drive || - drv->drive == FDRIVE_DRV_NONE) { + drv->drive == FLOPPY_DRIVE_TYPE_NONE) { size = (parse->max_head + 1) * parse->max_track * parse->last_sect; if (nb_sectors == size) { @@ -288,7 +297,7 @@ static void pick_geometry(FDrive *drv) drv->max_track = parse->max_track; drv->last_sect = parse->last_sect; drv->drive = parse->drive; - drv->disk = drv->media_inserted ? parse->drive : FDRIVE_DRV_NONE; + drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE; drv->media_rate = parse->rate; if (drv->media_inserted) { @@ -566,6 +575,9 @@ struct FDCtrl { /* Timers state */ uint8_t timer0; uint8_t timer1; + + FloppyDriveType typeA; + FloppyDriveType typeB; }; #define TYPE_SYSBUS_FDC "base-sysbus-fdc" @@ -2396,7 +2408,7 @@ static void sysbus_fdc_common_realize(DeviceState *dev, Error **errp) fdctrl_realize_common(fdctrl, errp); } -FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i) +FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i) { FDCtrlISABus *isa = ISA_FDC(fdc); @@ -2421,6 +2433,10 @@ static Property isa_fdc_properties[] = { DEFINE_PROP_DRIVE("driveB", FDCtrlISABus, state.drives[1].blk), DEFINE_PROP_BIT("check_media_rate", FDCtrlISABus, state.check_media_rate, 0, true), + DEFINE_PROP_DEFAULT("fdtypeA", FDCtrlISABus, state.typeA, FDRIVE_DEFAULT, + qdev_prop_fdc_drive_type, FloppyDriveType), + DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlISABus, state.typeB, FDRIVE_DEFAULT, + qdev_prop_fdc_drive_type, FloppyDriveType), DEFINE_PROP_END_OF_LIST(), }; @@ -2469,6 +2485,10 @@ static const VMStateDescription vmstate_sysbus_fdc ={ static Property sysbus_fdc_properties[] = { DEFINE_PROP_DRIVE("driveA", FDCtrlSysBus, state.drives[0].blk), DEFINE_PROP_DRIVE("driveB", FDCtrlSysBus, state.drives[1].blk), + DEFINE_PROP_DEFAULT("fdtypeA", FDCtrlSysBus, state.typeA, FDRIVE_DEFAULT, + qdev_prop_fdc_drive_type, FloppyDriveType), + DEFINE_PROP_DEFAULT("fdtypeB", FDCtrlSysBus, state.typeB, FDRIVE_DEFAULT, + qdev_prop_fdc_drive_type, FloppyDriveType), DEFINE_PROP_END_OF_LIST(), }; @@ -2489,6 +2509,8 @@ static const TypeInfo sysbus_fdc_info = { static Property sun4m_fdc_properties[] = { DEFINE_PROP_DRIVE("drive", FDCtrlSysBus, state.drives[0].blk), + DEFINE_PROP_DEFAULT("fdtype", FDCtrlSysBus, state.typeA, FDRIVE_DEFAULT, + qdev_prop_fdc_drive_type, FloppyDriveType), DEFINE_PROP_END_OF_LIST(), }; diff --git a/hw/core/qdev-properties.c b/hw/core/qdev-properties.c index 33e245e..b43943e 100644 --- a/hw/core/qdev-properties.c +++ b/hw/core/qdev-properties.c @@ -541,6 +541,17 @@ PropertyInfo qdev_prop_bios_chs_trans = { .set = set_enum, }; +/* --- FDC default drive types */ + +PropertyInfo qdev_prop_fdc_drive_type = { + .name = "FdcDriveType", + .description = "FDC drive type, " + "144/288/120/none/auto", + .enum_table = FloppyDriveType_lookup, + .get = get_enum, + .set = set_enum +}; + /* --- pci address --- */ /* diff --git a/hw/i386/pc.c b/hw/i386/pc.c index 5e20e07..af48af8 100644 --- a/hw/i386/pc.c +++ b/hw/i386/pc.c @@ -207,24 +207,24 @@ static void pic_irq_request(void *opaque, int irq, int level) #define REG_EQUIPMENT_BYTE 0x14 -static int cmos_get_fd_drive_type(FDriveType fd0) +static int cmos_get_fd_drive_type(FloppyDriveType fd0) { int val; switch (fd0) { - case FDRIVE_DRV_144: + case FLOPPY_DRIVE_TYPE_144: /* 1.44 Mb 3"5 drive */ val = 4; break; - case FDRIVE_DRV_288: + case FLOPPY_DRIVE_TYPE_288: /* 2.88 Mb 3"5 drive */ val = 5; break; - case FDRIVE_DRV_120: + case FLOPPY_DRIVE_TYPE_120: /* 1.2 Mb 5"5 drive */ val = 2; break; - case FDRIVE_DRV_NONE: + case FLOPPY_DRIVE_TYPE_NONE: default: val = 0; break; @@ -295,7 +295,8 @@ static void pc_boot_set(void *opaque, const char *boot_device, Error **errp) static void pc_cmos_init_floppy(ISADevice *rtc_state, ISADevice *floppy) { int val, nb, i; - FDriveType fd_type[2] = { FDRIVE_DRV_NONE, FDRIVE_DRV_NONE }; + FloppyDriveType fd_type[2] = { FLOPPY_DRIVE_TYPE_NONE, + FLOPPY_DRIVE_TYPE_NONE }; /* floppy type */ if (floppy) { @@ -309,10 +310,10 @@ static void pc_cmos_init_floppy(ISADevice *rtc_state, ISADevice *floppy) val = rtc_get_memory(rtc_state, REG_EQUIPMENT_BYTE); nb = 0; - if (fd_type[0] < FDRIVE_DRV_NONE) { + if (fd_type[0] != FLOPPY_DRIVE_TYPE_NONE) { nb++; } - if (fd_type[1] < FDRIVE_DRV_NONE) { + if (fd_type[1] != FLOPPY_DRIVE_TYPE_NONE) { nb++; } switch (nb) { diff --git a/include/hw/block/fdc.h b/include/hw/block/fdc.h index d48b2f8..adce14f 100644 --- a/include/hw/block/fdc.h +++ b/include/hw/block/fdc.h @@ -6,13 +6,6 @@ /* fdc.c */ #define MAX_FD 2 -typedef enum FDriveType { - FDRIVE_DRV_144 = 0x00, /* 1.44 MB 3"5 drive */ - FDRIVE_DRV_288 = 0x01, /* 2.88 MB 3"5 drive */ - FDRIVE_DRV_120 = 0x02, /* 1.2 MB 5"25 drive */ - FDRIVE_DRV_NONE = 0x03, /* No drive connected */ -} FDriveType; - #define TYPE_ISA_FDC "isa-fdc" ISADevice *fdctrl_init_isa(ISABus *bus, DriveInfo **fds); @@ -21,6 +14,6 @@ void fdctrl_init_sysbus(qemu_irq irq, int dma_chann, void sun4m_fdctrl_init(qemu_irq irq, hwaddr io_base, DriveInfo **fds, qemu_irq *fdc_tc); -FDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i); +FloppyDriveType isa_fdc_get_drive_type(ISADevice *fdc, int i); #endif diff --git a/include/hw/qdev-properties.h b/include/hw/qdev-properties.h index 77538a8..c26797e 100644 --- a/include/hw/qdev-properties.h +++ b/include/hw/qdev-properties.h @@ -20,6 +20,7 @@ extern PropertyInfo qdev_prop_ptr; extern PropertyInfo qdev_prop_macaddr; extern PropertyInfo qdev_prop_losttickpolicy; extern PropertyInfo qdev_prop_bios_chs_trans; +extern PropertyInfo qdev_prop_fdc_drive_type; extern PropertyInfo qdev_prop_drive; extern PropertyInfo qdev_prop_netdev; extern PropertyInfo qdev_prop_vlan; diff --git a/qapi/block.json b/qapi/block.json index 84022f1..36d4bac 100644 --- a/qapi/block.json +++ b/qapi/block.json @@ -40,6 +40,22 @@ 'data': ['auto', 'none', 'lba', 'large', 'rechs']} ## +# @FloppyDriveType +# +# Type of Floppy drive to be emulated by the Floppy Disk Controller. +# +# @144: 1.44MB 3.5" drive +# @288: 2.88MB 3.5" drive +# @120: 1.5MB 5.25" drive +# @none: No drive connected +# @auto: Automatically determined by inserted media at boot +# +# Since: 2.6 +## +{ 'enum': 'FloppyDriveType', + 'data': ['144', '288', '120', 'none', 'auto']} + +## # @BlockdevSnapshotInternal # # @device: the name of the device to generate the snapshot from -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option John Snow @ 2015-12-07 23:56 ` Eric Blake 2015-12-14 20:05 ` John Snow 0 siblings, 1 reply; 17+ messages in thread From: Eric Blake @ 2015-12-07 23:56 UTC (permalink / raw) To: John Snow, qemu-block; +Cc: kwolf, armbru, qemu-devel [-- Attachment #1: Type: text/plain, Size: 2019 bytes --] On 12/07/2015 04:34 PM, John Snow wrote: > This patch adds a new explicit Floppy Drive Type option. The existing > behavior in QEMU is to automatically guess a drive type based on the > media inserted, or if a diskette is not present, arbitrarily assign one. > > This behavior can be described as "auto." This patch adds explicit > behaviors: 120, 144, 288, auto, and none. The new "auto" behavior > is intended to mimick current behavior, while the other types pick s/mimick/mimic/ (one of those weird 'ic' verbs where the 'k' is necessary in past tense but not present tense) > one explicitly. > > In a future patch, the goal is to change the FDC's default drive type > from auto (falling back to 1.44MB) to auto (falling back to 2.88MB). > > In order to allow users to obtain the old behaviors, though, a mechanism > for specifying the exact type of drive we want is needed. > > This patch adds the properties, but it is not acted on yet in favor of > making those changes a little more explicitly clear in standalone patches > later in this patch set. > > Signed-off-by: John Snow <jsnow@redhat.com> > --- > +++ b/qapi/block.json > @@ -40,6 +40,22 @@ > 'data': ['auto', 'none', 'lba', 'large', 'rechs']} > > ## > +# @FloppyDriveType > +# > +# Type of Floppy drive to be emulated by the Floppy Disk Controller. > +# > +# @144: 1.44MB 3.5" drive > +# @288: 2.88MB 3.5" drive > +# @120: 1.5MB 5.25" drive Names start with a digit - not the prettiest, but also not the first instance, so qapi handles it just fine. And I don't have any suggestions for a better yet still concise name. > +# @none: No drive connected > +# @auto: Automatically determined by inserted media at boot > +# > +# Since: 2.6 > +## > +{ 'enum': 'FloppyDriveType', > + 'data': ['144', '288', '120', 'none', 'auto']} Reviewed-by: Eric Blake <eblake@redhat.com> -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option 2015-12-07 23:56 ` Eric Blake @ 2015-12-14 20:05 ` John Snow 2015-12-15 16:29 ` Eric Blake 0 siblings, 1 reply; 17+ messages in thread From: John Snow @ 2015-12-14 20:05 UTC (permalink / raw) To: Eric Blake, qemu-block; +Cc: kwolf, armbru, qemu-devel On 12/07/2015 06:56 PM, Eric Blake wrote: > On 12/07/2015 04:34 PM, John Snow wrote: >> This patch adds a new explicit Floppy Drive Type option. The existing >> behavior in QEMU is to automatically guess a drive type based on the >> media inserted, or if a diskette is not present, arbitrarily assign one. >> >> This behavior can be described as "auto." This patch adds explicit >> behaviors: 120, 144, 288, auto, and none. The new "auto" behavior >> is intended to mimick current behavior, while the other types pick > > s/mimick/mimic/ (one of those weird 'ic' verbs where the 'k' is > necessary in past tense but not present tense) > >> one explicitly. >> >> In a future patch, the goal is to change the FDC's default drive type >> from auto (falling back to 1.44MB) to auto (falling back to 2.88MB). >> >> In order to allow users to obtain the old behaviors, though, a mechanism >> for specifying the exact type of drive we want is needed. >> >> This patch adds the properties, but it is not acted on yet in favor of >> making those changes a little more explicitly clear in standalone patches >> later in this patch set. >> >> Signed-off-by: John Snow <jsnow@redhat.com> >> --- > >> +++ b/qapi/block.json >> @@ -40,6 +40,22 @@ >> 'data': ['auto', 'none', 'lba', 'large', 'rechs']} >> >> ## >> +# @FloppyDriveType >> +# >> +# Type of Floppy drive to be emulated by the Floppy Disk Controller. >> +# >> +# @144: 1.44MB 3.5" drive >> +# @288: 2.88MB 3.5" drive >> +# @120: 1.5MB 5.25" drive > > Names start with a digit - not the prettiest, but also not the first > instance, so qapi handles it just fine. And I don't have any > suggestions for a better yet still concise name. > >> +# @none: No drive connected >> +# @auto: Automatically determined by inserted media at boot >> +# >> +# Since: 2.6 >> +## >> +{ 'enum': 'FloppyDriveType', >> + 'data': ['144', '288', '120', 'none', 'auto']} > > Reviewed-by: Eric Blake <eblake@redhat.com> > I was actually contemplating re-spinning this for a v3: Instead of having a "typeA" and "typeB" properties of the FDC, I'll just spin the properties in such a way that they write directly to FDCtrl.drives[n].drive, which avoids the need for two new members and a method designed to "fetch" the default from the controller. I also want to add a "fallback" member to the FDC as a CLI configurable parameter such that when using "auto" you can configure directly what type of drive you'll get. This way the default behavior will be "auto" (but configurable) and the fallback if the drive is empty or it cannot make a confident guess will be whatever the user chose as "fallback" -- presumed "144" before this series and "288" afterwards. I believe this also opens up the possibility for having "fallback" default to "144" for machine types created prior to 2.6 ... I think. (I'm less familiar with machine compat code as of yet.) Sound reasonable? --js ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option 2015-12-14 20:05 ` John Snow @ 2015-12-15 16:29 ` Eric Blake 0 siblings, 0 replies; 17+ messages in thread From: Eric Blake @ 2015-12-15 16:29 UTC (permalink / raw) To: John Snow, qemu-block; +Cc: kwolf, armbru, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1408 bytes --] On 12/14/2015 01:05 PM, John Snow wrote: > I was actually contemplating re-spinning this for a v3: > > Instead of having a "typeA" and "typeB" properties of the FDC, I'll just > spin the properties in such a way that they write directly to > FDCtrl.drives[n].drive, which avoids the need for two new members and a > method designed to "fetch" the default from the controller. > > I also want to add a "fallback" member to the FDC as a CLI configurable > parameter such that when using "auto" you can configure directly what > type of drive you'll get. > > This way the default behavior will be "auto" (but configurable) and the > fallback if the drive is empty or it cannot make a confident guess will > be whatever the user chose as "fallback" -- presumed "144" before this > series and "288" afterwards. > > I believe this also opens up the possibility for having "fallback" > default to "144" for machine types created prior to 2.6 ... I think. > (I'm less familiar with machine compat code as of yet.) > > Sound reasonable? On paper, yes that sounds reasonable. I'm also not familiar enough with machine types to know if it will let you keep 2.5 and earlier machines with a fallback of 144, and newer machines with a fallback of 288, but sounds promising. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 604 bytes --] ^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 05/10] fdc: do not call revalidate on eject 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow ` (3 preceding siblings ...) 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 06/10] fdc: implement new drive type property John Snow ` (4 subsequent siblings) 9 siblings, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel Currently, fd_revalidate is called in two different places, with two different expectations of behavior: (1) On initialization, as a routine to help pick the drive type and initial geometries as a side-effect of the pick_geometry routine (2) On insert/eject, which either sets the geometries to a default value or chooses new geometries based on the inserted diskette. Break this nonsense apart by creating a new function dedicated towards picking the drive type on initialization. This has a few results: (1) fd_revalidate does not get called on boot anymore for drives with no diskette. (2) pick_geometry will actually get called twice if we have a diskette inserted, but this is harmless. (Once for the drive type, and once as part of the media callback.) Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 498eb9c..39e680b 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -170,6 +170,7 @@ static void fd_init(FDrive *drv) drv->disk = FLOPPY_DRIVE_TYPE_NONE; drv->last_sect = 0; drv->max_track = 0; + drv->ro = true; } #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1) @@ -252,13 +253,21 @@ static void fd_recalibrate(FDrive *drv) fd_seek(drv, 0, 0, 1, 1); } -static void pick_geometry(FDrive *drv) +/** + * Determine geometry based on inserted diskette. + */ +static bool pick_geometry(FDrive *drv) { BlockBackend *blk = drv->blk; const FDFormat *parse; uint64_t nb_sectors, size; int i, first_match, match; + /* We can only pick a geometry if we have a diskette. */ + if (!drv->media_inserted) { + return false; + } + blk_get_geometry(blk, &nb_sectors); match = -1; first_match = -1; @@ -296,8 +305,7 @@ static void pick_geometry(FDrive *drv) } drv->max_track = parse->max_track; drv->last_sect = parse->last_sect; - drv->drive = parse->drive; - drv->disk = drv->media_inserted ? parse->drive : FLOPPY_DRIVE_TYPE_NONE; + drv->disk = parse->drive; drv->media_rate = parse->rate; if (drv->media_inserted) { @@ -306,6 +314,14 @@ static void pick_geometry(FDrive *drv) drv->max_track, drv->last_sect, drv->ro ? "ro" : "rw"); } + return true; +} + +static void pick_drive_type(FDrive *drv) +{ + if (pick_geometry(drv)) { + drv->drive = drv->disk; + } } /* Revalidate a disk drive after a disk change */ @@ -314,15 +330,18 @@ static void fd_revalidate(FDrive *drv) FLOPPY_DPRINTF("revalidate\n"); if (drv->blk != NULL) { drv->ro = blk_is_read_only(drv->blk); - pick_geometry(drv); if (!drv->media_inserted) { FLOPPY_DPRINTF("No disk in drive\n"); + drv->disk = FLOPPY_DRIVE_TYPE_NONE; + } else { + pick_geometry(drv); } } else { FLOPPY_DPRINTF("No drive connected\n"); drv->last_sect = 0; drv->max_track = 0; drv->flags &= ~FDISK_DBL_SIDES; + drv->disk = FLOPPY_DRIVE_TYPE_NONE; } } @@ -2194,9 +2213,11 @@ static void fdctrl_change_cb(void *opaque, bool load) FDrive *drive = opaque; drive->media_inserted = load && drive->blk && blk_is_inserted(drive->blk); - drive->media_changed = 1; - fd_revalidate(drive); + + if (load) { + fd_revalidate(drive); + } } static bool fdctrl_is_tray_open(void *opaque) @@ -2232,11 +2253,12 @@ static void fdctrl_connect_drives(FDCtrl *fdctrl, Error **errp) } fd_init(drive); - fdctrl_change_cb(drive, 0); if (drive->blk) { blk_set_dev_ops(drive->blk, &fdctrl_block_ops, drive); drive->media_inserted = blk_is_inserted(drive->blk); + pick_drive_type(drive); } + fdctrl_change_cb(drive, drive->media_inserted); } } -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 06/10] fdc: implement new drive type property 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow ` (4 preceding siblings ...) 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 05/10] fdc: do not call revalidate on eject John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 07/10] fdc: add physical disk sizes John Snow ` (3 subsequent siblings) 9 siblings, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel Respect the drive type as given via the CLI. Set the type given by the CLI during fd_init. If the type remains the default (auto), we'll attempt to scan an inserted diskette if present to determine a type. If auto is selected but no diskette is present, we fall back to a predetermined default (currently 1.44MB to match legacy QEMU behavior.) The pick_geometry algorithm is modified to only allow matches outside of the existing drive type for the new auto behavior. If a user specifies the "none" type, QEMU will not report this drive to the CMOS. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 45 ++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 39e680b..9bb3021 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -161,10 +161,12 @@ typedef struct FDrive { bool media_inserted; /* Is there a medium in the tray */ } FDrive; +static FloppyDriveType get_default_drive_type(FDrive *drv); + static void fd_init(FDrive *drv) { /* Drive */ - drv->drive = FLOPPY_DRIVE_TYPE_NONE; + drv->drive = get_default_drive_type(drv); drv->perpendicular = 0; /* Disk */ drv->disk = FLOPPY_DRIVE_TYPE_NONE; @@ -277,7 +279,7 @@ static bool pick_geometry(FDrive *drv) break; } if (drv->drive == parse->drive || - drv->drive == FLOPPY_DRIVE_TYPE_NONE) { + drv->drive == FLOPPY_DRIVE_TYPE_AUTO) { size = (parse->max_head + 1) * parse->max_track * parse->last_sect; if (nb_sectors == size) { @@ -291,13 +293,17 @@ static bool pick_geometry(FDrive *drv) } if (match == -1) { if (first_match == -1) { - match = 1; + /* No entry found: drive_type was NONE or we neglected to add any + * candidate geometries for our drive type into the fd_formats table + */ + match = ARRAY_SIZE(fd_formats) - 1; } else { match = first_match; } parse = &fd_formats[match]; } + out: if (parse->max_head == 0) { drv->flags &= ~FDISK_DBL_SIDES; } else { @@ -319,8 +325,16 @@ static bool pick_geometry(FDrive *drv) static void pick_drive_type(FDrive *drv) { - if (pick_geometry(drv)) { - drv->drive = drv->disk; + if (drv->drive != FLOPPY_DRIVE_TYPE_AUTO) { + return; + } + + if (!drv->media_inserted) { + drv->drive = FDRIVE_AUTO_FALLBACK; + } else { + if (pick_geometry(drv)) { + drv->drive = drv->disk; + } } } @@ -623,6 +637,27 @@ typedef struct FDCtrlISABus { int32_t bootindexB; } FDCtrlISABus; +static FloppyDriveType get_default_drive_type(FDrive *drv) +{ + FDCtrl *fdctrl = drv->fdctrl; + + g_assert_cmpint(MAX_FD, ==, 2); + + if (!drv->blk) { + return FLOPPY_DRIVE_TYPE_NONE; + } + + if (drv == &fdctrl->drives[0]) { + return fdctrl->typeA; + } + + if (drv == &fdctrl->drives[1]) { + return fdctrl->typeB; + } + + return FDRIVE_DEFAULT; +} + static uint32_t fdctrl_read (void *opaque, uint32_t reg) { FDCtrl *fdctrl = opaque; -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 07/10] fdc: add physical disk sizes 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow ` (5 preceding siblings ...) 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 06/10] fdc: implement new drive type property John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 08/10] fdc: rework pick_geometry John Snow ` (2 subsequent siblings) 9 siblings, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel 2.88MB capable drives can accept 1.44MB floppies, for instance. To rework the pick_geometry function, we need to know if our current drive can even accept the type of disks we're considering. NB: This allows us to distinguish between all of the "total sectors" collisions between 1.20MB and 1.44MB diskette types, by using the physical drive size as a differentiator. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 40 ++++++++++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 8 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 9bb3021..12a2595 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -59,6 +59,12 @@ typedef enum FDriveRate { FDRIVE_RATE_1M = 0x03, /* 1 Mbps */ } FDriveRate; +typedef enum FDriveSize { + FDRIVE_SIZE_UNKNOWN, + FDRIVE_SIZE_350, + FDRIVE_SIZE_525, +} FDriveSize; + typedef struct FDFormat { FloppyDriveType drive; uint8_t last_sect; @@ -75,11 +81,15 @@ typedef struct FDFormat { #define FDRIVE_DEFAULT FLOPPY_DRIVE_TYPE_AUTO #define FDRIVE_AUTO_FALLBACK FLOPPY_DRIVE_TYPE_144 +/* In many cases, the total sector size of a format is enough to uniquely + * identify it. However, there are some total sector collisions between + * formats of different physical size, and these are noted below by + * highlighting the total sector size for entries with collisions. */ static const FDFormat fd_formats[] = { /* First entry is default format */ /* 1.44 MB 3"1/2 floppy disks */ - { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, }, - { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_144, 18, 80, 1, FDRIVE_RATE_500K, }, /* 3.5" 2880 */ + { FLOPPY_DRIVE_TYPE_144, 20, 80, 1, FDRIVE_RATE_500K, }, /* 3.5" 3200 */ { FLOPPY_DRIVE_TYPE_144, 21, 80, 1, FDRIVE_RATE_500K, }, { FLOPPY_DRIVE_TYPE_144, 21, 82, 1, FDRIVE_RATE_500K, }, { FLOPPY_DRIVE_TYPE_144, 21, 83, 1, FDRIVE_RATE_500K, }, @@ -93,7 +103,7 @@ static const FDFormat fd_formats[] = { { FLOPPY_DRIVE_TYPE_288, 44, 80, 1, FDRIVE_RATE_1M, }, { FLOPPY_DRIVE_TYPE_288, 48, 80, 1, FDRIVE_RATE_1M, }, /* 720 kB 3"1/2 floppy disks */ - { FLOPPY_DRIVE_TYPE_144, 9, 80, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_144, 9, 80, 1, FDRIVE_RATE_250K, }, /* 3.5" 1400 */ { FLOPPY_DRIVE_TYPE_144, 10, 80, 1, FDRIVE_RATE_250K, }, { FLOPPY_DRIVE_TYPE_144, 10, 82, 1, FDRIVE_RATE_250K, }, { FLOPPY_DRIVE_TYPE_144, 10, 83, 1, FDRIVE_RATE_250K, }, @@ -101,15 +111,15 @@ static const FDFormat fd_formats[] = { { FLOPPY_DRIVE_TYPE_144, 14, 80, 1, FDRIVE_RATE_250K, }, /* 1.2 MB 5"1/4 floppy disks */ { FLOPPY_DRIVE_TYPE_120, 15, 80, 1, FDRIVE_RATE_500K, }, - { FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_120, 18, 80, 1, FDRIVE_RATE_500K, }, /* 5.25" 2880 */ { FLOPPY_DRIVE_TYPE_120, 18, 82, 1, FDRIVE_RATE_500K, }, { FLOPPY_DRIVE_TYPE_120, 18, 83, 1, FDRIVE_RATE_500K, }, - { FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, }, + { FLOPPY_DRIVE_TYPE_120, 20, 80, 1, FDRIVE_RATE_500K, }, /* 5.25" 3200 */ /* 720 kB 5"1/4 floppy disks */ - { FLOPPY_DRIVE_TYPE_120, 9, 80, 1, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_120, 9, 80, 1, FDRIVE_RATE_250K, }, /* 5.25" 1440 */ { FLOPPY_DRIVE_TYPE_120, 11, 80, 1, FDRIVE_RATE_250K, }, /* 360 kB 5"1/4 floppy disks */ - { FLOPPY_DRIVE_TYPE_120, 9, 40, 1, FDRIVE_RATE_300K, }, + { FLOPPY_DRIVE_TYPE_120, 9, 40, 1, FDRIVE_RATE_300K, }, /* 5.25" 720 */ { FLOPPY_DRIVE_TYPE_120, 9, 40, 0, FDRIVE_RATE_300K, }, { FLOPPY_DRIVE_TYPE_120, 10, 41, 1, FDRIVE_RATE_300K, }, { FLOPPY_DRIVE_TYPE_120, 10, 42, 1, FDRIVE_RATE_300K, }, @@ -117,11 +127,25 @@ static const FDFormat fd_formats[] = { { FLOPPY_DRIVE_TYPE_120, 8, 40, 1, FDRIVE_RATE_250K, }, { FLOPPY_DRIVE_TYPE_120, 8, 40, 0, FDRIVE_RATE_250K, }, /* 360 kB must match 5"1/4 better than 3"1/2... */ - { FLOPPY_DRIVE_TYPE_144, 9, 80, 0, FDRIVE_RATE_250K, }, + { FLOPPY_DRIVE_TYPE_144, 9, 80, 0, FDRIVE_RATE_250K, }, /* 3.5" 720 */ /* end */ { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, }, }; +__attribute__((__unused__)) +static FDriveSize drive_size(FloppyDriveType drive) +{ + switch (drive) { + case FLOPPY_DRIVE_TYPE_120: + return FDRIVE_SIZE_525; + case FLOPPY_DRIVE_TYPE_144: + case FLOPPY_DRIVE_TYPE_288: + return FDRIVE_SIZE_350; + default: + return FDRIVE_SIZE_UNKNOWN; + } +} + #define GET_CUR_DRV(fdctrl) ((fdctrl)->cur_drv) #define SET_CUR_DRV(fdctrl, drive) ((fdctrl)->cur_drv = (drive)) -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 08/10] fdc: rework pick_geometry 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow ` (6 preceding siblings ...) 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 07/10] fdc: add physical disk sizes John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 09/10] qtest/fdc: Support for 2.88MB drives John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 10/10] fdc: change auto fallback drive to 288 John Snow 9 siblings, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel This one is the crazy one. fd_revalidate currently uses pick_geometry to tell if the diskette geometry has changed upon an eject/insert event, but it won't allow us to insert a 1.44MB diskette into a 2.88MB drive. This is inflexible. The new algorithm applies a new heuristic to guessing disk geometries that allows us to switch diskette types as long as the physical size matches before falling back to the old heuristic. The old one is roughly: - If the size and type matches, choose it. - Fall back to the first geometry that matched our type. The new one is: - If the size and type matches, choose it. - If the size (sectors) and physical size match, choose it. - If the size (sectors) matches at all, choose it begrudgingly. - Fall back to the first geometry that matched our type. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 63 +++++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 23 deletions(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 12a2595..246bd83 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -132,7 +132,6 @@ static const FDFormat fd_formats[] = { { FLOPPY_DRIVE_TYPE_NONE, -1, -1, 0, 0, }, }; -__attribute__((__unused__)) static FDriveSize drive_size(FloppyDriveType drive) { switch (drive) { @@ -287,45 +286,63 @@ static bool pick_geometry(FDrive *drv) BlockBackend *blk = drv->blk; const FDFormat *parse; uint64_t nb_sectors, size; - int i, first_match, match; + int i; + int match, size_match, type_match; + bool magic = drv->drive == FLOPPY_DRIVE_TYPE_AUTO; /* We can only pick a geometry if we have a diskette. */ if (!drv->media_inserted) { return false; } + /* We need to determine the likely geometry of the inserted medium. + * In order of preference, we look for: + * (1) The same drive type and number of sectors, + * (2) The same diskette size and number of sectors, + * (3) The same number of sectors, + * (4) The same drive type. + * + * In all cases, matches that occur higher in the drive table will take + * precedence over matches that occur later in the table. + */ blk_get_geometry(blk, &nb_sectors); - match = -1; - first_match = -1; + match = size_match = type_match = -1; for (i = 0; ; i++) { parse = &fd_formats[i]; if (parse->drive == FLOPPY_DRIVE_TYPE_NONE) { break; } - if (drv->drive == parse->drive || - drv->drive == FLOPPY_DRIVE_TYPE_AUTO) { - size = (parse->max_head + 1) * parse->max_track * - parse->last_sect; - if (nb_sectors == size) { - match = i; - break; - } - if (first_match == -1) { - first_match = i; + size = (parse->max_head + 1) * parse->max_track * parse->last_sect; + if (nb_sectors == size) { + if (magic || parse->drive == drv->drive) { + /* (1) perfect match */ + goto out; + } else if (drive_size(parse->drive) == drive_size(drv->drive)) { + /* (2) physical size match */ + match = (match == -1) ? i : match; + } else { + /* (3) nsectors match only */ + size_match = (size_match == -1) ? i : size_match; } + } else if ((type_match == -1) && + ((parse->drive == drv->drive) || + (magic && (parse->drive == FDRIVE_AUTO_FALLBACK)))) { + /* (4) type matches, or type matches the autodetect default if we + * are using the autodetect mechanism. */ + type_match = i; } } + if (match == -1) { - if (first_match == -1) { - /* No entry found: drive_type was NONE or we neglected to add any - * candidate geometries for our drive type into the fd_formats table - */ - match = ARRAY_SIZE(fd_formats) - 1; - } else { - match = first_match; - } - parse = &fd_formats[match]; + match = (size_match != -1) ? size_match : type_match; + } + + if (match == -1) { + /* No entry found: drive_type was NONE or we neglected to add any + * candidate geometries for our drive type into the fd_formats table. */ + match = ARRAY_SIZE(fd_formats) - 1; } + parse = &(fd_formats[match]); out: if (parse->max_head == 0) { -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 09/10] qtest/fdc: Support for 2.88MB drives 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow ` (7 preceding siblings ...) 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 08/10] fdc: rework pick_geometry John Snow @ 2015-12-07 23:34 ` John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 10/10] fdc: change auto fallback drive to 288 John Snow 9 siblings, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel The old test assumes a 1.44MB drive. Assert that the QEMU default drive is now either 1.44 or 2.88. Signed-off-by: John Snow <jsnow@redhat.com> --- tests/fdc-test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/fdc-test.c b/tests/fdc-test.c index b5a4696..526d459 100644 --- a/tests/fdc-test.c +++ b/tests/fdc-test.c @@ -267,7 +267,7 @@ static void test_cmos(void) uint8_t cmos; cmos = cmos_read(CMOS_FLOPPY); - g_assert(cmos == 0x40); + g_assert(cmos == 0x40 || cmos == 0x50); } static void test_no_media_on_start(void) -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH for-2.6 v2 10/10] fdc: change auto fallback drive to 288 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow ` (8 preceding siblings ...) 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 09/10] qtest/fdc: Support for 2.88MB drives John Snow @ 2015-12-07 23:34 ` John Snow 9 siblings, 0 replies; 17+ messages in thread From: John Snow @ 2015-12-07 23:34 UTC (permalink / raw) To: qemu-block; +Cc: kwolf, John Snow, armbru, qemu-devel The 2.88 drive is more suitable as a default because it can still read 1.44 images correctly, but the reverse is not true. Since there exist virtio-win drivers that are shipped on 2.88 floppy images, this patch will allow VMs booted without a floppy disk inserted to later insert a 2.88MB floppy and have that work. This patch has been tested with msdos, freedos, fedora, windows 8 and windows 10 without issue: if problems do arise for certain guests being unable to cope with 2.88MB drives as the default, they are in the minority and can use type=144 as needed (or insert a proper boot medium and omit type=144/288 or use type=auto) to obtain different drive types. Signed-off-by: John Snow <jsnow@redhat.com> --- hw/block/fdc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 246bd83..a82ddd0 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -79,7 +79,7 @@ typedef struct FDFormat { * no media is inserted. */ #define FDRIVE_DEFAULT FLOPPY_DRIVE_TYPE_AUTO -#define FDRIVE_AUTO_FALLBACK FLOPPY_DRIVE_TYPE_144 +#define FDRIVE_AUTO_FALLBACK FLOPPY_DRIVE_TYPE_288 /* In many cases, the total sector size of a format is enough to uniquely * identify it. However, there are some total sector collisions between -- 2.4.3 ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2015-12-16 21:14 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-07 23:34 [Qemu-devel] [PATCH for-2.6 v2 00/10] fdc: fix 2.88mb floppy diskette support John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 01/10] fdc: move pick_geometry John Snow 2015-12-15 21:51 ` Hervé Poussineau 2015-12-15 21:56 ` John Snow 2015-12-16 21:14 ` John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 02/10] fdc: refactor pick_geometry John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 03/10] fdc: add disk field John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 04/10] fdc: add default drive type option John Snow 2015-12-07 23:56 ` Eric Blake 2015-12-14 20:05 ` John Snow 2015-12-15 16:29 ` Eric Blake 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 05/10] fdc: do not call revalidate on eject John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 06/10] fdc: implement new drive type property John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 07/10] fdc: add physical disk sizes John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 08/10] fdc: rework pick_geometry John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 09/10] qtest/fdc: Support for 2.88MB drives John Snow 2015-12-07 23:34 ` [Qemu-devel] [PATCH for-2.6 v2 10/10] fdc: change auto fallback drive to 288 John Snow
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.