From patchwork Wed Jun 15 14:03:06 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 100537 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [140.186.70.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id E6DF7B6FA0 for ; Thu, 16 Jun 2011 00:51:47 +1000 (EST) Received: from localhost ([::1]:53438 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QWrRW-0007fp-GT for incoming@patchwork.ozlabs.org; Wed, 15 Jun 2011 10:51:42 -0400 Received: from eggs.gnu.org ([140.186.70.92]:35102) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QWqe6-0003ji-Qs for qemu-devel@nongnu.org; Wed, 15 Jun 2011 10:00:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QWqe1-0005sU-88 for qemu-devel@nongnu.org; Wed, 15 Jun 2011 10:00:38 -0400 Received: from mx1.redhat.com ([209.132.183.28]:22426) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QWqe0-0005s3-E1 for qemu-devel@nongnu.org; Wed, 15 Jun 2011 10:00:32 -0400 Received: from int-mx02.intmail.prod.int.phx2.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p5FE0Uo5006050 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Wed, 15 Jun 2011 10:00:30 -0400 Received: from dhcp-5-188.str.redhat.com (dhcp-5-175.str.redhat.com [10.32.5.175]) by int-mx02.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p5FE0Krf024634; Wed, 15 Jun 2011 10:00:29 -0400 From: Kevin Wolf To: anthony@codemonkey.ws Date: Wed, 15 Jun 2011 16:03:06 +0200 Message-Id: <1308146593-19842-8-git-send-email-kwolf@redhat.com> In-Reply-To: <1308146593-19842-1-git-send-email-kwolf@redhat.com> References: <1308146593-19842-1-git-send-email-kwolf@redhat.com> X-Scanned-By: MIMEDefang 2.67 on 10.5.11.12 X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 209.132.183.28 Cc: kwolf@redhat.com, qemu-devel@nongnu.org Subject: [Qemu-devel] [PATCH 07/14] ide: Split error status from status register 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+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org When adding the werror=stop mode, some flags were added to s->status which are used to determine what kind of operation should be restarted when the VM is continued. Unfortunately, it turns out that s->status is in fact a device register and as such is visible to the guest (some of the abused bits are even writable for the guest). For migration we keep on using the old VMState field (renamed to migration_compat_status) if the status register doesn't use any of the previously abused bits. If it does, we use a subsection with a clean copy of the status register. The error status is always sent in a subsection if there is any error. It can't use the old field because errors happen even without PCI. Signed-off-by: Kevin Wolf --- hw/ide/core.c | 28 +++++++++++++++++++- hw/ide/internal.h | 8 ++++++ hw/ide/pci.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++----- hw/ide/pci.h | 4 +++ 4 files changed, 105 insertions(+), 8 deletions(-) diff --git a/hw/ide/core.c b/hw/ide/core.c index 95beb17..da250ac 100644 --- a/hw/ide/core.c +++ b/hw/ide/core.c @@ -446,7 +446,7 @@ static int ide_handle_rw_error(IDEState *s, int error, int op) if ((error == ENOSPC && action == BLOCK_ERR_STOP_ENOSPC) || action == BLOCK_ERR_STOP_ANY) { s->bus->dma->ops->set_unit(s->bus->dma, s->unit); - s->bus->dma->ops->add_status(s->bus->dma, op); + s->bus->error_status = op; bdrv_mon_event(s->bs, BDRV_ACTION_STOP, is_read); vm_stop(VMSTOP_DISKFULL); } else { @@ -1847,6 +1847,13 @@ static bool ide_atapi_gesn_needed(void *opaque) return s->events.new_media || s->events.eject_request; } +static bool ide_error_needed(void *opaque) +{ + IDEBus *bus = opaque; + + return (bus->error_status != 0); +} + /* Fields for GET_EVENT_STATUS_NOTIFICATION ATAPI command */ const VMStateDescription vmstate_ide_atapi_gesn_state = { .name ="ide_drive/atapi/gesn_state", @@ -1921,6 +1928,17 @@ const VMStateDescription vmstate_ide_drive = { } }; +const VMStateDescription vmstate_ide_error_status = { + .name ="ide_bus/error", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField []) { + VMSTATE_INT32(error_status, IDEBus), + VMSTATE_END_OF_LIST() + } +}; + const VMStateDescription vmstate_ide_bus = { .name = "ide_bus", .version_id = 1, @@ -1930,6 +1948,14 @@ const VMStateDescription vmstate_ide_bus = { VMSTATE_UINT8(cmd, IDEBus), VMSTATE_UINT8(unit, IDEBus), VMSTATE_END_OF_LIST() + }, + .subsections = (VMStateSubsection []) { + { + .vmsd = &vmstate_ide_error_status, + .needed = ide_error_needed, + }, { + /* empty */ + } } }; diff --git a/hw/ide/internal.h b/hw/ide/internal.h index c2b35ec..8d18cc3 100644 --- a/hw/ide/internal.h +++ b/hw/ide/internal.h @@ -486,6 +486,8 @@ struct IDEBus { uint8_t unit; uint8_t cmd; qemu_irq irq; + + int error_status; }; struct IDEDevice { @@ -505,11 +507,17 @@ struct IDEDeviceInfo { #define BM_STATUS_DMAING 0x01 #define BM_STATUS_ERROR 0x02 #define BM_STATUS_INT 0x04 + +/* FIXME These are not status register bits */ #define BM_STATUS_DMA_RETRY 0x08 #define BM_STATUS_PIO_RETRY 0x10 #define BM_STATUS_RETRY_READ 0x20 #define BM_STATUS_RETRY_FLUSH 0x40 +#define BM_MIGRATION_COMPAT_STATUS_BITS \ + (BM_STATUS_DMA_RETRY | BM_STATUS_PIO_RETRY | \ + BM_STATUS_RETRY_READ | BM_STATUS_RETRY_FLUSH) + #define BM_CMD_START 0x01 #define BM_CMD_READ 0x08 diff --git a/hw/ide/pci.c b/hw/ide/pci.c index a4726ad..7fa32bd 100644 --- a/hw/ide/pci.c +++ b/hw/ide/pci.c @@ -183,27 +183,33 @@ static void bmdma_restart_dma(BMDMAState *bm, int is_read) bmdma_start_dma(&bm->dma, s, bm->dma_cb); } +/* TODO This should be common IDE code */ static void bmdma_restart_bh(void *opaque) { BMDMAState *bm = opaque; + IDEBus *bus = bm->bus; int is_read; qemu_bh_delete(bm->bh); bm->bh = NULL; - is_read = !!(bm->status & BM_STATUS_RETRY_READ); + if (bm->unit == (uint8_t) -1) { + return; + } - if (bm->status & BM_STATUS_DMA_RETRY) { - bm->status &= ~(BM_STATUS_DMA_RETRY | BM_STATUS_RETRY_READ); + is_read = !!(bus->error_status & BM_STATUS_RETRY_READ); + + if (bus->error_status & BM_STATUS_DMA_RETRY) { + bus->error_status &= ~(BM_STATUS_DMA_RETRY | BM_STATUS_RETRY_READ); bmdma_restart_dma(bm, is_read); - } else if (bm->status & BM_STATUS_PIO_RETRY) { - bm->status &= ~(BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ); + } else if (bus->error_status & BM_STATUS_PIO_RETRY) { + bus->error_status &= ~(BM_STATUS_PIO_RETRY | BM_STATUS_RETRY_READ); if (is_read) { ide_sector_read(bmdma_active_if(bm)); } else { ide_sector_write(bmdma_active_if(bm)); } - } else if (bm->status & BM_STATUS_RETRY_FLUSH) { + } else if (bus->error_status & BM_STATUS_RETRY_FLUSH) { ide_flush_cache(bmdma_active_if(bm)); } } @@ -351,6 +357,43 @@ static bool ide_bmdma_current_needed(void *opaque) return (bm->cur_prd_len != 0); } +static bool ide_bmdma_status_needed(void *opaque) +{ + BMDMAState *bm = opaque; + + /* Older versions abused some bits in the status register for internal + * error state. If any of these bits are set, we must add a subsection to + * transfer the real status register */ + uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS; + + return ((bm->status & abused_bits) != 0); +} + +static void ide_bmdma_pre_save(void *opaque) +{ + BMDMAState *bm = opaque; + uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS; + + bm->migration_compat_status = + (bm->status & ~abused_bits) | (bm->bus->error_status & abused_bits); +} + +/* This function accesses bm->bus->error_status which is loaded only after + * BMDMA itself. This is why the function is called from ide_pci_post_load + * instead of being registered with VMState where it would run too early. */ +static int ide_bmdma_post_load(void *opaque, int version_id) +{ + BMDMAState *bm = opaque; + uint8_t abused_bits = BM_MIGRATION_COMPAT_STATUS_BITS; + + if (bm->status == 0) { + bm->status = bm->migration_compat_status & ~abused_bits; + bm->bus->error_status |= bm->migration_compat_status & abused_bits; + } + + return 0; +} + static const VMStateDescription vmstate_bmdma_current = { .name = "ide bmdma_current", .version_id = 1, @@ -365,15 +408,26 @@ static const VMStateDescription vmstate_bmdma_current = { } }; +const VMStateDescription vmstate_bmdma_status = { + .name ="ide bmdma/status", + .version_id = 1, + .minimum_version_id = 1, + .minimum_version_id_old = 1, + .fields = (VMStateField []) { + VMSTATE_UINT8(status, BMDMAState), + VMSTATE_END_OF_LIST() + } +}; static const VMStateDescription vmstate_bmdma = { .name = "ide bmdma", .version_id = 3, .minimum_version_id = 0, .minimum_version_id_old = 0, + .pre_save = ide_bmdma_pre_save, .fields = (VMStateField []) { VMSTATE_UINT8(cmd, BMDMAState), - VMSTATE_UINT8(status, BMDMAState), + VMSTATE_UINT8(migration_compat_status, BMDMAState), VMSTATE_UINT32(addr, BMDMAState), VMSTATE_INT64(sector_num, BMDMAState), VMSTATE_UINT32(nsector, BMDMAState), @@ -385,6 +439,9 @@ static const VMStateDescription vmstate_bmdma = { .vmsd = &vmstate_bmdma_current, .needed = ide_bmdma_current_needed, }, { + .vmsd = &vmstate_bmdma_status, + .needed = ide_bmdma_status_needed, + }, { /* empty */ } } @@ -399,7 +456,9 @@ static int ide_pci_post_load(void *opaque, int version_id) /* current versions always store 0/1, but older version stored bigger values. We only need last bit */ d->bmdma[i].unit &= 1; + ide_bmdma_post_load(&d->bmdma[i], -1); } + return 0; } diff --git a/hw/ide/pci.h b/hw/ide/pci.h index cd72cba..b4f3691 100644 --- a/hw/ide/pci.h +++ b/hw/ide/pci.h @@ -22,6 +22,10 @@ typedef struct BMDMAState { IORange addr_ioport; QEMUBH *bh; qemu_irq irq; + + /* Bit 0-2 and 7: BM status register + * Bit 3-6: bus->error_status */ + uint8_t migration_compat_status; } BMDMAState; typedef struct PCIIDEState {