On 11/06/2021 12:38, Paolo Bonzini wrote:

On 09/06/21 17:31, Mark Cave-Ayland wrote:
Due to the absence of the IDENTIFY byte in the S case I'm guessing from the patch that the LUN is in encoded in buf[1] (the top bits being "Reserved" according to my copy of the specification).

They used to be the LUN many years ago.

Got it :)

  static void s_without_satn_pdma_cb(ESPState *s)
  {
+    uint8_t busid;
+
      if (!esp_get_tc(s) && !fifo8_is_empty(&s->cmdfifo)) {
          s->cmdfifo_cdb_offset = 0;
          s->do_cmd = 0;
-        do_busid_cmd(s, 0);
+        busid = s->cmdfifo.data[1] >> 5;
+        do_busid_cmd(s, busid);
      }
  }

Considering how obsolete the LUN-in-CDB case is (and now that I actually understand that the first byte is a message-phase byte), it is probably be more correct to keep the previous busid: with no message phase, the previously-selected LUN would be addressed.  I can send a patch for this, but it's orthogonal to this one so I queued it as well.

That was going to be my next question - how do you determine the LUN if there is no message phase byte? But that makes sense to me. I don't think that I have a LUN-in-CDB example in my set of ESP test images, but I'm happy to run them against your patch and make sure there are no regressions.


ATB,

Mark.

Reply via email to