Re: [kvm-unit-tests PATCH v5 1/2] s390x: Add specification exception test

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

 



On 8/26/22 13:23, Janis Schoetterl-Glausch wrote:
On Wed, 2022-08-24 at 11:35 +0200, Janosch Frank wrote:
On 7/20/22 16:25, Janis Schoetterl-Glausch wrote:
Generate specification exceptions and check that they occur.

Signed-off-by: Janis Schoetterl-Glausch <scgl@xxxxxxxxxxxxx>
---
   s390x/Makefile           |   1 +
   lib/s390x/asm/arch_def.h |   5 ++
   s390x/spec_ex.c          | 180 +++++++++++++++++++++++++++++++++++++++
   s390x/unittests.cfg      |   3 +
   4 files changed, 189 insertions(+)
   create mode 100644 s390x/spec_ex.c


+
+/*
+ * Load possibly invalid psw, but setup fixup_psw before,
+ * so that fixup_invalid_psw() can bring us back onto the right track.

Not sure if the second line is needed as fixup_psw is a descriptive name already.

+ * Also acts as compiler barrier, -> none required in expect/check_invalid_psw
+ */
+static void load_psw(struct psw psw)
+{
+	uint64_t scratch;
+

[...]

/*
Store a valid mask and the address of the nop into the fixup PSW.
Then load the possibly invalid PSW.
*/

This seems a bit redundant given the function comment, but I can
drop a comment in here describing how the fixup psw is computed.

Well, I skipped the function comment, got confused by the addr asm variable and then decided to propose the comment.

It's a bit confusing since you have the invalid PSW and the global fixup PSW in one function.

Maybe something like:
/* From here to the lpswe we're computing and setting the fixup PSW */



+	fixup_psw.mask = extract_psw_mask();
+	asm volatile ( "larl	%[scratch],0f\n"
+		"	stg	%[scratch],%[addr]\n"
+		"	lpswe	%[psw]\n"
+		"0:	nop\n"
+		: [scratch] "=&d"(scratch),
+		  [addr] "=&T"(fixup_psw.addr)

s/addr/psw_addr/ ?

+		: [psw] "Q"(psw)
+		: "cc", "memory"
+	);
+}
+
+static void load_short_psw(struct short_psw psw)
+{
+	uint64_t scratch;
+
+	fixup_psw.mask = extract_psw_mask();
+	asm volatile ( "larl	%[scratch],0f\n"
+		"	stg	%[scratch],%[addr]\n"
+		"	lpsw	%[psw]\n"
+		"0:	nop\n"
+		: [scratch] "=&d"(scratch),
+		  [addr] "=&T"(fixup_psw.addr)
+		: [psw] "Q"(psw)
+		: "cc", "memory"
+	);

Same story.

Do you want me to repeat the comments here or just rename addr?

Just rename addr


[...]

+static int not_even(void)
+{
+	uint64_t quad[2] __attribute__((aligned(16))) = {0};
+
+	asm volatile (".insn	rxy,0xe3000000008f,%%r7,%[quad]" /* lpq %%r7,%[quad] */
+		      : : [quad] "T"(quad)

Is there a reason you never put a space after the constraint?

TBH I never noticed I'm unusual in that regard. I guess I tend to think
of the operand and constraint as one entity.
I'll add the spaces.


+		      : "%r7", "%r8"
+	);
+	return 0;
+}
+

[...]




[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