2022-10-20 08:54:41

by Zhao Liu

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

From: Zhao Liu <[email protected]>

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 <[email protected]>
Suggested-by: Ira Weiny <[email protected]>
Reviewed-by: Ira Weiny <[email protected]>
Signed-off-by: Zhao Liu <[email protected]>
---
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


2022-10-26 15:23:36

by Wei Liu

[permalink] [raw]
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 <[email protected]>
>
> 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 <[email protected]>
> Suggested-by: Ira Weiny <[email protected]>
> Reviewed-by: Ira Weiny <[email protected]>
> Signed-off-by: Zhao Liu <[email protected]>

Applied to hyperv-fixes. Thanks.

2022-10-27 10:03:37

by Zhao Liu

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

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 <[email protected]>
> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > Suggested-by: Ira Weiny <[email protected]>
> > Reviewed-by: Ira Weiny <[email protected]>
> > Signed-off-by: Zhao Liu <[email protected]>
>
> 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

2022-10-27 12:11:14

by Wei Liu

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

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 <[email protected]>
> > 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 <[email protected]>
> > >
> > > 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 <[email protected]>
> > > Suggested-by: Ira Weiny <[email protected]>
> > > Reviewed-by: Ira Weiny <[email protected]>
> > > Signed-off-by: Zhao Liu <[email protected]>
> >
> > 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.