Re: [PATCH v2] s390/mm: add missing secure storage access fixups for donated memory

[Claudio Imbrenda]

On Tue, 10 Mar 2026 15:02:42 +0000
Janosch Frank <frankja@xxxxxxxxxxxxx> wrote:

> There are special cases where secure storage access exceptions happen
> in a kernel context for pages that don't have the PG_arch_1 bit
> set. That bit is set for non-exported guest secure storage (memory)
> but is absent on storage donated to the Ultravisor since the kernel
> isn't allowed to export donated pages.
> 
> Prior to this patch we would try to export the page by calling
> arch_make_folio_accessible() which would instantly return since the
> arch bit is absent signifying that the page was already exported and
> no further action is necessary. This leads to secure storage access
> exception loops which can never be resolved.
> 
> With this patch we unconditionally try to export and if that fails we
> fixup.
> 
> Fixes: 084ea4d611a3 ("s390/mm: add (non)secure page access exceptions handlers")
> Reported-by: Heiko Carstens <hca@xxxxxxxxxxxxx>
> Suggested-by: Heiko Carstens <hca@xxxxxxxxxxxxx>
> Signed-off-by: Janosch Frank <frankja@xxxxxxxxxxxxx>

Reviewed-by: Claudio Imbrenda <imbrenda@xxxxxxxxxxxxx>

> ---
> 
> Changed fault error handling to nolock. (Heiko)
> Added PG_arch_1 cleanup requested off-list. (Claudio)
> 
> ---
>  arch/s390/mm/fault.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/s390/mm/fault.c b/arch/s390/mm/fault.c
> index a52aa7a99b6b..191cc53caead 100644
> --- a/arch/s390/mm/fault.c
> +++ b/arch/s390/mm/fault.c
> @@ -441,10 +441,17 @@ void do_secure_storage_access(struct pt_regs *regs)
>  		folio = phys_to_folio(addr);
>  		if (unlikely(!folio_try_get(folio)))
>  			return;
> -		rc = arch_make_folio_accessible(folio);
> +		rc = uv_convert_from_secure(folio_to_phys(folio));
> +		if (!rc)
> +			clear_bit(PG_arch_1, &folio->flags.f);
>  		folio_put(folio);
> +		/*
> +		 * There are some valid fixup types for kernel
> +		 * accesses to donated secure memory. zeropad is one
> +		 * of them.
> +		 */
>  		if (rc)
> -			BUG();
> +			return handle_fault_error_nolock(regs, 0);
>  	} else {
>  		if (faulthandler_disabled())
>  			return handle_fault_error_nolock(regs, 0);