Received-SPF: pass (google.com: domain of linux-kernel+bounces-93235-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Wed, 6 Mar 2024 09:49:00 +0800
From: Yan Zhao <yan.y.zhao@intel.com>
To: Sagi Shahar <sagis@google.com>
CC: <linux-kselftest@vger.kernel.org>, Ackerley Tng <ackerleytng@google.com>,
	Ryan Afranji <afranji@google.com>, Erdem Aktas <erdemaktas@google.com>,
	"Isaku Yamahata" <isaku.yamahata@intel.com>, Sean Christopherson
	<seanjc@google.com>, Paolo Bonzini <pbonzini@redhat.com>, Shuah Khan
	<shuah@kernel.org>, "Peter Gonda" <pgonda@google.com>, Haibo Xu
	<haibo1.xu@intel.com>, Chao Peng <chao.p.peng@linux.intel.com>, Vishal
 Annapurve <vannapurve@google.com>, Roger Wang <runanwang@google.com>, Vipin
 Sharma <vipinsh@google.com>, <jmattson@google.com>, <dmatlack@google.com>,
	<linux-kernel@vger.kernel.org>, <kvm@vger.kernel.org>, <linux-mm@kvack.org>
Subject: Re: [RFC PATCH v5 24/29] KVM: selftests: Expose _vm_vaddr_alloc
Message-ID: <ZefLjLa1KAHE0Toa@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <20231212204647.2170650-1-sagis@google.com>
 <20231212204647.2170650-25-sagis@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20231212204647.2170650-25-sagis@google.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?3gdBFs/evYC4N9hImEde5/m7o8vqVGiCXEFByTXhQ1Je2LbKdy4x+L5ZTv+E?=
 =?us-ascii?Q?RhAyeD5zOhq64lZJQjVHPBGQgay8lavXYEfIfHjMAekozoiYOU0iyjA+vmQb?=
 =?us-ascii?Q?xjT/77FDYxSYVD1E7IEBGPFUvCLPW/KJoDxS32it95SXeJT4OI4dMfUHUlDf?=
 =?us-ascii?Q?DlMYELUB6l19ZHNuqPMVAt60UhEQxLJ0KOHZ9r23OWhTu98jpIJyZMxD1a5P?=
 =?us-ascii?Q?O8wYra2mgb4JfXsheo0EpD9yi0LaAVdorPgp1AsPJ55QJ4IBxbIeRn/13FS+?=
 =?us-ascii?Q?eIGqDocMe1TuhRJcfgC0mluWK6zE2TTKOWkAuRABfRbK35c8DWf2/1CvOrgN?=
 =?us-ascii?Q?V8vz9iHjjUtRPilpr5AuzK7oo7QZ/XosOgHoiWOZ1nLeFDvqUGDuv3SeCv6X?=
 =?us-ascii?Q?XNoCboMWYIXGcTPnxAXs5bn9xWA9sK8yqfPX51AeCxQV5i9Jqfj58BZhr6Hq?=
 =?us-ascii?Q?+80htmLBmA/651mhN//auox9xM8zeYdy/r/cAUfNRfqKx8szz8o0lu7MVK5h?=
 =?us-ascii?Q?dQPGajQk26NimBu+pFdoOMixDl1SXxrnqvHHCV9HmM+bzNUcrEMCOH3i8oWe?=
 =?us-ascii?Q?mfW6cLuGLc4pPRF9Uw9ynZrRPwzQTFqvo6ECp7fMnmI5iDBn1GuqJITVKgdx?=
 =?us-ascii?Q?SkrRtyLDnTTVFR7O18EvZl2fJaHaE9pmB5ValNcOSwYYmoeRHbhGBiUb4M/s?=
 =?us-ascii?Q?e9/ArYcbmvItN2f57ZBYgUh0l+rB83mi0dz6hVS45Hu3b/93kPB6FKCD0odx?=
 =?us-ascii?Q?3JLQkLgJMvl9d4rO50H/L2oLT8QNWgZt4rkfZzKY8yWLLPEAsN2ymBwjYsXf?=
 =?us-ascii?Q?w3N+gQXiOOmcpakMrsj6BMlkVd6MbXQx9MXSIvrLUaRf1fOyL35kBzawXxJw?=
 =?us-ascii?Q?LLdqDZAXqSDjrwZUo6KXBz/88+SEIA1xFQQCAKKgMVkQ2JxH7tDnZOoTI6YZ?=
 =?us-ascii?Q?FAYgeAUMfOZmZx0bF8JWRe8Kt8x22XnFGWjO5DkcPsmDiLOI6Dbm6+V0wRyh?=
 =?us-ascii?Q?Tdumc1n/TZKjNKLNgwvtW8FFmM6FHasLL5lT48BJ43vTNJ6h7V7PGrix2SL4?=
 =?us-ascii?Q?ck7QOciVYXqV4aciuThc9ZMhzm3I81ggN/XlLWYNRMFNqOianCO7wwWTVMJu?=
 =?us-ascii?Q?omAStZDoiiQjdZ0W3PgTY4xrC4wple9Ypzpc8I2vsYPP8TaCM+ZPXLY3xS7p?=
 =?us-ascii?Q?wFvqgXug4efjhXgBlh6isbzT7vgPgaQbMY3zX9hAHDLMTPm3b3xSjuAg1hFr?=
 =?us-ascii?Q?eZ4+NfhzTT/KtU1aPkOSOPioJMkMJayWNgkEkxA5iH12h7/6ee7ujtrBEipx?=
 =?us-ascii?Q?syWV3xMJwWIIhOOegFMBRBTqFJxO5Ga2A5MVOCn+f32Z16JnaeWbMMn1QDU9?=
 =?us-ascii?Q?g+AxPhsG2Wuu4J9DMxWy13gRLPs0i/ipH+lFgZ2TXpnoaM+y1tmJzBOT9GLm?=
 =?us-ascii?Q?JmjWHznXe+LN+1C+4H+7AknMNzHmmV40Yn9HnQfmSLzj8ROUygC46fkFL36O?=
 =?us-ascii?Q?l3CjErYf5AWml197xQMjeDPiR802NEi0pny2mpQszpwEtaRpt5g93Njr5+ZU?=
 =?us-ascii?Q?yYnfN056O0FfSP2xZnKEAEzmZLRWwOfpeD/GV3Ok?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 29bf49e5-74a5-4b94-4c83-08dc3d83c420
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Mar 2024 02:18:52.4584
 (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: fVtm8SGqZj/dnwWJHMamWl0Qjuv5Tg/Hln2wCITDC5F2hM0Ng9BdWABOffGis5AXo+H5PC63Ajm55xpGMcm/VQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4548
X-OriginatorOrg: intel.com

On Tue, Dec 12, 2023 at 12:46:39PM -0800, Sagi Shahar wrote:
> From: Ackerley Tng <ackerleytng@google.com>
> 
> vm_vaddr_alloc always allocates memory in memslot 0. This allows users
> of this function to choose which memslot to allocate virtual memory
> in.
> 
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> Signed-off-by: Ryan Afranji <afranji@google.com>
> Signed-off-by: Sagi Shahar <sagis@google.com>
> ---
>  tools/testing/selftests/kvm/include/kvm_util_base.h | 3 +++
>  tools/testing/selftests/kvm/lib/kvm_util.c          | 6 +++---
>  2 files changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index efd7ae8abb20..5dbebf5cfd07 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -561,6 +561,9 @@ void vm_mem_region_delete(struct kvm_vm *vm, uint32_t slot);
>  struct kvm_vcpu *__vm_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id);
>  void vm_populate_vaddr_bitmap(struct kvm_vm *vm);
>  vm_vaddr_t vm_vaddr_unused_gap(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min);
> +vm_vaddr_t ____vm_vaddr_alloc(struct kvm_vm *vm, size_t sz,
> +			      vm_vaddr_t vaddr_min, vm_paddr_t paddr_min,
> +			      uint32_t data_memslot, bool encrypt);
>  vm_vaddr_t vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min);
>  vm_vaddr_t __vm_vaddr_alloc(struct kvm_vm *vm, size_t sz, vm_vaddr_t vaddr_min,
>  			    enum kvm_mem_region_type type);
> diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c b/tools/testing/selftests/kvm/lib/kvm_util.c
> index 28780fa1f0f2..d024abc5379c 100644
> --- a/tools/testing/selftests/kvm/lib/kvm_util.c
> +++ b/tools/testing/selftests/kvm/lib/kvm_util.c
> @@ -1410,9 +1410,9 @@ vm_vaddr_t vm_vaddr_unused_gap(struct kvm_vm *vm, size_t sz,
>   * a unique set of pages, with the minimum real allocation being at least
>   * a page.
>   */
> -static vm_vaddr_t ____vm_vaddr_alloc(struct kvm_vm *vm, size_t sz,
> -				     vm_vaddr_t vaddr_min, vm_paddr_t paddr_min,
> -				     uint32_t data_memslot, bool encrypt)
> +vm_vaddr_t ____vm_vaddr_alloc(struct kvm_vm *vm, size_t sz,
> +			      vm_vaddr_t vaddr_min, vm_paddr_t paddr_min,
> +			      uint32_t data_memslot, bool encrypt)
Is it better to expose a helper function with meaningful name?
It's annoying to count the number of "_" to figure out which version is
used, especially when there're "__", "____" versions :)

>  {
>  	uint64_t pages = (sz >> vm->page_shift) + ((sz % vm->page_size) != 0);
>  
> -- 
> 2.43.0.472.g3155946c3a-goog
> 
>