[PATCH v2] x86/hyperv: Remove BUG_ON() for kmap_local_page()

[PATCH v2] x86/hyperv: Remove BUG_ON() for kmap_local_page()

[Zhao Liu]

From: Zhao Liu <zhao1.liu@intel.com>

The commit 154fb14df7a3c ("x86/hyperv: Replace kmap() with
kmap_local_page()") keeps the BUG_ON() to check if kmap_local_page()
fails.

But in fact, kmap_local_page() always returns a valid kernel address
and won't return NULL here. It will BUG on its own if it fails. [1]

So directly use memcpy_to_page() which creates local mapping to copy.

[1]: https://lore.kernel.org/lkml/YztFEyUA48et0yTt@iweiny-mobl/

Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Suggested-by: Ira Weiny <ira.weiny@intel.com>
Reviewed-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
---
Change in v2:
 - Removed "Fixes" tag in patch message.
---
 arch/x86/hyperv/hv_init.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
index 29774126e931..f66c5709324f 100644
--- a/arch/x86/hyperv/hv_init.c
+++ b/arch/x86/hyperv/hv_init.c
@@ -459,13 +459,11 @@ void __init hyperv_init(void)
 		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
 
 		pg = vmalloc_to_page(hv_hypercall_pg);
-		dst = kmap_local_page(pg);
 		src = memremap(hypercall_msr.guest_physical_address << PAGE_SHIFT, PAGE_SIZE,
 				MEMREMAP_WB);
-		BUG_ON(!(src && dst));
-		memcpy(dst, src, HV_HYP_PAGE_SIZE);
+		BUG_ON(!src);
+		memcpy_to_page(pg, 0, src, HV_HYP_PAGE_SIZE);
 		memunmap(src);
-		kunmap_local(dst);
 	} else {
 		hypercall_msr.guest_physical_address = vmalloc_to_pfn(hv_hypercall_pg);
 		wrmsrl(HV_X64_MSR_HYPERCALL, hypercall_msr.as_uint64);
-- 
2.34.1

Re: [PATCH v2] x86/hyperv: Remove BUG_ON() for kmap_local_page()

[Wei Liu]

On Thu, Oct 20, 2022 at 04:38:20PM +0800, Zhao Liu wrote:
> From: Zhao Liu <zhao1.liu@intel.com>
> 
> The commit 154fb14df7a3c ("x86/hyperv: Replace kmap() with
> kmap_local_page()") keeps the BUG_ON() to check if kmap_local_page()
> fails.
> 
> But in fact, kmap_local_page() always returns a valid kernel address
> and won't return NULL here. It will BUG on its own if it fails. [1]
> 
> So directly use memcpy_to_page() which creates local mapping to copy.
> 
> [1]: https://lore.kernel.org/lkml/YztFEyUA48et0yTt@iweiny-mobl/
> 
> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Zhao Liu <zhao1.liu@intel.com>

Applied to hyperv-fixes. Thanks.

Re: [PATCH v2] x86/hyperv: Remove BUG_ON() for kmap_local_page()

[Zhao Liu]

On Wed, Oct 26, 2022 at 03:17:18PM +0000, Wei Liu wrote:
> Date: Wed, 26 Oct 2022 15:17:18 +0000
> From: Wei Liu <wei.liu@kernel.org>
> Subject: Re: [PATCH v2] x86/hyperv: Remove BUG_ON() for kmap_local_page()
> 
> On Thu, Oct 20, 2022 at 04:38:20PM +0800, Zhao Liu wrote:
> > From: Zhao Liu <zhao1.liu@intel.com>
> > 
> > The commit 154fb14df7a3c ("x86/hyperv: Replace kmap() with
> > kmap_local_page()") keeps the BUG_ON() to check if kmap_local_page()
> > fails.
> > 
> > But in fact, kmap_local_page() always returns a valid kernel address
> > and won't return NULL here. It will BUG on its own if it fails. [1]
> > 
> > So directly use memcpy_to_page() which creates local mapping to copy.
> > 
> > [1]: https://lore.kernel.org/lkml/YztFEyUA48et0yTt@iweiny-mobl/
> > 
> > Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> 
> Applied to hyperv-fixes. Thanks.

Sorry Wei, I appology for not deleting unused variables, which caused
the next tree break. Do I need to send another v3 to make up for my
carelessness?

Zhao

Re: [PATCH v2] x86/hyperv: Remove BUG_ON() for kmap_local_page()

[Wei Liu]

On Thu, Oct 27, 2022 at 06:05:48PM +0800, Zhao Liu wrote:
> On Wed, Oct 26, 2022 at 03:17:18PM +0000, Wei Liu wrote:
> > Date: Wed, 26 Oct 2022 15:17:18 +0000
> > From: Wei Liu <wei.liu@kernel.org>
> > Subject: Re: [PATCH v2] x86/hyperv: Remove BUG_ON() for kmap_local_page()
> > 
> > On Thu, Oct 20, 2022 at 04:38:20PM +0800, Zhao Liu wrote:
> > > From: Zhao Liu <zhao1.liu@intel.com>
> > > 
> > > The commit 154fb14df7a3c ("x86/hyperv: Replace kmap() with
> > > kmap_local_page()") keeps the BUG_ON() to check if kmap_local_page()
> > > fails.
> > > 
> > > But in fact, kmap_local_page() always returns a valid kernel address
> > > and won't return NULL here. It will BUG on its own if it fails. [1]
> > > 
> > > So directly use memcpy_to_page() which creates local mapping to copy.
> > > 
> > > [1]: https://lore.kernel.org/lkml/YztFEyUA48et0yTt@iweiny-mobl/
> > > 
> > > Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > > Reviewed-by: Ira Weiny <ira.weiny@intel.com>
> > > Signed-off-by: Zhao Liu <zhao1.liu@intel.com>
> > 
> > Applied to hyperv-fixes. Thanks.
> 
> Sorry Wei, I appology for not deleting unused variables, which caused
> the next tree break. Do I need to send another v3 to make up for my
> carelessness?

There is no need to send v3. I've dealt with it.

Thanks,
Wei.