Re: [kvm-unit-tests PATCH v2 3/5] s390x: css: implementing Set CHannel Monitor

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/12/21 11:53 AM, Cornelia Huck wrote:
On Wed, 10 Feb 2021 14:20:12 +0100
Pierre Morel <pmorel@xxxxxxxxxxxxx> wrote:

We implement the call of the Set CHannel Monitor instruction,
starting the monitoring of the all Channel Sub System, and
initializing channel subsystem monitoring.

An initial test reports the presence of the extended measurement
block feature.

Several tests on SCHM verify the error reporting of the hypervisor.

Combine these two into one sentence?

"Initial tests report the presence of the extended measurement block
feature, and verify the error reporting of the hypervisor for SCHM."

Also, you add the infrastructure for enabling measurements at the
subchannel -- either mention this in the patch description or move it
to a separate patch or the first user?

yes, I change for one of these solutions, thanks.



Signed-off-by: Pierre Morel <pmorel@xxxxxxxxxxxxx>
---
  lib/s390x/css.h     | 19 +++++++++++-
  lib/s390x/css_lib.c | 74 +++++++++++++++++++++++++++++++++++++++++++++
  s390x/css.c         | 36 ++++++++++++++++++++++
  3 files changed, 128 insertions(+), 1 deletion(-)


(...)

diff --git a/lib/s390x/css_lib.c b/lib/s390x/css_lib.c
index 5426a6b..355881d 100644
--- a/lib/s390x/css_lib.c
+++ b/lib/s390x/css_lib.c
@@ -267,6 +267,80 @@ retry:
  	return -1;
  }
+static bool schib_update(int schid, uint64_t mb, uint16_t mbi, uint16_t flags,
+		  bool format1)

Maybe schib_update_mb()?

yes, it is dedicated.


+{
+	struct pmcw *pmcw = &schib.pmcw;
+	int cc;
+
+	/* Read the SCHIB for this subchannel */
+	cc = stsch(schid, &schib);
+	if (cc) {
+		report_info("stsch: sch %08x failed with cc=%d", schid, cc);
+		return false;
+	}
+
+	/* Update the SCHIB to enable the measurement block */
+	pmcw->flags |= flags;

Do we also want to be able to disable it again?

Yes, I can add the disabling of the measurement on a channel.
In the test I disable it for the system but we may need this later.



+
+	if (format1)
+		pmcw->flags2 |= PMCW_MBF1;
+	else
+		pmcw->flags2 &= ~PMCW_MBF1;
+
+	pmcw->mbi = mbi;
+	schib.mbo = mb;
+
+	/* Tell the CSS we want to modify the subchannel */
+	cc = msch(schid, &schib);
+	if (cc) {
+		/*
+		 * If the subchannel is status pending or
+		 * if a function is in progress,
+		 * we consider both cases as errors.
+		 */
+		report_info("msch: sch %08x failed with cc=%d", schid, cc);
+		return false;
+	}
+
+	/*
+	 * Read the SCHIB again to verify the measurement block origin
+	 */
+	cc = stsch(schid, &schib);
+	if (cc) {
+		report_info("stsch: updating sch %08x failed with cc=%d",
+			    schid, cc);
+		return false;
+	}

Hm, you only do the stsch, but do not check the result (that is done by
the caller) -- remove the misleading comment or replace it with "Read
the SCHIB again"?

right, "Read the SCHIB again"


+
+	return true;
+}
+

(...)

Otherwise, LGTM.


Thanks,
Pierre

--
Pierre Morel
IBM Lab Boeblingen



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux