On 11/04/17 10:57, Auger Eric wrote: > Hi Marc, > > On 10/04/2017 11:55, Marc Zyngier wrote: >> On 27/03/17 10:31, Eric Auger wrote: >>> The flush path copies the collection entries into guest RAM >>> at the GPA specified in the BASER register. This obviously >>> requires the BASER to be set. The last written element >>> is a dummy collection table entry. >>> >>> We do not index by collection ID as the collection entry >>> can fit into 8 bytes while containing the collection ID. >>> >>> On restore path we re-allocate the collection objects. >>> >>> Signed-off-by: Eric Auger <eric.auger@xxxxxxxxxx> >>> >>> --- >>> >>> v3 -> v4: >>> - replaced u64 *ptr by gpa_t gpa >>> - check the collection does not exist before allocating it >>> >>> v1 -> v2: >>> - reword commit message and directly use 8 as entry size >>> - no kvm parameter anymore >>> - add helper for flush/restore cte >>> - table size computed here >>> - add le64/cpu conversions >>> --- >>> virt/kvm/arm/vgic/vgic-its.c | 99 +++++++++++++++++++++++++++++++++++++++++++- >>> 1 file changed, 97 insertions(+), 2 deletions(-) >>> >>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c >>> index 8eaeba4..df984b6 100644 >>> --- a/virt/kvm/arm/vgic/vgic-its.c >>> +++ b/virt/kvm/arm/vgic/vgic-its.c >>> @@ -1828,13 +1828,89 @@ static int vgic_its_restore_device_tables(struct vgic_its *its) >>> return -ENXIO; >>> } >>> >>> +static int vgic_its_flush_cte(struct vgic_its *its, >>> + struct its_collection *collection, gpa_t gpa) >> >> Given that the pendent of this function is name ..._restore_..., should >> this be called ..._save_... ? >> >>> +{ >>> + u64 val; >>> + int ret; >>> + >>> + val = ((u64)1 << 63 | ((u64)collection->target_addr << 16) | >>> + collection->collection_id); >> >> Please provide some #defines that describe the encoding. This will be >> specially useful if we need to export this to userspace (KVM/TCG >> interoperability?). >> >>> + val = cpu_to_le64(val); >>> + ret = kvm_write_guest(its->dev->kvm, gpa, &val, 8); >>> + return ret; >>> +} >>> + >>> +static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, bool *valid) >>> +{ >>> + struct its_collection *collection; >>> + u32 target_addr; >>> + u32 coll_id; >>> + u64 val; >>> + int ret; >>> + >>> + *valid = false; >>> + >>> + ret = kvm_read_guest(its->dev->kvm, gpa, &val, 8); >>> + if (ret) >>> + return ret; >>> + val = le64_to_cpu(val); >>> + *valid = val & BIT_ULL(63); >>> + >>> + if (!*valid) >>> + return 0; >>> + >>> + target_addr = (u32)(val >> 16); >> >> u32? > At the moment its_collection.target_addr is u32 and corresponds to the > PE number. Only GITS_TYPER.PTA = 0 is supported. > > Or do I miss something? No, you're right. I'm getting confused between the model I'm working with and the ITS emulation... Ignore me. Thanks, M. -- Jazz is not dead. It just smells funny...