[PATCH v2 0/4] KVM: s390: replace page allocator calls with kzalloc()

[PATCH v2 0/4] KVM: s390: replace page allocator calls with kzalloc()

[Mike Rapoport (Microsoft)]

This is a (small) part of larger work of replacing page allocator calls
with kmalloc.

My initial intention a few month ago was to remove ugly casts [1], but then
willy pointed out that Linus objected to something like this [2] and it
looks like more than a decade old technical debt.

Largely, anything that doesn't need struct page (or a memdesc in the
future) should just use kmalloc() or kvmalloc() to allocate memory.
kmalloc() guarantees alignment, physical contiguity and working
virt_to_phys() and beside nicer API that returns void * on alloc and
doesn't require to know the allocation size on free, kmalloc() provides
better debugging capabilities than page allocator.

Another thing is that touching these allocation sites gives the reviewers
opportunity to see if a PAGE_SIZE buffer is actually needed or maybe
another size is appropriate.

For larger allocations that don't need physically contiguous memory
kvmalloc() can be a better option that __get_free_pages() because under
memory pressure it's is easier to allocate several order-0 pages than a
physically contiguous chunk with the same number of pages.

And last, but not least, removing needless calls to page allocator should
help with memdesc (aka project folio) conversion. There will be way less
places to audit to see if the user was actually using struct page.

Also in git:
https://git.kernel.org/pub/scm/linux/kernel/git/rppt/linux.git gfp-to-kmalloc/s390-kvm

[1] https://lore.kernel.org/all/20251018093002.3660549-1-rppt@kernel.org/
[2] https://lore.kernel.org/all/CA+55aFwp4iy4rtX2gE2WjBGFL=NxMVnoFeHqYa2j1dYOMMGqxg@mail.gmail.com/

---
v2 changes:
* Mask out the next CBR entry offset SIE stores in the low bits of cbrlo
  before handing the address to kfree()
* Use __free(kfree) in handle_sthyi()

v1: https://patch.msgid.link/20260902-s390-kvm-v1-0-3bc0986550b1@kernel.org

---
Mike Rapoport (Microsoft) (4):
      KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer
      KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer
      KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB
      KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA

 arch/s390/kvm/s390/intercept.c | 23 ++++++++++-------------
 arch/s390/kvm/s390/interrupt.c |  8 ++++----
 arch/s390/kvm/s390/priv.c      | 19 ++++++++++---------
 arch/s390/kvm/s390/s390.c      | 12 ++++++------
 4 files changed, 30 insertions(+), 32 deletions(-)
---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260828-s390-kvm-2015b0d777dc

--
Sincerely yours,
Mike.

