[PATCH v3 0/8] misc AHCI cleanups

Niklas Cassel posted 8 patches 10 months, 3 weeks ago
hw/ide/ahci.c             | 112 +++++++++++++++++++++++++++-----------
hw/ide/core.c             |   2 +-
tests/qtest/libqos/ahci.c | 106 +++++++++++++++++++++++++++---------
tests/qtest/libqos/ahci.h |   8 +--
4 files changed, 164 insertions(+), 64 deletions(-)
[PATCH v3 0/8] misc AHCI cleanups
Posted by Niklas Cassel 10 months, 3 weeks ago
From: Niklas Cassel <niklas.cassel@wdc.com>

Hello John,

Here comes some misc AHCI cleanups.

Most are related to error handling.

Please review.

Changes since v2:
-Squashed in the test commits that were sent out as a separate series into
 the patch "hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set",
 and reordered some of the patches, such that each and every commit passes
 the ahci test suite as a separate unit. This way it will be possible to
 perform a git bisect without seeing any failures in the ahci test suite.


Kind regards,
Niklas

Niklas Cassel (8):
  hw/ide/ahci: remove stray backslash
  hw/ide/core: set ERR_STAT in unsupported command completion
  hw/ide/ahci: write D2H FIS when processing NCQ command
  hw/ide/ahci: simplify and document PxCI handling
  hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared
  hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
  hw/ide/ahci: fix ahci_write_fis_sdb()
  hw/ide/ahci: fix broken SError handling

 hw/ide/ahci.c             | 112 +++++++++++++++++++++++++++-----------
 hw/ide/core.c             |   2 +-
 tests/qtest/libqos/ahci.c | 106 +++++++++++++++++++++++++++---------
 tests/qtest/libqos/ahci.h |   8 +--
 4 files changed, 164 insertions(+), 64 deletions(-)

-- 
2.40.1
Re: [PATCH v3 0/8] misc AHCI cleanups
Posted by Niklas Cassel 9 months, 1 week ago
On Fri, Jun 09, 2023 at 04:08:36PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
> 
> Hello John,
> 
> Here comes some misc AHCI cleanups.
> 
> Most are related to error handling.
> 
> Please review.
> 
> Changes since v2:
> -Squashed in the test commits that were sent out as a separate series into
>  the patch "hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set",
>  and reordered some of the patches, such that each and every commit passes
>  the ahci test suite as a separate unit. This way it will be possible to
>  perform a git bisect without seeing any failures in the ahci test suite.
> 
> 
> Kind regards,
> Niklas
> 
> Niklas Cassel (8):
>   hw/ide/ahci: remove stray backslash
>   hw/ide/core: set ERR_STAT in unsupported command completion
>   hw/ide/ahci: write D2H FIS when processing NCQ command
>   hw/ide/ahci: simplify and document PxCI handling
>   hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared
>   hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
>   hw/ide/ahci: fix ahci_write_fis_sdb()
>   hw/ide/ahci: fix broken SError handling
> 
>  hw/ide/ahci.c             | 112 +++++++++++++++++++++++++++-----------
>  hw/ide/core.c             |   2 +-
>  tests/qtest/libqos/ahci.c | 106 +++++++++++++++++++++++++++---------
>  tests/qtest/libqos/ahci.h |   8 +--
>  4 files changed, 164 insertions(+), 64 deletions(-)
> 
> -- 
> 2.40.1
> 
> 

Hello Philippe,

Considering that you picked up my patch,
"hw/ide/ahci: remove stray backslash" (patch 1/8 in this series),
and since John seems to have gone silent for 40+ days,
could you please consider taking this series through your misc tree?

The series still applies, you just need to skip patch 1/8
(which has already landed in qemu master).


Kind regards,
Niklas
Re: [PATCH v3 0/8] misc AHCI cleanups
Posted by Philippe Mathieu-Daudé 9 months ago
Hi Niklas, John, Paolo, Kevin,

On 19/7/23 12:47, Niklas Cassel wrote:

>> Niklas Cassel (8):
>>    hw/ide/ahci: remove stray backslash
>>    hw/ide/core: set ERR_STAT in unsupported command completion
>>    hw/ide/ahci: write D2H FIS when processing NCQ command
>>    hw/ide/ahci: simplify and document PxCI handling
>>    hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared
>>    hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
>>    hw/ide/ahci: fix ahci_write_fis_sdb()
>>    hw/ide/ahci: fix broken SError handling
>>
>>   hw/ide/ahci.c             | 112 +++++++++++++++++++++++++++-----------
>>   hw/ide/core.c             |   2 +-
>>   tests/qtest/libqos/ahci.c | 106 +++++++++++++++++++++++++++---------
>>   tests/qtest/libqos/ahci.h |   8 +--
>>   4 files changed, 164 insertions(+), 64 deletions(-)
>>
>> -- 
>> 2.40.1
>>
>>
> 
> Hello Philippe,
> 
> Considering that you picked up my patch,
> "hw/ide/ahci: remove stray backslash" (patch 1/8 in this series),
> and since John seems to have gone silent for 40+ days,
> could you please consider taking this series through your misc tree?

