Re: [PATCH 1/1] hw/ide/core.c: fix handling of unsupported commands

[John Snow <jsnow@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 3131 bytes --]

On Thu, Jun 1, 2023, 10:03 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:

> On Wed, May 24, 2023 at 11:20:41AM -0400, John Snow wrote:
> > On Wed, May 24, 2023, 4:17 AM Niklas Cassel <nks@flawful.org> wrote:
> >
> > > On Tue, May 23, 2023 at 10:35:56AM -0400, John Snow wrote:
> > > > On Mon, May 22, 2023 at 5:16???PM Mateusz Albecki <
> > > mateusz.p.albecki@gmail.com>
> > > > wrote:
> > > > >
> > > > > Certainly seems like my patch is wrong as it will make the abort
> path
> > > > execute ide_cmd_done twice. During debug I came to the conclusion
> that
> > > > ide_cmd_done is not called at all as I was getting timeouts on the
> driver
> > > > side while waiting for D2H FIS. I am still not sure how I was getting
> > > this
> > > > behavior if the problem was actually with setting correct error bits.
> > > Even
> > > > so I think it can be safely assumed that Niklas' change will solve
> the
> > > > issue, I will try to verify it in a couple of days and if I see any
> > > problem
> > > > I will come back to you.
> > > > >
> > > > > Mateusz
> > > >
> > > > Great, thanks :)
> > > >
> > > > I'm waiting to hear back from Niklas, but I'm hoping to take their
> > > patches
> > > > this cycle as I think they look quite good.
> > >
> > > Hello John,
> > >
> > > Unfortunately, I've noticed an increase in boot time during
> > > the initial SeaBIOS part of QEMU with my changes.
> > >
> > > Will need to debug to see which change is causing this.
> > >
> > > I'm at a conference this week, so it might take until next
> > > week until I have time to figure out why this is happening.
> > >
> > > So unfortunately, I think we need to hold off with my series
> > > for now.
> > >
> >
> > Shame. Feel free to resend it once you've isolated the problem and I'll
> try
> > to fast-track it, since it'd be nice to have my more embarrassing
> mistakes
> > for NCQ fixed :)
> >
> > (If I become hard to reach, please ping me with a direct, non-patch email
> > so it bubbles up to the top of my inbox.)
> >
> > ((...by the way, what are you working on? What motivates the interest in
> > AHCI/SATA now? Can you say?))
>
> Hello John,
>
> It is simple, Western Digital is still selling a lot of SATA drives :)
>

Haha! Fair enough. I built my first computer without a single AHCI device
in it last year, but you can't say the same for my Synology downstairs in
my networking closet :)


> And we used QEMU to verify some of the support for Command Duration Limits
> in linux:
> https://lore.kernel.org/linux-scsi/20230511011356.227789-1-nks@flawful.org/


Ah-ha, that helps explain it :)

If you find other problems, even if you don't intend to fix them, please
file a gitlab issue to document that there's work that needs done.


>
> Right now, I don't intend to upstream all my hacky patches to QEMU,
> just the things that might be useful for everyone, e.g. GPL and NCQ error
> log
> (and possibly Sense Data Reporting feature set support, and NCQ autosense).
>

Got it, thanks! I'll try to set aside time to look over your patches
"soon", I'd like then for 8.1. If I go quiet, *please* ping me.


>
> Kind regards,
> Niklas
>

[-- Attachment #2: Type: text/html, Size: 5059 bytes --]