diff mbox series

[v6,8/8] linux-user: Add support for FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls

Message ID 1567601968-26946-9-git-send-email-aleksandar.markovic@rt-rk.com (mailing list archive)
State New, archived
Headers show
Series linux-user: Misc patches for 4.2 | expand

Commit Message

Aleksandar Markovic Sept. 4, 2019, 12:59 p.m. UTC 
From: Aleksandar Markovic <amarkovic@wavecomp.com>

FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls are commands
for controlling error reporting of a floppy drive.

Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
---
 linux-user/ioctls.h        |  2 ++
 linux-user/syscall_defs.h  | 19 +++++++++++++++++++
 linux-user/syscall_types.h |  7 +++++++
 3 files changed, 28 insertions(+)

Comments

Laurent Vivier Sept. 6, 2019, 10:47 a.m. UTC | #1 
Le 04/09/2019 à 14:59, Aleksandar Markovic a écrit :
> From: Aleksandar Markovic <amarkovic@wavecomp.com>
> 
> FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls are commands
> for controlling error reporting of a floppy drive.
> 
> Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> ---
>  linux-user/ioctls.h        |  2 ++
>  linux-user/syscall_defs.h  | 19 +++++++++++++++++++
>  linux-user/syscall_types.h |  7 +++++++
>  3 files changed, 28 insertions(+)
> 
> diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> index 622874b..0c75d03 100644
> --- a/linux-user/ioctls.h
> +++ b/linux-user/ioctls.h
> @@ -118,6 +118,8 @@
>       IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
>       IOCTL(FDFMTEND, 0, TYPE_NULL)
>       IOCTL(FDFLUSH, 0, TYPE_NULL)
> +     IOCTL(FDSETMAXERRS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
> +     IOCTL(FDGETMAXERRS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))

where is FDSETEMSGTRESH?

