Re: [Qemu-devel] [PATCH 00/10] AHCI emulation support v2

[Kevin Wolf]

Hi Alex,

Am 18.11.2010 04:27, schrieb Alexander Graf:
> This patch adds support for AHCI emulation. I have tested and verified it 
> works
> in Linux, OpenBSD, Windows Vista and Windows 7. This AHCI emulation supports
> NCQ, so multiple read or write requests can be outstanding at the same time.
> 
> The code is however not fully optimized yet. I'm fairly sure that there are
> low hanging performance fruits to be found still :). In my simple benchmarks
> I achieved about 2/3rd of virtio performance.
> 
> Also, this AHCI emulation layer does not support legacy mode. So if you're
> using a disk with this emulation, you do not get it exposed using the legacy
> IDE interfaces.
> 
> Another nitpick is CD-ROM support in Windows. Somehow it doesn't detect a
> CD-ROM drive attached to AHCI. At least it doesn't list it.
> 
> To attach an AHCI disk to your VM, please use
> 
>   -drive file=...,if=sata
> 
> This should do the trick for x86. On other platforms, you might need to add
> the ahci host controller using -device.
> 
> 
> This patch set is based on work done during the Google Summer of Code. I was
> mentoring a student, Roland Elek, who wrote most of the AHCI emulation code
> based on a patch from Chong Qiao. A bunch of other people were also involved,
> so everybody who I didn't mention - thanks a lot!

I'm not completely sure about the relationship between the AHCI
emulation and our existing IDE emulation. First thing I noticed is that
AHCI wants to be independent and resides in hw/ instead of hw/ide/, but
it still include ide/internal.h. Do you think it would make sense to
move AHCI into hw/ide?

Then I believe that core.c is now a mixture of some generic ATA code
(that is also used by SATA) and the Legacy IDE code. SATA doesn't seem
to interact with the generic code through clean interfaces, but by
accessing internal data structures and calls to somewhere in the middle
of the existing IDE emultion. I think we should get a clean abstraction
there and have a clean split between SATA, PATA and common code, with
each of them sitting in its own file in hw/ide.

I haven't reviewed the patches in detail but just had a quick look at
them, so my impressions might be wrong. If so, please correct me.

Kevin