Re: [PATCH v3 1/2] s390/setup: diag318: remove bit check and refactor struct

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

 



On 4/3/19 8:33 AM, Cornelia Huck wrote:
On Wed, 3 Apr 2019 14:03:21 +0200
David Hildenbrand <david@xxxxxxxxxx> wrote:

On 02.04.19 19:46, Collin Walling wrote:
Execution of DIAGNOSE 0x318 is fenced by checking an SCLP bit
for the availability of hardware support for the instruction.

In order to support this instruction for a KVM/QEMU guest, we
would need to provide modifications to the SCLP Read SCP Info
data, which will in turn reduce the maximum number of CPUs that
may be provided to the guest. This issue introduces compatability
and legacy concerns.

Let's circumvent this issue by removing the bit check and blindly
executing the instruction. An exception table rule is in place to
catch the case where hardware does not support this instruction.

While we're at it, let's condense the version code fields in the
diag318_info struct until we can determine how it will be used.

This modifies commit 4ad78b8651aacf26b3ab6d1e784952eb70469c43

Signed-off-by: Collin Walling <walling@xxxxxxxxxxxxx>
---
  arch/s390/include/asm/diag.h |  6 ++----
  arch/s390/kernel/setup.c     | 12 ++++++------
  2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/arch/s390/include/asm/diag.h b/arch/s390/include/asm/diag.h
index 19562be22b7e..215516284175 100644
--- a/arch/s390/include/asm/diag.h
+++ b/arch/s390/include/asm/diag.h
@@ -298,10 +298,8 @@ struct diag26c_mac_resp {
  union diag318_info {
  	unsigned long val;
  	struct {
-		unsigned int cpnc : 8;
-		unsigned int cpvc_linux : 24;
-		unsigned char cpvc_distro[3];
-		unsigned char zero;
+		unsigned long cpnc : 8;
+		unsigned long cpvc : 56;

That part looks reasonable (we don't have a proper convention yet, have
we?)

  	};
  };
diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
index 2c642af526ce..fe70201f8b5d 100644
--- a/arch/s390/kernel/setup.c
+++ b/arch/s390/kernel/setup.c
@@ -1011,15 +1011,15 @@ static void __init setup_control_program_code(void)
  {
  	union diag318_info diag318_info = {
  		.cpnc = CPNC_LINUX,
-		.cpvc_linux = 0,
-		.cpvc_distro = {0},
+		.cpvc = 0,
  	};
- if (!sclp.has_diag318)
-		return;
-
  	diag_stat_inc(DIAG_STAT_X318);
-	asm volatile("diag %0,0,0x318\n" : : "d" (diag318_info.val));
+	asm volatile(
+		"	diag	%0,0,0x318\n"
+		"0:	nopr	%%r7\n"
+		EX_TABLE(0b,0b)
+		: : "d" (diag318_info.val));
  }
/*

That smells like a nasty hack to not expose new features in QEMU and
deal with the issue of handling CPU limits. No, I don't like this.

Fix QEMU, not the kernel.


I agree. The compat handling is a bit annoying, but I don't think we
can get around it.


Thanks for the feedback, everyone.

The consensus here is to keep the bit check, so I'll throw that back in.
I'll squeeze in a "clean-up" patch for the diag318_info struct in v4.

I'll see that QEMU 4.1 has a max cpu limit of at most 247 (one less than
the current max). If anyone has a suggestion on a better limit
(Christian mentioned 240), please let me know. Otherwise we can discuss
that value in the next version.




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Kernel Development]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Info]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Linux Media]     [Device Mapper]

  Powered by Linux