>       IOCTL(FDRESET, 0, TYPE_NULL)
>       IOCTL(FDRAWCMD, 0, TYPE_NULL)
>       IOCTL(FDTWADDLE, 0, TYPE_NULL)
> diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> index 834a085..7c5b614 100644
> --- a/linux-user/syscall_defs.h
> +++ b/linux-user/syscall_defs.h
> @@ -890,12 +890,31 @@ struct target_pollfd {
>  
>  /* From <linux/fd.h> */
>  
> +struct target_floppy_max_errors {
> +    abi_uint        abort;
> +    abi_uint        read_track;
> +    abi_uint        reset;
> +    abi_uint        recal;
> +    abi_uint        reporting;
> +};

You don't need this, you can use floppy_max_errors from <linux/fd.h>.

But you can define it if you want because it is used to know the size of
the target structure (and if alignment or data types differ it can
mismatch. With "int" it's not the case).

> +struct target_format_descr {
> +    abi_uint        device;
> +    abi_uint        head;
> +    abi_uint        track;
> +};
> +

This one is for the previous patch. Same comment as above.

Thanks,
Laurent
Aleksandar Markovic Sept. 10, 2019, 6:59 p.m. UTC | #2 
06.09.2019. 12.47, "Laurent Vivier" <laurent@vivier.eu> је написао/ла:
>
> Le 04/09/2019 à 14:59, Aleksandar Markovic a écrit :
> > From: Aleksandar Markovic <amarkovic@wavecomp.com>
> >
> > FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls are commands
> > for controlling error reporting of a floppy drive.
> >
> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> > ---
> >  linux-user/ioctls.h        |  2 ++
> >  linux-user/syscall_defs.h  | 19 +++++++++++++++++++
> >  linux-user/syscall_types.h |  7 +++++++
> >  3 files changed, 28 insertions(+)
> >
> > diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> > index 622874b..0c75d03 100644
> > --- a/linux-user/ioctls.h
> > +++ b/linux-user/ioctls.h
> > @@ -118,6 +118,8 @@
> >       IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
> >       IOCTL(FDFMTEND, 0, TYPE_NULL)
> >       IOCTL(FDFLUSH, 0, TYPE_NULL)
> > +     IOCTL(FDSETMAXERRS, IOC_W,
MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
> > +     IOCTL(FDGETMAXERRS, IOC_R,
MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
>
> where is FDSETEMSGTRESH?
>
> >       IOCTL(FDRESET, 0, TYPE_NULL)
> >       IOCTL(FDRAWCMD, 0, TYPE_NULL)
> >       IOCTL(FDTWADDLE, 0, TYPE_NULL)
> > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> > index 834a085..7c5b614 100644
> > --- a/linux-user/syscall_defs.h
> > +++ b/linux-user/syscall_defs.h
> > @@ -890,12 +890,31 @@ struct target_pollfd {
> >
> >  /* From <linux/fd.h> */
> >
> > +struct target_floppy_max_errors {
> > +    abi_uint        abort;
> > +    abi_uint        read_track;
> > +    abi_uint        reset;
> > +    abi_uint        recal;
> > +    abi_uint        reporting;
> > +};
>
> You don't need this, you can use floppy_max_errors from <linux/fd.h>.
>
> But you can define it if you want because it is used to know the size of
> the target structure (and if alignment or data types differ it can
> mismatch. With "int" it's not the case).
>

Laurent, thanks for the review, I'll correct this in the next version.

Just a follow-up question:

If the structure of related to a (not-yet-supported in QEMU) ioctl was:

struct hd_geometry {
      unsigned char heads;
      unsigned char sectors;
      unsigned short cylinders;
      unsigned long start;
};

... would "target_hd_geometry" be needed, or not?

Regards,

Aleksandar

> > +struct target_format_descr {
> > +    abi_uint        device;
> > +    abi_uint        head;
> > +    abi_uint        track;
> > +};
> > +
>
> This one is for the previous patch. Same comment as above.
>
> Thanks,
> Laurent
>
Aleksandar Markovic Sept. 10, 2019, 7:15 p.m. UTC | #3 
10.09.2019. 20.58, aleksandar.m.mail@gmail.com је написао/ла:
>
>
> 06.09.2019. 12.47, "Laurent Vivier" <laurent@vivier.eu> је написао/ла:
> >
> > Le 04/09/2019 à 14:59, Aleksandar Markovic a écrit :
> > > From: Aleksandar Markovic <amarkovic@wavecomp.com>
> > >
> > > FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls are commands
> > > for controlling error reporting of a floppy drive.
> > >
> > > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com>
> > > ---
> > >  linux-user/ioctls.h        |  2 ++
> > >  linux-user/syscall_defs.h  | 19 +++++++++++++++++++
> > >  linux-user/syscall_types.h |  7 +++++++
> > >  3 files changed, 28 insertions(+)
> > >
> > > diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
> > > index 622874b..0c75d03 100644
> > > --- a/linux-user/ioctls.h
> > > +++ b/linux-user/ioctls.h
> > > @@ -118,6 +118,8 @@
> > >       IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
> > >       IOCTL(FDFMTEND, 0, TYPE_NULL)
> > >       IOCTL(FDFLUSH, 0, TYPE_NULL)
> > > +     IOCTL(FDSETMAXERRS, IOC_W,
MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
> > > +     IOCTL(FDGETMAXERRS, IOC_R,
MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
> >
> > where is FDSETEMSGTRESH?
> >
> > >       IOCTL(FDRESET, 0, TYPE_NULL)
> > >       IOCTL(FDRAWCMD, 0, TYPE_NULL)
> > >       IOCTL(FDTWADDLE, 0, TYPE_NULL)
> > > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
> > > index 834a085..7c5b614 100644
> > > --- a/linux-user/syscall_defs.h
> > > +++ b/linux-user/syscall_defs.h
> > > @@ -890,12 +890,31 @@ struct target_pollfd {
> > >
> > >  /* From <linux/fd.h> */
> > >
> > > +struct target_floppy_max_errors {
> > > +    abi_uint        abort;
> > > +    abi_uint        read_track;
> > > +    abi_uint        reset;
> > > +    abi_uint        recal;
> > > +    abi_uint        reporting;
> > > +};
> >
> > You don't need this, you can use floppy_max_errors from <linux/fd.h>.
> >
> > But you can define it if you want because it is used to know the size of
> > the target structure (and if alignment or data types differ it can
> > mismatch. With "int" it's not the case).
> >
>
> Laurent, thanks for the review, I'll correct this in the next version.
>
> Just a follow-up question:
>
> If the structure of related to a (not-yet-supported in QEMU) ioctl was:
>
> struct hd_geometry {
>       unsigned char heads;
>       unsigned char sectors;
>       unsigned short cylinders;
>       unsigned long start;
> };
>
> ... would "target_hd_geometry" be needed, or not?
>

Actually, that ioctl is already implemented in QEMU (HDIO_GETGEO), without
defining target_hd_geometry. Is this fine?

> Regards,
>
> Aleksandar
>
> > > +struct target_format_descr {
> > > +    abi_uint        device;
> > > +    abi_uint        head;
> > > +    abi_uint        track;
> > > +};
> > > +
> >
> > This one is for the previous patch. Same comment as above.
> >
> > Thanks,
> > Laurent
> >
Laurent Vivier Sept. 10, 2019, 7:15 p.m. UTC | #4 
Le 10/09/2019 à 20:59, Aleksandar Markovic a écrit :
> 
> 06.09.2019. 12.47, "Laurent Vivier" <laurent@vivier.eu
> <mailto:laurent@vivier.eu>> је написао/ла:
>>
>> Le 04/09/2019 à 14:59, Aleksandar Markovic a écrit :
>> > From: Aleksandar Markovic <amarkovic@wavecomp.com
> <mailto:amarkovic@wavecomp.com>>
>> >
>> > FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls are commands
>> > for controlling error reporting of a floppy drive.
>> >
>> > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com
> <mailto:amarkovic@wavecomp.com>>
>> > ---
>> >  linux-user/ioctls.h        |  2 ++
>> >  linux-user/syscall_defs.h  | 19 +++++++++++++++++++
>> >  linux-user/syscall_types.h |  7 +++++++
>> >  3 files changed, 28 insertions(+)
>> >
>> > diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
>> > index 622874b..0c75d03 100644
>> > --- a/linux-user/ioctls.h
>> > +++ b/linux-user/ioctls.h
>> > @@ -118,6 +118,8 @@
>> >       IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
>> >       IOCTL(FDFMTEND, 0, TYPE_NULL)
>> >       IOCTL(FDFLUSH, 0, TYPE_NULL)
>> > +     IOCTL(FDSETMAXERRS, IOC_W,
> MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
>> > +     IOCTL(FDGETMAXERRS, IOC_R,
> MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
>>
>> where is FDSETEMSGTRESH?
>>
>> >       IOCTL(FDRESET, 0, TYPE_NULL)
>> >       IOCTL(FDRAWCMD, 0, TYPE_NULL)
>> >       IOCTL(FDTWADDLE, 0, TYPE_NULL)
>> > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> > index 834a085..7c5b614 100644
>> > --- a/linux-user/syscall_defs.h
>> > +++ b/linux-user/syscall_defs.h
>> > @@ -890,12 +890,31 @@ struct target_pollfd {
>> > 
>> >  /* From <linux/fd.h> */
>> > 
>> > +struct target_floppy_max_errors {
>> > +    abi_uint        abort;
>> > +    abi_uint        read_track;
>> > +    abi_uint        reset;
>> > +    abi_uint        recal;
>> > +    abi_uint        reporting;
>> > +};
>>
>> You don't need this, you can use floppy_max_errors from <linux/fd.h>.
>>
>> But you can define it if you want because it is used to know the size of
>> the target structure (and if alignment or data types differ it can
>> mismatch. With "int" it's not the case).
>>
> 
> Laurent, thanks for the review, I'll correct this in the next version.
> 
> Just a follow-up question:
> 
> If the structure of related to a (not-yet-supported in QEMU) ioctl was:
> 
> struct hd_geometry {
>       unsigned char heads;
>       unsigned char sectors;
>       unsigned short cylinders;
>       unsigned long start;
> };
> 
> ... would "target_hd_geometry" be needed, or not?

In this case I think it is needed:
"unsigned long start" can be needed to be aligned on 64bit on some 64bit
architectures whereas on other it can be needed to be aligned on 32bit
or 16bit (m68k). So host and guest can behave differently.

Thanks,
Laurent
Laurent Vivier Sept. 10, 2019, 7:24 p.m. UTC | #5 
Le 10/09/2019 à 21:15, Aleksandar Markovic a écrit :
> 
> 10.09.2019. 20.58, aleksandar.m.mail@gmail.com
> <mailto:aleksandar.m.mail@gmail.com> је написао/ла:
>>
>>
>> 06.09.2019. 12.47, "Laurent Vivier" <laurent@vivier.eu
> <mailto:laurent@vivier.eu>> је написао/ла:
>> >
>> > Le 04/09/2019 à 14:59, Aleksandar Markovic a écrit :
>> > > From: Aleksandar Markovic <amarkovic@wavecomp.com
> <mailto:amarkovic@wavecomp.com>>
>> > >
>> > > FDSETEMSGTRESH, FDSETMAXERRS, and FDGETMAXERRS ioctls are commands
>> > > for controlling error reporting of a floppy drive.
>> > >
>> > > Signed-off-by: Aleksandar Markovic <amarkovic@wavecomp.com
> <mailto:amarkovic@wavecomp.com>>
>> > > ---
>> > >  linux-user/ioctls.h        |  2 ++
>> > >  linux-user/syscall_defs.h  | 19 +++++++++++++++++++
>> > >  linux-user/syscall_types.h |  7 +++++++
>> > >  3 files changed, 28 insertions(+)
>> > >
>> > > diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
>> > > index 622874b..0c75d03 100644
>> > > --- a/linux-user/ioctls.h
>> > > +++ b/linux-user/ioctls.h
>> > > @@ -118,6 +118,8 @@
>> > >       IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
>> > >       IOCTL(FDFMTEND, 0, TYPE_NULL)
>> > >       IOCTL(FDFLUSH, 0, TYPE_NULL)
>> > > +     IOCTL(FDSETMAXERRS, IOC_W,
> MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
>> > > +     IOCTL(FDGETMAXERRS, IOC_R,
> MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
>> >
>> > where is FDSETEMSGTRESH?
>> >
>> > >       IOCTL(FDRESET, 0, TYPE_NULL)
>> > >       IOCTL(FDRAWCMD, 0, TYPE_NULL)
>> > >       IOCTL(FDTWADDLE, 0, TYPE_NULL)
>> > > diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
>> > > index 834a085..7c5b614 100644
>> > > --- a/linux-user/syscall_defs.h
>> > > +++ b/linux-user/syscall_defs.h
>> > > @@ -890,12 +890,31 @@ struct target_pollfd {
>> > > 
>> > >  /* From <linux/fd.h> */
>> > > 
>> > > +struct target_floppy_max_errors {
>> > > +    abi_uint        abort;
>> > > +    abi_uint        read_track;
>> > > +    abi_uint        reset;
>> > > +    abi_uint        recal;
>> > > +    abi_uint        reporting;
>> > > +};
>> >
>> > You don't need this, you can use floppy_max_errors from <linux/fd.h>.
>> >
>> > But you can define it if you want because it is used to know the size of
>> > the target structure (and if alignment or data types differ it can
>> > mismatch. With "int" it's not the case).
>> >
>>
>> Laurent, thanks for the review, I'll correct this in the next version.
>>
>> Just a follow-up question:
>>
>> If the structure of related to a (not-yet-supported in QEMU) ioctl was:
>>
>> struct hd_geometry {
>>       unsigned char heads;
>>       unsigned char sectors;
>>       unsigned short cylinders;
>>       unsigned long start;
>> };
>>
>> ... would "target_hd_geometry" be needed, or not?
>>
> 
> Actually, that ioctl is already implemented in QEMU (HDIO_GETGEO),
> without defining target_hd_geometry. Is this fine?

No, for instance you can check it:

#include <stdio.h>
#include <stddef.h>
#include <linux/hdreg.h>

int main(void)
{
    printf("heads %zd\n", offsetof(struct hd_geometry, heads));
    printf("sectors %zd\n", offsetof(struct hd_geometry, sectors));
    printf("cylinders %zd\n", offsetof(struct hd_geometry, cylinders));
    printf("start %zd\n", offsetof(struct hd_geometry, start));
}

$ cc -o test_align test_align.c

on i386:

$ ./test_align
heads 0
sectors 1
cylinders 2
start 4

on x86_64:

$ ./test_align
heads 0
sectors 1
cylinders 2
start 8

Thanks,
Laurent
diff mbox series

Patch

diff --git a/linux-user/ioctls.h b/linux-user/ioctls.h
index 622874b..0c75d03 100644
--- a/linux-user/ioctls.h
+++ b/linux-user/ioctls.h
@@ -118,6 +118,8 @@ 
      IOCTL(FDFMTTRK, IOC_W, MK_PTR(MK_STRUCT(STRUCT_format_descr)))
      IOCTL(FDFMTEND, 0, TYPE_NULL)
      IOCTL(FDFLUSH, 0, TYPE_NULL)
+     IOCTL(FDSETMAXERRS, IOC_W, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
+     IOCTL(FDGETMAXERRS, IOC_R, MK_PTR(MK_STRUCT(STRUCT_floppy_max_errors)))
      IOCTL(FDRESET, 0, TYPE_NULL)
      IOCTL(FDRAWCMD, 0, TYPE_NULL)
      IOCTL(FDTWADDLE, 0, TYPE_NULL)
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 834a085..7c5b614 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -890,12 +890,31 @@  struct target_pollfd {
 
 /* From <linux/fd.h> */
 
+struct target_floppy_max_errors {
+    abi_uint        abort;
+    abi_uint        read_track;
+    abi_uint        reset;
+    abi_uint        recal;
+    abi_uint        reporting;
+};
+
+struct target_format_descr {
+    abi_uint        device;
+    abi_uint        head;
+    abi_uint        track;
+};
+
 #define TARGET_FDMSGON        TARGET_IO(2, 0x45)
 #define TARGET_FDMSGOFF       TARGET_IO(2, 0x46)
 #define TARGET_FDFMTBEG       TARGET_IO(2, 0x47)
 #define TARGET_FDFMTTRK      TARGET_IOW(2, 0x48, struct target_format_descr)
 #define TARGET_FDFMTEND       TARGET_IO(2, 0x49)
+#define TARGET_FDSETEMSGTRESH TARGET_IO(2, 0x4a)
 #define TARGET_FDFLUSH        TARGET_IO(2, 0x4b)
+#define TARGET_FDSETMAXERRS  TARGET_IOW(2, 0x4c,                               \
+                                        struct target_floppy_max_errors)
+#define TARGET_FDGETMAXERRS  TARGET_IOR(2, 0x0e,                               \
+                                        struct target_floppy_max_errors)
 #define TARGET_FDRESET        TARGET_IO(2, 0x54)
 #define TARGET_FDRAWCMD       TARGET_IO(2, 0x58)
 #define TARGET_FDTWADDLE      TARGET_IO(2, 0x59)
diff --git a/linux-user/syscall_types.h b/linux-user/syscall_types.h
index d82d1a5..5ba7c34 100644
--- a/linux-user/syscall_types.h
+++ b/linux-user/syscall_types.h
@@ -261,6 +261,13 @@  STRUCT(blkpg_ioctl_arg,
        TYPE_INT, /* datalen */
        TYPE_PTRVOID) /* data */
 
+STRUCT(floppy_max_errors,
+       TYPE_INT,     /* abort */
+       TYPE_INT,     /* read_track */
+       TYPE_INT,     /* reset */
+       TYPE_INT,     /* recal */
+       TYPE_INT)     /* reporting */
+
 STRUCT(format_descr,
        TYPE_INT,     /* device */
        TYPE_INT,     /* head */