[{"id":1761401,"web_url":"http://patchwork.ozlabs.org/comment/1761401/","msgid":"","list_archive_url":null,"date":"2017-09-01T00:27:46","subject":"Re: [Qemu-devel] [PATCH v3 5/9] IDE: replace DEBUG_AIO with trace\n\tevents","submitter":{"id":70924,"url":"http://patchwork.ozlabs.org/api/people/70924/","name":"Philippe Mathieu-Daudé","email":"f4bug@amsat.org"},"content":"On 08/31/2017 09:14 PM, John Snow wrote:\n> Signed-off-by: John Snow \n\nReviewed-by: Philippe Mathieu-Daudé \n\n> ---\n> hw/ide/atapi.c | 5 +----\n> hw/ide/core.c | 24 +++++++++++++++++-------\n> hw/ide/trace-events | 3 +++\n> include/hw/ide/internal.h | 6 ++++--\n> 4 files changed, 25 insertions(+), 13 deletions(-)\n> \n> diff --git a/hw/ide/atapi.c b/hw/ide/atapi.c\n> index 9b84a1b..c0509c8 100644\n> --- a/hw/ide/atapi.c\n> +++ b/hw/ide/atapi.c\n> @@ -416,10 +416,7 @@ static void ide_atapi_cmd_read_dma_cb(void *opaque, int ret)\n> s->io_buffer_size = n * 2048;\n> data_offset = 0;\n> }\n> -#ifdef DEBUG_AIO\n> - printf(\"aio_read_cd: lba=%u n=%d\\n\", s->lba, n);\n> -#endif\n> -\n> + trace_ide_atapi_cmd_read_dma_cb_aio(s, s->lba, n);\n> s->bus->dma->iov.iov_base = (void *)(s->io_buffer + data_offset);\n> s->bus->dma->iov.iov_len = n * ATAPI_SECTOR_SIZE;\n> qemu_iovec_init_external(&s->bus->dma->qiov, &s->bus->dma->iov, 1);\n> diff --git a/hw/ide/core.c b/hw/ide/core.c\n> index 82a19b1..2cc2a08 100644\n> --- a/hw/ide/core.c\n> +++ b/hw/ide/core.c\n> @@ -58,6 +58,21 @@ static const int smart_attributes[][12] = {\n> { 190, 0x03, 0x00, 0x45, 0x45, 0x1f, 0x00, 0x1f, 0x1f, 0x00, 0x00, 0x32},\n> };\n> \n> +const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT] = {\n> + [IDE_DMA_READ] = \"DMA READ\",\n> + [IDE_DMA_WRITE] = \"DMA WRITE\",\n> + [IDE_DMA_TRIM] = \"DMA TRIM\",\n> + [IDE_DMA_ATAPI] = \"DMA ATAPI\"\n> +};\n> +\n> +static const char *IDE_DMA_CMD_str(enum ide_dma_cmd enval)\n> +{\n> + if (enval >= 0 && enval < IDE_DMA__COUNT) {\n> + return IDE_DMA_CMD_lookup[enval];\n> + }\n> + return \"DMA UNKNOWN CMD\";\n> +}\n> +\n> static void ide_dummy_transfer_stop(IDEState *s);\n> \n> static void padstr(char *str, const char *src, int len)\n> @@ -860,10 +875,7 @@ static void ide_dma_cb(void *opaque, int ret)\n> goto eot;\n> }\n> \n> -#ifdef DEBUG_AIO\n> - printf(\"ide_dma_cb: sector_num=%\" PRId64 \" n=%d, cmd_cmd=%d\\n\",\n> - sector_num, n, s->dma_cmd);\n> -#endif\n> + trace_ide_dma_cb(s, sector_num, n, IDE_DMA_CMD_str(s->dma_cmd));\n> \n> if ((s->dma_cmd == IDE_DMA_READ || s->dma_cmd == IDE_DMA_WRITE) &&\n> !ide_sect_range_ok(s, sector_num, n)) {\n> @@ -2391,9 +2403,7 @@ void ide_bus_reset(IDEBus *bus)\n> \n> /* pending async DMA */\n> if (bus->dma->aiocb) {\n> -#ifdef DEBUG_AIO\n> - printf(\"aio_cancel\\n\");\n> -#endif\n> + trace_ide_bus_reset_aio();\n> blk_aio_cancel(bus->dma->aiocb);\n> bus->dma->aiocb = NULL;\n> }\n> diff --git a/hw/ide/trace-events b/hw/ide/trace-events\n> index 8c79a6c..cc8949c 100644\n> --- a/hw/ide/trace-events\n> +++ b/hw/ide/trace-events\n> @@ -18,6 +18,8 @@ ide_cancel_dma_sync_remaining(void) \"draining all remaining requests\"\n> ide_sector_read(int64_t sector_num, int nsectors) \"sector=%\"PRId64\" nsectors=%d\"\n> ide_sector_write(int64_t sector_num, int nsectors) \"sector=%\"PRId64\" nsectors=%d\"\n> ide_reset(void *s) \"IDEstate %p\"\n> +ide_bus_reset_aio(void) \"aio_cancel\"\n> +ide_dma_cb(void *s, int64_t sector_num, int n, const char *dma) \"IDEState %p; sector_num=%\"PRId64\" n=%d cmd=%s\"\n> \n> # BMDMA HBAs:\n> \n> @@ -51,5 +53,6 @@ ide_atapi_cmd_reply_end_new(void *s, int status) \"IDEState: %p; new transfer sta\n> ide_atapi_cmd_check_status(void *s) \"IDEState: %p\"\n> ide_atapi_cmd_read(void *s, const char *method, int lba, int nb_sectors) \"IDEState: %p; read %s: LBA=%d nb_sectors=%d\"\n> ide_atapi_cmd(void *s, uint8_t cmd) \"IDEState: %p; cmd: 0x%02x\"\n> +ide_atapi_cmd_read_dma_cb_aio(void *s, int lba, int n) \"IDEState: %p; aio read: lba=%d n=%d\"\n> # Warning: Verbose\n> ide_atapi_cmd_packet(void *s, uint16_t limit, const char *packet) \"IDEState: %p; limit=0x%x packet: %s\"\n> diff --git a/include/hw/ide/internal.h b/include/hw/ide/internal.h\n> index 74efe8a..db9fde0 100644\n> --- a/include/hw/ide/internal.h\n> +++ b/include/hw/ide/internal.h\n> @@ -14,7 +14,6 @@\n> #include \"block/scsi.h\"\n> \n> /* debug IDE devices */\n> -//#define DEBUG_AIO\n> #define USE_DMA_CDROM\n> \n> typedef struct IDEBus IDEBus;\n> @@ -333,12 +332,15 @@ struct unreported_events {\n> };\n> \n> enum ide_dma_cmd {\n> - IDE_DMA_READ,\n> + IDE_DMA_READ = 0,\n> IDE_DMA_WRITE,\n> IDE_DMA_TRIM,\n> IDE_DMA_ATAPI,\n> + IDE_DMA__COUNT\n> };\n> \n> +extern const char *IDE_DMA_CMD_lookup[IDE_DMA__COUNT];\n> +\n> #define ide_cmd_is_read(s) \\\n> \t((s)->dma_cmd == IDE_DMA_READ)\n> \n>","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"P7+oBw+n\"; dkim-atps=neutral"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xk0TV27J7z9s81\n\tfor ;\n\tFri, 1 Sep 2017 10:28:30 +1000 (AEST)","from localhost ([::1]:35103 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dnZoy-0000l6-Cy\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 20:28:28 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:54463)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from )\n\tid 1dnZoS-0000jQ-DE\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 20:27:57 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from )\n\tid 1dnZoN-0001gQ-A1\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 20:27:56 -0400","from mail-qk0-x241.google.com ([2607:f8b0:400d:c09::241]:36806)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1dnZoN-0001g1-3f; Thu, 31 Aug 2017 20:27:51 -0400","by mail-qk0-x241.google.com with SMTP id l65so889700qkc.3;\n\tThu, 31 Aug 2017 17:27:51 -0700 (PDT)","from [192.168.1.10] ([181.93.89.178])\n\tby smtp.gmail.com with ESMTPSA id\n\tm83sm6315514qki.26.2017.08.31.17.27.48\n\t(version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128);\n\tThu, 31 Aug 2017 17:27:49 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=sender:subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=V88x81/Xbj3Rraz7OQgu2e3Y5d1JwTsXucbe5CIMly0=;\n\tb=P7+oBw+nframJFx5dv3gjnleZEIxnKm8lnVkIwvGtsQ40v1l5g7fGInImwhuOMYMIn\n\t7PQjIJQrjEhT3d9Azg0/a/G+9rPs2AK3a4/FaLMmmlZnMFFPv17tJSqCTnTHZdMskbmm\n\ttytX2Htcjx+ewG0DPKPIv5NcfnU4fQPC5Q71F9BJt3Dwz9yHdbjTc1w6QgPmzPVCLSOU\n\t1gCU84KTWd+lvc7k/a9E6lP3xOCugIwHJUDD+tbScUar1K5uSfZ7ZuYDuE17wRkMZWg5\n\tE7mu72F2XyyRqXL35xbJ0mgVoL/KNyM+JQSsVvOHPoUaeaEWZWjgK2btftDCqeb0xzws\n\tuA/A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:sender:subject:to:cc:references:from:message-id\n\t:date:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=V88x81/Xbj3Rraz7OQgu2e3Y5d1JwTsXucbe5CIMly0=;\n\tb=ipe1Z3XeXq2pckiQ0z6k64zVaa8KkexUd1GbYtPj4BMQa9H2dQsdG/Itu48AslWYUH\n\tlAhjsb/K7UIGFP3lC/3xlLNbvbgvLSjcORmLUm8QwS6b2Wxzh2KFnATRH29lRrnG4q0C\n\tYIEuTMAzNE+Zc6TrD+vf12vcuZVCIUlx9a9Nh4STfvx9duEWNQ6I1zsyQKClrJBJYUCB\n\tDnq2P9Hnd77YcXmx7oOVb9B5Ads+xQwII8dnpx11KpRc2Jb/QSPE+0uLDnJ8NRKKC/GV\n\tw8xM6ANjZek9BsjmY2L2sVYZpyOKmGyBy4VoegY8iMhi9G2oARCffSa/k904sk/n6ZXE\n\tIhCA==","X-Gm-Message-State":"AHPjjUjpLb2IdxNf5aEalaIdn6c8vqrjUKwQEeoddloHhEucdsNorCuu\n\tXt3bK+kE4hB42Q==","X-Google-Smtp-Source":"ADKCNb5IFoCg1dx6II1AZiQF3rnljUACAymwwHG0/9LSK/d1oHtCiCae9EmowMxV0TkDBq7WkyFBCA==","X-Received":"by 10.55.118.132 with SMTP id r126mr309954qkc.169.1504225670398; \n\tThu, 31 Aug 2017 17:27:50 -0700 (PDT)","To":"John Snow , qemu-block@nongnu.org","References":"<20170901001502.29915-1-jsnow@redhat.com>\n\t<20170901001502.29915-6-jsnow@redhat.com>","From":"=?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= ","Message-ID":"","Date":"Thu, 31 Aug 2017 21:27:46 -0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.3.0","MIME-Version":"1.0","In-Reply-To":"<20170901001502.29915-6-jsnow@redhat.com>","Content-Type":"text/plain; charset=utf-8; format=flowed","Content-Language":"en-US","Content-Transfer-Encoding":"8bit","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2607:f8b0:400d:c09::241","Subject":"Re: [Qemu-devel] [PATCH v3 5/9] IDE: replace DEBUG_AIO with trace\n\tevents","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, stefanha@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1761413,"web_url":"http://patchwork.ozlabs.org/comment/1761413/","msgid":"<8234fbad-76d8-bb9d-6972-871578fca59e@redhat.com>","list_archive_url":null,"date":"2017-09-01T00:40:28","subject":"Re: [Qemu-devel] [PATCH v3 5/9] IDE: replace DEBUG_AIO with trace\n\tevents","submitter":{"id":64343,"url":"http://patchwork.ozlabs.org/api/people/64343/","name":"John Snow","email":"jsnow@redhat.com"},"content":"On 08/31/2017 08:27 PM, Philippe Mathieu-Daudé wrote:\n> On 08/31/2017 09:14 PM, John Snow wrote:\n>> Signed-off-by: John Snow \n> \n> Reviewed-by: Philippe Mathieu-Daudé \n> \n\nTY, and btw the reasoning behind what I went with:\n\nSince s->dma_cmd is itself a type `enum ide_dma_cmd` then by definition\ns->dma_cmd (even if corrupted or wrong) must fit inside the width of\nthat type. Performing the range checking in the getter is adequate for\nprotecting the table in this case, if I understand Laszlo's rationales\ncorrectly.\n\nIn case case of the AHCI table, I do not use a getter since the IRQBIT\nproperty is passed in statically for each instance and shouldn't have a\nchance to get out-of-spec. (If it does, something MASSIVELY bad has\ncorrupted everything, and then we've got worse problems.)\n\nThanks,\nJohn","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=pass (mailfrom) smtp.mailfrom=nongnu.org\n\t(client-ip=2001:4830:134:3::11; helo=lists.gnu.org;\n\tenvelope-from=qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org;\n\treceiver=)","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx01.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=jsnow@redhat.com"],"Received":["from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11])\n\t(using TLSv1 with cipher AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby ozlabs.org (Postfix) with ESMTPS id 3xk0m10XDQz9s7F\n\tfor ;\n\tFri, 1 Sep 2017 10:41:05 +1000 (AEST)","from localhost ([::1]:35635 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1dna19-0005ma-7t\n\tfor incoming@patchwork.ozlabs.org; Thu, 31 Aug 2017 20:41:03 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:56119)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1dna0l-0005jw-PH\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 20:40:40 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1dna0k-0007V9-JO\n\tfor qemu-devel@nongnu.org; Thu, 31 Aug 2017 20:40:39 -0400","from mx1.redhat.com ([209.132.183.28]:39462)\n\tby eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32)\n\t(Exim 4.71) (envelope-from )\n\tid 1dna0e-0007PK-IY; Thu, 31 Aug 2017 20:40:32 -0400","from smtp.corp.redhat.com\n\t(int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16])\n\t(using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits))\n\t(No client certificate requested)\n\tby mx1.redhat.com (Postfix) with ESMTPS id 9BFBE81E1F;\n\tFri, 1 Sep 2017 00:40:31 +0000 (UTC)","from [10.18.17.130] (dhcp-17-130.bos.redhat.com [10.18.17.130])\n\tby smtp.corp.redhat.com (Postfix) with ESMTP id 070E1A01E0;\n\tFri, 1 Sep 2017 00:40:29 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 9BFBE81E1F","To":"=?utf-8?q?Philippe_Mathieu-Daud=C3=A9?= ,\n\tqemu-block@nongnu.org","References":"<20170901001502.29915-1-jsnow@redhat.com>\n\t<20170901001502.29915-6-jsnow@redhat.com>\n\t","From":"John Snow ","Message-ID":"<8234fbad-76d8-bb9d-6972-871578fca59e@redhat.com>","Date":"Thu, 31 Aug 2017 20:40:28 -0400","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101\n\tThunderbird/52.2.1","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-US","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.16","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.25]);\n\tFri, 01 Sep 2017 00:40:31 +0000 (UTC)","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic]\n\t[fuzzy]","X-Received-From":"209.132.183.28","Subject":"Re: [Qemu-devel] [PATCH v3 5/9] IDE: replace DEBUG_AIO with trace\n\tevents","X-BeenThere":"qemu-devel@nongnu.org","X-Mailman-Version":"2.1.21","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"qemu-devel@nongnu.org, stefanha@redhat.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]