[PATCH] drm/i915/gvt: Refine new_mmio_info to override the added MMIO_reg

[Colin Xu]

On 05/31/2018 10:25 AM, Zhao, Yakui wrote:

> Hi,
>       Any comment about this patch?
>       After the overriding is supported, it can reduce the effect on the other platform when adding one
> mmio handle for one specific platform.
>
> Thanks
>       Yakui

trade-off against the convenience:
- Unnecessary delete and re-add.
- For a specific mmio, it's unclear how it's handled until go through all init functions, which
transfers the effort to debugging stage, especially for issue with low fail rate.

BR
Colin

>> -----Original Message-----
>> From: Zhao, Yakui
>> Sent: Thursday, May 17, 2018 11:58 AM
>> To: intel-gvt-dev at lists.freedesktop.org
>> Cc: Zhao, Yakui <yakui.zhao at intel.com>
>> Subject: [PATCH] drm/i915/gvt: Refine new_mmio_info to override the added
>> MMIO_reg
>>
>> GVT-g uses new_mmio_info function to track the required the mmio_reg. If
>> the required mmio_reg is already added, error is returned. If one platform
>> hopes to add new specific MMIO read/write callback for one mmio_reg,  the
>> change needs to be applied on all the platforms.
>>
>> So the new_mmio_info is implemented as override. If it is already added, it
>> will be replaced by the new attributes(mask, read/write callback and so on).
>>
>> Signed-off-by: Zhao Yakui <yakui.zhao at intel.com>
>> ---
>> drivers/gpu/drm/i915/gvt/handlers.c | 25 +++++++++++--------------
>> 1 file changed, 11 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gvt/handlers.c
>> b/drivers/gpu/drm/i915/gvt/handlers.c
>> index 653ba2b..a7bf6d5 100644
>> --- a/drivers/gpu/drm/i915/gvt/handlers.c
>> +++ b/drivers/gpu/drm/i915/gvt/handlers.c
>> @@ -107,22 +107,19 @@ static int new_mmio_info(struct intel_gvt *gvt,
>> 	end = offset + size;
>>
>> 	for (i = start; i < end; i += 4) {
>> -		info = kzalloc(sizeof(*info), GFP_KERNEL);
>> -		if (!info)
>> -			return -ENOMEM;
>> -
>> -		info->offset = i;
>> -		p = find_mmio_info(gvt, info->offset);
>> +		p = find_mmio_info(gvt, i);
>> 		if (p) {
>> -			WARN(1, "dup mmio definition offset %x\n",
>> -				info->offset);
>> -			kfree(info);
>> +			/* It will be removed from hash_list if found */
>> +			hash_del(&p->node);
>> +			info = p;
>> +			gvt_dbg_mmio("VGPU MMIO_reg %x is added.
>> Override\n",
>> +					p->offset);
>> +		} else {
>> +			info = kzalloc(sizeof(*info), GFP_KERNEL);
>> +			if (!info)
>> +				return -ENOMEM;
>>
>> -			/* We return -EEXIST here to make GVT-g load fail.
>> -			 * So duplicated MMIO can be found as soon as
>> -			 * possible.
>> -			 */
>> -			return -EEXIST;
>> +			info->offset = i;
>> 		}
>>
>> 		info->ro_mask = ro_mask;
>> --
>> 2.7.4
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
Best Regards,
Colin Xu