diff mbox

[v2,02/29] target/s390x: remove some Linux assumptions from IPTE

Message ID 20170529192440.5990-3-aurelien@aurel32.net (mailing list archive)
State New, archived
Headers show

Commit Message

Aurelien Jarno May 29, 2017, 7:24 p.m. UTC
Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
---
 target/s390x/cpu.h        |  2 ++
 target/s390x/mem_helper.c | 17 ++++++++++-------
 target/s390x/mmu_helper.c |  4 +---
 3 files changed, 13 insertions(+), 10 deletions(-)

Comments

Thomas Huth May 30, 2017, 8:20 a.m. UTC | #1
On 29.05.2017 21:24, Aurelien Jarno wrote:
> Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> ---
>  target/s390x/cpu.h        |  2 ++
>  target/s390x/mem_helper.c | 17 ++++++++++-------
>  target/s390x/mmu_helper.c |  4 +---
>  3 files changed, 13 insertions(+), 10 deletions(-)
> 
> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> index 79235cfa45..d89ad83e71 100644
> --- a/target/s390x/cpu.h
> +++ b/target/s390x/cpu.h
> @@ -1033,6 +1033,8 @@ struct sysib_322 {
>  #define _SEGMENT_ENTRY_RO       0x200     /* page protection bit              */
>  #define _SEGMENT_ENTRY_INV      0x20      /* invalid segment table entry      */
>  
> +#define _VADDR_PX               0xff000   /* page index bits                  */

Even though the other #defines start with an underscore, we should not
use this for new code anymore. #defines that start with an underscore +
capital letter are reserved for the C compiler. If you're afraid that
this "VADDR_PX" is not unique enough, maybe rename it to "MMU_VADDR_PX"
instead?

>  #define _PAGE_RO        0x200            /* HW read-only bit  */
>  #define _PAGE_INVALID   0x400            /* HW invalid bit    */
>  #define _PAGE_RES0      0x800            /* bit must be zero  */
[...]

 Thomas
Aurelien Jarno May 30, 2017, 11:19 a.m. UTC | #2
On 2017-05-30 10:20, Thomas Huth wrote:
> On 29.05.2017 21:24, Aurelien Jarno wrote:
> > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > ---
> >  target/s390x/cpu.h        |  2 ++
> >  target/s390x/mem_helper.c | 17 ++++++++++-------
> >  target/s390x/mmu_helper.c |  4 +---
> >  3 files changed, 13 insertions(+), 10 deletions(-)
> > 
> > diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
> > index 79235cfa45..d89ad83e71 100644
> > --- a/target/s390x/cpu.h
> > +++ b/target/s390x/cpu.h
> > @@ -1033,6 +1033,8 @@ struct sysib_322 {
> >  #define _SEGMENT_ENTRY_RO       0x200     /* page protection bit              */
> >  #define _SEGMENT_ENTRY_INV      0x20      /* invalid segment table entry      */
> >  
> > +#define _VADDR_PX               0xff000   /* page index bits                  */
> 
> Even though the other #defines start with an underscore, we should not
> use this for new code anymore. #defines that start with an underscore +
> capital letter are reserved for the C compiler. If you're afraid that
> this "VADDR_PX" is not unique enough, maybe rename it to "MMU_VADDR_PX"
> instead?

Ok, I'll change that, possibly renaming a few other defines.
diff mbox

Patch

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 79235cfa45..d89ad83e71 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1033,6 +1033,8 @@  struct sysib_322 {
 #define _SEGMENT_ENTRY_RO       0x200     /* page protection bit              */
 #define _SEGMENT_ENTRY_INV      0x20      /* invalid segment table entry      */
 
+#define _VADDR_PX               0xff000   /* page index bits                  */
+
 #define _PAGE_RO        0x200            /* HW read-only bit  */
 #define _PAGE_INVALID   0x400            /* HW invalid bit    */
 #define _PAGE_RES0      0x800            /* bit must be zero  */
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e35571e342..0ebd65d9ab 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1073,19 +1073,22 @@  uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 }
 
 /* invalidate pte */
-void HELPER(ipte)(CPUS390XState *env, uint64_t pte_addr, uint64_t vaddr)
+void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr)
 {
     CPUState *cs = CPU(s390_env_get_cpu(env));
     uint64_t page = vaddr & TARGET_PAGE_MASK;
-    uint64_t pte = 0;
+    uint64_t pte_addr, pte;
 
     /* XXX broadcast to other CPUs */
 
-    /* XXX Linux is nice enough to give us the exact pte address.
-       According to spec we'd have to find it out ourselves */
-    /* XXX Linux is fine with overwriting the pte, the spec requires
-       us to only set the invalid bit */
-    stq_phys(cs->as, pte_addr, pte | _PAGE_INVALID);
+    /* Compute the page table entry address */
+    pte_addr = (pto & _SEGMENT_ENTRY_ORIGIN);
+    pte_addr += (vaddr & _VADDR_PX) >> 9;
+
+    /* Mark the page table entry as invalid */
+    pte = ldq_phys(cs->as, pte_addr);
+    pte |= _PAGE_INVALID;
+    stq_phys(cs->as, pte_addr, pte);
 
     /* XXX we exploit the fact that Linux passes the exact virtual
        address here - it's not obliged to! */
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 31eb9efa9b..fb712c8d66 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -143,8 +143,6 @@  static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
     return 0;
 }
 
-#define VADDR_PX    0xff000         /* Page index bits */
-
 /* Decode segment table entry */
 static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
                                  uint64_t asc, uint64_t st_entry,
@@ -167,7 +165,7 @@  static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
 
     /* Look up 4KB page entry */
     origin = st_entry & _SEGMENT_ENTRY_ORIGIN;
-    offs  = (vaddr & VADDR_PX) >> 9;
+    offs  = (vaddr & _VADDR_PX) >> 9;
     pt_entry = ldq_phys(cs->as, origin + offs);
     PTE_DPRINTF("%s: 0x%" PRIx64 " + 0x%" PRIx64 " => 0x%016" PRIx64 "\n",
                 __func__, origin, offs, pt_entry);