Hi Christoffer, On 27/04/2017 10:57, Christoffer Dall wrote: > On Wed, Apr 26, 2017 at 05:48:32PM +0200, Auger Eric wrote: >> Hi Christoffer, >> >> On 26/04/2017 14:31, Christoffer Dall wrote: >>> On Fri, Apr 14, 2017 at 12:15:13PM +0200, Eric Auger wrote: >>>> Add description for how to access ITS registers and how to save/restore >>>> ITS tables into/from memory. >>>> >>>> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> >>>> >>>> --- >>>> v4 -> v5: >>>> - take into account Christoffer's comments >>>> - pending table save on GICV3 side now >>>> >>>> v3 -> v4: >>>> - take into account Peter's comments: >>>> - typos >>>> - KVM_DEV_ARM_VGIC_GRP_ITS_TABLES kvm_device_attr = 0 >>>> - add a validity bit in DTE >>>> - document all fields in CTE and ITE >>>> - document ABI revision >>>> - take into account Andre's comments: >>>> - document restrictions about GITS_CREADR writing and GITS_IIDR >>>> - document -EBUSY error if one or more VCPUS are runnning >>>> - document 64b registers only can be accessed with 64b access >>>> - itt_addr field matches bits [51:8] of the itt_addr >>>> >>>> v1 -> v2: >>>> - DTE and ITE now are 8 bytes >>>> - DTE and ITE now indexed by deviceid/eventid >>>> - use ITE name instead of ITTE >>>> - mentions ITT_addr matches bits [51:8] of the actual address >>>> - mentions LE layout >>>> --- >>>> Documentation/virtual/kvm/devices/arm-vgic-its.txt | 99 ++++++++++++++++++++++ >>>> 1 file changed, 99 insertions(+) >>>> >>>> diff --git a/Documentation/virtual/kvm/devices/arm-vgic-its.txt b/Documentation/virtual/kvm/devices/arm-vgic-its.txt >>>> index 6081a5b..b5f010d 100644 >>>> --- a/Documentation/virtual/kvm/devices/arm-vgic-its.txt >>>> +++ b/Documentation/virtual/kvm/devices/arm-vgic-its.txt >>>> @@ -32,7 +32,106 @@ Groups: >>>> KVM_DEV_ARM_VGIC_CTRL_INIT >>>> request the initialization of the ITS, no additional parameter in >>>> kvm_device_attr.addr. >>>> + >>>> + 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. >>>> + >>>> + The layout of the tables in guest memory defines an ABI. The entries >>>> + are laid out in little endian format as described in the last paragraph. >>>> + >>>> + KVM_DEV_ARM_ITS_RESTORE_TABLES >>>> + restore the ITS tables from guest RAM to ITS internal structures. >>>> + >>>> + The GICV3 must be restored before the ITS and all ITS registers but >>>> + the GITS_CTLR must be restored before restoring the ITS tables. >>>> + >>>> + The GITS_IIDR read-only register must also be restored before >>>> + the table restore as the IIDR revision field encodes the ABI revision. >>>> + >>> >>> what is the expected sequence of operations. For example, to restore >>> the ITS, do I call KVM_DEV_ARM_VGIC_CTRL_INIT first, then restore all >>> the memory and registers, and finally call KVM_DEV_ARM_ITS_RESTORE_TABLES? >> Yes KVM_DEV_ARM_VGIC_CTRL_INIT comes first, then restore all registers >> except GITS_CTLR, then table restore, then GITS_CTLR >>> >>> Is there any interaction between when you call KVM_DEV_ARM_ITS_RESTORE_TABLES >>> and restore GITS_CTLR (which enables the ITS)? >> >> Yep, when GITS_CTLR is set, LPIs may be enabled and this on that event >> that the pending table is read. But the whole pending table is not read >> as we only iterate on registered LPIs. So the ITT must have been >> restored previously. >> >> I became aware that the pending table sync is done twice, once in the >> pending table restore, and once in the GITS_CTLR restore. So if we >> leave this order specification, I should be able to remove the sync on >> table restore. This was the original reason why GITS_CTLR restore has >> been done at the very end. > > I'm sorry, I'm a bit confused. Do we not need > KVM_DEV_ARM_ITS_RESTORE_TABLES at all then? Yes you do. I was talking about the RDIST pending table sync. The save is explicit using GICV3 device KVM_DEV_ARM_VGIC_SAVE_PENDING_TABLES. However the sync is implicit on GITS_CTLR restore if LPIs are enabled. and today I do it also on ITS device KVM_DEV_ARM_ITS_RESTORE_TABLES which is not requested I think since GITS_CTLR restore does it already. KVM_DEV_ARM_ITS_RESTORE_TABLES restores all the ITS tables (device, collection, ITT) Hope it clarifies. Thanks Eric > >>> >>>> Errors: >>>> -ENXIO: ITS not properly configured as required prior to setting >>>> this attribute >>>> -ENOMEM: Memory shortage when allocating ITS internal data >>>> + -EINVAL: Inconsistent restored data >>>> + -EFAULT: Invalid guest ram access >>>> + -EBUSY: One or more VCPUS are running >>>> + >>>> + KVM_DEV_ARM_VGIC_GRP_ITS_REGS >>>> + Attributes: >>>> + The attr field of kvm_device_attr encodes the offset of the >>>> + ITS register, relative to the ITS control frame base address >>>> + (ITS_base). >>>> + >>>> + kvm_device_attr.addr points to a __u64 value whatever the width >>>> + of the addressed register (32/64 bits). 64 bit registers can only >>>> + be accessed with full length. >>>> + >>>> + Writes to read-only registers are ignored by the kernel except for: >>>> + - GITS_READR. It needs to be restored otherwise commands in the queue >>>> + will be re-executed after restoring CWRITER. GITS_READR must be restored >>>> + before restoring the GITS_CTLR which is likely to enable the ITS. >>>> + Also it needs to be restored after GITS_CBASER since a write to >>>> + GITS_CBASER resets GITS_CREADR. >>>> + - GITS_IIDR. Its Revision field encodes the table layout ABI revision. >>>> + In the future we might implement direct injection of virtual LPIS. >>>> + This will require an upgrade of the table layout and an evolution of >>>> + the ABI. GITS_IIDR must be restored before the table restoration. >>>> + >>>> + For other registers, getting or setting a register has the same >>>> + effect as reading/writing the register on real hardware. >>>> + Errors: >>>> + -ENXIO: Offset does not correspond to any supported register >>>> + -EFAULT: Invalid user pointer for attr->addr >>>> + -EINVAL: Offset is not 64-bit aligned >>>> + -EBUSY: one or more VCPUS are running >>> >>> >>> It may be helpful to state the ordering requirements somewhere: >>> >>> Restoring the ITS: >>> ------------------ >>> Restoring the ITS requires certain things to happen in order. >>> Specifically: >>> 1. Initialize the ITS (KVM_DEV_ARM_VGIC_CTRL_INIT) >>> 2. Restore GITS_IIDR >>> 3. Restore GITS_CBASER >>> 4. Restore GITS_READR >>> 5. Restore remainin registers except GITS_CTLR >>> 6. Make sure all guest memory is restored >>> 7. Load the ITS table data (KVM_DEV_ARM_ITS_RESTORE_TABLES) >> >> OK I will try to fit that description somewhere. >> > > Thanks, > -Christoffer > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel >
