[qemu-xen stable-4.19] hw/ide/ahci: fix broken SError handling

[patchbot@xxxxxxx]

commit 4f6c55371743ba9d6512fcf4a88a5b5b8936037e
Author:     Niklas Cassel <niklas.cassel@xxxxxxx>
AuthorDate: Fri Jun 9 16:08:44 2023 +0200
Commit:     Michael Tokarev <mjt@xxxxxxxxxx>
CommitDate: Sun Sep 10 19:40:11 2023 +0300

    hw/ide/ahci: fix broken SError handling
    
    When encountering an NCQ error, you should not write the NCQ tag to the
    SError register. This is completely wrong.
    
    The SError register has a clear definition, where each bit represents a
    different error, see PxSERR definition in AHCI 1.3.1.
    
    If we write a random value (like the NCQ tag) in SError, e.g. Linux will
    read SError, and will trigger arbitrary error handling depending on the
    NCQ tag that happened to be executing.
    
    In case of success, ncq_cb() will call ncq_finish().
    In case of error, ncq_cb() will call ncq_err() (which will clear
    ncq_tfs->used), and then call ncq_finish(), thus using ncq_tfs->used is
    sufficient to tell if finished should get set or not.
    
    Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
    Reviewed-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
    Message-id: 20230609140844.202795-9-nks@xxxxxxxxxxx
    Signed-off-by: John Snow <jsnow@xxxxxxxxxx>
    (cherry picked from commit 9f89423537653de07ca40c18b5ff5b70b104cc93)
    Signed-off-by: Michael Tokarev <mjt@xxxxxxxxxx>
---
 hw/ide/ahci.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
index 8ce3dd796e..29ddd4b6c4 100644
--- a/hw/ide/ahci.c
+++ b/hw/ide/ahci.c
@@ -1012,7 +1012,6 @@ static void ncq_err(NCQTransferState *ncq_tfs)
 
     ide_state->error = ABRT_ERR;
     ide_state->status = READY_STAT | ERR_STAT;
-    ncq_tfs->drive->port_regs.scr_err |= (1 << ncq_tfs->tag);
     qemu_sglist_destroy(&ncq_tfs->sglist);
     ncq_tfs->used = 0;
 }
@@ -1022,7 +1021,7 @@ static void ncq_finish(NCQTransferState *ncq_tfs)
     /* If we didn't error out, set our finished bit. Errored commands
      * do not get a bit set for the SDB FIS ACT register, nor do they
      * clear the outstanding bit in scr_act (PxSACT). */
-    if (!(ncq_tfs->drive->port_regs.scr_err & (1 << ncq_tfs->tag))) {
+    if (ncq_tfs->used) {
         ncq_tfs->drive->finished |= (1 << ncq_tfs->tag);
     }
 
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#stable-4.19