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

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

Posted by Hervé Poussineau 9 months, 1 week ago

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;
                 }
-- 
2.40.1

Re: [PATCH] hw/block/fdc: do not set SEEK status bit in multi track commands

Posted by Hervé Poussineau 4 months, 2 weeks ago

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;
>                   }

Re: [PATCH] hw/block/fdc: do not set SEEK status bit in multi track commands

Posted by John Snow 4 months, 1 week ago

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?