* [PATCH] semihosting: Write back semihosting data before completion callback
@ 2022-10-12 1:48 Keith Packard via
2022-10-13 18:12 ` Richard Henderson
2023-01-06 13:52 ` Alex Bennée
0 siblings, 2 replies; 3+ messages in thread
From: Keith Packard via @ 2022-10-12 1:48 UTC (permalink / raw)
To: qemu-devel; +Cc: Alex Bennée, Keith Packard
'lock_user' allocates a host buffer to shadow a target buffer,
'unlock_user' copies that host buffer back to the target and frees the
host memory. If the completion function uses the target buffer, it
must be called after unlock_user to ensure the data are present.
This caused the arm-compatible TARGET_SYS_READC to fail as the
completion function, common_semi_readc_cb, pulled data from the target
buffer which would not have been gotten the console data.
I decided to fix all instances of this pattern instead of just the
console_read function to make things consistent and potentially fix
bugs in other cases.
Signed-off-by: Keith Packard <keithp@keithp.com>
---
semihosting/syscalls.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
index 508a0ad88c..78ba97d7ab 100644
--- a/semihosting/syscalls.c
+++ b/semihosting/syscalls.c
@@ -321,11 +321,11 @@ static void host_read(CPUState *cs, gdb_syscall_complete_cb complete,
ret = read(gf->hostfd, ptr, len);
} while (ret == -1 && errno == EINTR);
if (ret == -1) {
- complete(cs, -1, errno);
unlock_user(ptr, buf, 0);
+ complete(cs, -1, errno);
} else {
- complete(cs, ret, 0);
unlock_user(ptr, buf, ret);
+ complete(cs, ret, 0);
}
}
@@ -341,8 +341,8 @@ static void host_write(CPUState *cs, gdb_syscall_complete_cb complete,
return;
}
ret = write(gf->hostfd, ptr, len);
- complete(cs, ret, ret == -1 ? errno : 0);
unlock_user(ptr, buf, 0);
+ complete(cs, ret, ret == -1 ? errno : 0);
}
static void host_lseek(CPUState *cs, gdb_syscall_complete_cb complete,
@@ -428,8 +428,8 @@ static void host_stat(CPUState *cs, gdb_syscall_complete_cb complete,
ret = -1;
}
}
- complete(cs, ret, err);
unlock_user(name, fname, 0);
+ complete(cs, ret, err);
}
static void host_remove(CPUState *cs, gdb_syscall_complete_cb complete,
@@ -446,8 +446,8 @@ static void host_remove(CPUState *cs, gdb_syscall_complete_cb complete,
}
ret = remove(p);
- complete(cs, ret, ret ? errno : 0);
unlock_user(p, fname, 0);
+ complete(cs, ret, ret ? errno : 0);
}
static void host_rename(CPUState *cs, gdb_syscall_complete_cb complete,
@@ -471,9 +471,9 @@ static void host_rename(CPUState *cs, gdb_syscall_complete_cb complete,
}
ret = rename(ostr, nstr);
- complete(cs, ret, ret ? errno : 0);
unlock_user(ostr, oname, 0);
unlock_user(nstr, nname, 0);
+ complete(cs, ret, ret ? errno : 0);
}
static void host_system(CPUState *cs, gdb_syscall_complete_cb complete,
@@ -490,8 +490,8 @@ static void host_system(CPUState *cs, gdb_syscall_complete_cb complete,
}
ret = system(p);
- complete(cs, ret, ret == -1 ? errno : 0);
unlock_user(p, cmd, 0);
+ complete(cs, ret, ret == -1 ? errno : 0);
}
static void host_gettimeofday(CPUState *cs, gdb_syscall_complete_cb complete,
@@ -556,8 +556,8 @@ static void staticfile_read(CPUState *cs, gdb_syscall_complete_cb complete,
}
memcpy(ptr, gf->staticfile.data + gf->staticfile.off, len);
gf->staticfile.off += len;
- complete(cs, len, 0);
unlock_user(ptr, buf, len);
+ complete(cs, len, 0);
}
static void staticfile_lseek(CPUState *cs, gdb_syscall_complete_cb complete,
@@ -610,8 +610,8 @@ static void console_read(CPUState *cs, gdb_syscall_complete_cb complete,
return;
}
ret = qemu_semihosting_console_read(cs, ptr, len);
- complete(cs, ret, 0);
unlock_user(ptr, buf, ret);
+ complete(cs, ret, 0);
}
static void console_write(CPUState *cs, gdb_syscall_complete_cb complete,
@@ -626,8 +626,8 @@ static void console_write(CPUState *cs, gdb_syscall_complete_cb complete,
return;
}
ret = qemu_semihosting_console_write(ptr, len);
- complete(cs, ret ? ret : -1, ret ? 0 : EIO);
unlock_user(ptr, buf, 0);
+ complete(cs, ret ? ret : -1, ret ? 0 : EIO);
}
static void console_fstat(CPUState *cs, gdb_syscall_complete_cb complete,
--
2.37.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] semihosting: Write back semihosting data before completion callback
2022-10-12 1:48 [PATCH] semihosting: Write back semihosting data before completion callback Keith Packard via
@ 2022-10-13 18:12 ` Richard Henderson
2023-01-06 13:52 ` Alex Bennée
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2022-10-13 18:12 UTC (permalink / raw)
To: Keith Packard, qemu-devel; +Cc: Alex Bennée
On 10/12/22 18:48, Keith Packard via wrote:
> 'lock_user' allocates a host buffer to shadow a target buffer,
> 'unlock_user' copies that host buffer back to the target and frees the
> host memory. If the completion function uses the target buffer, it
> must be called after unlock_user to ensure the data are present.
>
> This caused the arm-compatible TARGET_SYS_READC to fail as the
> completion function, common_semi_readc_cb, pulled data from the target
> buffer which would not have been gotten the console data.
>
> I decided to fix all instances of this pattern instead of just the
> console_read function to make things consistent and potentially fix
> bugs in other cases.
>
> Signed-off-by: Keith Packard<keithp@keithp.com>
> ---
> semihosting/syscalls.c | 20 ++++++++++----------
> 1 file changed, 10 insertions(+), 10 deletions(-)
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
r~
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] semihosting: Write back semihosting data before completion callback
2022-10-12 1:48 [PATCH] semihosting: Write back semihosting data before completion callback Keith Packard via
2022-10-13 18:12 ` Richard Henderson
@ 2023-01-06 13:52 ` Alex Bennée
1 sibling, 0 replies; 3+ messages in thread
From: Alex Bennée @ 2023-01-06 13:52 UTC (permalink / raw)
To: Keith Packard; +Cc: qemu-devel
Keith Packard <keithp@keithp.com> writes:
> 'lock_user' allocates a host buffer to shadow a target buffer,
> 'unlock_user' copies that host buffer back to the target and frees the
> host memory. If the completion function uses the target buffer, it
> must be called after unlock_user to ensure the data are present.
>
> This caused the arm-compatible TARGET_SYS_READC to fail as the
> completion function, common_semi_readc_cb, pulled data from the target
> buffer which would not have been gotten the console data.
>
> I decided to fix all instances of this pattern instead of just the
> console_read function to make things consistent and potentially fix
> bugs in other cases.
>
> Signed-off-by: Keith Packard <keithp@keithp.com>
Queued to semihosting/next, thanks.
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-01-06 13:52 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-10-12 1:48 [PATCH] semihosting: Write back semihosting data before completion callback Keith Packard via
2022-10-13 18:12 ` Richard Henderson
2023-01-06 13:52 ` Alex Bennée
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).