On Fri, Oct 13, 2017 at 05:41:10PM +0200, Auger Eric wrote: > 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 <eric.auger@xxxxxxxxxx> > >> Reported-by: wanghaibin <wanghaibin.wang@xxxxxxxxxx> > >> > >> --- > >> > >> 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? I think used is more plainspoken and intuitively understood. > > > >> + GITS_BASER<n>.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 :) -Christoffer
