Move and improve the struct definitions for DAT tables from gaccess.c
to a new header.
Once in a separate header, the structs become available everywhere. One
possible usecase is to merge them in the s390 pte_t and p?d_t
definitions, which is left as an exercise for the reader.
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
---
arch/s390/include/asm/dat-bits.h | 170 +++++++++++++++++++++++++++++++
arch/s390/kvm/gaccess.c | 163 +----------------------------
2 files changed, 173 insertions(+), 160 deletions(-)
create mode 100644 arch/s390/include/asm/dat-bits.h
diff --git a/arch/s390/include/asm/dat-bits.h b/arch/s390/include/asm/dat-bits.h
new file mode 100644
index 000000000000..d8afd13be48c
--- /dev/null
+++ b/arch/s390/include/asm/dat-bits.h
@@ -0,0 +1,170 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * DAT table and related structures
+ *
+ * Copyright IBM Corp. 2024
+ *
+ */
+
+#ifndef _S390_DAT_BITS_H
+#define _S390_DAT_BITS_H
+
+union asce {
+ unsigned long val;
+ struct {
+ unsigned long rsto: 52;/* Region- or Segment-Table Origin */
+ unsigned long : 2;
+ unsigned long g : 1; /* Subspace Group control */
+ unsigned long p : 1; /* Private Space control */
+ unsigned long s : 1; /* Storage-Alteration-Event control */
+ unsigned long x : 1; /* Space-Switch-Event control */
+ unsigned long r : 1; /* Real-Space control */
+ unsigned long : 1;
+ unsigned long dt : 2; /* Designation-Type control */
+ unsigned long tl : 2; /* Region- or Segment-Table Length */
+ };
+};
+
+enum {
+ ASCE_TYPE_SEGMENT = 0,
+ ASCE_TYPE_REGION3 = 1,
+ ASCE_TYPE_REGION2 = 2,
+ ASCE_TYPE_REGION1 = 3
+};
+
+union region1_table_entry {
+ unsigned long val;
+ struct {
+ unsigned long rto: 52;/* Region-Table Origin */
+ unsigned long : 2;
+ unsigned long p : 1; /* DAT-Protection Bit */
+ unsigned long : 1;
+ unsigned long tf : 2; /* Region-Second-Table Offset */
+ unsigned long i : 1; /* Region-Invalid Bit */
+ unsigned long : 1;
+ unsigned long tt : 2; /* Table-Type Bits */
+ unsigned long tl : 2; /* Region-Second-Table Length */
+ };
+};
+
+union region2_table_entry {
+ unsigned long val;
+ struct {
+ unsigned long rto: 52;/* Region-Table Origin */
+ unsigned long : 2;
+ unsigned long p : 1; /* DAT-Protection Bit */
+ unsigned long : 1;
+ unsigned long tf : 2; /* Region-Third-Table Offset */
+ unsigned long i : 1; /* Region-Invalid Bit */
+ unsigned long : 1;
+ unsigned long tt : 2; /* Table-Type Bits */
+ unsigned long tl : 2; /* Region-Third-Table Length */
+ };
+};
+
+struct region3_table_entry_fc0 {
+ unsigned long sto: 52;/* Segment-Table Origin */
+ unsigned long : 1;
+ unsigned long fc : 1; /* Format-Control */
+ unsigned long p : 1; /* DAT-Protection Bit */
+ unsigned long : 1;
+ unsigned long tf : 2; /* Segment-Table Offset */
+ unsigned long i : 1; /* Region-Invalid Bit */
+ unsigned long cr : 1; /* Common-Region Bit */
+ unsigned long tt : 2; /* Table-Type Bits */
+ unsigned long tl : 2; /* Segment-Table Length */
+};
+
+struct region3_table_entry_fc1 {
+ unsigned long rfaa: 33;/* Region-Frame Absolute Address */
+ unsigned long : 14;
+ unsigned long av : 1; /* ACCF-Validity Control */
+ unsigned long acc : 4; /* Access-Control Bits */
+ unsigned long f : 1; /* Fetch-Protection Bit */
+ unsigned long fc : 1; /* Format-Control */
+ unsigned long p : 1; /* DAT-Protection Bit */
+ unsigned long iep : 1; /* Instruction-Execution-Protection */
+ unsigned long : 2;
+ unsigned long i : 1; /* Region-Invalid Bit */
+ unsigned long cr : 1; /* Common-Region Bit */
+ unsigned long tt : 2; /* Table-Type Bits */
+ unsigned long : 2;
+};
+
+union region3_table_entry {
+ unsigned long val;
+ struct region3_table_entry_fc0 fc0;
+ struct region3_table_entry_fc1 fc1;
+ struct {
+ unsigned long : 53;
+ unsigned long fc: 1; /* Format-Control */
+ unsigned long : 4;
+ unsigned long i : 1; /* Region-Invalid Bit */
+ unsigned long cr: 1; /* Common-Region Bit */
+ unsigned long tt: 2; /* Table-Type Bits */
+ unsigned long : 2;
+ };
+};
+
+struct segment_table_entry_fc0 {
+ unsigned long pto: 53;/* Page-Table Origin */
+ unsigned long fc : 1; /* Format-Control */
+ unsigned long p : 1; /* DAT-Protection Bit */
+ unsigned long : 3;
+ unsigned long i : 1; /* Segment-Invalid Bit */
+ unsigned long cs : 1; /* Common-Segment Bit */
+ unsigned long tt : 2; /* Table-Type Bits */
+ unsigned long : 2;
+};
+
+struct segment_table_entry_fc1 {
+ unsigned long sfaa: 44;/* Segment-Frame Absolute Address */
+ unsigned long : 3;
+ unsigned long av : 1; /* ACCF-Validity Control */
+ unsigned long acc : 4; /* Access-Control Bits */
+ unsigned long f : 1; /* Fetch-Protection Bit */
+ unsigned long fc : 1; /* Format-Control */
+ unsigned long p : 1; /* DAT-Protection Bit */
+ unsigned long iep : 1; /* Instruction-Execution-Protection */
+ unsigned long : 2;
+ unsigned long i : 1; /* Segment-Invalid Bit */
+ unsigned long cs : 1; /* Common-Segment Bit */
+ unsigned long tt : 2; /* Table-Type Bits */
+ unsigned long : 2;
+};
+
+union segment_table_entry {
+ unsigned long val;
+ struct segment_table_entry_fc0 fc0;
+ struct segment_table_entry_fc1 fc1;
+ struct {
+ unsigned long : 53;
+ unsigned long fc: 1; /* Format-Control */
+ unsigned long : 4;
+ unsigned long i : 1; /* Segment-Invalid Bit */
+ unsigned long cs: 1; /* Common-Segment Bit */
+ unsigned long tt: 2; /* Table-Type Bits */
+ unsigned long : 2;
+ };
+};
+
+union page_table_entry {
+ unsigned long val;
+ struct {
+ unsigned long pfra: 52;/* Page-Frame Real Address */
+ unsigned long z : 1; /* Zero Bit */
+ unsigned long i : 1; /* Page-Invalid Bit */
+ unsigned long p : 1; /* DAT-Protection Bit */
+ unsigned long iep : 1; /* Instruction-Execution-Protection */
+ unsigned long : 8;
+ };
+};
+
+enum {
+ TABLE_TYPE_SEGMENT = 0,
+ TABLE_TYPE_REGION3 = 1,
+ TABLE_TYPE_REGION2 = 2,
+ TABLE_TYPE_REGION1 = 3
+};
+
+#endif /* _S390_DAT_BITS_H */
diff --git a/arch/s390/kvm/gaccess.c b/arch/s390/kvm/gaccess.c
index 5bf3d94e9dda..e65f597e3044 100644
--- a/arch/s390/kvm/gaccess.c
+++ b/arch/s390/kvm/gaccess.c
@@ -14,167 +14,10 @@
#include <asm/access-regs.h>
#include <asm/fault.h>
#include <asm/gmap.h>
+#include <asm/dat-bits.h>
#include "kvm-s390.h"
#include "gaccess.h"
-union asce {
- unsigned long val;
- struct {
- unsigned long origin : 52; /* Region- or Segment-Table Origin */
- unsigned long : 2;
- unsigned long g : 1; /* Subspace Group Control */
- unsigned long p : 1; /* Private Space Control */
- unsigned long s : 1; /* Storage-Alteration-Event Control */
- unsigned long x : 1; /* Space-Switch-Event Control */
- unsigned long r : 1; /* Real-Space Control */
- unsigned long : 1;
- unsigned long dt : 2; /* Designation-Type Control */
- unsigned long tl : 2; /* Region- or Segment-Table Length */
- };
-};
-
-enum {
- ASCE_TYPE_SEGMENT = 0,
- ASCE_TYPE_REGION3 = 1,
- ASCE_TYPE_REGION2 = 2,
- ASCE_TYPE_REGION1 = 3
-};
-
-union region1_table_entry {
- unsigned long val;
- struct {
- unsigned long rto: 52;/* Region-Table Origin */
- unsigned long : 2;
- unsigned long p : 1; /* DAT-Protection Bit */
- unsigned long : 1;
- unsigned long tf : 2; /* Region-Second-Table Offset */
- unsigned long i : 1; /* Region-Invalid Bit */
- unsigned long : 1;
- unsigned long tt : 2; /* Table-Type Bits */
- unsigned long tl : 2; /* Region-Second-Table Length */
- };
-};
-
-union region2_table_entry {
- unsigned long val;
- struct {
- unsigned long rto: 52;/* Region-Table Origin */
- unsigned long : 2;
- unsigned long p : 1; /* DAT-Protection Bit */
- unsigned long : 1;
- unsigned long tf : 2; /* Region-Third-Table Offset */
- unsigned long i : 1; /* Region-Invalid Bit */
- unsigned long : 1;
- unsigned long tt : 2; /* Table-Type Bits */
- unsigned long tl : 2; /* Region-Third-Table Length */
- };
-};
-
-struct region3_table_entry_fc0 {
- unsigned long sto: 52;/* Segment-Table Origin */
- unsigned long : 1;
- unsigned long fc : 1; /* Format-Control */
- unsigned long p : 1; /* DAT-Protection Bit */
- unsigned long : 1;
- unsigned long tf : 2; /* Segment-Table Offset */
- unsigned long i : 1; /* Region-Invalid Bit */
- unsigned long cr : 1; /* Common-Region Bit */
- unsigned long tt : 2; /* Table-Type Bits */
- unsigned long tl : 2; /* Segment-Table Length */
-};
-
-struct region3_table_entry_fc1 {
- unsigned long rfaa : 33; /* Region-Frame Absolute Address */
- unsigned long : 14;
- unsigned long av : 1; /* ACCF-Validity Control */
- unsigned long acc: 4; /* Access-Control Bits */
- unsigned long f : 1; /* Fetch-Protection Bit */
- unsigned long fc : 1; /* Format-Control */
- unsigned long p : 1; /* DAT-Protection Bit */
- unsigned long iep: 1; /* Instruction-Execution-Protection */
- unsigned long : 2;
- unsigned long i : 1; /* Region-Invalid Bit */
- unsigned long cr : 1; /* Common-Region Bit */
- unsigned long tt : 2; /* Table-Type Bits */
- unsigned long : 2;
-};
-
-union region3_table_entry {
- unsigned long val;
- struct region3_table_entry_fc0 fc0;
- struct region3_table_entry_fc1 fc1;
- struct {
- unsigned long : 53;
- unsigned long fc : 1; /* Format-Control */
- unsigned long : 4;
- unsigned long i : 1; /* Region-Invalid Bit */
- unsigned long cr : 1; /* Common-Region Bit */
- unsigned long tt : 2; /* Table-Type Bits */
- unsigned long : 2;
- };
-};
-
-struct segment_entry_fc0 {
- unsigned long pto: 53;/* Page-Table Origin */
- unsigned long fc : 1; /* Format-Control */
- unsigned long p : 1; /* DAT-Protection Bit */
- unsigned long : 3;
- unsigned long i : 1; /* Segment-Invalid Bit */
- unsigned long cs : 1; /* Common-Segment Bit */
- unsigned long tt : 2; /* Table-Type Bits */
- unsigned long : 2;
-};
-
-struct segment_entry_fc1 {
- unsigned long sfaa : 44; /* Segment-Frame Absolute Address */
- unsigned long : 3;
- unsigned long av : 1; /* ACCF-Validity Control */
- unsigned long acc: 4; /* Access-Control Bits */
- unsigned long f : 1; /* Fetch-Protection Bit */
- unsigned long fc : 1; /* Format-Control */
- unsigned long p : 1; /* DAT-Protection Bit */
- unsigned long iep: 1; /* Instruction-Execution-Protection */
- unsigned long : 2;
- unsigned long i : 1; /* Segment-Invalid Bit */
- unsigned long cs : 1; /* Common-Segment Bit */
- unsigned long tt : 2; /* Table-Type Bits */
- unsigned long : 2;
-};
-
-union segment_table_entry {
- unsigned long val;
- struct segment_entry_fc0 fc0;
- struct segment_entry_fc1 fc1;
- struct {
- unsigned long : 53;
- unsigned long fc : 1; /* Format-Control */
- unsigned long : 4;
- unsigned long i : 1; /* Segment-Invalid Bit */
- unsigned long cs : 1; /* Common-Segment Bit */
- unsigned long tt : 2; /* Table-Type Bits */
- unsigned long : 2;
- };
-};
-
-enum {
- TABLE_TYPE_SEGMENT = 0,
- TABLE_TYPE_REGION3 = 1,
- TABLE_TYPE_REGION2 = 2,
- TABLE_TYPE_REGION1 = 3
-};
-
-union page_table_entry {
- unsigned long val;
- struct {
- unsigned long pfra : 52; /* Page-Frame Real Address */
- unsigned long z : 1; /* Zero Bit */
- unsigned long i : 1; /* Page-Invalid Bit */
- unsigned long p : 1; /* DAT-Protection Bit */
- unsigned long iep: 1; /* Instruction-Execution-Protection */
- unsigned long : 8;
- };
-};
-
/*
* vaddress union in order to easily decode a virtual address into its
* region first index, region second index etc. parts.
@@ -632,7 +475,7 @@ static unsigned long guest_translate(struct kvm_vcpu *vcpu, unsigned long gva,
iep = ctlreg0.iep && test_kvm_facility(vcpu->kvm, 130);
if (asce.r)
goto real_address;
- ptr = asce.origin * PAGE_SIZE;
+ ptr = asce.rsto * PAGE_SIZE;
switch (asce.dt) {
case ASCE_TYPE_REGION1:
if (vaddr.rfx01 > asce.tl)
@@ -1379,7 +1222,7 @@ static int kvm_s390_shadow_tables(struct gmap *sg, unsigned long saddr,
parent = sg->parent;
vaddr.addr = saddr;
asce.val = sg->orig_asce;
- ptr = asce.origin * PAGE_SIZE;
+ ptr = asce.rsto * PAGE_SIZE;
if (asce.r) {
*fake = 1;
ptr = 0;
--
2.45.2On Wed, Jul 03, 2024 at 05:59:00PM +0200, Claudio Imbrenda wrote: Hi Claudio, > Once in a separate header, the structs become available everywhere. One > possible usecase is to merge them in the s390 > definitions, which is left as an exercise for the reader. Is my understanding correct that you potentially see page_table_entry::val / region?_table_entry.*::val / segment_table_entry.* merged with pte_t::pte / p?d_t::p?d? Thanks!
On Thu, 11 Jul 2024 17:16:23 +0200
Alexander Gordeev <agordeev@linux.ibm.com> wrote:
> On Wed, Jul 03, 2024 at 05:59:00PM +0200, Claudio Imbrenda wrote:
>
> Hi Claudio,
>
> > Once in a separate header, the structs become available everywhere. One
> > possible usecase is to merge them in the s390
> > definitions, which is left as an exercise for the reader.
>
> Is my understanding correct that you potentially see page_table_entry::val /
> region?_table_entry.*::val / segment_table_entry.* merged with pte_t::pte /
> p?d_t::p?d?
>
> Thanks!
that depends on how you want to do the merge
you could do:
typedef union {
unsigned long pte;
union page_table_entry hw;
union page_table_entry_softbits sw;
} pte_t;
then you would have pte_t::pte and pte_t::hw::val; unfortunately it's
not possible to anonymously merge a named type..
this would be great but can't be done*:
typedef union {
unsigned long pte;
union page_table_entry;
} pte_t;
[*] gcc actually supports it with an additional feature switch, but
it's not standard C and I seriously doubt we should even think about
doing it
another possibility is a plain
typedef union page_table_entry pte_t;
and then fix pte_val() and similar, but then you won't have the
softbits.
in the end, it's up to you how you want to merge them. I will
have my own unions that I will use only inside KVM, that's enough for
me.
On Thu, Jul 11, 2024 at 06:23:48PM +0200, Claudio Imbrenda wrote:
> On Thu, 11 Jul 2024 17:16:23 +0200
> Alexander Gordeev <agordeev@linux.ibm.com> wrote:
> > Hi Claudio,
> >
> > > Once in a separate header, the structs become available everywhere. One
> > > possible usecase is to merge them in the s390
> > > definitions, which is left as an exercise for the reader.
> >
> > Is my understanding correct that you potentially see page_table_entry::val /
> > region?_table_entry.*::val / segment_table_entry.* merged with pte_t::pte /
> > p?d_t::p?d?
>
> that depends on how you want to do the merge
>
> you could do:
>
> typedef union {
> unsigned long pte;
> union page_table_entry hw;
> union page_table_entry_softbits sw;
> } pte_t;
>
> then you would have pte_t::pte and pte_t::hw::val; unfortunately it's
> not possible to anonymously merge a named type..
>
> this would be great but can't be done*:
>
> typedef union {
> unsigned long pte;
> union page_table_entry;
> } pte_t;
>
> [*] gcc actually supports it with an additional feature switch, but
> it's not standard C and I seriously doubt we should even think about
> doing it
>
> another possibility is a plain
>
> typedef union page_table_entry pte_t;
>
> and then fix pte_val() and similar, but then you won't have the
> softbits.
>
>
> in the end, it's up to you how you want to merge them. I will
> have my own unions that I will use only inside KVM, that's enough for
> me.
We discussed this, and I really don't think we want this for the software
defined bits. In general I do like the idea of having bit fields and let
gcc do the decoding. But we have so many masks we use every for ptes and
friends that a single assignment won't work in most cases. So we eiter have
to do several assignments (which sometimes is a no-go when block concurrent
updates are required), or we have to stay with using both the existing
defines plus the new unions - which makes things even more complicated.
There might be uses cases where the hardware structures are also useful for
s390 core code, but I don't think we should go the route outlined above.
Quoting Claudio Imbrenda (2024-07-03 17:59:00) > Move and improve the struct definitions for DAT tables from gaccess.c > to a new header. > > Once in a separate header, the structs become available everywhere. One > possible usecase is to merge them in the s390 pte_t and p?d_t > definitions, which is left as an exercise for the reader. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
© 2016 - 2026 Red Hat, Inc.