qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/3] ide: don't loose pending dma state


From: John Snow
Subject: Re: [Qemu-devel] [PATCH 1/3] ide: don't loose pending dma state
Date: Wed, 30 Mar 2016 13:35:29 -0400
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0


On 03/23/2016 06:26 AM, Denis V. Lunev wrote:
> From: Pavel Butsykin <address@hidden>
> 
> If the migration occurs after the IDE DMA has been set up but before it
> has been initiated, the state gets lost upon save/restore. Specifically,
> ->dma_cb callback gets cleared, so, when the guest eventually starts bus
> mastering, the DMA never completes, causing the guest to time out the
> operation.
> 
> OTOH all the infrastructure is already in place to restart the DMA if
> the migration happens while the DMA is in progress.
> 
> So reuse that infrastructure, by calling the DMA callback with an
> artificial error code in pre_save if the callback is already set but
> DMAING is clear. The callback then sets bus->error_status to indicate
> the need for restart; however since DMAING is clear the state upon
> restore will be exactly "ready forDMA" as before the save.
> 
> This patch only fixes the IDE DMA case; the ATAPI DMA is left with a stub
> for now since its restart hadn't been implemented yet. It will be
> addressed in the followup patch.
> 
> Signed-off-by: Pavel Butsykin <address@hidden>
> Reviewed-by: Roman Kagan <address@hidden>
> Signed-off-by: Denis V. Lunev <address@hidden>
> CC: John Snow <address@hidden>
> ---
>  hw/ide/atapi.c    | 9 ++++++++-
>  hw/ide/core.c     | 9 ++++++++-
>  hw/ide/internal.h | 7 +++++++
>  hw/ide/pci.c      | 3 +++
>  4 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c
> index 1fe58ab..268220d 100644
> --- a/hw/ide/atapi.c
> +++ b/hw/ide/atapi.c
> @@ -381,7 +381,14 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int 
> ret)
>      IDEState *s = opaque;
>      int data_offset, n;
>  
> -    if (ret < 0) {
> +    if (ret < 0) { /* XXX: handle -ESTATESAVE for migration */
> +        if (ret == -ESTATESAVE) {
> +            /*
> +             * This case is not really an error
> +             * but a request to save the state.
> +             */
> +            return;
> +        }
>          ide_atapi_io_error(s, ret);
>          goto eot;
>      }
> diff --git a/hw/ide/core.c b/hw/ide/core.c
> index 241e840..872e11f 100644
> --- a/hw/ide/core.c
> +++ b/hw/ide/core.c
> @@ -810,7 +810,14 @@ static void ide_dma_cb(void *opaque, int ret)
>          else if (s->dma_cmd == IDE_DMA_TRIM)
>              op |= IDE_RETRY_TRIM;
>  
> -        if (ide_handle_rw_error(s, -ret, op)) {
> +        if (ret == -ESTATESAVE) {
> +            /*
> +             * This case is not really an error
> +             * but a request to save the state.
> +             */
> +            s->bus->error_status = op;
> +            return;
> +        } else if (ide_handle_rw_error(s, -ret, op)) {
>              return;
>          }
>      }
> diff --git a/hw/ide/internal.h b/hw/ide/internal.h
> index 86bde26..dcd8627 100644
> --- a/hw/ide/internal.h
> +++ b/hw/ide/internal.h
> @@ -513,6 +513,13 @@ struct IDEDevice {
>  #define IDE_RETRY_TRIM 0x80
>  #define IDE_RETRY_HBA  0x100
>  
> +/*
> + * a code to trigger entering error path and save/restore the "ready to DMA"
> + * state just like DMA-ing state. (Ab)use EINPROGRESS as it's not supposed to
> + * come from the block layer
> + */
> +#define ESTATESAVE EINPROGRESS
> +
>  static inline IDEState *idebus_active_if(IDEBus *bus)
>  {
>      return bus->ifs + bus->unit;
> diff --git a/hw/ide/pci.c b/hw/ide/pci.c
> index 92ffee7..e1f89fa 100644
> --- a/hw/ide/pci.c
> +++ b/hw/ide/pci.c
> @@ -308,6 +308,9 @@ static void ide_bmdma_pre_save(void *opaque)
>      BMDMAState *bm = opaque;
>      uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS;
>  
> +    if (!(bm->status & BM_STATUS_DMAING) && bm->dma_cb) {
> +        bm->dma_cb(bmdma_active_if(bm), -ESTATESAVE);
> +    }
>      bm->migration_retry_unit = bm->bus->retry_unit;
>      bm->migration_retry_sector_num = bm->bus->retry_sector_num;
>      bm->migration_retry_nsector = bm->bus->retry_nsector;
> 

Reviewed-by: John Snow <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]