Received-SPF: pass (google.com: domain of linux-kernel+bounces-95031-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <6b453972-2723-47c5-981e-56c150f217d7@intel.com>
Date: Thu, 7 Mar 2024 15:01:11 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v19 014/130] KVM: Add KVM vcpu ioctl to pre-populate guest
 memory
To: <isaku.yamahata@intel.com>, <kvm@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>
CC: <isaku.yamahata@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>,
	<erdemaktas@google.com>, Sean Christopherson <seanjc@google.com>, Sagi Shahar
	<sagis@google.com>, Kai Huang <kai.huang@intel.com>, <chen.bo@intel.com>,
	<hang.yuan@intel.com>, <tina.zhang@intel.com>
References: <cover.1708933498.git.isaku.yamahata@intel.com>
 <8b7380f1b02f8e3995f18bebb085e43165d5d682.1708933498.git.isaku.yamahata@intel.com>
Content-Language: en-US
From: Yin Fengwei <fengwei.yin@intel.com>
In-Reply-To: <8b7380f1b02f8e3995f18bebb085e43165d5d682.1708933498.git.isaku.yamahata@intel.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RC9wdzJQODNmSjBBY1V2RFVDQ2NKcUpVUy93Y0xwZGt1eWZTVWlHWDhJeitF?=
 =?utf-8?B?MUlTUmNlbjJINjFIM1k3Snhtd1hoVytKVmREdzM4UmxXOS9vYk5tbHZtZ1pI?=
 =?utf-8?B?Q2xMQW5VSlRuNEpmMkxZMlV5cTRZUnptTU9OZlFWaXUyNXFrcHVEYmJORDNV?=
 =?utf-8?B?c25aMmpMVXhua2dPc2ZGMG54TC91aTJvWWY1VnovbEViRGlxaURUcnJ6RDg3?=
 =?utf-8?B?MldWVzdCbDlveGI2OE9XMWlnNW5yMFo2UitUT3kzdkw0V0hzQTNJbjBoQlUw?=
 =?utf-8?B?ZFNLbkJaU3NSWFltaWFmOXZ5cWVubXVnOGZnRnViR2NkTFM0U0NSSFQ4emcy?=
 =?utf-8?B?NWtGNTFWSS85djNIeXc1SDV4OTVFcjVGdnQ3bXp2UWNqckYwZm5JbVRkRk9o?=
 =?utf-8?B?Q2QvcUtQaDVYS202cHVGaHVzRmVLSFRJSDh2aHZzYThKMFZHaHRLeEVqODdL?=
 =?utf-8?B?V1lxUWZtN0Y5cjFzK2RQdlBiNzUvMHJTbHhqek4wam5GQWI1TkNhOHYwOTJD?=
 =?utf-8?B?SFA5Q1dBb3czOFp2MjIxS2RvRnY4d3lHVVJRbHNRTWNmanA3U3pobFNyMXJN?=
 =?utf-8?B?Tk5NSm9yenRqSVVsQlhpUVQ0c3k3MkVFanlMRVFIckZPRlNZUC9CV1phYkdJ?=
 =?utf-8?B?M0F3T2R6UC9KSm9FSFhJcU16OHhTTTdwMXdRdWIwODJMSFAxOGVPYlorblY5?=
 =?utf-8?B?WWdIZjdsaXlJZTREVUJKd3BhN2JEemxvQ09KTTNsRnl2SE8wbUd5akdMK0xM?=
 =?utf-8?B?Y05XRTViais1d0hQSXRWQ1hKSlhINUtMODh1cWNKUVoxQUx4ZGk2YlFyTitJ?=
 =?utf-8?B?MHhjcTNocUxYcXNadml3T1J2NkU3ZnhHaWgvSThNM2J2NjdZK2h5dVd6R3lX?=
 =?utf-8?B?NUp2Vi9zRVpyb1V1YXdxd3pEbThiMjlhdEFiZFZWb09oT3l6ZzNqcUY0cC9E?=
 =?utf-8?B?b1VXSjRnQ01sTDg0amVrNGg1MVpHeC9vU2JIM3pDb3h4LzNxMnkrK1ljdEdk?=
 =?utf-8?B?WURTc011dkNTbVFKN0VRSDhUcUxhZWlPdk9EWHJMWlZCUHZpdXo4S2dtZklt?=
 =?utf-8?B?czQ5dFNTdUUwUXppWE1QN1hMTnoyVlhwS3pBUUZzM040UXYrazZHYmhweWdQ?=
 =?utf-8?B?ZVFuL0R3WC9RQ3RlVlhTdDJ2Q0tPSmx2NE5tbllSWGJXY2dCVnN2c29GVUVs?=
 =?utf-8?B?bjE5RmVObUhGYUJsYUpMOUdKcmhxYk8vRUsvUmx3ZVQ3Y1NVenhiajY1NWlV?=
 =?utf-8?B?T3BaV3N5YmUxSDhobEticXNlNW5RMnl3M1dYN3pDUUJsdjRvcTRMcGYyeTFC?=
 =?utf-8?B?Q29CdW9kaWwrSE9kQUsyWkRlT0p3bm1BcThrNlJ1WG9zKzRTamNwVDVseDhs?=
 =?utf-8?B?VWJnUzIzN2pOakhTMmduNVB6S08vRStMcWtpdDZvUTBoWnRWQi9zang5Slc3?=
 =?utf-8?B?UC9KYUhGRkNBMlZ6dUhKY29vY3JUVlErWlNuTnNoa1IwMXFZaVFLMUFRdGtj?=
 =?utf-8?B?Tk1TYTdoTDJSZ3RhdDZBbGhWQnJKdGtIZTRUVVk2NG5wVXNncW1yQzAxMGZy?=
 =?utf-8?B?RHh0ZmlQV0h1U0JFRTlFc09hTjdaYnc0RHpvV1FPOVUzTkVuVGZCTC9WYWNn?=
 =?utf-8?B?UlFLcWUycVErckgwdUY4ZENDdDdWeU1pWkdWZ1FVVEtxajJJQSt3Zlp2cjhh?=
 =?utf-8?B?MEY5Ujd6MG5NWkF4M3I2Y05TMk5PYzNnUThyK3BQcWJDSU8yTmZ1ekZqclRk?=
 =?utf-8?B?M3N2SExFaTc2Q2p2VzU4TzFXa2JtV0grTWtCdjdvbEtlOUN0c2JCbW56UndN?=
 =?utf-8?B?OGpreVVtZ1V0cHRlMEhXU0hPRjd4UCtGc2ZSTXRDNERkdWF3UlJ4YnA1SENE?=
 =?utf-8?B?bWg3Q0puM2FrblJPOW1HVVVFOWhveDVFaVYxK0xDd1JReXNoTmtuSnBBSGtQ?=
 =?utf-8?B?SXkzNGlDZXB1bDFXZ3VOY2dkOTZQck5CVzdjb3B4eFo1b2ZxN1F0Zk5KZnVy?=
 =?utf-8?B?Mm15RDErY0FSeXJjSXBjSTA0U1pualhyb2NuYkhSWTN4OWZIaldUMmQxaDFy?=
 =?utf-8?B?TnAxY09SZlJPL0NyQXZLcVUyME9kL3Z2a21MVEk3eCtjLzdwZ3hpWEhaR2k1?=
 =?utf-8?Q?omwHEP9lhi7rqiQeFtdxqA6MC?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 297f0830-86c6-4fe4-c2b0-08dc3e745fdf
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB4820.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2024 07:01:13.0195
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: h52/X9menYv3bFDIk3WockyJh7ZVgJwXTm79LanETCWm92bkWSteIn7f1AMG9SWoss4lAWJowlDST/3yPyedvQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR11MB8601
X-OriginatorOrg: intel.com


