lib/test_kho: Check if KHO is enabled

[PATCH] lib/test_kho: Check if KHO is enabled

Posted by Pasha Tatashin 1 month, 1 week ago

We must check whether KHO is enabled prior to issuing KHO commands,
otherwise KHO internal data structures are not initialized.

Fixes: b753522bed0b ("kho: add test for kexec handover")

Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202511061629.e242724-lkp@intel.com

Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
---
 lib/test_kho.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/lib/test_kho.c b/lib/test_kho.c
index 025ea251a186..85b60d87a50a 100644
--- a/lib/test_kho.c
+++ b/lib/test_kho.c
@@ -315,6 +315,9 @@ static int __init kho_test_init(void)
 	phys_addr_t fdt_phys;
 	int err;
 
+	if (!kho_is_enabled())
+		return 0;
+
 	err = kho_retrieve_subtree(KHO_TEST_FDT, &fdt_phys);
 	if (!err)
 		return kho_test_restore(fdt_phys);
-- 
2.51.2.1041.gc1ab5b90ca-goog

Re: [PATCH] lib/test_kho: Check if KHO is enabled

Posted by Mike Rapoport 1 month, 1 week ago

On Thu, Nov 06, 2025 at 05:06:35PM -0500, Pasha Tatashin wrote:
> We must check whether KHO is enabled prior to issuing KHO commands,
> otherwise KHO internal data structures are not initialized.
> 
> Fixes: b753522bed0b ("kho: add test for kexec handover")
> 
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202511061629.e242724-lkp@intel.com
> 
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>

Reviewed-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

> ---
>  lib/test_kho.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/lib/test_kho.c b/lib/test_kho.c
> index 025ea251a186..85b60d87a50a 100644
> --- a/lib/test_kho.c
> +++ b/lib/test_kho.c
> @@ -315,6 +315,9 @@ static int __init kho_test_init(void)
>  	phys_addr_t fdt_phys;
>  	int err;
>  
> +	if (!kho_is_enabled())
> +		return 0;
> +
>  	err = kho_retrieve_subtree(KHO_TEST_FDT, &fdt_phys);
>  	if (!err)
>  		return kho_test_restore(fdt_phys);
> -- 
> 2.51.2.1041.gc1ab5b90ca-goog
> 

-- 
Sincerely yours,
Mike.

Re: [PATCH] lib/test_kho: Check if KHO is enabled

Posted by Pratyush Yadav 1 month, 1 week ago

On Thu, Nov 06 2025, Pasha Tatashin wrote:

> We must check whether KHO is enabled prior to issuing KHO commands,
> otherwise KHO internal data structures are not initialized.

Should we have this check in the KHO APIs instead? This check is easy
enough to miss.

>
> Fixes: b753522bed0b ("kho: add test for kexec handover")
>

Nit: these blank lines would probably mess up trailer parsing for
tooling.

> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202511061629.e242724-lkp@intel.com
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
>  lib/test_kho.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/lib/test_kho.c b/lib/test_kho.c
> index 025ea251a186..85b60d87a50a 100644
> --- a/lib/test_kho.c
> +++ b/lib/test_kho.c
> @@ -315,6 +315,9 @@ static int __init kho_test_init(void)
>  	phys_addr_t fdt_phys;
>  	int err;
>  
> +	if (!kho_is_enabled())
> +		return 0;
> +
>  	err = kho_retrieve_subtree(KHO_TEST_FDT, &fdt_phys);
>  	if (!err)
>  		return kho_test_restore(fdt_phys);

-- 
Regards,
Pratyush Yadav

Re: [PATCH] lib/test_kho: Check if KHO is enabled

Posted by Pasha Tatashin 1 month, 1 week ago

On Fri, Nov 7, 2025 at 5:24 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>
> On Thu, Nov 06 2025, Pasha Tatashin wrote:
>
> > We must check whether KHO is enabled prior to issuing KHO commands,
> > otherwise KHO internal data structures are not initialized.
>
> Should we have this check in the KHO APIs instead? This check is easy
> enough to miss.

I considered adding a kho_is_enabled() check to every KHO API, but it
seems unnecessary.

In-kernel users of KHO, like reserve_mem and the upcoming LUO, are
already expected to check if KHO is enabled before doing extra
preservation work. I anticipate any future in-kernel users will follow
the same pattern.

We could add a WARN_ON(!kho_is_enabled()) to the internal API calls,
but I don't think it's needed. We already catch this condition with
other WARN_ONs, as shown by this report.

>
> >
> > Fixes: b753522bed0b ("kho: add test for kexec handover")
> >
>
> Nit: these blank lines would probably mess up trailer parsing for
> tooling.

Hm, if so,  the blank line should be removed.

Thank you,
Pasha

>
> > Reported-by: kernel test robot <oliver.sang@intel.com>
> > Closes: https://lore.kernel.org/oe-lkp/202511061629.e242724-lkp@intel.com
> >
> > Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> > ---
> >  lib/test_kho.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/lib/test_kho.c b/lib/test_kho.c
> > index 025ea251a186..85b60d87a50a 100644
> > --- a/lib/test_kho.c
> > +++ b/lib/test_kho.c
> > @@ -315,6 +315,9 @@ static int __init kho_test_init(void)
> >       phys_addr_t fdt_phys;
> >       int err;
> >
> > +     if (!kho_is_enabled())
> > +             return 0;
> > +
> >       err = kho_retrieve_subtree(KHO_TEST_FDT, &fdt_phys);
> >       if (!err)
> >               return kho_test_restore(fdt_phys);
>
> --
> Regards,
> Pratyush Yadav

Re: [PATCH] lib/test_kho: Check if KHO is enabled

Posted by Pratyush Yadav 1 month, 1 week ago

On Fri, Nov 07 2025, Pasha Tatashin wrote:

> On Fri, Nov 7, 2025 at 5:24 AM Pratyush Yadav <pratyush@kernel.org> wrote:
>>
>> On Thu, Nov 06 2025, Pasha Tatashin wrote:
>>
>> > We must check whether KHO is enabled prior to issuing KHO commands,
>> > otherwise KHO internal data structures are not initialized.
>>
>> Should we have this check in the KHO APIs instead? This check is easy
>> enough to miss.
>
> I considered adding a kho_is_enabled() check to every KHO API, but it
> seems unnecessary.
>
> In-kernel users of KHO, like reserve_mem and the upcoming LUO, are
> already expected to check if KHO is enabled before doing extra
> preservation work. I anticipate any future in-kernel users will follow
> the same pattern.

Hmm, fair enough. I suppose we can always change this later if it causes
more pain.

Reviewed-by: Pratyush Yadav <pratyush@kernel.org>

[...]

-- 
Regards,
Pratyush Yadav