(First patch was a cleanup)

Niklas, I don't feel confident enough :/

John, Paolo, Kevin, do you Ack?

Regards,

Phil.
Re: [PATCH v3 0/8] misc AHCI cleanups
Posted by John Snow 9 months ago
On Tue, Jul 25, 2023 at 9:04 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> Hi Niklas, John, Paolo, Kevin,
>
> On 19/7/23 12:47, Niklas Cassel wrote:
>
> >> Niklas Cassel (8):
> >>    hw/ide/ahci: remove stray backslash
> >>    hw/ide/core: set ERR_STAT in unsupported command completion
> >>    hw/ide/ahci: write D2H FIS when processing NCQ command
> >>    hw/ide/ahci: simplify and document PxCI handling
> >>    hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared
> >>    hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
> >>    hw/ide/ahci: fix ahci_write_fis_sdb()
> >>    hw/ide/ahci: fix broken SError handling
> >>
> >>   hw/ide/ahci.c             | 112 +++++++++++++++++++++++++++-----------
> >>   hw/ide/core.c             |   2 +-
> >>   tests/qtest/libqos/ahci.c | 106 +++++++++++++++++++++++++++---------
> >>   tests/qtest/libqos/ahci.h |   8 +--
> >>   4 files changed, 164 insertions(+), 64 deletions(-)
> >>
> >> --
> >> 2.40.1
> >>
> >>
> >
> > Hello Philippe,
> >
> > Considering that you picked up my patch,
> > "hw/ide/ahci: remove stray backslash" (patch 1/8 in this series),
> > and since John seems to have gone silent for 40+ days,
> > could you please consider taking this series through your misc tree?
>

40 days, ouch. I kept thinking it had been a week. Don't trust me with time.

> (First patch was a cleanup)
>
> Niklas, I don't feel confident enough :/
>
> John, Paolo, Kevin, do you Ack?
>
> Regards,
>
> Phil.

I'm staging it, but it's for next release. We'll get it in early and
it gives us a chance to fix anything that's amiss before the next RC
window.
Re: [PATCH v3 0/8] misc AHCI cleanups
Posted by Niklas Cassel 8 months, 3 weeks ago
On Tue, Jul 25, 2023 at 03:00:56PM -0400, John Snow wrote:
> On Tue, Jul 25, 2023 at 9:04 AM Philippe Mathieu-Daudé
> <philmd@linaro.org> wrote:
> >
> > Hi Niklas, John, Paolo, Kevin,
> >
> > On 19/7/23 12:47, Niklas Cassel wrote:
> >
> > >> Niklas Cassel (8):
> > >>    hw/ide/ahci: remove stray backslash
> > >>    hw/ide/core: set ERR_STAT in unsupported command completion
> > >>    hw/ide/ahci: write D2H FIS when processing NCQ command
> > >>    hw/ide/ahci: simplify and document PxCI handling
> > >>    hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared
> > >>    hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
> > >>    hw/ide/ahci: fix ahci_write_fis_sdb()
> > >>    hw/ide/ahci: fix broken SError handling
> > >>
> > >>   hw/ide/ahci.c             | 112 +++++++++++++++++++++++++++-----------
> > >>   hw/ide/core.c             |   2 +-
> > >>   tests/qtest/libqos/ahci.c | 106 +++++++++++++++++++++++++++---------
> > >>   tests/qtest/libqos/ahci.h |   8 +--
> > >>   4 files changed, 164 insertions(+), 64 deletions(-)
> > >>
> > >> --
> > >> 2.40.1
> > >>
> > >>
> > >
> > > Hello Philippe,
> > >
> > > Considering that you picked up my patch,
> > > "hw/ide/ahci: remove stray backslash" (patch 1/8 in this series),
> > > and since John seems to have gone silent for 40+ days,
> > > could you please consider taking this series through your misc tree?
> >
> 
> 40 days, ouch. I kept thinking it had been a week. Don't trust me with time.

Well, it is summer vacation times, so the days tend to fly by quite
quickly :)


