[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [qemu-s390x] [PATCH v1 2/2] s390x/tcg: low-address protection suppor
From: |
Cornelia Huck |
Subject: |
Re: [qemu-s390x] [PATCH v1 2/2] s390x/tcg: low-address protection support |
Date: |
Thu, 19 Oct 2017 10:56:26 +0200 |
On Wed, 18 Oct 2017 21:34:07 +0200
David Hildenbrand <address@hidden> wrote:
> On 18.10.2017 20:21, Thomas Huth wrote:
> > On 16.10.2017 22:23, David Hildenbrand wrote:
> >> This is a neat way to implement low address protection, whereby
> >> only the first 512 bytes of the first two pages (each 4096 bytes) of
> >> every address space are protected.
> >>
> >> Store a tec of 0 for the access exception, this is what is defined by
> >> Enhanced Suppression on Protection in case of a low address protection
> >> (Bit 61 set to 0, rest undefined).
> >>
> >> We have to make sure to to pass the access address, not the masked page
> >> address into mmu_translate*().
> >>
> >> Drop the check from testblock. So we can properly test this via
> >> kvm-unit-tests.
> >>
> >> This will check every access going through one of the MMUs.
> >>
> >> Reviewed-by: Richard Henderson <address@hidden>
> >> Signed-off-by: David Hildenbrand <address@hidden>
> >> ---
> >> target/s390x/excp_helper.c | 3 +-
> >> target/s390x/mem_helper.c | 8 ----
> >> target/s390x/mmu_helper.c | 94
> >> +++++++++++++++++++++++++++++-----------------
> >> 3 files changed, 60 insertions(+), 45 deletions(-)
> > [...]
> >> diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
> >> index 9daa0fd8e2..9806685bee 100644
> >> --- a/target/s390x/mmu_helper.c
> >> +++ b/target/s390x/mmu_helper.c
> >> @@ -106,6 +106,35 @@ static void trigger_page_fault(CPUS390XState *env,
> >> target_ulong vaddr,
> >> trigger_access_exception(env, type, ilen, tec);
> >> }
> >>
> >> +/* check whether the address would be proteted by Low-Address Protection
> >> */
> >> +static bool is_low_address(uint64_t addr)
> >> +{
> >> + return addr < 512 || (addr >= 4096 && addr <= 4607);
> >> +}
> >
> > Just cosmetic, but I'd rather either use "<=" or "<" both times, so:
> >
> > return addr <= 511 || (addr >= 4096 && addr <= 4607);
> >
>
> That one then, as it matches the wording in the PoP.
>
> >> + /* Check the private-space control bit */
> >> + switch (asc) {
> >> + case PSW_ASC_PRIMARY:
> >> + return !(env->cregs[1] & _ASCE_PRIVATE_SPACE);
> >> + case PSW_ASC_SECONDARY:
> >> + return !(env->cregs[7] & _ASCE_PRIVATE_SPACE);
> >> + case PSW_ASC_HOME:
> >> + return !(env->cregs[13] & _ASCE_PRIVATE_SPACE);
> >> + default:
> >> + g_assert_not_reached();
> >
> > Well, this is certainly reachable - if the guest was running in access
> > register mode. So it might be nicer to the user if you keep the
> > error_report() here?
>
> Right, this would be reachable via translate_pages(), but not via the
> tlb. Although unlikely to hit it at that point, we can keep the error.
>
> Conny, can you fix these two up or do you want me to resend?
Fixed it up. Can you please verify that
git://github.com/cohuck/qemu lap
looks sane?
Re: [qemu-s390x] [PATCH v1 0/2] s390x/tcg: LAP support using immediate TLB invalidation, Cornelia Huck, 2017/10/17
Re: [qemu-s390x] [PATCH v1 0/2] s390x/tcg: LAP support using immediate TLB invalidation, Cornelia Huck, 2017/10/17