The TARGET_MTIOCTOP/TARGET_MTIOCGET/TARGET_MTIOCPOS values
were being defined in terms of host struct types, but
these structures are such that their size might differ
on different hosts. Switch to using a target struct
definition instead.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
linux-user/syscall_defs.h | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index f7cc9f9..16d56fa 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -2706,9 +2706,34 @@ struct target_f_owner_ex {
#define TARGET_VFAT_IOCTL_READDIR_BOTH TARGET_IORU('r', 1)
#define TARGET_VFAT_IOCTL_READDIR_SHORT TARGET_IORU('r', 2)
-#define TARGET_MTIOCTOP TARGET_IOW('m', 1, struct mtop)
-#define TARGET_MTIOCGET TARGET_IOR('m', 2, struct mtget)
-#define TARGET_MTIOCPOS TARGET_IOR('m', 3, struct mtpos)
+struct target_mtop {
+ abi_short mt_op;
+ abi_int mt_count;
+};
+
+#if defined(TARGET_SPARC) || defined(TARGET_MIPS)
+typedef abi_long target_kernel_daddr_t;
+#else
+typedef abi_int target_kernel_daddr_t;
+#endif
+
+struct target_mtget {
+ abi_long mt_type;
+ abi_long mt_resid;
+ abi_long mt_dsreg;
+ abi_long mt_gstat;
+ abi_long mt_erreg;
+ target_kernel_daddr_t mt_fileno;
+ target_kernel_daddr_t mt_blkno;
+};
+
+struct target_mtpos {
+ abi_long mt_blkno;
+};
+
+#define TARGET_MTIOCTOP TARGET_IOW('m', 1, struct target_mtop)
+#define TARGET_MTIOCGET TARGET_IOR('m', 2, struct target_mtget)
+#define TARGET_MTIOCPOS TARGET_IOR('m', 3, struct target_mtpos)
struct target_sysinfo {
abi_long uptime; /* Seconds since boot */
--
2.7.4
Le 12/10/2017 à 17:30, Peter Maydell a écrit :
> The TARGET_MTIOCTOP/TARGET_MTIOCGET/TARGET_MTIOCPOS values
> were being defined in terms of host struct types, but
> these structures are such that their size might differ
> on different hosts. Switch to using a target struct
> definition instead.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> linux-user/syscall_defs.h | 31 ++++++++++++++++++++++++++++---
> 1 file changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index f7cc9f9..16d56fa 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -2706,9 +2706,34 @@ struct target_f_owner_ex {
> #define TARGET_VFAT_IOCTL_READDIR_BOTH TARGET_IORU('r', 1)
> #define TARGET_VFAT_IOCTL_READDIR_SHORT TARGET_IORU('r', 2)
>
> -#define TARGET_MTIOCTOP TARGET_IOW('m', 1, struct mtop)
> -#define TARGET_MTIOCGET TARGET_IOR('m', 2, struct mtget)
> -#define TARGET_MTIOCPOS TARGET_IOR('m', 3, struct mtpos)
> +struct target_mtop {
> + abi_short mt_op;
> + abi_int mt_count;
> +};
> +
> +#if defined(TARGET_SPARC) || defined(TARGET_MIPS)
> +typedef abi_long target_kernel_daddr_t;
> +#else
> +typedef abi_int target_kernel_daddr_t;
> +#endif
Perhaps you can add these ones into include/exec/user/abitypes.h ?
> +struct target_mtget {
> + abi_long mt_type;
> + abi_long mt_resid;
> + abi_long mt_dsreg;
> + abi_long mt_gstat;
> + abi_long mt_erreg;
> + target_kernel_daddr_t mt_fileno;
> + target_kernel_daddr_t mt_blkno;
> +};
I think you need to update STRUCT(mtget, ...) in
linux-user/syscall_types.h to reflect the size difference for MIPS and
SPARC.
Thanks,
Laurent
On 12 October 2017 at 17:49, Laurent Vivier <laurent@vivier.eu> wrote:
> Le 12/10/2017 à 17:30, Peter Maydell a écrit :
>> +#if defined(TARGET_SPARC) || defined(TARGET_MIPS)
>> +typedef abi_long target_kernel_daddr_t;
>> +#else
>> +typedef abi_int target_kernel_daddr_t;
>> +#endif
>
> Perhaps you can add these ones into include/exec/user/abitypes.h ?
I don't think they belong there -- that file is for basic
CPU ABI dependent types, not things which are just part of
the kernel interface.
>> +struct target_mtget {
>> + abi_long mt_type;
>> + abi_long mt_resid;
>> + abi_long mt_dsreg;
>> + abi_long mt_gstat;
>> + abi_long mt_erreg;
>> + target_kernel_daddr_t mt_fileno;
>> + target_kernel_daddr_t mt_blkno;
>> +};
>
> I think you need to update STRUCT(mtget, ...) in
> linux-user/syscall_types.h to reflect the size difference for MIPS and
> SPARC.
I thought about that but I wasn't feeling too enthusiastic
due to not having a test case... I suppose it's better to
change them both though.
thanks
-- PMM
Le 12/10/2017 à 18:53, Peter Maydell a écrit : > On 12 October 2017 at 17:49, Laurent Vivier <laurent@vivier.eu> wrote: >> Le 12/10/2017 à 17:30, Peter Maydell a écrit : >>> +#if defined(TARGET_SPARC) || defined(TARGET_MIPS) >>> +typedef abi_long target_kernel_daddr_t; >>> +#else >>> +typedef abi_int target_kernel_daddr_t; >>> +#endif >> >> Perhaps you can add these ones into include/exec/user/abitypes.h ? > > I don't think they belong there -- that file is for basic > CPU ABI dependent types, not things which are just part of > the kernel interface. I agree Laurent
On Thu, Oct 12, 2017 at 07:08:55PM +0200, Laurent Vivier wrote: > Le 12/10/2017 à 18:53, Peter Maydell a écrit : > > On 12 October 2017 at 17:49, Laurent Vivier <laurent@vivier.eu> wrote: > >> Le 12/10/2017 à 17:30, Peter Maydell a écrit : > >>> +#if defined(TARGET_SPARC) || defined(TARGET_MIPS) > >>> +typedef abi_long target_kernel_daddr_t; > >>> +#else > >>> +typedef abi_int target_kernel_daddr_t; > >>> +#endif > >> > >> Perhaps you can add these ones into include/exec/user/abitypes.h ? > > > > I don't think they belong there -- that file is for basic > > CPU ABI dependent types, not things which are just part of > > the kernel interface. > I agree So we should go with the patch as-is? Riku
Le 16/10/2017 à 15:06, Riku Voipio a écrit : > On Thu, Oct 12, 2017 at 07:08:55PM +0200, Laurent Vivier wrote: >> Le 12/10/2017 à 18:53, Peter Maydell a écrit : >>> On 12 October 2017 at 17:49, Laurent Vivier <laurent@vivier.eu> wrote: >>>> Le 12/10/2017 à 17:30, Peter Maydell a écrit : >>>>> +#if defined(TARGET_SPARC) || defined(TARGET_MIPS) >>>>> +typedef abi_long target_kernel_daddr_t; >>>>> +#else >>>>> +typedef abi_int target_kernel_daddr_t; >>>>> +#endif >>>> >>>> Perhaps you can add these ones into include/exec/user/abitypes.h ? >>> >>> I don't think they belong there -- that file is for basic >>> CPU ABI dependent types, not things which are just part of >>> the kernel interface. > >> I agree > > So we should go with the patch as-is? For this part, yes, but I think for the other comment, STRUCT(mtget, ...) needs to be updated. Thanks, Laurent
On 16 October 2017 at 14:09, Laurent Vivier <laurent@vivier.eu> wrote: > For this part, yes, but I think for the other comment, STRUCT(mtget, > ...) needs to be updated. Looking more closely I don't think it is as simple as just adjusting the STRUCT() definition. For a basic type that can be different on host and guest, I think we'd need to handle it the way we do the TYPE_OLDDEVT, with special support in the thunk code for figuring out how large it is and so on. That seems like a lot of work for a magtape ioctl that I suspect nobody's using and which we can't test... Applying this patch as-is would at least fix the ioctl for everything except MIPS and SPARC hosts and guests. thanks -- PMM
Le 16/10/2017 à 15:26, Peter Maydell a écrit : > On 16 October 2017 at 14:09, Laurent Vivier <laurent@vivier.eu> wrote: >> For this part, yes, but I think for the other comment, STRUCT(mtget, >> ...) needs to be updated. > > Looking more closely I don't think it is as simple as just > adjusting the STRUCT() definition. For a basic type that can > be different on host and guest, I think we'd need to handle > it the way we do the TYPE_OLDDEVT, with special support > in the thunk code for figuring out how large it is and so on. > That seems like a lot of work for a magtape ioctl that I > suspect nobody's using and which we can't test... I agree. > Applying this patch as-is would at least fix the ioctl > for everything except MIPS and SPARC hosts and guests. Yes, so I think it can be applied as-is. Thanks, Laurent
© 2016 - 2026 Red Hat, Inc.