Re: [Qemu-devel] [WIP PATCH 00/24] IDE cleanups, initial work on AHCI rerror/werror=stop

[Paolo Bonzini]

Il 28/10/2013 18:16, Michael S. Tsirkin ha scritto:
> On Mon, Oct 28, 2013 at 05:43:16PM +0100, Paolo Bonzini wrote:
>> Hi all,
>>
>> this is some cleanup to the IDE code that I started long ago
>> and picked up again on my flight back from KVM Forum...
>>
>> The patches clean up the callbacks to prepare for implementing request
>> retry for AHCI.  After these patches, PIO retry should work for AHCI and
>> non-NCQ retry should be very close (you need an implementation of the
>> new restart_dma callback).  NCQ retry and save/restore is AHCI-specific
>> and has to be done separately, similar to how we do it for SCSI already.
>>
>> Patches 1-4 set up a new (incomplete) testcase.  Patches 5-13 are no-op
>> cleanups, or so I hope.  Patches 14-23 genericize retry and make it
>> "just work" for all adapters for PIO.  Patch 24 is another cleanup that
>> could probably be moved earlier in the series.
>>
>> Throwing it out since I know Kevin and mst are working on AHCI, to avoid
>> doing duplicate work.  Patches 2 and 4 are known to be racy and
>> incomplete.  Extremely light testing, if something breaks tells me and
>> I'll look at it.
> 
> Well ATM win7 boot is broken with non NCQ (crash on boot).
> Do you patches help?

No, these are for another feature that is missing for AHCI, that is
support for stopping the VM on I/O errors.  It is
https://bugzilla.redhat.com/show_bug.cgi?id=887844

> Also, without NCQ win 7 boot will sometime hang.
> The following patch by Alex makes it behave better:
> it still sometimes hangs for a very long time but with the patch it
> will after a long time proceed. Sometimes it proceeds normally.

Yeah, I saw you and Kevin discuss it on IRC.

Paolo

> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
> index a8be62c..fbea9e8 100644
> --- a/hw/ide/ahci.c
> +++ b/hw/ide/ahci.c
> @@ -961,7 +961,8 @@ static int handle_cmd(AHCIState *s, int port, int slot)
>          /* We're ready to process the command in FIS byte 2. */
>          ide_exec_cmd(&s->dev[port].port, cmd_fis[2]);
>  
> -        if (s->dev[port].port.ifs[0].status & READY_STAT) {
> +        if ((s->dev[port].port.ifs[0].status & 
> (READY_STAT|DRQ_STAT|BUSY_STAT)) ==
> +            READY_STAT) {
>              ahci_write_fis_d2h(&s->dev[port], cmd_fis);
>          }
>      }
>