> 
> > (First patch was a cleanup)
> >
> > Niklas, I don't feel confident enough :/
> >
> > John, Paolo, Kevin, do you Ack?
> >
> > Regards,
> >
> > Phil.
> 
> I'm staging it, but it's for next release. We'll get it in early and
> it gives us a chance to fix anything that's amiss before the next RC
> window.
> 

Thank you John!

I don't expect any (further) issues, but I will of course be available
in case something unexpectedly pops up.


Kind regards,
Niklas
Re: [PATCH v3 0/8] misc AHCI cleanups
Posted by John Snow 8 months, 3 weeks ago
On Mon, Aug 7, 2023, 6:33 AM Niklas Cassel <Niklas.Cassel@wdc.com> wrote:

> On Tue, Jul 25, 2023 at 03:00:56PM -0400, John Snow wrote:
> > On Tue, Jul 25, 2023 at 9:04 AM Philippe Mathieu-Daudé
> > <philmd@linaro.org> wrote:
> > >
> > > Hi Niklas, John, Paolo, Kevin,
> > >
> > > On 19/7/23 12:47, Niklas Cassel wrote:
> > >
> > > >> Niklas Cassel (8):
> > > >>    hw/ide/ahci: remove stray backslash
> > > >>    hw/ide/core: set ERR_STAT in unsupported command completion
> > > >>    hw/ide/ahci: write D2H FIS when processing NCQ command
> > > >>    hw/ide/ahci: simplify and document PxCI handling
> > > >>    hw/ide/ahci: PxSACT and PxCI is cleared when PxCMD.ST is cleared
> > > >>    hw/ide/ahci: PxCI should not get cleared when ERR_STAT is set
> > > >>    hw/ide/ahci: fix ahci_write_fis_sdb()
> > > >>    hw/ide/ahci: fix broken SError handling
> > > >>
> > > >>   hw/ide/ahci.c             | 112
> +++++++++++++++++++++++++++-----------
> > > >>   hw/ide/core.c             |   2 +-
> > > >>   tests/qtest/libqos/ahci.c | 106
> +++++++++++++++++++++++++++---------
> > > >>   tests/qtest/libqos/ahci.h |   8 +--
> > > >>   4 files changed, 164 insertions(+), 64 deletions(-)
> > > >>
> > > >> --
> > > >> 2.40.1
> > > >>
> > > >>
> > > >
> > > > Hello Philippe,
> > > >
> > > > Considering that you picked up my patch,
> > > > "hw/ide/ahci: remove stray backslash" (patch 1/8 in this series),
> > > > and since John seems to have gone silent for 40+ days,
> > > > could you please consider taking this series through your misc tree?
> > >
> >
> > 40 days, ouch. I kept thinking it had been a week. Don't trust me with
> time.
>
> Well, it is summer vacation times, so the days tend to fly by quite
> quickly :)
>
>
> >
> > > (First patch was a cleanup)
> > >
> > > Niklas, I don't feel confident enough :/
> > >
> > > John, Paolo, Kevin, do you Ack?
> > >
> > > Regards,
> > >
> > > Phil.
> >
> > I'm staging it, but it's for next release. We'll get it in early and
> > it gives us a chance to fix anything that's amiss before the next RC
> > window.
> >
>
> Thank you John!
>
> I don't expect any (further) issues, but I will of course be available
> in case something unexpectedly pops up.
>
>
> Kind regards,
> Niklas


Apologies again for the delay. I tested it lightly and it seems fine to me
(and in general I trust your commits as they've got meticulous references
to the spec, so it'll be easy to fix if something goes wrong)

It's my fault we'll miss this release window, but putting it in early next
window gives us a lot of time for people to notice accidental regressions.

Thanks for the patches and I hope to see more from you soon ;)

--js
Re: [PATCH v3 0/8] misc AHCI cleanups
Posted by Philippe Mathieu-Daudé 7 months, 4 weeks ago
Hi John,

On 7/8/23 19:37, John Snow wrote:

> Apologies again for the delay. I tested it lightly and it seems fine to 
> me (and in general I trust your commits as they've got meticulous 
> references to the spec, so it'll be easy to fix if something goes wrong)
> 
> It's my fault we'll miss this release window, but putting it in early 
> next window gives us a lot of time for people to notice accidental 
> regressions.
> 
> Thanks for the patches and I hope to see more from you soon ;)

I've been doing some testing (x86/mips), but I don't think I have the
same coverage as your maintainer test suite. Still, if you are busy,
I can merge this series now that we are at the beginning of the
development cycle.

Regards,

Phil.