On 2/26/24 16:25, isaku.yamahata@intel.com wrote:
> From: Isaku Yamahata <isaku.yamahata@intel.com>
> 
> Add new ioctl KVM_MEMORY_MAPPING in the kvm common code. It iterates on the
> memory range and call arch specific function.  Add stub function as weak
> symbol.
> 
> Signed-off-by: Isaku Yamahata <isaku.yamahata@intel.com>
> ---
> v19:
> - newly added
> ---
>  include/linux/kvm_host.h |  4 +++
>  include/uapi/linux/kvm.h | 10 ++++++
>  virt/kvm/kvm_main.c      | 67 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 81 insertions(+)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 0520cd8d03cc..eeaf4e73317c 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -2389,4 +2389,8 @@ static inline int kvm_gmem_get_pfn(struct kvm *kvm,
>  }
>  #endif /* CONFIG_KVM_PRIVATE_MEM */
>  
> +void kvm_arch_vcpu_pre_memory_mapping(struct kvm_vcpu *vcpu);
> +int kvm_arch_vcpu_memory_mapping(struct kvm_vcpu *vcpu,
> +				 struct kvm_memory_mapping *mapping);
> +
>  #endif
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index c3308536482b..5e2b28934aa9 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1155,6 +1155,7 @@ struct kvm_ppc_resize_hpt {
>  #define KVM_CAP_MEMORY_ATTRIBUTES 233
>  #define KVM_CAP_GUEST_MEMFD 234
>  #define KVM_CAP_VM_TYPES 235
> +#define KVM_CAP_MEMORY_MAPPING 236
>  
>  #ifdef KVM_CAP_IRQ_ROUTING
>  
> @@ -2227,4 +2228,13 @@ struct kvm_create_guest_memfd {
>  	__u64 reserved[6];
>  };
>  
> +#define KVM_MEMORY_MAPPING	_IOWR(KVMIO, 0xd5, struct kvm_memory_mapping)
> +
> +struct kvm_memory_mapping {
> +	__u64 base_gfn;
> +	__u64 nr_pages;
> +	__u64 flags;
> +	__u64 source;
> +};
> +
>  #endif /* __LINUX_KVM_H */
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 0349e1f241d1..2f0a8e28795e 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -4409,6 +4409,62 @@ static int kvm_vcpu_ioctl_get_stats_fd(struct kvm_vcpu *vcpu)
>  	return fd;
>  }
>  
> +__weak void kvm_arch_vcpu_pre_memory_mapping(struct kvm_vcpu *vcpu)
> +{
> +}
> +
> +__weak int kvm_arch_vcpu_memory_mapping(struct kvm_vcpu *vcpu,
> +					struct kvm_memory_mapping *mapping)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> +static int kvm_vcpu_memory_mapping(struct kvm_vcpu *vcpu,
> +				   struct kvm_memory_mapping *mapping)
> +{
> +	bool added = false;
> +	int idx, r = 0;
> +
> +	/* flags isn't used yet. */
> +	if (mapping->flags)
> +		return -EINVAL;
> +
> +	/* Sanity check */
> +	if (!IS_ALIGNED(mapping->source, PAGE_SIZE) ||
> +	    !mapping->nr_pages ||
> +	    mapping->nr_pages & GENMASK_ULL(63, 63 - PAGE_SHIFT) ||
> +	    mapping->base_gfn + mapping->nr_pages <= mapping->base_gfn)
I suppose !mapping->nr_pages can be deleted as this line can cover it.
> +		return -EINVAL;
> +
> +	vcpu_load(vcpu);
> +	idx = srcu_read_lock(&vcpu->kvm->srcu);
> +	kvm_arch_vcpu_pre_memory_mapping(vcpu);
> +
> +	while (mapping->nr_pages) {
> +		if (signal_pending(current)) {
> +			r = -ERESTARTSYS;
> +			break;
> +		}
> +
> +		if (need_resched())
> +			cond_resched();
> +
> +		r = kvm_arch_vcpu_memory_mapping(vcpu, mapping);
> +		if (r)
> +			break;
> +
> +		added = true;
> +	}
> +
> +	srcu_read_unlock(&vcpu->kvm->srcu, idx);
> +	vcpu_put(vcpu);
> +
> +	if (added && mapping->nr_pages > 0)
> +		r = -EAGAIN;
> +
> +	return r;
> +}
> +
>  static long kvm_vcpu_ioctl(struct file *filp,
>  			   unsigned int ioctl, unsigned long arg)
>  {
> @@ -4610,6 +4666,17 @@ static long kvm_vcpu_ioctl(struct file *filp,
>  		r = kvm_vcpu_ioctl_get_stats_fd(vcpu);
>  		break;
>  	}
> +	case KVM_MEMORY_MAPPING: {
> +		struct kvm_memory_mapping mapping;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&mapping, argp, sizeof(mapping)))
> +			break;
> +		r = kvm_vcpu_memory_mapping(vcpu, &mapping);
return value r should be checked before copy_to_user


Regards
Yin, Fengwei

> +		if (copy_to_user(argp, &mapping, sizeof(mapping)))
> +			r = -EFAULT;
> +		break;
> +	}
>  	default:
>  		r = kvm_arch_vcpu_ioctl(filp, ioctl, arg);
>  	}