From patchwork Wed Mar 23 10:26:32 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Denis V. Lunev" X-Patchwork-Id: 8648911 Return-Path: X-Original-To: patchwork-qemu-devel@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 0A0CA9FC93 for ; Wed, 23 Mar 2016 10:27:16 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 352CE203B1 for ; Wed, 23 Mar 2016 10:27:15 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2D1CD203B7 for ; Wed, 23 Mar 2016 10:27:14 +0000 (UTC) Received: from localhost ([::1]:42432 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aig0P-0004yo-J1 for patchwork-qemu-devel@patchwork.kernel.org; Wed, 23 Mar 2016 06:27:13 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37278) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aig0C-0004yN-Ls for qemu-devel@nongnu.org; Wed, 23 Mar 2016 06:27:02 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aig08-00009H-MZ for qemu-devel@nongnu.org; Wed, 23 Mar 2016 06:27:00 -0400 Received: from mailhub.sw.ru ([195.214.232.25]:8047 helo=relay.sw.ru) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aig08-00007I-46 for qemu-devel@nongnu.org; Wed, 23 Mar 2016 06:26:56 -0400 Received: from irbis.sw.ru ([10.30.2.139]) by relay.sw.ru (8.13.4/8.13.4) with ESMTP id u2NAQbni021776; Wed, 23 Mar 2016 13:26:43 +0300 (MSK) From: "Denis V. Lunev" To: qemu-devel@nongnu.org Date: Wed, 23 Mar 2016 13:26:32 +0300 Message-Id: <1458728792-15779-4-git-send-email-den@openvz.org> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1458728792-15779-1-git-send-email-den@openvz.org> References: <1458728792-15779-1-git-send-email-den@openvz.org> X-detected-operating-system: by eggs.gnu.org: OpenBSD 3.x X-Received-From: 195.214.232.25 Cc: den@openvz.org, John Snow , Pavel Butsykin Subject: [Qemu-devel] [PATCH 3/3] ide: really restart pending and in-flight atapi dma X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org X-Spam-Status: No, score=-6.9 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_HI, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Pavel Butsykin Restart of ATAPI DMA used to be unreachable, because the request to do so wasn't indicated in bus->error_status due to the lack of spare bits, and ide_restart_bh() would return early doing nothing. This patch makes use of the observation that not all bit combinations were possible in ->error_status. In particular, IDE_RETRY_READ only made sense together with IDE_RETRY_DMA or IDE_RETRY_PIO. This allows to re-use IDE_RETRY_READ alone as an indicator of ATAPI DMA restart request. As a means against confusion, macros are added to test the state of ->error_status. The patch fixes the restart of both in-flight and pending ATAPI DMA, following the scheme similar to that of IDE DMA. Signed-off-by: Pavel Butsykin Reviewed-by: Roman Kagan Signed-off-by: Denis V. Lunev CC: John Snow --- hw/ide/atapi.c | 15 +++++++-------- hw/ide/core.c | 23 ++++++++++------------- hw/ide/internal.h | 18 ++++++++++++++++++ 3 files changed, 35 insertions(+), 21 deletions(-) diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c index 8816b38..0d51b44 100644 --- a/hw/ide/atapi.c +++ b/hw/ide/atapi.c @@ -375,22 +375,25 @@ static void ide_atapi_cmd_check_status(IDEState *s) } /* ATAPI DMA support */ -/* XXX: handle read errors */ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret) { IDEState *s = opaque; int data_offset, n; - if (ret < 0) { /* XXX: handle -ESTATESAVE for migration */ + if (ret < 0) { if (ret == -ESTATESAVE) { /* * This case is not really an error * but a request to save the state. */ + s->bus->error_status = IDE_RETRY_ATAPI; return; + } else if (ide_handle_rw_error(s, -ret, IDE_RETRY_ATAPI)) { + if (s->bus->error_status) { + return; + } + goto eot; } - ide_atapi_io_error(s, ret); - goto eot; } if (s->io_buffer_size > 0) { @@ -488,10 +491,6 @@ static void ide_atapi_cmd_read(IDEState *s, int lba, int nb_sectors, } } - -/* Called by *_restart_bh when the transfer function points - * to ide_atapi_cmd - */ void ide_atapi_dma_restart(IDEState *s) { /* diff --git a/hw/ide/core.c b/hw/ide/core.c index 872e11f..6d7533f 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -56,7 +56,6 @@ static const int smart_attributes[][12] = { { 190, 0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32}, }; -static int ide_handle_rw_error(IDEState *s, int error, int op); static void ide_dummy_transfer_stop(IDEState *s); static void padstr(char *str, const char *src, int len) @@ -772,7 +771,7 @@ void ide_dma_error(IDEState *s) ide_set_irq(s->bus); } -static int ide_handle_rw_error(IDEState *s, int error, int op) +int ide_handle_rw_error(IDEState *s, int error, int op) { bool is_read = (op & IDE_RETRY_READ) != 0; BlockErrorAction action = blk_get_error_action(s->blk, is_read, error); @@ -782,8 +781,10 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) s->bus->error_status = op; } else if (action == BLOCK_ERROR_ACTION_REPORT) { block_acct_failed(blk_get_stats(s->blk), &s->acct); - if (op & IDE_RETRY_DMA) { + if (IS_IDE_RETRY_DMA(op)) { ide_dma_error(s); + } else if (IS_IDE_RETRY_ATAPI(op)) { + ide_atapi_io_error(s, -error); } else { ide_rw_error(s); } @@ -2533,13 +2534,13 @@ static void ide_restart_bh(void *opaque) } } - if (error_status & IDE_RETRY_DMA) { + if (IS_IDE_RETRY_DMA(error_status)) { if (error_status & IDE_RETRY_TRIM) { ide_restart_dma(s, IDE_DMA_TRIM); } else { ide_restart_dma(s, is_read ? IDE_DMA_READ : IDE_DMA_WRITE); } - } else if (error_status & IDE_RETRY_PIO) { + } else if (IS_IDE_RETRY_PIO(error_status)) { if (is_read) { ide_sector_read(s); } else { @@ -2547,15 +2548,11 @@ static void ide_restart_bh(void *opaque) } } else if (error_status & IDE_RETRY_FLUSH) { ide_flush_cache(s); + } else if (IS_IDE_RETRY_ATAPI(error_status)) { + assert(s->end_transfer_func == ide_atapi_cmd); + ide_atapi_dma_restart(s); } else { - /* - * We've not got any bits to tell us about ATAPI - but - * we do have the end_transfer_func that tells us what - * we're trying to do. - */ - if (s->end_transfer_func == ide_atapi_cmd) { - ide_atapi_dma_restart(s); - } + abort(); } } diff --git a/hw/ide/internal.h b/hw/ide/internal.h index dcd8627..2e845d0 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -508,11 +508,27 @@ struct IDEDevice { /* These are used for the error_status field of IDEBus */ #define IDE_RETRY_DMA 0x08 #define IDE_RETRY_PIO 0x10 +#define IDE_RETRY_ATAPI 0x20 /* reused IDE_RETRY_READ bit */ #define IDE_RETRY_READ 0x20 #define IDE_RETRY_FLUSH 0x40 #define IDE_RETRY_TRIM 0x80 #define IDE_RETRY_HBA 0x100 +#define IS_IDE_RETRY_DMA(_status) \ + ((_status) & IDE_RETRY_DMA) + +#define IS_IDE_RETRY_PIO(_status) \ + ((_status) & IDE_RETRY_PIO) + +/* + * The method of the IDE_RETRY_ATAPI determination is to use a previously + * impossible bit combination as a new status value. + */ +#define IS_IDE_RETRY_ATAPI(_status) \ + (((_status) & IDE_RETRY_ATAPI) && \ + !IS_IDE_RETRY_DMA(_status) && \ + !IS_IDE_RETRY_PIO(_status)) + /* * 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 @@ -604,4 +620,6 @@ void ide_bus_new(IDEBus *idebus, size_t idebus_size, DeviceState *dev, int bus_id, int max_units); IDEDevice *ide_create_drive(IDEBus *bus, int unit, DriveInfo *drive); +int ide_handle_rw_error(IDEState *s, int error, int op); + #endif /* HW_IDE_INTERNAL_H */