[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compat
From: |
Alex Bennée |
Subject: |
Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility |
Date: |
Fri, 06 Jan 2023 15:33:12 +0000 |
User-agent: |
mu4e 1.9.11; emacs 29.0.60 |
Peter Maydell <peter.maydell@linaro.org> writes:
> On Fri, 6 Jan 2023 at 10:21, Evgeny Iakovlev
> <eiakovlev@linux.microsoft.com> wrote:
>>
>> Windows open(2) implementation opens files in text mode by default and
>> needs a Windows-only O_BINARY flag to open files as binary. QEMU already
>> knows about that flag in osdep and it is defined to 0 on non-Windows,
>> so we can just add it to the host_flags for better compatibility.
>>
>> Signed-off-by: Evgeny Iakovlev <eiakovlev@linux.microsoft.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
>> ---
>> semihosting/syscalls.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/semihosting/syscalls.c b/semihosting/syscalls.c
>> index 508a0ad88c..b621d78c2d 100644
>> --- a/semihosting/syscalls.c
>> +++ b/semihosting/syscalls.c
>> @@ -253,7 +253,7 @@ static void host_open(CPUState *cs,
>> gdb_syscall_complete_cb complete,
>> {
>> CPUArchState *env G_GNUC_UNUSED = cs->env_ptr;
>> char *p;
>> - int ret, host_flags;
>> + int ret, host_flags = O_BINARY;
>
> The semihosting API, at least for Arm, has a modeflags string so the
> guest can say whether it wants to open O_BINARY or not:
> https://github.com/ARM-software/abi-aa/blob/main/semihosting/semihosting.rst#sys-open-0x01
>
> So we need to plumb that down through the common semihosting code
> into this function and set O_BINARY accordingly. Otherwise guest
> code that asks for a text-mode file won't get one.
We used to, in fact we still have a remnant of the code where we do:
#ifndef O_BINARY
#define O_BINARY 0
#endif
I presume because the only places it exists in libc is wrapped in stuff
like:
#if defined (__CYGWIN__)
#define O_BINARY _FBINARY
So the mapping got removed in a1a2a3e609 (semihosting: Remove
GDB_O_BINARY) because GDB knows nothing of this and as far as I can tell
neither does Linux whatever ISO C might say about it.
Is this a host detail leakage to the guest? Should a semihosting app be
caring about what fopen() modes the underlying host supports? At least a
default O_BINARY for windows is most likely to DTRT.
> I don't know about other semihosting APIs, so those would need
> to be checked to see what they should do.
>
> thanks
> -- PMM
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
- [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, Evgeny Iakovlev, 2023/01/06
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, Alex Bennée, 2023/01/06
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, Peter Maydell, 2023/01/06
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility,
Alex Bennée <=
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, Peter Maydell, 2023/01/06
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, Evgeny Iakovlev, 2023/01/06
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, Peter Maydell, 2023/01/06
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, eiakovlev, 2023/01/16
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, Alex Bennée, 2023/01/16
- Re: [PATCH v2] semihosting: add O_BINARY flag in host_open for NT compatibility, Peter Maydell, 2023/01/16