mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v1] KVM: s390: pv: remove misleading gaddr output from KVM_PV_DUMP_CONFIG_STOR_STATE
@ 2026-10-28 11:16 Gautam Gala
  2026-11-05 15:23 ` Christian Borntraeger
  0 siblings, 1 reply; 2+ messages in thread
From: Gautam Gala @ 2026-10-28 11:16 UTC (permalink / raw)
  To: borntraeger, frankja, imbrenda
  Cc: david, hca, gor, agordeev, svens, kvm, linux-s390, linux-kernel

A comment in kvm_s390_pv_dmp() suggested gaddr is an output parameter
for KVM_PV_DUMP_CONFIG_STOR_STATE, and kvm_s390_pv_dump_stor_state()
wrote back uvcb.gaddr through the pointer accordingly.

However, api.rst does not document that KVM_PV_DUMP_CONFIG_STOR_STATE
writes back dump.gaddr and QEMU does not use it. Furthermore, the
write-back would not have reached userspace in any case: dmp was passed
by value to kvm_s390_pv_dmp() so copy_to_user() in the caller always
copied the unmodified input gaddr, not the updated one.

Drop the write-back and the misleading comment, and change the gaddr
parameter of kvm_s390_pv_dump_stor_state() to u64 to reflect its
input-only role.

Fixes: 0460eb35b443 ("KVM: s390: Add configuration dump functionality")
Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com>
Signed-off-by: Gautam Gala <ggala@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
---
 arch/s390/kvm/s390/pv.c   | 16 ++++++----------
 arch/s390/kvm/s390/s390.c |  7 +------
 arch/s390/kvm/s390/s390.h |  2 +-
 3 files changed, 8 insertions(+), 17 deletions(-)

diff --git a/arch/s390/kvm/s390/pv.c b/arch/s390/kvm/s390/pv.c
index b18abd0e29ef..a9bc32d5be6f 100644
--- a/arch/s390/kvm/s390/pv.c
+++ b/arch/s390/kvm/s390/pv.c
@@ -901,10 +901,9 @@ int kvm_s390_pv_dump_cpu(struct kvm_vcpu *vcpu, void *buff, u16 *rc, u16 *rrc)
  *
  * Stores buff_len bytes of tweak component values to buff_user
  * starting with the 1MB block specified by the absolute guest address
- * (gaddr). The gaddr pointer will be updated with the last address
- * for which data was written when returning to userspace. buff_user
- * might be written to even if an error rc is returned. For instance
- * if we encounter a fault after writing the first page of data.
+ * (gaddr). buff_user might be written to even if an error rc is
+ * returned. For instance if we encounter a fault after writing the
+ * first page of data.
  *
  * Context: kvm->lock needs to be held
  *
@@ -917,13 +916,13 @@ int kvm_s390_pv_dump_cpu(struct kvm_vcpu *vcpu, void *buff, u16 *rc, u16 *rrc)
  *  -EFAULT if copying the result to buff_user failed
  */
 int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
-				u64 *gaddr, u64 buff_user_len, u16 *rc, u16 *rrc)
+				u64 gaddr, u64 buff_user_len, u16 *rc, u16 *rrc)
 {
 	struct uv_cb_dump_stor_state uvcb = {
 		.header.cmd = UVC_CMD_DUMP_CONF_STOR_STATE,
 		.header.len = sizeof(uvcb),
 		.config_handle = kvm->arch.pv.handle,
-		.gaddr = *gaddr,
+		.gaddr = gaddr,
 		.dump_area_origin = 0,
 	};
 	const u64 increment_len = uv_info.conf_dump_storage_state_len;
@@ -934,7 +933,7 @@ int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
 
 	ret = -EINVAL;
 	/* UV call processes 1MB guest storage chunks at a time */
-	if (!IS_ALIGNED(*gaddr, HPAGE_SIZE))
+	if (!IS_ALIGNED(gaddr, HPAGE_SIZE))
 		goto out;
 
 	/*
@@ -989,9 +988,6 @@ int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
 		}
 	} while (buff_user_len);
 
-	/* Report back where we ended dumping */
-	*gaddr = uvcb.gaddr;
-
 	/* Lets only log errors, we don't want to spam */
 out:
 	if (ret)
diff --git a/arch/s390/kvm/s390/s390.c b/arch/s390/kvm/s390/s390.c
index e4912db67adc..1f975ebeca82 100644
--- a/arch/s390/kvm/s390/s390.c
+++ b/arch/s390/kvm/s390/s390.c
@@ -2522,12 +2522,7 @@ static int kvm_s390_pv_dmp(struct kvm *kvm, struct kvm_pv_cmd *cmd,
 		if (!kvm->arch.pv.dumping)
 			break;
 
-		/*
-		 * gaddr is an output parameter since we might stop
-		 * early. As dmp will be copied back in our caller, we
-		 * don't need to do it ourselves.
-		 */
-		r = kvm_s390_pv_dump_stor_state(kvm, result_buff, &dmp.gaddr, dmp.buff_len,
+		r = kvm_s390_pv_dump_stor_state(kvm, result_buff, dmp.gaddr, dmp.buff_len,
 						&cmd->rc, &cmd->rrc);
 		break;
 	}
diff --git a/arch/s390/kvm/s390/s390.h b/arch/s390/kvm/s390/s390.h
index aa0d1d062f8d..5d6c36c6ba34 100644
--- a/arch/s390/kvm/s390/s390.h
+++ b/arch/s390/kvm/s390/s390.h
@@ -316,7 +316,7 @@ int kvm_s390_pv_unpack(struct kvm *kvm, unsigned long addr, unsigned long size,
 int kvm_s390_pv_set_cpu_state(struct kvm_vcpu *vcpu, u8 state);
 int kvm_s390_pv_dump_cpu(struct kvm_vcpu *vcpu, void *buff, u16 *rc, u16 *rrc);
 int kvm_s390_pv_dump_stor_state(struct kvm *kvm, void __user *buff_user,
-				u64 *gaddr, u64 buff_user_len, u16 *rc, u16 *rrc);
+				u64 gaddr, u64 buff_user_len, u16 *rc, u16 *rrc);
 int kvm_s390_pv_dump_complete(struct kvm *kvm, void __user *buff_user,
 			      u16 *rc, u16 *rrc);
 int kvm_s390_pv_destroy_page(struct kvm *kvm, unsigned long gaddr);
-- 
2.56.0


^ permalink raw reply	[flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-11-05 15:23 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-28 11:16 [PATCH v1] KVM: s390: pv: remove misleading gaddr output from KVM_PV_DUMP_CONFIG_STOR_STATE Gautam Gala
2026-11-05 15:23 ` Christian Borntraeger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®