qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v1 2/2] s390x/tcg: low-address protection suppor


From: Cornelia Huck
Subject: Re: [Qemu-devel] [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?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]