[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [PATCH-for-4.2 v1 3/9] s390x/mmu: DAT translation rewri
From: |
David Hildenbrand |
Subject: |
Re: [qemu-s390x] [PATCH-for-4.2 v1 3/9] s390x/mmu: DAT translation rewrite |
Date: |
Mon, 19 Aug 2019 13:58:53 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 19.08.19 13:40, Thomas Huth wrote:
> On 8/5/19 5:29 PM, David Hildenbrand wrote:
>> Let's rewrite the DAT translation in a non-recursive way, similar to
>> arch/s390/kvm/gaccess.c:guest_translate() in KVM. This makes the
>> code much easier to read, compare and maintain.
>
> Ok, I just had another look at this patch, and even if I don't like the
> complete rewrite just for the sake of it, the new code looks ok to me.
>
> [...]
>> + switch (asce & ASCE_TYPE_MASK) {
>> + case ASCE_TYPE_REGION1:
>> + if (read_table_entry(gaddr, &entry)) {
>> + return PGM_ADDRESSING;
>> + }
>> + if (entry & REGION_ENTRY_I) {
>> + return PGM_REG_FIRST_TRANS;
>> + }
>> + if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION1) {
>> + return PGM_TRANS_SPEC;
>> + }
>> + if (VADDR_REGION2_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
>> + VADDR_REGION2_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
>> + return PGM_REG_SEC_TRANS;
>> + }
>> + if (edat1 && (entry & REGION_ENTRY_P)) {
>> + *flags &= ~PAGE_WRITE;
>> + }
>> + gaddr = (entry & REGION_ENTRY_ORIGIN) + VADDR_REGION2_TX(vaddr) * 8;
>> + /* FALL THROUGH */
>> + case ASCE_TYPE_REGION2:
>> + if (read_table_entry(gaddr, &entry)) {
>> + return PGM_ADDRESSING;
>> + }
>> + if (entry & REGION_ENTRY_I) {
>> + return PGM_REG_SEC_TRANS;
>> + }
>> + if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION2) {
>> + return PGM_TRANS_SPEC;
>> + }
>> + if (VADDR_REGION3_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
>> + VADDR_REGION3_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
>> + return PGM_REG_THIRD_TRANS;
>> + }
>> + if (edat1 && (entry & REGION_ENTRY_P)) {
>> + *flags &= ~PAGE_WRITE;
>> + }
>> + gaddr = (entry & REGION_ENTRY_ORIGIN) + VADDR_REGION3_TX(vaddr) * 8;
>> + /* FALL THROUGH */
>> + case ASCE_TYPE_REGION3:
>> + if (read_table_entry(gaddr, &entry)) {
>> + return PGM_ADDRESSING;
>> + }
>> + if (entry & REGION_ENTRY_I) {
>> + return PGM_REG_THIRD_TRANS;
>> + }
>> + if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) {
>> + return PGM_TRANS_SPEC;
>> + }
>> + if (edat1 && (entry & REGION_ENTRY_P)) {
>> + *flags &= ~PAGE_WRITE;
>> + }
>> + if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
>> + VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
>> + return PGM_SEGMENT_TRANS;
>> + }
>> + gaddr = (entry & REGION_ENTRY_ORIGIN) + VADDR_SEGMENT_TX(vaddr) * 8;
>> + /* FALL THROUGH */
>
> If you don't like recursion, maybe you could at least use a for-loop for
> the region tables? ... the code is really quite repetitive here... just
> my 0.02 €.
I'll split this patch further up once I have time. With the unrolling I
am not yet quite sure - the tables tend to get more different with every
new HW release (especially, see the other patches in this series, like
edat2 and IEP) and we want our branch predictor to produce sane
predictions (especially when processing consecutive pages).
>
> Thomas
>
--
Thanks,
David / dhildenb
- Re: [qemu-s390x] [PATCH-for-4.2 v1 5/9] s390x/mmu: Implement access-exception-fetch/store-indication facility, (continued)
[qemu-s390x] [PATCH-for-4.2 v1 3/9] s390x/mmu: DAT translation rewrite, David Hildenbrand, 2019/08/05
Re: [qemu-s390x] [PATCH-for-4.2 v1 3/9] s390x/mmu: DAT translation rewrite, Thomas Huth, 2019/08/19
[qemu-s390x] [PATCH-for-4.2 v1 6/9] s390x/mmu: Implement enhanced suppression-on-protection facility 2, David Hildenbrand, 2019/08/05
[qemu-s390x] [PATCH-for-4.2 v1 7/9] s390x/mmu: Implement Instruction-Execution-Protection Facility, David Hildenbrand, 2019/08/05
[qemu-s390x] [PATCH-for-4.2 v1 8/9] s390x/cpumodel: Prepare for changes of QEMU model, David Hildenbrand, 2019/08/05
[qemu-s390x] [PATCH-for-4.2 v1 9/9] s390x/cpumodel: Add new TCG features to QEMU cpu model, David Hildenbrand, 2019/08/05