From patchwork Mon Sep 24 18:33:12 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Peter Maydell X-Patchwork-Id: 11679 Return-Path: X-Original-To: patchwork@peony.canonical.com Delivered-To: patchwork@peony.canonical.com Received: from fiordland.canonical.com (fiordland.canonical.com [91.189.94.145]) by peony.canonical.com (Postfix) with ESMTP id 94C5E23E57 for ; Mon, 24 Sep 2012 18:33:20 +0000 (UTC) Received: from mail-ie0-f180.google.com (mail-ie0-f180.google.com [209.85.223.180]) by fiordland.canonical.com (Postfix) with ESMTP id 0AEA6A18F31 for ; Mon, 24 Sep 2012 18:33:19 +0000 (UTC) Received: by ieje10 with SMTP id e10so10800256iej.11 for ; Mon, 24 Sep 2012 11:33:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-forwarded-to:x-forwarded-for:delivered-to:received-spf:from:to:cc :subject:date:message-id:x-mailer:in-reply-to:references :x-gm-message-state; bh=rBr4xOcinVQ/kEb/Jl1Q/0ItQiHny0BOs4oWP84yom4=; b=e6VaEGY7RbdtW9aFqmklF0OzusEkQbHoek83Q05l0Ja331492KqoyfvHLczizWWpcx /nvf7fOGvLOOS/DPJMpTH+z3lzeqawWkKtuqKMSkO6Wsj92HYNKF9tfps83NmHKCc4nA fiZcIenruKR5GIDrObTC6dRN+PGFHpX+ReiThndjKVFWywWHGpV43YjCy/BoXlZdzcoC i/wc6aMOqsQMd3LI2m7vbkFPvjQ4FfORgruq5DAli4lMmEEly8R5MDbmHmkzwxynQmVI 4AWZf/5NcxA79QZW88+w6nH2EBBNYu3Rbtuv4aEQsBObHs1XQYXrkv5NhSm4H25RArsD lSZA== Received: by 10.50.7.212 with SMTP id l20mr6021884iga.43.1348511599330; Mon, 24 Sep 2012 11:33:19 -0700 (PDT) X-Forwarded-To: linaro-patchwork@canonical.com X-Forwarded-For: patch@linaro.org linaro-patchwork@canonical.com Delivered-To: patches@linaro.org Received: by 10.50.184.232 with SMTP id ex8csp257079igc; Mon, 24 Sep 2012 11:33:18 -0700 (PDT) Received: by 10.180.73.76 with SMTP id j12mr16123774wiv.11.1348511597318; Mon, 24 Sep 2012 11:33:17 -0700 (PDT) Received: from mnementh.archaic.org.uk (1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.d.1.0.0.b.8.0.1.0.0.2.ip6.arpa. [2001:8b0:1d0::1]) by mx.google.com with ESMTPS id g6si19615955wiy.38.2012.09.24.11.33.16 (version=TLSv1/SSLv3 cipher=OTHER); Mon, 24 Sep 2012 11:33:17 -0700 (PDT) Received-SPF: neutral (google.com: 2001:8b0:1d0::1 is neither permitted nor denied by best guess record for domain of pm215@archaic.org.uk) client-ip=2001:8b0:1d0::1; Authentication-Results: mx.google.com; spf=neutral (google.com: 2001:8b0:1d0::1 is neither permitted nor denied by best guess record for domain of pm215@archaic.org.uk) smtp.mail=pm215@archaic.org.uk Received: from pm215 by mnementh.archaic.org.uk with local (Exim 4.72) (envelope-from ) id 1TGDT1-0003TQ-M2; Mon, 24 Sep 2012 19:33:15 +0100 From: Peter Maydell To: qemu-devel@nongnu.org Cc: patches@linaro.org Subject: [PATCH 1/4] hw/ds1338: Fix mishandling of register pointer Date: Mon, 24 Sep 2012 19:33:12 +0100 Message-Id: <1348511595-13327-2-git-send-email-peter.maydell@linaro.org> X-Mailer: git-send-email 1.7.2.5 In-Reply-To: <1348511595-13327-1-git-send-email-peter.maydell@linaro.org> References: <1348511595-13327-1-git-send-email-peter.maydell@linaro.org> X-Gm-Message-State: ALoCoQmeR4/bmYJTnKhcSAM6bsp1SPbE6zZsmYWzk8ewWD8SvdWHjIxit0JXuQwJY+sxHhqgtSfJ Correct several deficiencies in the handling of the register pointer: * it should wrap around after 0x3f, not 0xff * guard against the caller handing us an out of range pointer (on h/w this can never happen, because only a 7 bit value is transferred over the I2C bus) * there was confusion over whether nvram[] holds only the 56 bytes of guest-accessible NVRAM, or also the secondary registers which hold the value of the clock captured at the start of a multibyte read. Correct to consistently be the latter, by fixing the array size and the offset used for NVRAM writes. * ds1338_send was attempting to use 'data' as both the data and the register offset simultaneously, which meant that writes to any register were broken; fix to use the register pointer. Signed-off-by: Peter Maydell --- hw/ds1338.c | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/hw/ds1338.c b/hw/ds1338.c index d590d9c..be68140 100644 --- a/hw/ds1338.c +++ b/hw/ds1338.c @@ -12,11 +12,16 @@ #include "i2c.h" +/* Size of NVRAM including both the user-accessible area and the + * secondary register area. + */ +#define NVRAM_SIZE 64 + typedef struct { I2CSlave i2c; time_t offset; struct tm now; - uint8_t nvram[56]; + uint8_t nvram[NVRAM_SIZE]; int ptr; int addr_byte; } DS1338State; @@ -57,7 +62,7 @@ static int ds1338_recv(I2CSlave *i2c) uint8_t res; res = s->nvram[s->ptr]; - s->ptr = (s->ptr + 1) & 0xff; + s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1); return res; } @@ -65,14 +70,13 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data) { DS1338State *s = FROM_I2C_SLAVE(DS1338State, i2c); if (s->addr_byte) { - s->ptr = data; + s->ptr = data & (NVRAM_SIZE - 1); s->addr_byte = 0; return 0; } - s->nvram[s->ptr - 8] = data; - if (data < 8) { + if (s->ptr < 8) { qemu_get_timedate(&s->now, s->offset); - switch(data) { + switch(s->ptr) { case 0: /* TODO: Implement CH (stop) bit. */ s->now.tm_sec = from_bcd(data & 0x7f); @@ -109,8 +113,10 @@ static int ds1338_send(I2CSlave *i2c, uint8_t data) break; } s->offset = qemu_timedate_diff(&s->now); + } else { + s->nvram[s->ptr] = data; } - s->ptr = (s->ptr + 1) & 0xff; + s->ptr = (s->ptr + 1) & (NVRAM_SIZE - 1); return 0; }