diff mbox

[4/4] tcg-hppa: Compute is_write in cpu_signal_handler.

Message ID 14f1f6f787a15f6d6a5c3ce7f49a2e682aa339e7.1268754659.git.rth@twiddle.net
State New
Headers show

Commit Message

Richard Henderson March 12, 2010, 2:58 p.m. UTC
Signed-off-by: Richard Henderson <rth@twiddle.net>
---
 cpu-exec.c |   38 +++++++++++++++++++++++++++++++-------
 1 files changed, 31 insertions(+), 7 deletions(-)

Comments

Stuart Brady March 17, 2010, 2:10 a.m. UTC | #1
On Fri, Mar 12, 2010 at 03:58:08PM +0100, Richard Henderson wrote:
> Signed-off-by: Richard Henderson <rth@twiddle.net>

Acked-by: Stuart Brady <stuart.brady@gmail.com>

Argh.  It just seems mind bogglingly silly that is_write doesn't seem to
be included in the siginfo on archs where doing so would make sense...

It's at least something I'd have hoped libc could take care of in the case
where the hardware doesn't indicate whether the fault was due to a read
or a write...

(Right up there with the use of our own caching flushing code instead of
calls to mprotect(), etc. and the rather dubious use of qemu-lock.h...)

> ---
>  cpu-exec.c |   38 +++++++++++++++++++++++++++++++-------
>  1 files changed, 31 insertions(+), 7 deletions(-)
> 
> diff --git a/cpu-exec.c b/cpu-exec.c
> index bcfcda2..14204f4 100644
> --- a/cpu-exec.c
> +++ b/cpu-exec.c
> @@ -1193,15 +1193,39 @@ int cpu_signal_handler(int host_signum, void *pinfo,
>  {
>      struct siginfo *info = pinfo;
>      struct ucontext *uc = puc;
> -    unsigned long pc;
> -    int is_write;
> +    unsigned long pc = uc->uc_mcontext.sc_iaoq[0];
> +    uint32_t insn = *(uint32_t *)pc;
> +    int is_write = 0;
> +
> +    /* XXX: need kernel patch to get write flag faster.  */
> +    switch (insn >> 26) {
> +    case 0x1a: /* STW */
> +    case 0x19: /* STH */
> +    case 0x18: /* STB */
> +    case 0x1b: /* STWM */
> +        is_write = 1;
> +        break;
> +
> +    case 0x09: /* CSTWX, FSTWX, FSTWS */
> +    case 0x0b: /* CSTDX, FSTDX, FSTDS */
> +        /* Distinguish from coprocessor load ... */
> +        is_write = (insn >> 9) & 1;
> +        break;
> +
> +    case 0x03:
> +        switch ((insn >> 6) & 15) {
> +        case 0xa: /* STWS */
> +        case 0x9: /* STHS */
> +        case 0x8: /* STBS */
> +        case 0xe: /* STWAS */
> +        case 0xc: /* STBYS */
> +            is_write = 1;
> +        }
> +        break;
> +    }
>  
> -    pc = uc->uc_mcontext.sc_iaoq[0];
> -    /* FIXME: compute is_write */
> -    is_write = 0;
>      return handle_cpu_signal(pc, (unsigned long)info->si_addr, 
> -                             is_write,
> -                             &uc->uc_sigmask, puc);
> +                             is_write, &uc->uc_sigmask, puc);
>  }
>  
>  #else
> -- 
> 1.6.6.1
> 
>
Stuart Brady March 17, 2010, 2:23 a.m. UTC | #2
On Wed, Mar 17, 2010 at 02:10:43AM +0000, Stuart Brady wrote:
> Argh.  It just seems mind bogglingly silly that is_write doesn't seem to
> be included in the siginfo on archs where doing so would make sense...

> It's at least something I'd have hoped libc could take care of in the case
> where the hardware doesn't indicate whether the fault was due to a read
> or a write...

Oh, actually, I guess you'd just test the equivalent of
'(info->si_isr >> 33) & 1' or 'DSISR_sig(uc) & 0x00800000)' on such
an arch, but it still seems a shame that this isn't done for us on
archs without that ability...

Cheers,
Richard Henderson March 17, 2010, 2:49 p.m. UTC | #3
On 03/16/2010 07:23 PM, Stuart Brady wrote:
> On Wed, Mar 17, 2010 at 02:10:43AM +0000, Stuart Brady wrote:
>> Argh.  It just seems mind bogglingly silly that is_write doesn't seem to
>> be included in the siginfo on archs where doing so would make sense...
> 
>> It's at least something I'd have hoped libc could take care of in the case
>> where the hardware doesn't indicate whether the fault was due to a read
>> or a write...
> 
> Oh, actually, I guess you'd just test the equivalent of
> '(info->si_isr >> 33) & 1' or 'DSISR_sig(uc) & 0x00800000)' on such
> an arch, but it still seems a shame that this isn't done for us on
> archs without that ability...

Unfortunately there's no reserved space in the hppa sigcontext, which
is what would need to be expanded in order to store the additional info
from the kernel.  So in addition to a kernel patch, userland would need
to change its abi.  All kinda nasty, that.


r~
diff mbox

Patch

diff --git a/cpu-exec.c b/cpu-exec.c
index bcfcda2..14204f4 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -1193,15 +1193,39 @@  int cpu_signal_handler(int host_signum, void *pinfo,
 {
     struct siginfo *info = pinfo;
     struct ucontext *uc = puc;
-    unsigned long pc;
-    int is_write;
+    unsigned long pc = uc->uc_mcontext.sc_iaoq[0];
+    uint32_t insn = *(uint32_t *)pc;
+    int is_write = 0;
+
+    /* XXX: need kernel patch to get write flag faster.  */
+    switch (insn >> 26) {
+    case 0x1a: /* STW */
+    case 0x19: /* STH */
+    case 0x18: /* STB */
+    case 0x1b: /* STWM */
+        is_write = 1;
+        break;
+
+    case 0x09: /* CSTWX, FSTWX, FSTWS */
+    case 0x0b: /* CSTDX, FSTDX, FSTDS */
+        /* Distinguish from coprocessor load ... */
+        is_write = (insn >> 9) & 1;
+        break;
+
+    case 0x03:
+        switch ((insn >> 6) & 15) {
+        case 0xa: /* STWS */
+        case 0x9: /* STHS */
+        case 0x8: /* STBS */
+        case 0xe: /* STWAS */
+        case 0xc: /* STBYS */
+            is_write = 1;
+        }
+        break;
+    }
 
-    pc = uc->uc_mcontext.sc_iaoq[0];
-    /* FIXME: compute is_write */
-    is_write = 0;
     return handle_cpu_signal(pc, (unsigned long)info->si_addr, 
-                             is_write,
-                             &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask, puc);
 }
 
 #else