[PATCH] KVM: PPC: Book3S HV: Free the queue pages in error path

[PATCH] KVM: PPC: Book3S HV: Free the queue pages in error path

[Gautam Menghani]

If the xive_native_configure_queue() call fails, the qpage allocated is
not freed, which results in wasted memory. Fix this by freeing the qpage
in the error path.

Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller")
Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
---
 arch/powerpc/kvm/book3s_xive.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
index 1d67237783b7..6f8648d9302b 100644
--- a/arch/powerpc/kvm/book3s_xive.c
+++ b/arch/powerpc/kvm/book3s_xive.c
@@ -961,9 +961,11 @@ static int xive_provision_queue(struct kvm_vcpu *vcpu, u8 prio)
 	 */
 	rc = xive_native_configure_queue(xc->vp_id, q, prio, qpage,
 					 xive->q_order, true);
-	if (rc)
+	if (rc) {
+		free_pages((unsigned long)qpage, xive->q_page_order);
 		pr_err("Failed to configure queue %d for VCPU %d\n",
 		       prio, xc->server_num);
+	}
 	return rc;
 }
 
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] KVM: PPC: Book3S HV: Free the queue pages in error path

[Amit Machhiwal]

On 2026/09/16 04:42 PM, Gautam Menghani wrote:
> If the xive_native_configure_queue() call fails, the qpage allocated is
> not freed, which results in wasted memory. Fix this by freeing the qpage
> in the error path.
> 
> Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller")
> Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> ---
>  arch/powerpc/kvm/book3s_xive.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index 1d67237783b7..6f8648d9302b 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -961,9 +961,11 @@ static int xive_provision_queue(struct kvm_vcpu *vcpu, u8 prio)
>  	 */
>  	rc = xive_native_configure_queue(xc->vp_id, q, prio, qpage,
>  					 xive->q_order, true);
> -	if (rc)
> +	if (rc) {
> +		free_pages((unsigned long)qpage, xive->q_page_order);
>  		pr_err("Failed to configure queue %d for VCPU %d\n",
>  		       prio, xc->server_num);

Fix looks correct.  xive_native_configure_queue() only sets q->qpage on success,
so without this fix the cleanup path in kvmppc_xive_cleanup_vcpu() would never
reclaim the pages.  The use of q_page_order for freeing is consistent with the
rest of the file.

One nit: I would prefer emitting the error log first and then perform cleanup
which I think looks more consistent:

	if (rc) {
		pr_err("Failed to configure queue %d for VCPU %d\n",
		       prio, xc->server_num);
		free_pages((unsigned long)qpage, xive->q_page_order);
	}

Not a blocker though.

Reviewed-by: Amit Machhiwal <amachhiw@linux.ibm.com>

Thanks,
Amit

Re: [PATCH] KVM: PPC: Book3S HV: Free the queue pages in error path

[Gautam Menghani]

On Wed, Sep 16, 2026 at 08:42:15PM +0530, Amit Machhiwal wrote:
> On 2026/09/16 04:42 PM, Gautam Menghani wrote:
> > If the xive_native_configure_queue() call fails, the qpage allocated is
> > not freed, which results in wasted memory. Fix this by freeing the qpage
> > in the error path.
> > 
> > Fixes: 5af50993850a ("KVM: PPC: Book3S HV: Native usage of the XIVE interrupt controller")
> > Signed-off-by: Gautam Menghani <gautam@linux.ibm.com>
> > ---
> >  arch/powerpc/kvm/book3s_xive.c | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> > index 1d67237783b7..6f8648d9302b 100644
> > --- a/arch/powerpc/kvm/book3s_xive.c
> > +++ b/arch/powerpc/kvm/book3s_xive.c
> > @@ -961,9 +961,11 @@ static int xive_provision_queue(struct kvm_vcpu *vcpu, u8 prio)
> >  	 */
> >  	rc = xive_native_configure_queue(xc->vp_id, q, prio, qpage,
> >  					 xive->q_order, true);
> > -	if (rc)
> > +	if (rc) {
> > +		free_pages((unsigned long)qpage, xive->q_page_order);
> >  		pr_err("Failed to configure queue %d for VCPU %d\n",
> >  		       prio, xc->server_num);
> 
> Fix looks correct.  xive_native_configure_queue() only sets q->qpage on success,
> so without this fix the cleanup path in kvmppc_xive_cleanup_vcpu() would never
> reclaim the pages.  The use of q_page_order for freeing is consistent with the
> rest of the file.
> 
> One nit: I would prefer emitting the error log first and then perform cleanup
> which I think looks more consistent:

Yes agreed, will send a v2.

Thanks,
Gautam