Re: [PATCH v6 19/24] KVM: arm64: vgic-its: Collection table save/restore

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 05/05/2017 16:28, Auger Eric wrote:
> Hi
> 
> On 05/05/2017 14:28, Christoffer Dall wrote:
>> On Thu, May 04, 2017 at 01:44:39PM +0200, Eric Auger wrote:
>>> The save 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>
>>>
>>> ---
>>> v5 -> v6:
>>> - remove the valid pointer from vgic_its_restore_cte and
>>>   return +1 if no error and last elt not found
>>> - check the BASER valid bit
>>> - add BUG_ON to check esz versus val size
>>>
>>> v4 -> v5:
>>> - add macros for field encoding/decoding
>>> - use abi->cte_esz
>>> - rename flush into save
>>> - check the target_addr is valid
>>>
>>> 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 | 102 ++++++++++++++++++++++++++++++++++++++++++-
>>>  virt/kvm/arm/vgic/vgic.h     |   9 ++++
>>>  2 files changed, 109 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
>>> index 1db7e38..6bef9147 100644
>>> --- a/virt/kvm/arm/vgic/vgic-its.c
>>> +++ b/virt/kvm/arm/vgic/vgic-its.c
>>> @@ -1804,13 +1804,91 @@ static int vgic_its_restore_device_tables(struct vgic_its *its)
>>>  	return -ENXIO;
>>>  }
>>>  
>>> +static int vgic_its_save_cte(struct vgic_its *its,
>>> +			     struct its_collection *collection,
>>> +			     gpa_t gpa, int esz)
>>> +{
>>> +	u64 val;
>>> +
>>> +	val = (1ULL << KVM_ITS_CTE_VALID_SHIFT |
>>> +	       ((u64)collection->target_addr << KVM_ITS_CTE_RDBASE_SHIFT) |
>>> +	       collection->collection_id);
>>> +	val = cpu_to_le64(val);
>>> +	return kvm_write_guest(its->dev->kvm, gpa, &val, esz);
>>> +}
>>> +
>>> +static int vgic_its_restore_cte(struct vgic_its *its, gpa_t gpa, int esz)
>>> +{
>>> +	struct its_collection *collection;
>>> +	struct kvm *kvm = its->dev->kvm;
>>> +	u32 target_addr, coll_id;
>>> +	u64 val;
>>> +	int ret;
>>> +
>>> +	BUG_ON(esz > sizeof(val));
>>> +	ret = kvm_read_guest(kvm, gpa, &val, esz);
>>> +	if (ret)
>>> +		return ret;
>>> +	val = le64_to_cpu(val);
>>> +	if (!(val & KVM_ITS_CTE_VALID_MASK))
>>> +		return 0;
>>> +
>>> +	target_addr = (u32)(val >> KVM_ITS_CTE_RDBASE_SHIFT);
>>> +	coll_id = val & KVM_ITS_CTE_ICID_MASK;
>>> +
>>> +	if (target_addr >= atomic_read(&kvm->online_vcpus))
>>> +		return -EINVAL;
>>> +
>>> +	collection = find_collection(its, coll_id);
>>> +	if (collection)
>>> +		return -EEXIST;
>>> +	ret = vgic_its_alloc_collection(its, &collection, coll_id);
>>> +	if (ret)
>>> +		return ret;
>>> +	collection->target_addr = target_addr;
>>> +	return 1;
>>> +}
>>> +
>>>  /**
>>>   * vgic_its_save_collection_table - Save the collection table into
>>>   * guest RAM
>>>   */
>>>  static int vgic_its_save_collection_table(struct vgic_its *its)
>>>  {
>>> -	return -ENXIO;
>>> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
>>> +	struct its_collection *collection;
>>> +	u64 val;
>>> +	gpa_t gpa;
>>> +	size_t max_size, filled = 0;
>>> +	int ret, cte_esz = abi->cte_esz;
>>> +
>>> +	gpa = BASER_ADDRESS(its->baser_coll_table);
>>> +	if (!gpa)
>>> +		return 0;
>>> +
>>> +	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
>>> +
>>> +	list_for_each_entry(collection, &its->collection_list, coll_list) {
>>> +		if (filled == max_size)
>>> +			return -ENOSPC;
>>
>> should this ever happen?  Shouldn't we check limit in
>> vgic_its_cmd_handle_mapi() ?
> Yes you're right. This should be done in vgic_its_cmd_handle_mapc()
>>
>> We can fix that later, and it shouldn't block this patch, just asking
>> the question here because I'm noticing it.
> 
> OK, I will fix that later then.
actually the check already is done in
vgic_its_alloc_collection/vgic_its_check_id called from both
mapi and mapti.

so I can directly remove the above check.

Thanks

Eric
> 
> Thanks
> 
> Eric
>>
>>> +		ret = vgic_its_save_cte(its, collection, gpa, cte_esz);
>>> +		if (ret)
>>> +			return ret;
>>> +		gpa += cte_esz;
>>> +		filled += cte_esz;
>>> +	}
>>> +
>>> +	if (filled == max_size)
>>> +		return 0;
>>> +
>>> +	/*
>>> +	 * table is not fully filled, add a last dummy element
>>> +	 * with valid bit unset
>>> +	 */
>>> +	val = 0;
>>> +	BUG_ON(cte_esz > sizeof(val));
>>> +	ret = kvm_write_guest(its->dev->kvm, gpa, &val, cte_esz);
>>> +	return ret;
>>>  }
>>>  
>>>  /**
>>> @@ -1820,7 +1898,27 @@ static int vgic_its_save_collection_table(struct vgic_its *its)
>>>   */
>>>  static int vgic_its_restore_collection_table(struct vgic_its *its)
>>>  {
>>> -	return -ENXIO;
>>> +	const struct vgic_its_abi *abi = vgic_its_get_abi(its);
>>> +	int cte_esz = abi->cte_esz;
>>> +	size_t max_size, read = 0;
>>> +	gpa_t gpa;
>>> +	int ret;
>>> +
>>> +	if (!(its->baser_coll_table & GITS_BASER_VALID))
>>> +		return 0;
>>> +
>>> +	gpa = BASER_ADDRESS(its->baser_coll_table);
>>> +
>>> +	max_size = GITS_BASER_NR_PAGES(its->baser_coll_table) * SZ_64K;
>>> +
>>> +	while (read < max_size) {
>>> +		ret = vgic_its_restore_cte(its, gpa, cte_esz);
>>> +		if (ret <= 0)
>>> +			break;
>>> +		gpa += cte_esz;
>>> +		read += cte_esz;
>>> +	}
>>> +	return ret;
>>>  }
>>>  
>>>  /**
>>> diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
>>> index 309ab64..58adcae 100644
>>> --- a/virt/kvm/arm/vgic/vgic.h
>>> +++ b/virt/kvm/arm/vgic/vgic.h
>>> @@ -73,6 +73,15 @@
>>>  				      KVM_REG_ARM_VGIC_SYSREG_CRM_MASK | \
>>>  				      KVM_REG_ARM_VGIC_SYSREG_OP2_MASK)
>>>  
>>> +/*
>>> + * As per Documentation/virtual/kvm/devices/arm-vgic-its.txt,
>>> + * below macros are defined for ITS table entry encoding.
>>> + */
>>> +#define KVM_ITS_CTE_VALID_SHIFT		63
>>> +#define KVM_ITS_CTE_VALID_MASK		BIT_ULL(63)
>>> +#define KVM_ITS_CTE_RDBASE_SHIFT	16
>>> +#define KVM_ITS_CTE_ICID_MASK		GENMASK_ULL(15, 0)
>>> +
>>>  static inline bool irq_is_pending(struct vgic_irq *irq)
>>>  {
>>>  	if (irq->config == VGIC_CONFIG_EDGE)
>>> -- 
>>> 2.5.5
>>>
>>
>> Reviewed-by: Christoffer Dall <cdall@xxxxxxxxxx>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux