On 01.10.19 06:54, Thomas Huth wrote: > On 27/09/2019 11.58, David Hildenbrand wrote: >> Let's use consitent names for the region/section/page table entries and >> for the macros to extract relevant parts from virtual address. Make them >> match the definitions in the PoP - e.g., how the relevant bits are actually >> called. >> >> Introduce defines for all bits declared in the PoP. This will come in >> handy in follow-up patches. >> >> Add a note where additional information about s390x and the used >> definitions can be found. >> >> Signed-off-by: David Hildenbrand <da...@redhat.com> >> --- >> target/s390x/cpu.h | 81 +++++++++++++++++++++++++++++---------- >> target/s390x/mem_helper.c | 12 +++--- >> target/s390x/mmu_helper.c | 37 ++++++++++-------- >> 3 files changed, 87 insertions(+), 43 deletions(-) >> >> diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h >> index 163dae13d7..690b94c8ea 100644 >> --- a/target/s390x/cpu.h >> +++ b/target/s390x/cpu.h >> @@ -1,6 +1,10 @@ >> /* >> * S/390 virtual CPU header >> * >> + * For details on the s390x architecture and used definitions (e.g., >> + * PSW, PER and DAT (Dynamic Address Translation)), please refer to >> + * the "z/Architecture Principles of Operations" - a.k.a. PoP. >> + * >> * Copyright (c) 2009 Ulrich Hecht >> * Copyright IBM Corp. 2012, 2018 >> * >> @@ -558,26 +562,63 @@ QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096); >> #define ASCE_TYPE_SEGMENT 0x00 /* segment table type >> */ >> #define ASCE_TABLE_LENGTH 0x03 /* region table length >> */ >> >> -#define REGION_ENTRY_ORIGIN (~0xfffULL) /* region/segment table origin >> */ >> -#define REGION_ENTRY_RO 0x200 /* region/segment protection bit >> */ >> -#define REGION_ENTRY_TF 0xc0 /* region/segment table offset >> */ >> -#define REGION_ENTRY_INV 0x20 /* invalid region table entry >> */ >> -#define REGION_ENTRY_TYPE_MASK 0x0c /* region/segment table type mask >> */ >> -#define REGION_ENTRY_TYPE_R1 0x0c /* region first table type >> */ >> -#define REGION_ENTRY_TYPE_R2 0x08 /* region second table type >> */ >> -#define REGION_ENTRY_TYPE_R3 0x04 /* region third table type >> */ >> -#define REGION_ENTRY_LENGTH 0x03 /* region third length >> */ >> - >> -#define SEGMENT_ENTRY_ORIGIN (~0x7ffULL) /* segment table origin */ >> -#define SEGMENT_ENTRY_FC 0x400 /* format control */ >> -#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 */ >> +#define REGION_ENTRY_ORIGIN 0xfffffffffffff000ULL >> +#define REGION_ENTRY_P 0x0000000000000200ULL >> +#define REGION_ENTRY_TF 0x00000000000000c0ULL >> +#define REGION_ENTRY_I 0x0000000000000020ULL >> +#define REGION_ENTRY_TT 0x000000000000000cULL >> +#define REGION_ENTRY_TL 0x0000000000000003ULL >> + >> +#define REGION_ENTRY_TT_REGION1 0x000000000000000cULL >> +#define REGION_ENTRY_TT_REGION2 0x0000000000000008ULL >> +#define REGION_ENTRY_TT_REGION3 0x0000000000000004ULL >> + >> +#define REGION3_ENTRY_RFAA 0xffffffff80000000ULL >> +#define REGION3_ENTRY_AV 0x0000000000010000ULL >> +#define REGION3_ENTRY_ACC 0x000000000000f000ULL >> +#define REGION3_ENTRY_F 0x0000000000000800ULL >> +#define REGION3_ENTRY_FC 0x0000000000000400ULL >> +#define REGION3_ENTRY_IEP 0x0000000000000100ULL >> +#define REGION3_ENTRY_CR 0x0000000000000010ULL >> + >> +#define SEGMENT_ENTRY_ORIGIN 0xfffffffffffff800ULL >> +#define SEGMENT_ENTRY_SFAA 0xfffffffffff80000ULL > > I think SFAA should be 0xfffffffffff00000ULL instead?
Indeed, very true. I think I finallz have to add EDAT-1/EDAT-2 support, so I can test these code paths :) > >> +#define SEGMENT_ENTRY_AV 0x0000000000010000ULL >> +#define SEGMENT_ENTRY_ACC 0x000000000000f000ULL >> +#define SEGMENT_ENTRY_F 0x0000000000000800ULL >> +#define SEGMENT_ENTRY_FC 0x0000000000000400ULL >> +#define SEGMENT_ENTRY_P 0x0000000000000200ULL >> +#define SEGMENT_ENTRY_IEP 0x0000000000000100ULL >> +#define SEGMENT_ENTRY_I 0x0000000000000020ULL >> +#define SEGMENT_ENTRY_CS 0x0000000000000010ULL >> +#define SEGMENT_ENTRY_TT 0x000000000000000cULL >> + >> +#define SEGMENT_ENTRY_TT_REGION1 0x000000000000000cULL >> +#define SEGMENT_ENTRY_TT_REGION2 0x0000000000000008ULL >> +#define SEGMENT_ENTRY_TT_REGION3 0x0000000000000004ULL > > The above three definitions do not make much sense. TT should always be > 00 for segment table entries. Right, I thought I would use them for error checking, but (entry & SEGMENT_ENTRY_TT) != SEGMENT_ENTRY_TT_SEGMENT Is enough. Will drop, thanks! -- Thanks, David / dhildenb
