* Re: [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both
2023-10-11 13:12 [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both Niklas Cassel
@ 2023-10-11 13:26 ` Philippe Mathieu-Daudé
2023-10-11 14:44 ` Michael Tokarev
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-11 13:26 UTC (permalink / raw)
To: Niklas Cassel, John Snow
Cc: qemu-block, qemu-devel, Damien Le Moal, Michael Tokarev,
Niklas Cassel
On 11/10/23 15:12, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> According to AHCI 1.3.1, 5.3.8.1 RegFIS:Entry, if ERR_STAT is set,
> we jump to state ERR:FatalTaskfile, which will raise a TFES IRQ
> unconditionally, regardless if the I bit is set in the FIS or not.
>
> Thus, we should never raise a normal IRQ after having sent an error
> IRQ.
>
> NOTE: for QEMU platforms that use SeaBIOS, this patch depends on QEMU
> commit 784155cdcb02 ("seabios: update submodule to git snapshot"), and
> QEMU commit 14f5a7bae4cb ("seabios: update binaries to git snapshot").
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> hw/ide/ahci.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both
2023-10-11 13:12 [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both Niklas Cassel
2023-10-11 13:26 ` Philippe Mathieu-Daudé
@ 2023-10-11 14:44 ` Michael Tokarev
2023-10-11 14:50 ` Niklas Cassel
2023-10-26 9:08 ` Niklas Cassel
2023-11-07 16:55 ` Kevin Wolf
3 siblings, 1 reply; 6+ messages in thread
From: Michael Tokarev @ 2023-10-11 14:44 UTC (permalink / raw)
To: Niklas Cassel, John Snow
Cc: qemu-block, qemu-devel, Damien Le Moal, Niklas Cassel
11.10.2023 16:12, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> According to AHCI 1.3.1, 5.3.8.1 RegFIS:Entry, if ERR_STAT is set,
> we jump to state ERR:FatalTaskfile, which will raise a TFES IRQ
> unconditionally, regardless if the I bit is set in the FIS or not.
>
> Thus, we should never raise a normal IRQ after having sent an error
> IRQ.
>
> NOTE: for QEMU platforms that use SeaBIOS, this patch depends on QEMU
> commit 784155cdcb02 ("seabios: update submodule to git snapshot"), and
> QEMU commit 14f5a7bae4cb ("seabios: update binaries to git snapshot").
Am I right that without commit 1281e340a "ahci: handle TFES irq correctly"
in seabios (which is included in this 784155cd update above), seabios
will hang at boot for 32s when qemu is booted with ahci drives?
I mean, is it the only implication of not updating seabios after
this patch?
Thanks!
/mjt
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both
2023-10-11 14:44 ` Michael Tokarev
@ 2023-10-11 14:50 ` Niklas Cassel
0 siblings, 0 replies; 6+ messages in thread
From: Niklas Cassel @ 2023-10-11 14:50 UTC (permalink / raw)
To: Michael Tokarev
Cc: Niklas Cassel, John Snow, qemu-block@nongnu.org,
qemu-devel@nongnu.org, Damien Le Moal
On Wed, Oct 11, 2023 at 05:44:28PM +0300, Michael Tokarev wrote:
> 11.10.2023 16:12, Niklas Cassel wrote:
> > From: Niklas Cassel <niklas.cassel@wdc.com>
> >
> > According to AHCI 1.3.1, 5.3.8.1 RegFIS:Entry, if ERR_STAT is set,
> > we jump to state ERR:FatalTaskfile, which will raise a TFES IRQ
> > unconditionally, regardless if the I bit is set in the FIS or not.
> >
> > Thus, we should never raise a normal IRQ after having sent an error
> > IRQ.
> >
> > NOTE: for QEMU platforms that use SeaBIOS, this patch depends on QEMU
> > commit 784155cdcb02 ("seabios: update submodule to git snapshot"), and
> > QEMU commit 14f5a7bae4cb ("seabios: update binaries to git snapshot").
>
> Am I right that without commit 1281e340a "ahci: handle TFES irq correctly"
> in seabios (which is included in this 784155cd update above), seabios
> will hang at boot for 32s when qemu is booted with ahci drives?
>
> I mean, is it the only implication of not updating seabios after
> this patch?
That is correct.
If you don't have the seabios patch, the seabios ATAPI command will timeout
after 30 seconds, after which QEMU will boot and behave like normal.
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both
2023-10-11 13:12 [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both Niklas Cassel
2023-10-11 13:26 ` Philippe Mathieu-Daudé
2023-10-11 14:44 ` Michael Tokarev
@ 2023-10-26 9:08 ` Niklas Cassel
2023-11-07 16:55 ` Kevin Wolf
3 siblings, 0 replies; 6+ messages in thread
From: Niklas Cassel @ 2023-10-26 9:08 UTC (permalink / raw)
To: Niklas Cassel
Cc: John Snow, qemu-block@nongnu.org, qemu-devel@nongnu.org,
Damien Le Moal, Michael Tokarev
On Wed, Oct 11, 2023 at 03:12:20PM +0200, Niklas Cassel wrote:
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> According to AHCI 1.3.1, 5.3.8.1 RegFIS:Entry, if ERR_STAT is set,
> we jump to state ERR:FatalTaskfile, which will raise a TFES IRQ
> unconditionally, regardless if the I bit is set in the FIS or not.
>
> Thus, we should never raise a normal IRQ after having sent an error
> IRQ.
>
> NOTE: for QEMU platforms that use SeaBIOS, this patch depends on QEMU
> commit 784155cdcb02 ("seabios: update submodule to git snapshot"), and
> QEMU commit 14f5a7bae4cb ("seabios: update binaries to git snapshot").
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
> ---
> hw/ide/ahci.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index fcc5476e9e..7676e2d871 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -897,11 +897,10 @@ static bool ahci_write_fis_d2h(AHCIDevice *ad, bool d2h_fis_i)
> pr->tfdata = (ad->port.ifs[0].error << 8) |
> ad->port.ifs[0].status;
>
> + /* TFES IRQ is always raised if ERR_STAT is set, regardless of I bit. */
> if (d2h_fis[2] & ERR_STAT) {
> ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_TFES);
> - }
> -
> - if (d2h_fis_i) {
> + } else if (d2h_fis_i) {
> ahci_trigger_irq(ad->hba, ad, AHCI_PORT_IRQ_BIT_DHRS);
> }
>
> --
> 2.41.0
>
Gentle ping :)
Kind regards,
Niklas
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both
2023-10-11 13:12 [PATCH] hw/ide/ahci: trigger either error IRQ or regular IRQ, not both Niklas Cassel
` (2 preceding siblings ...)
2023-10-26 9:08 ` Niklas Cassel
@ 2023-11-07 16:55 ` Kevin Wolf
3 siblings, 0 replies; 6+ messages in thread
From: Kevin Wolf @ 2023-11-07 16:55 UTC (permalink / raw)
To: Niklas Cassel
Cc: John Snow, qemu-block, qemu-devel, Damien Le Moal,
Michael Tokarev, Niklas Cassel
Am 11.10.2023 um 15:12 hat Niklas Cassel geschrieben:
> From: Niklas Cassel <niklas.cassel@wdc.com>
>
> According to AHCI 1.3.1, 5.3.8.1 RegFIS:Entry, if ERR_STAT is set,
> we jump to state ERR:FatalTaskfile, which will raise a TFES IRQ
> unconditionally, regardless if the I bit is set in the FIS or not.
>
> Thus, we should never raise a normal IRQ after having sent an error
> IRQ.
>
> NOTE: for QEMU platforms that use SeaBIOS, this patch depends on QEMU
> commit 784155cdcb02 ("seabios: update submodule to git snapshot"), and
> QEMU commit 14f5a7bae4cb ("seabios: update binaries to git snapshot").
>
> Signed-off-by: Niklas Cassel <niklas.cassel@wdc.com>
Thanks, applied to the block branch.
Kevin
^ permalink raw reply [flat|nested] 6+ messages in thread