[{"id":1783124,"web_url":"http://patchwork.ozlabs.org/comment/1783124/","msgid":"","list_archive_url":null,"date":"2017-10-09T18:17:06","subject":"Re: [Qemu-devel] [RFC 0/3] vITS Reset","submitter":{"id":5111,"url":"http://patchwork.ozlabs.org/api/people/5111/","name":"Peter Maydell","email":"peter.maydell@linaro.org"},"content":"On 27 September 2017 at 15:56, Eric Auger wrote:\n> At the moment the ITS is not properly reset. On System reset or\n> reboot, previous ITS register values and caches are left\n> unchanged. Some of the registers might point to some guest RAM\n> tables which are not provisionned. This leads to state\n> inconsistencies that are detected by the kernel save/restore\n> code. And eventually this may cause qemu abort on source or\n> destination.\n>\n> The 1st patch, suggested to be cc'ed stable proposes to remove\n> the abort in case of table save/restore failure. This is\n> definitively not ideal but looks the most reasonable until we\n> get a proper way to reset the ITS. Still a message is emitted\n> to report the save/restore did not happen correctly.\n>\n> Subsequent patches add the support of explicit reset using\n> a new kvm device group/attribute combo. The associated kernel\n> series is not upstream [1], hence the RFC.\n>\n> ITS specification is not very clear about reset. There is no\n> reset wire. Some register fields are documented to have\n> architecturally defined reset values and we use those here:\n> Most importantly the Valid bit of GITS_CBASER and GITS_BASER\n> are cleared and the GITS_CTLR.Enabled bit is cleared as well.\n\nThe ITS is a device, not part of the CPU, so its reset is\nimplementation-defined but typically via some signal that's\ncontrolled by the SoC (this is no different to the other\nparts of the GICv3 which aren't denoted as being in the\nreset domain of the CPU). For instance if you look at the TRM\nfor the GIC-500 it has a single 'resetn' reset signal which\nresets all of the GIC/ITS apart from the CPU interface parts:\nhttp://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100336_0002_01_en/index.html\n\nThe GICv3 spec 6.6 indicates what is supposed to happen to\nthe ITS on powerup. For QEMU's purposes reset is generally\nconsidered to be a cold reset and we should always reset our\nstate to the same thing it was when QEMU first started.\n\nIt's not clear to me why we need a new KVM device attribute\nfor doing ITS reset. The usual approach for this is:\n * system reset causes QEMU's device model reset code\n to reset state structure values to whatever their\n reset value is\n * we write those values up into the kernel, which is\n sufficient to reset it\n\nWhat goes wrong with that in the case of the ITS?\nIn particular, if GITS_CTLR.Enabled is 0 then I think the\nkernel should not be trying to read guest memory tables.\n\nthanks\n-- PMM","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\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"CWue6bzh\"; 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 3y9pQH3K4Cz9t64\n\tfor ;\n\tTue, 10 Oct 2017 05:18:14 +1100 (AEDT)","from localhost ([::1]:59210 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1e1ccv-0008D5-6k\n\tfor incoming@patchwork.ozlabs.org; Mon, 09 Oct 2017 14:18:05 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:37657)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1e1ccL-00082k-Tb\n\tfor qemu-devel@nongnu.org; Mon, 09 Oct 2017 14:17:31 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1e1ccK-0004ZH-Ml\n\tfor qemu-devel@nongnu.org; Mon, 09 Oct 2017 14:17:29 -0400","from mail-wm0-x236.google.com ([2a00:1450:400c:c09::236]:55089)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1e1ccK-0004Yd-Fw\n\tfor qemu-devel@nongnu.org; Mon, 09 Oct 2017 14:17:28 -0400","by mail-wm0-x236.google.com with SMTP id i124so25795444wmf.3\n\tfor ; Mon, 09 Oct 2017 11:17:28 -0700 (PDT)","by 10.223.128.207 with HTTP; Mon, 9 Oct 2017 11:17:06 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=K3ZOiJealwATcwFs6Kt4Pbom0yLIzh5J3HyGrMd6c0c=;\n\tb=CWue6bzhPmpjsmPQwheOZ7x7LDe7kkbXG7xLRjh9i41CkG/6uSI16t6LeVllPmtsh3\n\tPGKzfX9Dv8xis6szmC+QtFN8Vpy0Y3pV30MH4xu9wnl/xF40vvd9Xq9nN6IGKUiTWdXi\n\tTtteoYLYQ/b32BOwm4VvTUj9FGiaBSb344YR8=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=K3ZOiJealwATcwFs6Kt4Pbom0yLIzh5J3HyGrMd6c0c=;\n\tb=gP+UyXAEIMXn2NUBI7sZOVetaQn1mrS85U7PYUINa4X8zw2buUGL6XD9Ke7Kt7LzLz\n\t45DkEp6lEXwgpVQp7DlLc04JNQdBIqNFtpaBDhxqk4nmRWfKTCcdZa+9xAPjXx3UB+Ns\n\tOVa0+ArjqxXH3A4dLSJHWb31hAJB0cKjC9pcNZ3KMncxAVknxpgPznMBjejkCdaBOLQl\n\t7OZ4d0L+cyOXfWfrEPyt7VXopUkVhPjPmhclSNJMzF7mODO5IBOdwNG1BSyly+chBCf9\n\tiqKWOr3WMZq3xsXAtmor/nM8ZUoLX60/sMeSE6KD5v28GIXLe+i0+/AurNk1ZZzFUG5c\n\tbTuA==","X-Gm-Message-State":"AMCzsaUip/rzM9lWQ4asOcgCstNIHiGnMdsQyeEYbjOKCeB6HKqPoyYK\n\tntMibxZhK0gugIbGPjMgnjIvz0zo53iULH9M34RcIg==","X-Google-Smtp-Source":"AOwi7QCkqTTpW8WvafLqIHwx3N50fHnQFzqdioJbK43cIfeeSe2Q+GA9jLcO0vk14PCaUyr95ADjk53uA8u8KyW5n4c=","X-Received":"by 10.223.162.152 with SMTP id\n\ts24mr11541585wra.173.1507573047260; \n\tMon, 09 Oct 2017 11:17:27 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1506524205-20763-1-git-send-email-eric.auger@redhat.com>","References":"<1506524205-20763-1-git-send-email-eric.auger@redhat.com>","From":"Peter Maydell ","Date":"Mon, 9 Oct 2017 19:17:06 +0100","Message-ID":"","To":"Eric Auger ","Content-Type":"text/plain; charset=\"UTF-8\"","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2a00:1450:400c:c09::236","Subject":"Re: [Qemu-devel] [RFC 0/3] vITS Reset","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":"Wei Huang , Andrew Jones ,\n\tVijay Kilari ,\n\tJuan Quintela , \n\tQEMU Developers ,\n\t\"Dr. David Alan Gilbert\" ,\n\tqemu-arm , wu.wubin@huawei.com,\n\twanghaibin.wang@huawei.com,\n\tChristoffer Dall , eric.auger.pro@gmail.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1784768,"web_url":"http://patchwork.ozlabs.org/comment/1784768/","msgid":"<73b7916a-4bb3-3913-ed6f-23e5c3e4fa23@redhat.com>","list_archive_url":null,"date":"2017-10-11T16:06:18","subject":"Re: [Qemu-devel] [RFC 0/3] vITS Reset","submitter":{"id":69187,"url":"http://patchwork.ozlabs.org/api/people/69187/","name":"Eric Auger","email":"eric.auger@redhat.com"},"content":"Hi Peter,\n\nOn 09/10/2017 20:17, Peter Maydell wrote:\n> On 27 September 2017 at 15:56, Eric Auger wrote:\n>> At the moment the ITS is not properly reset. On System reset or\n>> reboot, previous ITS register values and caches are left\n>> unchanged. Some of the registers might point to some guest RAM\n>> tables which are not provisionned. This leads to state\n>> inconsistencies that are detected by the kernel save/restore\n>> code. And eventually this may cause qemu abort on source or\n>> destination.\n>>\n>> The 1st patch, suggested to be cc'ed stable proposes to remove\n>> the abort in case of table save/restore failure. This is\n>> definitively not ideal but looks the most reasonable until we\n>> get a proper way to reset the ITS. Still a message is emitted\n>> to report the save/restore did not happen correctly.\n>>\n>> Subsequent patches add the support of explicit reset using\n>> a new kvm device group/attribute combo. The associated kernel\n>> series is not upstream [1], hence the RFC.\n>>\n>> ITS specification is not very clear about reset. There is no\n>> reset wire. Some register fields are documented to have\n>> architecturally defined reset values and we use those here:\n>> Most importantly the Valid bit of GITS_CBASER and GITS_BASER\n>> are cleared and the GITS_CTLR.Enabled bit is cleared as well.\n> \n> The ITS is a device, not part of the CPU, so its reset is\n> implementation-defined but typically via some signal that's\n> controlled by the SoC (this is no different to the other\n> parts of the GICv3 which aren't denoted as being in the\n> reset domain of the CPU). For instance if you look at the TRM\n> for the GIC-500 it has a single 'resetn' reset signal which\n> resets all of the GIC/ITS apart from the CPU interface parts:\n> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.100336_0002_01_en/index.html\n> \n> The GICv3 spec 6.6 indicates what is supposed to happen to\n> the ITS on powerup. For QEMU's purposes reset is generally\n> considered to be a cold reset and we should always reset our\n> state to the same thing it was when QEMU first started.\n> \n> It's not clear to me why we need a new KVM device attribute\n> for doing ITS reset. The usual approach for this is:\n> * system reset causes QEMU's device model reset code\n> to reset state structure values to whatever their\n> reset value is\n> * we write those values up into the kernel, which is\n> sufficient to reset it\n> \n> What goes wrong with that in the case of the ITS?\n> In particular, if GITS_CTLR.Enabled is 0 then I think the\n> kernel should not be trying to read guest memory tables.\n\nWe discussed that on the kernel thread and Christoffer seemed to prefer\nan IOTCL instead of individual register writes\n(https://www.spinics.net/lists/kvm-arm/msg27211.html)\n\nThe IOCTL empties the ITS cached data (list of collection, list of\ndevices and list of LPIs) while it is not obvious the reset of BASER\nwould mandate that. Eventually in my kernel series I also voided the\nlist on BASER.valid toggling from 1 to 0.\n\nSpec also says:\nIf a write to GITS.CTLR changes the Enabled field from 1 to 0, the\nInterrupt Translation Space must ensure\nthat both:\n• Any caches containing mapping data are made consistent with external\nmemory.\n• GITS_CTLR.Quiescent == 0 until all caches are consistent with external\nmemory.\n\nI aknowledge I don't really get at which moment we are supposed to void\nthe caches.\n\nNevertheless, personnally I think we can achieve the same reset\nfunctionality with individual register writes.\n\nThanks\n\nEric\n\n\n\n\n\n\n> \n> thanks\n> -- PMM\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=)","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tdmarc=none (p=none dis=none) header.from=redhat.com","ext-mx06.extmail.prod.ext.phx2.redhat.com;\n\tspf=fail smtp.mailfrom=eric.auger@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 3yBzR22PTBz9s1h\n\tfor ;\n\tThu, 12 Oct 2017 03:07:58 +1100 (AEDT)","from localhost ([::1]:41710 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1e2JY4-0006ao-EB\n\tfor incoming@patchwork.ozlabs.org; Wed, 11 Oct 2017 12:07:56 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:46976)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1e2JWm-000623-9c\n\tfor qemu-devel@nongnu.org; Wed, 11 Oct 2017 12:06:43 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1e2JWg-0003MO-Ds\n\tfor qemu-devel@nongnu.org; Wed, 11 Oct 2017 12:06:36 -0400","from mx1.redhat.com ([209.132.183.28]:37196)\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 1e2JWb-0003IZ-I0; Wed, 11 Oct 2017 12:06:25 -0400","from smtp.corp.redhat.com\n\t(int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13])\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 7B5B33E2A5;\n\tWed, 11 Oct 2017 16:06:24 +0000 (UTC)","from localhost.localdomain (ovpn-117-172.ams2.redhat.com\n\t[10.36.117.172])\n\tby smtp.corp.redhat.com (Postfix) with ESMTPS id C77C360A9B;\n\tWed, 11 Oct 2017 16:06:19 +0000 (UTC)"],"DMARC-Filter":"OpenDMARC Filter v1.3.2 mx1.redhat.com 7B5B33E2A5","To":"Peter Maydell ","References":"<1506524205-20763-1-git-send-email-eric.auger@redhat.com>\n\t","From":"Auger Eric ","Message-ID":"<73b7916a-4bb3-3913-ed6f-23e5c3e4fa23@redhat.com>","Date":"Wed, 11 Oct 2017 18:06:18 +0200","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101\n\tThunderbird/45.4.0","MIME-Version":"1.0","In-Reply-To":"","Content-Type":"text/plain; charset=utf-8","X-Scanned-By":"MIMEDefang 2.79 on 10.5.11.13","X-Greylist":"Sender IP whitelisted, not delayed by milter-greylist-4.5.16\n\t(mx1.redhat.com [10.5.110.30]);\n\tWed, 11 Oct 2017 16:06:24 +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] [RFC 0/3] vITS Reset","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":"Wei Huang , Andrew Jones ,\n\tVijay Kilari ,\n\tJuan Quintela , \n\t\"Dr. David Alan Gilbert\" ,\n\tQEMU Developers , qemu-arm , \n\tChristoffer Dall ,\n\twanghaibin.wang@huawei.com, wu.wubin@huawei.com, eric.auger.pro@gmail.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1785303,"web_url":"http://patchwork.ozlabs.org/comment/1785303/","msgid":"","list_archive_url":null,"date":"2017-10-12T10:36:47","subject":"Re: [Qemu-devel] [RFC 0/3] vITS Reset","submitter":{"id":5111,"url":"http://patchwork.ozlabs.org/api/people/5111/","name":"Peter Maydell","email":"peter.maydell@linaro.org"},"content":"On 11 October 2017 at 17:06, Auger Eric wrote:\n> On 09/10/2017 20:17, Peter Maydell wrote:\n>> It's not clear to me why we need a new KVM device attribute\n>> for doing ITS reset. The usual approach for this is:\n>> * system reset causes QEMU's device model reset code\n>> to reset state structure values to whatever their\n>> reset value is\n>> * we write those values up into the kernel, which is\n>> sufficient to reset it\n>>\n>> What goes wrong with that in the case of the ITS?\n>> In particular, if GITS_CTLR.Enabled is 0 then I think the\n>> kernel should not be trying to read guest memory tables.\n>\n> We discussed that on the kernel thread and Christoffer seemed to prefer\n> an IOTCL instead of individual register writes\n> (https://www.spinics.net/lists/kvm-arm/msg27211.html)\n>\n> The IOCTL empties the ITS cached data (list of collection, list of\n> devices and list of LPIs) while it is not obvious the reset of BASER\n> would mandate that. Eventually in my kernel series I also voided the\n> list on BASER.valid toggling from 1 to 0.\n\nOK, I guess that makes sense -- in hardware, the device\nhas extra internal state (the cached stuff) which isn't\nvisible in its register interface. So we want to provide\nan API which is morally equivalent to the hardware reset line\nto tell the ITS to drop that internal state.\n\n> Spec also says:\n> If a write to GITS.CTLR changes the Enabled field from 1 to 0, the\n> Interrupt Translation Space must ensure\n> that both:\n> • Any caches containing mapping data are made consistent with external\n> memory.\n> • GITS_CTLR.Quiescent == 0 until all caches are consistent with external\n> memory.\n>\n> I aknowledge I don't really get at which moment we are supposed to void\n> the caches.\n\nThis is the interface for a software controlled clean powerdown:\n * OS writes 0 to Enabled\n * ITS finishes anything it is currently in the middle of\n * ITS updates any info that it currently has only in its internal\n cache so that it is also reflected in the external memory\n data structures\n * ITS sets Quiescent to 1 to tell the OS it has finished this process\n (and may no longer access guest RAM after this point)\n * OS can now unmap the RAM that the ITS tables are in,\n tell the h/w to power off the GIC/ITS, etc\n\nIt's a multistep process where the ITS gets the opportunity to\nwrite memory in the middle. I think in theory there's no obligation\nto actually drop the cached data when you set Quiescent to 1,\nprovided that you validate that when the OS sets Enabled back to\n1 all your cached data is still valid, because that's\nindistinguishable from dropping the cache and then repopulating it.\n\nFor the unclean powerdown (yanking the cord out of the back of the\nmachine), which is what qemu's reset is, we don't give the ITS\nthe option to write memory, it just has to effectively drop\nany internal data structures it was using. So it needs a KVM\nAPI that's different from the register-access API.\n\nthanks\n-- PMM","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\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"cU08Vuc/\"; 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 3yCS3P65tVz9sRq\n\tfor ;\n\tThu, 12 Oct 2017 21:37:37 +1100 (AEDT)","from localhost ([::1]:44715 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1e2arw-0006Rc-0a\n\tfor incoming@patchwork.ozlabs.org; Thu, 12 Oct 2017 06:37:36 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:54390)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1e2arW-0006OW-MB\n\tfor qemu-devel@nongnu.org; Thu, 12 Oct 2017 06:37:12 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1e2arV-0007gz-KQ\n\tfor qemu-devel@nongnu.org; Thu, 12 Oct 2017 06:37:10 -0400","from mail-wm0-x235.google.com ([2a00:1450:400c:c09::235]:46939)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1e2arV-0007gc-DZ\n\tfor qemu-devel@nongnu.org; Thu, 12 Oct 2017 06:37:09 -0400","by mail-wm0-x235.google.com with SMTP id m72so11942925wmc.1\n\tfor ; Thu, 12 Oct 2017 03:37:09 -0700 (PDT)","by 10.223.139.195 with HTTP; Thu, 12 Oct 2017 03:36:47 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc:content-transfer-encoding;\n\tbh=g9fukNC5suo+HUzH2QCCHMvOnWhgruNf2oQ+Bp1FtoY=;\n\tb=cU08Vuc/vxpfcO2WVnZm+lrqLek1oqZpeMHnhiWOO3VFBNvhiS5mu7YXAFUnX+Vl5V\n\t+9TEETejQMZal/r7CUB9GPmSQZk3TIkw1YnPLEaADZCcAhM1fmgCDDo+c7uzwncsJtNQ\n\tQUKVx/pepdX52uoYKR8sj/+bC2FejrvMptFaY=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc:content-transfer-encoding;\n\tbh=g9fukNC5suo+HUzH2QCCHMvOnWhgruNf2oQ+Bp1FtoY=;\n\tb=A3xdED5a97nH4La6ba8afu1KsR4mXHkNvly9uHwtvQB7jqxsZbMbT2gdSg9OmKP2VJ\n\tL+EptbWV2YZzqGvia8/7kmZvw4IDciBF4VTkY2o+cCDJhRl5LSJfwgaeYydQ0SI/TAAp\n\tVU+EKKhDewYXYoE3a8tHAurbiT+mzbtRL9S1X2dSDErhYjhsNqx3eTrtAQ3azslD38KU\n\tiuCyixQAs3e3nvNT9Axoy6oeas2OuW1ufE4lHMY7GcZJfcuny1kgV+mTkiUF9kc+N3zW\n\tddbmhsD9cs4cm6GA1/uOSgF8R9Cp7qyhLrQF6sXzibAhZciFQ3+f7h6ZOXTQvYVIhQ2N\n\tyiBA==","X-Gm-Message-State":"AMCzsaWAcyqTRKLWMnVfH/Ots+cP9gtnq/I8IMRTiUxMnFnPg+cxPeSH\n\t55ANRxnbX0kdon9U397a+Q9yig3n5lyesAvhGQj6Tg==","X-Google-Smtp-Source":"AOwi7QAW/QsOZzV0en5pqrJ5KbxMHDzv4Nxo6VWo6d8iFJO61qRINn01ZoKEfkofVy+kdfHfXywLKCII2nQOGrGVT6g=","X-Received":"by 10.28.236.203 with SMTP id h72mr1638672wmi.147.1507804628205; \n\tThu, 12 Oct 2017 03:37:08 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<73b7916a-4bb3-3913-ed6f-23e5c3e4fa23@redhat.com>","References":"<1506524205-20763-1-git-send-email-eric.auger@redhat.com>\n\t\n\t<73b7916a-4bb3-3913-ed6f-23e5c3e4fa23@redhat.com>","From":"Peter Maydell ","Date":"Thu, 12 Oct 2017 11:36:47 +0100","Message-ID":"","To":"Auger Eric ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2a00:1450:400c:c09::235","Subject":"Re: [Qemu-devel] [RFC 0/3] vITS Reset","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":"Wei Huang , Andrew Jones ,\n\tVijay Kilari ,\n\tJuan Quintela , \n\t\"Dr. David Alan Gilbert\" ,\n\tQEMU Developers , qemu-arm , \n\tChristoffer Dall ,\n\twanghaibin.wang@huawei.com, wu.wubin@huawei.com, eric.auger.pro@gmail.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}},{"id":1785380,"web_url":"http://patchwork.ozlabs.org/comment/1785380/","msgid":"","list_archive_url":null,"date":"2017-10-12T11:52:28","subject":"Re: [Qemu-devel] [RFC 3/3] hw/intc/arm_gicv3_its: Implement reset","submitter":{"id":5111,"url":"http://patchwork.ozlabs.org/api/people/5111/","name":"Peter Maydell","email":"peter.maydell@linaro.org"},"content":"On 27 September 2017 at 15:56, Eric Auger wrote:\n> Currently the ITS is not reset and this causes trouble\n> when state backup is initiated before the guest has initialized\n> the ITS registers and especially GITS_CBASER.\n>\n> We are likely to save register values set before the reset/\n> restart. The register values may not be consistent with the\n> data structures in RAM.\n>\n> So let's use the ITS KVM device new combo,\n> KVM_DEV_ARM_VGIC_GRP_CTRL/KVM_DEV_ARM_ITS_CTRL_RESET\n> to explicitly force the in-kernel emulated reset.\n>\n> Signed-off-by: Eric Auger \n> ---\n> hw/intc/arm_gicv3_its_common.c | 5 ++---\n> hw/intc/arm_gicv3_its_kvm.c | 22 ++++++++++++++++++----\n> include/hw/intc/arm_gicv3_its_common.h | 1 +\n> 3 files changed, 21 insertions(+), 7 deletions(-)\n>\n> diff --git a/hw/intc/arm_gicv3_its_common.c b/hw/intc/arm_gicv3_its_common.c\n> index 68b20fc..a2fe561 100644\n> --- a/hw/intc/arm_gicv3_its_common.c\n> +++ b/hw/intc/arm_gicv3_its_common.c\n> @@ -129,15 +129,14 @@ static void gicv3_its_common_reset(DeviceState *dev)\n> s->creadr = 0;\n> s->iidr = 0;\n> memset(&s->baser, 0, sizeof(s->baser));\n> -\n> - gicv3_its_post_load(s, 0);\n\nThis doesn't look right as it means we won't write the QEMU\ninitial device register values up to the kernel. I think we\nwant to do that as well as call the specific reset ioctl,\nso that both userspace and the kernel are consistent in\ntheir idea of what's going on.\n\n> }\n>\n> static void gicv3_its_common_class_init(ObjectClass *klass, void *data)\n> {\n> DeviceClass *dc = DEVICE_CLASS(klass);\n> + GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_CLASS(klass);\n>\n> - dc->reset = gicv3_its_common_reset;\n> + c->parent_reset = gicv3_its_common_reset;\n> dc->vmsd = &vmstate_its;\n> }\n\nThis isn't how we handle this for the arm_gicv3_kvm.c and arm_gic_kvm.c\ncode which has a subclass reset/parent class reset. What we do there is:\n * the parent_reset field is in the subclass's Class struct\n * the subclass's reset function calls the parent_reset function\n * the subclass's class_init function sets parent_reset to whatever\n the old dc->reset was before setting dc->reset to its own reset\n function\n\nI think we should be consistent in how we do this.\n\n> diff --git a/hw/intc/arm_gicv3_its_kvm.c b/hw/intc/arm_gicv3_its_kvm.c\n> index 120b86d..3c2e724 100644\n> --- a/hw/intc/arm_gicv3_its_kvm.c\n> +++ b/hw/intc/arm_gicv3_its_kvm.c\n> @@ -156,10 +156,6 @@ static void kvm_arm_its_post_load(GICv3ITSState *s)\n> Error *err = NULL;\n> int i;\n>\n> - if (!s->iidr) {\n> - return;\n> - }\n> -\n\nThis looks like an unrelated change, or at least not one mentioned\nin the commit message?\n\n> kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_ITS_REGS,\n> GITS_IIDR, &s->iidr, true, &error_abort);\n>\n> @@ -195,6 +191,23 @@ static void kvm_arm_its_post_load(GICv3ITSState *s)\n> GITS_CTLR, &s->ctlr, true, &error_abort);\n> }\n>\n> +static void kvm_arm_its_reset(DeviceState *dev)\n> +{\n> + GICv3ITSState *s = ARM_GICV3_ITS_COMMON(dev);\n> + GICv3ITSCommonClass *c = ARM_GICV3_ITS_COMMON_GET_CLASS(s);\n> +\n> + c->parent_reset(dev);\n> +\n> + if (!kvm_device_check_attr(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,\n> + KVM_DEV_ARM_ITS_CTRL_RESET)) {\n> + error_report(\"ITS KVM: reset is not supported by the kernel\");\n> + return;\n> + }\n> +\n> + kvm_device_access(s->dev_fd, KVM_DEV_ARM_VGIC_GRP_CTRL,\n> + KVM_DEV_ARM_ITS_CTRL_RESET, NULL, true, &error_abort);\n> +}\n> +\n> static Property kvm_arm_its_props[] = {\n> DEFINE_PROP_LINK(\"parent-gicv3\", GICv3ITSState, gicv3, \"kvm-arm-gicv3\",\n> GICv3State *),\n> @@ -211,6 +224,7 @@ static void kvm_arm_its_class_init(ObjectClass *klass, void *data)\n> icc->send_msi = kvm_its_send_msi;\n> icc->pre_save = kvm_arm_its_pre_save;\n> icc->post_load = kvm_arm_its_post_load;\n> + dc->reset = kvm_arm_its_reset;\n> }\n>\n> static const TypeInfo kvm_arm_its_info = {\n> diff --git a/include/hw/intc/arm_gicv3_its_common.h b/include/hw/intc/arm_gicv3_its_common.h\n> index fd1fe64..c158e9f 100644\n> --- a/include/hw/intc/arm_gicv3_its_common.h\n> +++ b/include/hw/intc/arm_gicv3_its_common.h\n> @@ -79,6 +79,7 @@ struct GICv3ITSCommonClass {\n> int (*send_msi)(GICv3ITSState *s, uint32_t data, uint16_t devid);\n> void (*pre_save)(GICv3ITSState *s);\n> void (*post_load)(GICv3ITSState *s);\n> + void (*parent_reset)(DeviceState *dev);\n> };\n>\n> typedef struct GICv3ITSCommonClass GICv3ITSCommonClass;\n> --\n> 2.5.5\n>\n\nthanks\n-- PMM","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\" (1024-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"EE+EWdK+\"; 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 3yCTkv18b6z9t2S\n\tfor ;\n\tThu, 12 Oct 2017 22:53:26 +1100 (AEDT)","from localhost ([::1]:44913 helo=lists.gnu.org)\n\tby lists.gnu.org with esmtp (Exim 4.71) (envelope-from\n\t)\n\tid 1e2c3H-0002tH-Sv\n\tfor incoming@patchwork.ozlabs.org; Thu, 12 Oct 2017 07:53:23 -0400","from eggs.gnu.org ([2001:4830:134:3::10]:49082)\n\tby lists.gnu.org with esmtp (Exim 4.71)\n\t(envelope-from ) id 1e2c2l-0002rn-CN\n\tfor qemu-devel@nongnu.org; Thu, 12 Oct 2017 07:52:52 -0400","from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)\n\t(envelope-from ) id 1e2c2k-0006FX-EM\n\tfor qemu-devel@nongnu.org; Thu, 12 Oct 2017 07:52:51 -0400","from mail-wm0-x234.google.com ([2a00:1450:400c:c09::234]:56165)\n\tby eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16)\n\t(Exim 4.71) (envelope-from )\n\tid 1e2c2k-0006Ed-3b\n\tfor qemu-devel@nongnu.org; Thu, 12 Oct 2017 07:52:50 -0400","by mail-wm0-x234.google.com with SMTP id u138so12678542wmu.4\n\tfor ; Thu, 12 Oct 2017 04:52:49 -0700 (PDT)","by 10.223.139.195 with HTTP; Thu, 12 Oct 2017 04:52:28 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:in-reply-to:references:from:date:message-id:subject:to\n\t:cc; bh=62M1zMD00Af9Dsiw2Pdk4KXbzwqSIIHnaCoGvXkmX/g=;\n\tb=EE+EWdK+Sv01V4pixb1bshPvVxafCaGhBJZTOxQknBefj46NDIX4xy4OHULfcvytYZ\n\tYOd6XMY5Zvp6Kg+wWMjRoUEjpvSqGUkBT2LMlP1d24Qvmexwmvry1xZwLFGeS/AwUz6y\n\tt/94ulEAoa7XmxfBSX+wZgA8gYCm1Er87/isM=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:in-reply-to:references:from:date\n\t:message-id:subject:to:cc;\n\tbh=62M1zMD00Af9Dsiw2Pdk4KXbzwqSIIHnaCoGvXkmX/g=;\n\tb=VG/YgE4mM0hLY6zqVKce7BM9Ka8QF3WxNTfZ3mulWS/ZuWmPF+w6XkQEdp7Ync4/CT\n\tXseY3/YflwFX2rDFMCoFVJfd2sS7tCbPHh0GkIiDcIqIgi68MbNL5zNwKpqulsbeAaWc\n\tQ0sZgGiyuLUHSzpOQscyXNFMxHqdBldQYSjKuyCLCAqUWNiMyptS390Tak50/vJjWKnZ\n\ttNohW8jbfRv22pZgsFSi1JVnCN75v4uC+cHIu01gjB4hAheG53uVk8g+vUyxPsoaog6F\n\tk+ptt2WBhWgG9512fahfJawdBSFT2jRnGgrTvJZafFByArkU3zv22blapM5/lLf/HMfy\n\tI3bA==","X-Gm-Message-State":"AMCzsaWKcM5k5CDR6Af8uppUfHx1Z40kixB9PbB9fVHG0vn0b3r2RiR9\n\tjHOhzRpvCHqMLeDRI0lUUKRqE6+sz0DAtVLkxVHbVw==","X-Google-Smtp-Source":"AOwi7QBtPR6IumrHn7CBATy2rTTLXfavihC2FPVQpJLgA4Ii5oVnfsBtPviYmIbhMqSUbIMDuWlcJJ4dtTis37yeSkc=","X-Received":"by 10.223.197.13 with SMTP id q13mr2130052wrf.272.1507809168735; \n\tThu, 12 Oct 2017 04:52:48 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<1506524205-20763-4-git-send-email-eric.auger@redhat.com>","References":"<1506524205-20763-1-git-send-email-eric.auger@redhat.com>\n\t<1506524205-20763-4-git-send-email-eric.auger@redhat.com>","From":"Peter Maydell ","Date":"Thu, 12 Oct 2017 12:52:28 +0100","Message-ID":"","To":"Eric Auger ","Content-Type":"text/plain; charset=\"UTF-8\"","X-detected-operating-system":"by eggs.gnu.org: Genre and OS details not\n\trecognized.","X-Received-From":"2a00:1450:400c:c09::234","Subject":"Re: [Qemu-devel] [RFC 3/3] hw/intc/arm_gicv3_its: Implement reset","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":"Wei Huang , Andrew Jones ,\n\tVijay Kilari ,\n\tJuan Quintela , \n\tQEMU Developers ,\n\t\"Dr. David Alan Gilbert\" ,\n\tqemu-arm , wu.wubin@huawei.com,\n\twanghaibin.wang@huawei.com,\n\tChristoffer Dall , eric.auger.pro@gmail.com","Errors-To":"qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org","Sender":"\"Qemu-devel\"\n\t"}}]