[PATCH v2 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer

[Mike Rapoport (Microsoft)]

handle_sthyi() allocates the buffer that receives the STHYI response
block before it is copied to the guest.

This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

The only goto in that function does not unwind anything, it merely skips
to the exit when the function code is invalid. Turn it into an early
return so that the buffer can be freed with __free(kfree).

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/kvm/s390/intercept.c | 23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/arch/s390/kvm/s390/intercept.c b/arch/s390/kvm/s390/intercept.c
index ca1205dfac8b0..f7ad111f259c9 100644
--- a/arch/s390/kvm/s390/intercept.c
+++ b/arch/s390/kvm/s390/intercept.c
@@ -11,6 +11,8 @@
 #include <linux/kvm_host.h>
 #include <linux/errno.h>
 #include <linux/pagemap.h>
+#include <linux/cleanup.h>
+#include <linux/slab.h>
 
 #include <asm/asm-offsets.h>
 #include <asm/irq.h>
@@ -411,9 +413,9 @@ static int handle_partial_execution(struct kvm_vcpu *vcpu)
  */
 int handle_sthyi(struct kvm_vcpu *vcpu)
 {
+	struct sthyi_sctns *sctns __free(kfree) = NULL;
 	int reg1, reg2, cc = 0, r = 0;
 	u64 code, addr, rc = 0;
-	struct sthyi_sctns *sctns = NULL;
 
 	if (!test_kvm_facility(vcpu->kvm, 74))
 		return kvm_s390_inject_program_int(vcpu, PGM_OPERATION);
@@ -430,37 +432,32 @@ int handle_sthyi(struct kvm_vcpu *vcpu)
 		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
 
 	if (code & 0xffff) {
-		cc = 3;
-		rc = 4;
-		goto out;
+		vcpu->run->s.regs.gprs[reg2 + 1] = 4;
+		kvm_s390_set_psw_cc(vcpu, 3);
+		return 0;
 	}
 
 	if (!kvm_s390_pv_cpu_is_protected(vcpu) && (addr & ~PAGE_MASK))
 		return kvm_s390_inject_program_int(vcpu, PGM_SPECIFICATION);
 
-	sctns = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+	sctns = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
 	if (!sctns)
 		return -ENOMEM;
 
 	cc = sthyi_fill(sctns, &rc);
-	if (cc < 0) {
-		free_page((unsigned long)sctns);
+	if (cc < 0)
 		return cc;
-	}
-out:
+
 	if (!cc) {
 		if (kvm_s390_pv_cpu_is_protected(vcpu)) {
 			memcpy(sida_addr(vcpu->arch.sie_block), sctns, PAGE_SIZE);
 		} else {
 			r = write_guest(vcpu, addr, reg2, sctns, PAGE_SIZE);
-			if (r) {
-				free_page((unsigned long)sctns);
+			if (r)
 				return kvm_s390_inject_prog_cond(vcpu, r);
-			}
 		}
 	}
 
-	free_page((unsigned long)sctns);
 	vcpu->run->s.regs.gprs[reg2 + 1] = rc;
 	kvm_s390_set_psw_cc(vcpu, cc);
 	return r;

-- 
2.53.0

[PATCH v2 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer

[Mike Rapoport (Microsoft)]

handle_stsi() allocates the buffer that receives the STSI response block
before it is copied to the guest.

This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

While on it, make the buffer a void pointer to get rid of the casts.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/kvm/s390/priv.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
index b3cb2c2c3aa71..856939acf012f 100644
--- a/arch/s390/kvm/s390/priv.c
+++ b/arch/s390/kvm/s390/priv.c
@@ -14,6 +14,7 @@
 #include <linux/mm_types.h>
 #include <linux/pgtable.h>
 #include <linux/io.h>
+#include <linux/slab.h>
 #include <asm/asm-offsets.h>
 #include <asm/facility.h>
 #include <asm/current.h>
@@ -869,7 +870,7 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 	int fc = (vcpu->run->s.regs.gprs[0] & 0xf0000000) >> 28;
 	int sel1 = vcpu->run->s.regs.gprs[0] & 0xff;
 	int sel2 = vcpu->run->s.regs.gprs[1] & 0xffff;
-	unsigned long mem = 0;
+	void *mem = NULL;
 	u64 operand2;
 	int rc = 0;
 	u8 ar;
@@ -911,19 +912,19 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 	switch (fc) {
 	case 1: /* same handling for 1 and 2 */
 	case 2:
-		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
+		mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
 		if (!mem)
 			goto out_no_data;
-		if (stsi((void *) mem, fc, sel1, sel2))
+		if (stsi(mem, fc, sel1, sel2))
 			goto out_no_data;
 		break;
 	case 3:
 		if (sel1 != 2 || sel2 != 2)
 			goto out_no_data;
-		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
+		mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
 		if (!mem)
 			goto out_no_data;
-		handle_stsi_3_2_2(vcpu, (void *) mem);
+		handle_stsi_3_2_2(vcpu, mem);
 		break;
 	case 15: /* fc 15 is fully handled in userspace */
 		insert_stsi_usr_data(vcpu, operand2, ar, fc, sel1, sel2);
@@ -931,10 +932,10 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 		return -EREMOTE;
 	}
 	if (kvm_s390_pv_cpu_is_protected(vcpu)) {
-		memcpy(sida_addr(vcpu->arch.sie_block), (void *)mem, PAGE_SIZE);
+		memcpy(sida_addr(vcpu->arch.sie_block), mem, PAGE_SIZE);
 		rc = 0;
 	} else {
-		rc = write_guest(vcpu, operand2, ar, (void *)mem, PAGE_SIZE);
+		rc = write_guest(vcpu, operand2, ar, mem, PAGE_SIZE);
 	}
 	if (rc) {
 		rc = kvm_s390_inject_prog_cond(vcpu, rc);
@@ -945,14 +946,14 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
 		rc = -EREMOTE;
 	}
 	trace_kvm_s390_handle_stsi(vcpu, fc, sel1, sel2, operand2);
-	free_page(mem);
+	kfree(mem);
 	kvm_s390_set_psw_cc(vcpu, 0);
 	vcpu->run->s.regs.gprs[0] = 0;
 	return rc;
 out_no_data:
 	kvm_s390_set_psw_cc(vcpu, 3);
 out:
-	free_page(mem);
+	kfree(mem);
 	return rc;
 }
 

-- 
2.53.0

[PATCH v2 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB

[Mike Rapoport (Microsoft)]

kvm_s390_gib_init() allocates the guest information block (GIB). The GIB
is passed to the hardware as a physical address and must be page aligned.

kmalloc() guarantees that a power of two sized allocation is aligned to
its size, so a PAGE_SIZE allocation is page aligned as well.

This buffer can be allocated with kmalloc() as there's nothing special
about it to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/kvm/s390/interrupt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/s390/kvm/s390/interrupt.c b/arch/s390/kvm/s390/interrupt.c
index 0381ae9817035..02065fb8d053e 100644
--- a/arch/s390/kvm/s390/interrupt.c
+++ b/arch/s390/kvm/s390/interrupt.c
@@ -3689,7 +3689,7 @@ void kvm_s390_gib_destroy(void)
 	}
 	chsc_sgib(0);
 	unregister_adapter_interrupt(&gib_alert_irq);
-	free_page((unsigned long)gib);
+	kfree(gib);
 	gib = NULL;
 }
 
@@ -3703,7 +3703,7 @@ int __init kvm_s390_gib_init(u8 nisc)
 		goto out;
 	}
 
