diff mbox

[1/2] ahci: Fix CD-ROM signature

Message ID 1436219392-31915-2-git-send-email-jsnow@redhat.com
State New
Headers show

Commit Message

John Snow July 6, 2015, 9:49 p.m. UTC
From: Hannes Reinecke <hare@suse.de>

The CD-ROM signature is 0xeb140101, not 0xeb140000.
Without this change OVMF/Duet runs into a timeout trying
to detect a SATA cdrom.

Signed-off-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: John Snow <jsnow@redhat.com>
---
 hw/ide/ahci.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

John Snow July 7, 2015, 5:19 p.m. UTC | #1
On 07/06/2015 05:49 PM, John Snow wrote:
> From: Hannes Reinecke <hare@suse.de>
> 
> The CD-ROM signature is 0xeb140101, not 0xeb140000.
> Without this change OVMF/Duet runs into a timeout trying
> to detect a SATA cdrom.
> 
> Signed-off-by: Hannes Reinecke <hare@suse.de>
> Signed-off-by: John Snow <jsnow@redhat.com>
> ---
>  hw/ide/ahci.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
> index 9f5b4d2..68d5074 100644
> --- a/hw/ide/ahci.h
> +++ b/hw/ide/ahci.h
> @@ -166,7 +166,7 @@
>  #define AHCI_CMD_HDR_CMD_FIS_LEN           0x1f
>  #define AHCI_CMD_HDR_PRDT_LEN              16
>  
> -#define SATA_SIGNATURE_CDROM               0xeb140000
> +#define SATA_SIGNATURE_CDROM               0xeb140101
>  #define SATA_SIGNATURE_DISK                0x00000101
>  
>  #define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
> 

FWIW for review purposes, this is based on ATA8 AC3, Table 184 "Device
Signatures for Normal Output" and is very straightforward.

For how the component fields there (LBA and Count) become a single 4
byte signature, see AHCI 1.3 section 3.3.9 (PxSIG) and this is the value
we are emulating here with the #define.

I gave this patch a soft ACK in the past, but it feels wrong to give it
an R-B when I'm sending it out myself :)

--js
Kevin Wolf July 8, 2015, 9:26 a.m. UTC | #2
Am 07.07.2015 um 19:19 hat John Snow geschrieben:
> 
> 
> On 07/06/2015 05:49 PM, John Snow wrote:
> > From: Hannes Reinecke <hare@suse.de>
> > 
> > The CD-ROM signature is 0xeb140101, not 0xeb140000.
> > Without this change OVMF/Duet runs into a timeout trying
> > to detect a SATA cdrom.
> > 
> > Signed-off-by: Hannes Reinecke <hare@suse.de>
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >  hw/ide/ahci.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
> > index 9f5b4d2..68d5074 100644
> > --- a/hw/ide/ahci.h
> > +++ b/hw/ide/ahci.h
> > @@ -166,7 +166,7 @@
> >  #define AHCI_CMD_HDR_CMD_FIS_LEN           0x1f
> >  #define AHCI_CMD_HDR_PRDT_LEN              16
> >  
> > -#define SATA_SIGNATURE_CDROM               0xeb140000
> > +#define SATA_SIGNATURE_CDROM               0xeb140101
> >  #define SATA_SIGNATURE_DISK                0x00000101
> >  
> >  #define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20
> > 
> 
> FWIW for review purposes, this is based on ATA8 AC3, Table 184 "Device
> Signatures for Normal Output" and is very straightforward.

It is. The only thing that confuses me about this is that I thought it
had been fixed for a while already. My AHCI driver has a comment "Broken
value in qemu < 2.2" at the workaround, and I seem to remember that I
didn't need it for a newer qemu version indeed. Strange. Or perhaps I
only fixed it locally and then forgot to send the patch?

Anyway, the fix is obviously correct:

Reviewed-by: Kevin Wolf <kwolf@redhat.com>
diff mbox

Patch

diff --git a/hw/ide/ahci.h b/hw/ide/ahci.h
index 9f5b4d2..68d5074 100644
--- a/hw/ide/ahci.h
+++ b/hw/ide/ahci.h
@@ -166,7 +166,7 @@ 
 #define AHCI_CMD_HDR_CMD_FIS_LEN           0x1f
 #define AHCI_CMD_HDR_PRDT_LEN              16
 
-#define SATA_SIGNATURE_CDROM               0xeb140000
+#define SATA_SIGNATURE_CDROM               0xeb140101
 #define SATA_SIGNATURE_DISK                0x00000101
 
 #define AHCI_GENERIC_HOST_CONTROL_REGS_MAX_ADDR 0x20