From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758577AbdJMPlR (ORCPT ); Fri, 13 Oct 2017 11:41:17 -0400 Received: from mx1.redhat.com ([209.132.183.28]:38172 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758534AbdJMPlO (ORCPT ); Fri, 13 Oct 2017 11:41:14 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 87EEA3E2C2 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=eric.auger@redhat.com Subject: Re: [PATCH v2 09/10] KVM: arm/arm64: Document KVM_DEV_ARM_ITS_CTRL_RESET To: Christoffer Dall References: <1506518920-18571-1-git-send-email-eric.auger@redhat.com> <1506518920-18571-10-git-send-email-eric.auger@redhat.com> <20171013152658.GL8927@cbox> Cc: eric.auger.pro@gmail.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, marc.zyngier@arm.com, peter.maydell@linaro.org, andre.przywara@arm.com, wanghaibin.wang@huawei.com, wu.wubin@huawei.com From: Auger Eric Message-ID: <30547d7f-fa04-a7c1-ee56-1fb720999c0f@redhat.com> Date: Fri, 13 Oct 2017 17:41:10 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.4.0 MIME-Version: 1.0 In-Reply-To: <20171013152658.GL8927@cbox> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 13 Oct 2017 15:41:14 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Christoffer, On 13/10/2017 17:26, Christoffer Dall wrote: > Hi Eric, > > On Wed, Sep 27, 2017 at 03:28:39PM +0200, Eric Auger wrote: >> At the moment, the in-kernel emulated ITS is not properly reset. >> On guest restart/reset some registers keep their old values and >> internal structures like device, ITE, collection lists are not freed. > > , and collection ... >> >> This may lead to various bugs. Among them, we can have incorrect state >> backup or failure when saving the ITS state at early guest boot stage. >> >> This patch documents a new attribute, KVM_DEV_ARM_ITS_CTRL_RESET in >> the KVM_DEV_ARM_VGIC_GRP_CTRL group. >> >> Upon this action, we can reset registers and especially those >> pointing to tables previously allocated by the guest and free >> the internal data structures storing the list of devices, collections >> and lpis. >> >> Signed-off-by: Eric Auger >> Reported-by: wanghaibin >> >> --- >> >> v1 -> v2: >> - Describe architecturally-defined reset values >> --- >> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 16 ++++++++++++++++ >> 1 file changed, 16 insertions(+) >> >> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt >> index eb06beb..047358c 100644 >> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt >> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt >> @@ -33,6 +33,10 @@ Groups: >> request the initialization of the ITS, no additional parameter in >> kvm_device_attr.addr. >> >> + KVM_DEV_ARM_ITS_CTRL_RESET >> + reset the ITS, no additional parameter in kvm_device_attr.addr. >> + See "ITS Reset State" section. >> + >> KVM_DEV_ARM_ITS_SAVE_TABLES >> save the ITS table data into guest RAM, at the location provisioned >> by the guest in corresponding registers/table entries. >> @@ -157,3 +161,15 @@ Then vcpus can be started. >> - pINTID is the physical LPI ID; if zero, it means the entry is not valid >> and other fields are not meaningful. >> - ICID is the collection ID >> + >> + ITS Reset State: >> + ---------------- > > Are the bullet points below the state after the device has been reset > (after KVM_DEV_ARM_ITS_CTRL_RESET returns) or ? I think this should be > clarified. yes this is after reset. > >> + >> +- the ITS is not enabled and quiescent: > > The > >> + GITS_CTLR.Enabled = 0 .Quiescent=1 >> +- caches are empty > Caches (would it make more sense to say that there is no > internally cached state?) >> +- No collection or device table is provisionned > > are used: referenced? > >> + GITS_BASER.Valid = 0 >> +- the command queue is not allocated: > > The > >> + GITS_CBASER = 0, GITS_CREADR = 0, GITS_CWRITER = 0 >> +- The ABI version corresponds to the one set before reset > > Do you mean that resetting the ITS cannot change the ABI version used > for save/restore, and therefore remains the same as it the version > configured when the device was first created ? Yes the ABI version stays the same. > >> -- >> 2.5.5 >> > > Thanks, (and sorry for being pedantic about ABI wordings) no worries, I know it is important ;-) Thanks Eric > -Christoffer >