-	gib = (struct kvm_s390_gib *)get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
+	gib = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT | GFP_DMA);
 	if (!gib) {
 		rc = -ENOMEM;
 		goto out;
@@ -3722,7 +3722,7 @@ int __init kvm_s390_gib_init(u8 nisc)
 	gib_origin = virt_to_phys(gib);
 	if (chsc_sgib(gib_origin)) {
 		pr_err("Associating the GIB with the AIV facility failed\n");
-		free_page((unsigned long)gib);
+		kfree(gib);
 		gib = NULL;
 		rc = -EIO;
 		goto out_unreg_gal;
@@ -3742,7 +3742,7 @@ int __init kvm_s390_gib_init(u8 nisc)
 out_unreg_gal:
 	unregister_adapter_interrupt(&gib_alert_irq);
 out_free_gib:
-	free_page((unsigned long)gib);
+	kfree(gib);
 	gib = NULL;
 out:
 	return rc;

-- 
2.53.0

[PATCH v2 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA

[Mike Rapoport (Microsoft)]

kvm_arch_init_vm() allocates struct sie_page2 that holds the facility
list and the crypto control block of a VM and kvm_s390_vcpu_setup_cmma()
allocates the CMMA collaborative memory management block referenced by
the SIE control block. Both are passed to the hardware as physical
addresses and must be page aligned.

kmalloc() guarantees that a power of two sized allocation is aligned to
its size, so a PAGE_SIZE allocation is page aligned as well.

These buffers can be allocated with kmalloc() as there's nothing special
about them to go directly to the page allocator.

kmalloc() provides a better API that does not require ugly casts and
kfree() does not need to know the size of the freed object.

Performance difference between kmalloc() and __get_free_pages() is not
measurable as both allocators take an object/page from a per-CPU list for
fast path allocations.

For the slow path the performance is anyway determined by the amount of
reclaim involved rather than by what allocator is used.

The sie_page allocated in kvm_arch_vcpu_create() is left alone because it
is mapped to userspace with virt_to_page() in kvm_arch_vcpu_fault() and
therefore really needs a struct page.

The SIE hardware stores the offset of the next CBR entry in the low bits
of cbrlo, so mask them out before converting cbrlo back to a virtual
address. free_page() did not care, kfree() needs the address kzalloc()
returned.

Replace use of get_zeroed_page() with kzalloc() and free_page() with
kfree().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
Assisted-by: copilot:claude-opus
Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>
---
 arch/s390/kvm/s390/s390.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/s390/kvm/s390/s390.c b/arch/s390/kvm/s390/s390.c
index b0839e887221e..1d9eea64f3dbd 100644
--- a/arch/s390/kvm/s390/s390.c
+++ b/arch/s390/kvm/s390/s390.c
@@ -3273,8 +3273,8 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 		goto out_err;
 
 	BUILD_BUG_ON(sizeof(struct sie_page2) != 4096);
-	kvm->arch.sie_page2 =
-	     (struct sie_page2 *) get_zeroed_page(GFP_KERNEL_ACCOUNT | GFP_DMA);
+	kvm->arch.sie_page2 = kzalloc_obj(*kvm->arch.sie_page2,
+					  GFP_KERNEL_ACCOUNT | GFP_DMA);
 	if (!kvm->arch.sie_page2)
 		goto out_err;
 
@@ -3369,7 +3369,7 @@ int kvm_arch_init_vm(struct kvm *kvm, unsigned long type)
 
 	return 0;
 out_err:
-	free_page((unsigned long)kvm->arch.sie_page2);
+	kfree(kvm->arch.sie_page2);
 	debug_unregister(kvm->arch.dbf);
 	sca_dispose(kvm);
 	KVM_EVENT(3, "creation of vm failed: %d", rc);
@@ -3427,7 +3427,7 @@ void kvm_arch_destroy_vm(struct kvm *kvm)
 		mmu_notifier_unregister(&kvm->arch.pv.mmu_notifier, kvm->mm);
 
 	debug_unregister(kvm->arch.dbf);
-	free_page((unsigned long)kvm->arch.sie_page2);
+	kfree(kvm->arch.sie_page2);
 	kvm_s390_destroy_adapters(kvm);
 	kvm_s390_clear_float_irqs(kvm);
 	kvm_s390_vsie_destroy(kvm);
@@ -3657,13 +3657,13 @@ static void kvm_s390_vcpu_crypto_setup(struct kvm_vcpu *vcpu)
 void kvm_s390_vcpu_unsetup_cmma(struct kvm_vcpu *vcpu)
 {
 	if (vcpu->arch.sie_block->cbrlo)
-		free_page((unsigned long)phys_to_virt(vcpu->arch.sie_block->cbrlo));
+		kfree(phys_to_virt(vcpu->arch.sie_block->cbrlo & PAGE_MASK));
 	vcpu->arch.sie_block->cbrlo = 0;
 }
 
 int kvm_s390_vcpu_setup_cmma(struct kvm_vcpu *vcpu)
 {
-	void *cbrlo_page = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
+	void *cbrlo_page = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);
 
 	if (!cbrlo_page)
 		return -ENOMEM;

-- 
2.53.0

Re: [PATCH v2 1/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STHYI buffer

[sashiko-bot]

> handle_sthyi() allocates the buffer that receives the STHYI response
> block before it is copied to the guest.
> 
> This buffer can be allocated with kmalloc() as there's nothing special
> about it to go directly to the page allocator.
> 
> kmalloc() provides a better API that does not require ugly casts and
> kfree() does not need to know the size of the freed object.
> 
> Performance difference between kmalloc() and __get_free_pages() is not
> measurable as both allocators take an object/page from a per-CPU list for
> fast path allocations.
> 
> For the slow path the performance is anyway determined by the amount of
> reclaim involved rather than by what allocator is used.
> 
> The only goto in that function does not unwind anything, it merely skips
> to the exit when the function code is invalid. Turn it into an early
> return so that the buffer can be freed with __free(kfree).
> 
> Replace use of get_zeroed_page() with kzalloc() and free_page() with
> kfree().
> 
> Assisted-by: copilot:claude-opus
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-s390-kvm-v2-0-2cf6434e6646@kernel.org?part=1

Re: [PATCH v2 2/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] kzalloc() does not guarantee the 4K-byte page alignment strictly required by the s390x STSI hardware instruction, leading to a host kernel panic.
--

commit 1bfe659dfe76809762877435521d74d061f7364a
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>
KVM: s390: Replace get_zeroed_page() with kzalloc() for the STSI buffer

This commit replaces get_zeroed_page() with kzalloc() when allocating the
STSI response buffer. The intention is to use a cleaner API that does not
require casting and does not need the size when freeing the memory.

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/arch/s390/kvm/s390/priv.c b/arch/s390/kvm/s390/priv.c
> index b3cb2c2c3aa71..856939acf012f 100644
> --- a/arch/s390/kvm/s390/priv.c
> +++ b/arch/s390/kvm/s390/priv.c

[ ... ]

> @@ -911,19 +912,19 @@ static int handle_stsi(struct kvm_vcpu *vcpu)
>  	switch (fc) {
>  	case 1: /* same handling for 1 and 2 */
>  	case 2:
> -		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
> +		mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);

[Severity: Critical]
In handle_stsi(), does kzalloc() guarantee the 4K-byte page alignment that
the s390x stsi hardware instruction requires? The previous get_zeroed_page()
guaranteed page alignment, but kzalloc() might not if kernel SLUB debugging
(e.g., CONFIG_SLUB_DEBUG with redzoning) is enabled.

>  		if (!mem)
>  			goto out_no_data;
> -		if (stsi((void *) mem, fc, sel1, sel2))
> +		if (stsi(mem, fc, sel1, sel2))

[Severity: Critical]
If the buffer is unaligned, will executing the native stsi instruction here
trigger a host specification exception? Since the stsi() inline assembly in
arch/s390/include/asm/sysinfo.h lacks exception table handling, it seems an
unprivileged guest could reliably trigger a host kernel panic by invoking the
stsi instruction.

>  			goto out_no_data;
>  		break;
>  	case 3:
>  		if (sel1 != 2 || sel2 != 2)
>  			goto out_no_data;
> -		mem = get_zeroed_page(GFP_KERNEL_ACCOUNT);
> +		mem = kzalloc(PAGE_SIZE, GFP_KERNEL_ACCOUNT);

[Severity: Critical]
Does this second allocation in handle_stsi() introduce the same alignment
issue when the buffer is passed to handle_stsi_3_2_2() and used in the
stsi() execution there?

>  		if (!mem)
>  			goto out_no_data;
> -		handle_stsi_3_2_2(vcpu, (void *) mem);
> +		handle_stsi_3_2_2(vcpu, mem);
>  		break;
>  	case 15: /* fc 15 is fully handled in userspace */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-s390-kvm-v2-0-2cf6434e6646@kernel.org?part=2

Re: [PATCH v2 4/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for sie_page2 and CMMA

[sashiko-bot]

> kvm_arch_init_vm() allocates struct sie_page2 that holds the facility
> list and the crypto control block of a VM and kvm_s390_vcpu_setup_cmma()
> allocates the CMMA collaborative memory management block referenced by
> the SIE control block. Both are passed to the hardware as physical
> addresses and must be page aligned.
> 
> kmalloc() guarantees that a power of two sized allocation is aligned to
> its size, so a PAGE_SIZE allocation is page aligned as well.
> 
> These buffers can be allocated with kmalloc() as there's nothing special
> about them to go directly to the page allocator.
> 
> kmalloc() provides a better API that does not require ugly casts and
> kfree() does not need to know the size of the freed object.
> 
> [ ... ]
> kfree().
> 
> Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com
> Assisted-by: copilot:claude-opus
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-s390-kvm-v2-0-2cf6434e6646@kernel.org?part=4

Re: [PATCH v2 3/4] KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing hardware teardown (chsc_sgib(0)) on PCI AEN initialization failure, causing a hardware Use-After-Free.
- [High] Premature free and nullification of `gib` creates a race condition leading to a NULL pointer dereference in the adapter interrupt handler.
--

commit 507ef89f01184c6a51cfce1ef86dd6ded4bac518
Author: Mike Rapoport (Microsoft) <rppt@kernel.org>

KVM: s390: Replace get_zeroed_page() with kzalloc() for the GIB

This patch replaces get_zeroed_page() with kzalloc() for the guest
information block (GIB) allocation, as the size is a page and it does not
strictly require the page allocator directly. It also updates the cleanup
paths to use kfree() instead of free_page().

Link: https://lore.kernel.org/all/635405e4-9423-4a25-a6e7-e03c8ea0bcbe@redhat.com

> diff --git a/arch/s390/kvm/s390/interrupt.c b/arch/s390/kvm/s390/interrupt.c
> index 0381ae9817035..02065fb8d053e 100644
> --- a/arch/s390/kvm/s390/interrupt.c
> +++ b/arch/s390/kvm/s390/interrupt.c

[ ... ]

> @@ -3722,7 +3722,7 @@ int __init kvm_s390_gib_init(u8 nisc)
>  	gib_origin = virt_to_phys(gib);
>  	if (chsc_sgib(gib_origin)) {
>  		pr_err("Associating the GIB with the AIV facility failed\n");
> -		free_page((unsigned long)gib);
> +		kfree(gib);
>  		gib = NULL;
>  		rc = -EIO;
>  		goto out_unreg_gal;

[Severity: High]
This is a pre-existing issue, but does nullifying gib here before
unregistering the adapter interrupt create a race condition?

If the chsc_sgib() call fails, the error path prematurely frees gib and
sets it to NULL before jumping to out_unreg_gal to unregister the interrupt.
Between setting gib to NULL and the completion of
unregister_adapter_interrupt() (which waits for active handlers via RCU),
the interrupt handler remains active.

If an adapter interrupt fires on the shared ISC during this window,
gib_alert_irq_handler() is invoked and calls process_gib_alert_list().
This function unconditionally executes xchg(&gib->alert_list_origin, ...),
which would dereference the NULL pointer.

[ ... ]

> @@ -3742,7 +3742,7 @@ int __init kvm_s390_gib_init(u8 nisc)
>  out_unreg_gal:
>  	unregister_adapter_interrupt(&gib_alert_irq);
>  out_free_gib:
> -	free_page((unsigned long)gib);
> +	kfree(gib);
>  	gib = NULL;
>  out:
>  	return rc;

[Severity: High]
This isn't a bug introduced by this patch, but does the error path here
forget to detach the GIB from the hardware?

In kvm_s390_gib_init(), if chsc_sgib(gib_origin) successfully registers the
GIB with the hardware, but kvm_s390_pci_aen_init() subsequently fails, the
error path jumps to out_unreg_gal.

This path frees the gib memory but fails to call chsc_sgib(0) to revoke
hardware access (unlike kvm_s390_gib_destroy() which correctly does so).
The hardware retains the physical address of the freed memory and might
continue to access it, resulting in a hardware use-after-free and potential
memory corruption.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260906-s390-kvm-v2-0-2cf6434e6646@kernel.org?part=3