diff mbox series

hw/block/fdc: do not set SEEK status bit in multi track commands

Message ID 20230812085957.408208-1-hpoussin@reactos.org
State New
Headers show
Series hw/block/fdc: do not set SEEK status bit in multi track commands | expand

Commit Message

Hervé Poussineau Aug. 12, 2023, 8:59 a.m. UTC 
I don't understand when SEEK must be set or not, but it seems to fix Minix...

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1522
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
---
 hw/block/fdc.c | 1 -
 1 file changed, 1 deletion(-)

Comments

Hervé Poussineau Jan. 1, 2024, 9:44 p.m. UTC | #1 
Ping.

Le 12/08/2023 à 10:59, Hervé Poussineau a écrit :
> I don't understand when SEEK must be set or not, but it seems to fix Minix...
> 
> Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1522
> Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> ---
>   hw/block/fdc.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> index d7cc4d3ec19..f627bbaf951 100644
> --- a/hw/block/fdc.c
> +++ b/hw/block/fdc.c
> @@ -1404,7 +1404,6 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
>               } else {
>                   new_head = 0;
>                   new_track++;
> -                fdctrl->status0 |= FD_SR0_SEEK;
>                   if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) {
>                       ret = 0;
>                   }
John Snow Jan. 8, 2024, 11:29 p.m. UTC | #2 
On Mon, Jan 1, 2024 at 4:45 PM Hervé Poussineau <hpoussin@reactos.org> wrote:
>
> Ping.
>
> Le 12/08/2023 à 10:59, Hervé Poussineau a écrit :
> > I don't understand when SEEK must be set or not, but it seems to fix Minix...
> >
> > Fixes: https://gitlab.com/qemu-project/qemu/-/issues/1522
> > Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
> > ---
> >   hw/block/fdc.c | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/hw/block/fdc.c b/hw/block/fdc.c
> > index d7cc4d3ec19..f627bbaf951 100644
> > --- a/hw/block/fdc.c
> > +++ b/hw/block/fdc.c
> > @@ -1404,7 +1404,6 @@ static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
> >               } else {
> >                   new_head = 0;
> >                   new_track++;
> > -                fdctrl->status0 |= FD_SR0_SEEK;
> >                   if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) {
> >                       ret = 0;
> >                   }
>

I'll be honest, I don't have the time to audit this and I don't have
the test suite necessary to prove that it's safe enough. Do you have
any suggestions for how we can prove or test this beyond 'works for
me'?

I could read the spec sheet for this controller until I'm blue in the
face, but it doesn't seem to necessarily correlate to how the
controller behaves IRL or with what real operating systems actually do
with that controller. I also don't have access to a physical
controller anymore to even begin to try and write my own hardware
probe for it.

We need a robust test suite for FDC behavior, but it seems unlikely
that anyone will want to actually write one (I sure don't). Are there
any good shortcuts to victory here?
diff mbox series

Patch

diff --git a/hw/block/fdc.c b/hw/block/fdc.c
index d7cc4d3ec19..f627bbaf951 100644
--- a/hw/block/fdc.c
+++ b/hw/block/fdc.c
@@ -1404,7 +1404,6 @@  static int fdctrl_seek_to_next_sect(FDCtrl *fdctrl, FDrive *cur_drv)
             } else {
                 new_head = 0;
                 new_track++;
-                fdctrl->status0 |= FD_SR0_SEEK;
                 if ((cur_drv->flags & FDISK_DBL_SIDES) == 0) {
                     ret = 0;
                 }