Re: [Qemu-block] [Qemu-devel] [PATCH] ahci: advertise HOST_CAP_64

[John Snow]

On 01/13/2017 06:02 AM, Ladi Prosek wrote:
> The AHCI emulation code supports 64-bit addressing and should advertise this
> fact in the Host Capabilities register. Both Linux and Windows drivers test
> this bit to decide if the upper 32 bits of various registers may be written
> to, and at least some versions of Windows have a bug where DMA is attempted
> with an address above 4GB but, in the absence of HOST_CAP_64, the upper 32
> bits are left unititialized which leads to a memory corruption.
> 
> Signed-off-by: Ladi Prosek <address@hidden>
> ---
>  hw/ide/ahci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index 3c19bda..6a17acf 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -488,7 +488,7 @@ static void ahci_reg_init(AHCIState *s)
>      s->control_regs.cap = (s->ports - 1) |
>                            (AHCI_NUM_COMMAND_SLOTS << 8) |
>                            (AHCI_SUPPORTED_SPEED_GEN1 << 
> AHCI_SUPPORTED_SPEED) |
> -                          HOST_CAP_NCQ | HOST_CAP_AHCI;
> +                          HOST_CAP_NCQ | HOST_CAP_AHCI | HOST_CAP_64;
>  
>      s->control_regs.impl = (1 << s->ports) - 1;
>  
> 

Was this tested? What was the use case that prompted this patch, and do
you have a public bugzilla to point to?

It looks correct via the spec, thank you for finding this oversight. It
looks like everywhere this bit would make a difference we already do the
correct thing for having this bit set.

>From what I can gather from the spec, it looks as if even 32bit guests
must ensure that the upper 32bit registers are cleared, so I think we're
all set here.

Reviewed-by: John Snow <address@hidden>