Subject: [PATCH] kill HDIO_GETGEO_BIG_RAW ioctl


Against 2.6.0-test2-bk8.

--bartlomiej


kill HDIO_GETGEO_BIG_RAW ioctl

HDIO_GETGEO_BIG_RAW is an ide specific hack introduced in 2.3.99-pre3.
There are no known programs using this ioctl.

Its aim was to provide current CHS translation to the user-space,
but very often it provides what driver thinks is a current translation
(drive with LBA have to support only one physical translation, also
drive may not support chosen translation and there is no return value check).

hdparm -I can be used instead, it provides correct information
(and bogus data is still accessible through /proc/ide/hdX/geometry).

arch/ppc64/kernel/ioctl32.c | 29 -----------------------------
arch/sparc64/kernel/ioctl32.c | 29 -----------------------------
drivers/ide/ide.c | 12 ------------
include/linux/hdreg.h | 10 +---------
4 files changed, 1 insertion(+), 79 deletions(-)

diff -puN arch/ppc64/kernel/ioctl32.c~kill-HDIO_GETGEO_BIG_RAW arch/ppc64/kernel/ioctl32.c
--- linux-2.6.0-test2-bk7/arch/ppc64/kernel/ioctl32.c~kill-HDIO_GETGEO_BIG_RAW 2003-08-08 23:53:47.012359456 +0200
+++ linux-2.6.0-test2-bk7-root/arch/ppc64/kernel/ioctl32.c 2003-08-08 23:53:47.024357632 +0200
@@ -28,34 +28,6 @@
#define CODE
#include "compat_ioctl.c"

-struct hd_big_geometry32 {
- unsigned char heads;
- unsigned char sectors;
- unsigned int cylinders;
- u32 start;
-};
-
-static int hdio_getgeo_big(unsigned int fd, unsigned int cmd, unsigned long arg)
-{
- mm_segment_t old_fs = get_fs();
- struct hd_big_geometry geo;
- int err;
-
- set_fs (KERNEL_DS);
- err = sys_ioctl(fd, cmd, (unsigned long)&geo);
- set_fs (old_fs);
- if (!err) {
- struct hd_big_geometry32 *up = (struct hd_big_geometry32 *) arg;
-
- if (put_user(geo.heads, &up->heads) ||
- __put_user(geo.sectors, &up->sectors) ||
- __put_user(geo.cylinders, &up->cylinders) ||
- __put_user(((u32) geo.start), &up->start))
- err = -EFAULT;
- }
- return err;
-}
-
struct ncp_ioctl_request_32 {
unsigned int function;
unsigned int size;
@@ -773,7 +745,6 @@ COMPATIBLE_IOCTL(_IOR('p', 20, int[7]))
COMPATIBLE_IOCTL(_IOW('p', 21, int[7])) /* RTCSET */

/* And these ioctls need translation */
-HANDLE_IOCTL(HDIO_GETGEO_BIG_RAW, hdio_getgeo_big)

/* NCPFS */
HANDLE_IOCTL(NCP_IOC_NCPREQUEST_32, do_ncp_ncprequest)
diff -puN arch/sparc64/kernel/ioctl32.c~kill-HDIO_GETGEO_BIG_RAW arch/sparc64/kernel/ioctl32.c
--- linux-2.6.0-test2-bk7/arch/sparc64/kernel/ioctl32.c~kill-HDIO_GETGEO_BIG_RAW 2003-08-08 23:53:47.015359000 +0200
+++ linux-2.6.0-test2-bk7-root/arch/sparc64/kernel/ioctl32.c 2003-08-08 23:53:47.026357328 +0200
@@ -40,34 +40,6 @@ static __inline__ void *alloc_user_space
#define CODE
#include "compat_ioctl.c"

-struct hd_big_geometry32 {
- unsigned char heads;
- unsigned char sectors;
- unsigned int cylinders;
- u32 start;
-};
-
-static int hdio_getgeo_big(unsigned int fd, unsigned int cmd, unsigned long arg)
-{
- mm_segment_t old_fs = get_fs();
- struct hd_big_geometry geo;
- int err;
-
- set_fs (KERNEL_DS);
- err = sys_ioctl(fd, cmd, (unsigned long)&geo);
- set_fs (old_fs);
- if (!err) {
- struct hd_big_geometry32 *up = (struct hd_big_geometry32 *) arg;
-
- if (put_user(geo.heads, &up->heads) ||
- __put_user(geo.sectors, &up->sectors) ||
- __put_user(geo.cylinders, &up->cylinders) ||
- __put_user(((u32) geo.start), &up->start))
- err = -EFAULT;
- }
- return err;
-}
-
struct fbcmap32 {
int index; /* first element (0 origin) */
int count;
@@ -1558,7 +1530,6 @@ COMPATIBLE_IOCTL(DM_DEV_STATUS)
COMPATIBLE_IOCTL(DM_TARGET_STATUS)
COMPATIBLE_IOCTL(DM_TARGET_WAIT)
/* And these ioctls need translation */
-HANDLE_IOCTL(HDIO_GETGEO_BIG_RAW, hdio_getgeo_big)
/* NCPFS */
HANDLE_IOCTL(NCP_IOC_NCPREQUEST_32, do_ncp_ncprequest)
HANDLE_IOCTL(NCP_IOC_GETMOUNTUID2_32, do_ncp_getmountuid2)
diff -puN drivers/ide/ide.c~kill-HDIO_GETGEO_BIG_RAW drivers/ide/ide.c
--- linux-2.6.0-test2-bk7/drivers/ide/ide.c~kill-HDIO_GETGEO_BIG_RAW 2003-08-08 23:53:47.018358544 +0200
+++ linux-2.6.0-test2-bk7-root/drivers/ide/ide.c 2003-08-08 23:53:47.027357176 +0200
@@ -1619,18 +1619,6 @@ int generic_ide_ioctl(struct block_devic
return 0;
}

- case HDIO_GETGEO_BIG_RAW:
- {
- struct hd_big_geometry *loc = (struct hd_big_geometry *) arg;
- if (!loc || (drive->media != ide_disk && drive->media != ide_floppy)) return -EINVAL;
- if (put_user(drive->head, (u8 *) &loc->heads)) return -EFAULT;
- if (put_user(drive->sect, (u8 *) &loc->sectors)) return -EFAULT;
- if (put_user(drive->cyl, (unsigned int *) &loc->cylinders)) return -EFAULT;
- if (put_user((unsigned)get_start_sect(bdev),
- (unsigned long *) &loc->start)) return -EFAULT;
- return 0;
- }
-
case HDIO_OBSOLETE_IDENTITY:
case HDIO_GET_IDENTITY:
if (bdev != bdev->bd_contains)
diff -puN include/linux/hdreg.h~kill-HDIO_GETGEO_BIG_RAW include/linux/hdreg.h
--- linux-2.6.0-test2-bk7/include/linux/hdreg.h~kill-HDIO_GETGEO_BIG_RAW 2003-08-08 23:53:47.021358088 +0200
+++ linux-2.6.0-test2-bk7-root/include/linux/hdreg.h 2003-08-08 23:53:47.028357024 +0200
@@ -395,14 +395,6 @@ struct hd_geometry {
unsigned long start;
};

-/* BIG GEOMETRY - dying, used only by HDIO_GETGEO_BIG_RAW */
-struct hd_big_geometry {
- unsigned char heads;
- unsigned char sectors;
- unsigned int cylinders;
- unsigned long start;
-};
-
/* hd/ide ctl's that pass (arg) ptrs to user space are numbered 0x030n/0x031n */
#define HDIO_GETGEO 0x0301 /* get device geometry */
#define HDIO_GET_UNMASKINTR 0x0302 /* get current unmask setting */
@@ -456,7 +448,7 @@ enum {

/* hd/ide ctl's that pass (arg) ptrs to user space are numbered 0x033n/0x033n */
/* 0x330 is reserved - used to be HDIO_GETGEO_BIG */
-#define HDIO_GETGEO_BIG_RAW 0x0331 /* */
+/* 0x331 is reserved - used to be HDIO_GETGEO_BIG_RAW */

#define HDIO_SET_IDE_SCSI 0x0338
#define HDIO_SET_SCSI_IDE 0x0339

_


2003-08-10 20:40:38

by Andre Hedrick

[permalink] [raw]
Subject: Re: [PATCH] kill HDIO_GETGEO_BIG_RAW ioctl


Besure to force all devices to LBA only because wrapping the cylinder
count tends to make a mess.

Cheers,

--a

On Sat, 9 Aug 2003, Bartlomiej Zolnierkiewicz wrote:

>
> Against 2.6.0-test2-bk8.
>
> --bartlomiej
>
>
> kill HDIO_GETGEO_BIG_RAW ioctl
>
> HDIO_GETGEO_BIG_RAW is an ide specific hack introduced in 2.3.99-pre3.
> There are no known programs using this ioctl.
>
> Its aim was to provide current CHS translation to the user-space,
> but very often it provides what driver thinks is a current translation
> (drive with LBA have to support only one physical translation, also
> drive may not support chosen translation and there is no return value check).
>
> hdparm -I can be used instead, it provides correct information
> (and bogus data is still accessible through /proc/ide/hdX/geometry).
>
> arch/ppc64/kernel/ioctl32.c | 29 -----------------------------
> arch/sparc64/kernel/ioctl32.c | 29 -----------------------------
> drivers/ide/ide.c | 12 ------------
> include/linux/hdreg.h | 10 +---------
> 4 files changed, 1 insertion(+), 79 deletions(-)
>
> diff -puN arch/ppc64/kernel/ioctl32.c~kill-HDIO_GETGEO_BIG_RAW arch/ppc64/kernel/ioctl32.c
> --- linux-2.6.0-test2-bk7/arch/ppc64/kernel/ioctl32.c~kill-HDIO_GETGEO_BIG_RAW 2003-08-08 23:53:47.012359456 +0200
> +++ linux-2.6.0-test2-bk7-root/arch/ppc64/kernel/ioctl32.c 2003-08-08 23:53:47.024357632 +0200
> @@ -28,34 +28,6 @@
> #define CODE
> #include "compat_ioctl.c"
>
> -struct hd_big_geometry32 {
> - unsigned char heads;
> - unsigned char sectors;
> - unsigned int cylinders;
> - u32 start;
> -};
> -
> -static int hdio_getgeo_big(unsigned int fd, unsigned int cmd, unsigned long arg)
> -{
> - mm_segment_t old_fs = get_fs();
> - struct hd_big_geometry geo;
> - int err;
> -
> - set_fs (KERNEL_DS);
> - err = sys_ioctl(fd, cmd, (unsigned long)&geo);
> - set_fs (old_fs);
> - if (!err) {
> - struct hd_big_geometry32 *up = (struct hd_big_geometry32 *) arg;
> -
> - if (put_user(geo.heads, &up->heads) ||
> - __put_user(geo.sectors, &up->sectors) ||
> - __put_user(geo.cylinders, &up->cylinders) ||
> - __put_user(((u32) geo.start), &up->start))
> - err = -EFAULT;
> - }
> - return err;
> -}
> -
> struct ncp_ioctl_request_32 {
> unsigned int function;
> unsigned int size;
> @@ -773,7 +745,6 @@ COMPATIBLE_IOCTL(_IOR('p', 20, int[7]))
> COMPATIBLE_IOCTL(_IOW('p', 21, int[7])) /* RTCSET */
>
> /* And these ioctls need translation */
> -HANDLE_IOCTL(HDIO_GETGEO_BIG_RAW, hdio_getgeo_big)
>
> /* NCPFS */
> HANDLE_IOCTL(NCP_IOC_NCPREQUEST_32, do_ncp_ncprequest)
> diff -puN arch/sparc64/kernel/ioctl32.c~kill-HDIO_GETGEO_BIG_RAW arch/sparc64/kernel/ioctl32.c
> --- linux-2.6.0-test2-bk7/arch/sparc64/kernel/ioctl32.c~kill-HDIO_GETGEO_BIG_RAW 2003-08-08 23:53:47.015359000 +0200
> +++ linux-2.6.0-test2-bk7-root/arch/sparc64/kernel/ioctl32.c 2003-08-08 23:53:47.026357328 +0200
> @@ -40,34 +40,6 @@ static __inline__ void *alloc_user_space
> #define CODE
> #include "compat_ioctl.c"
>
> -struct hd_big_geometry32 {
> - unsigned char heads;
> - unsigned char sectors;
> - unsigned int cylinders;
> - u32 start;
> -};
> -
> -static int hdio_getgeo_big(unsigned int fd, unsigned int cmd, unsigned long arg)
> -{
> - mm_segment_t old_fs = get_fs();
> - struct hd_big_geometry geo;
> - int err;
> -
> - set_fs (KERNEL_DS);
> - err = sys_ioctl(fd, cmd, (unsigned long)&geo);
> - set_fs (old_fs);
> - if (!err) {
> - struct hd_big_geometry32 *up = (struct hd_big_geometry32 *) arg;
> -
> - if (put_user(geo.heads, &up->heads) ||
> - __put_user(geo.sectors, &up->sectors) ||
> - __put_user(geo.cylinders, &up->cylinders) ||
> - __put_user(((u32) geo.start), &up->start))
> - err = -EFAULT;
> - }
> - return err;
> -}
> -
> struct fbcmap32 {
> int index; /* first element (0 origin) */
> int count;
> @@ -1558,7 +1530,6 @@ COMPATIBLE_IOCTL(DM_DEV_STATUS)
> COMPATIBLE_IOCTL(DM_TARGET_STATUS)
> COMPATIBLE_IOCTL(DM_TARGET_WAIT)
> /* And these ioctls need translation */
> -HANDLE_IOCTL(HDIO_GETGEO_BIG_RAW, hdio_getgeo_big)
> /* NCPFS */
> HANDLE_IOCTL(NCP_IOC_NCPREQUEST_32, do_ncp_ncprequest)
> HANDLE_IOCTL(NCP_IOC_GETMOUNTUID2_32, do_ncp_getmountuid2)
> diff -puN drivers/ide/ide.c~kill-HDIO_GETGEO_BIG_RAW drivers/ide/ide.c
> --- linux-2.6.0-test2-bk7/drivers/ide/ide.c~kill-HDIO_GETGEO_BIG_RAW 2003-08-08 23:53:47.018358544 +0200
> +++ linux-2.6.0-test2-bk7-root/drivers/ide/ide.c 2003-08-08 23:53:47.027357176 +0200
> @@ -1619,18 +1619,6 @@ int generic_ide_ioctl(struct block_devic
> return 0;
> }
>
> - case HDIO_GETGEO_BIG_RAW:
> - {
> - struct hd_big_geometry *loc = (struct hd_big_geometry *) arg;
> - if (!loc || (drive->media != ide_disk && drive->media != ide_floppy)) return -EINVAL;
> - if (put_user(drive->head, (u8 *) &loc->heads)) return -EFAULT;
> - if (put_user(drive->sect, (u8 *) &loc->sectors)) return -EFAULT;
> - if (put_user(drive->cyl, (unsigned int *) &loc->cylinders)) return -EFAULT;
> - if (put_user((unsigned)get_start_sect(bdev),
> - (unsigned long *) &loc->start)) return -EFAULT;
> - return 0;
> - }
> -
> case HDIO_OBSOLETE_IDENTITY:
> case HDIO_GET_IDENTITY:
> if (bdev != bdev->bd_contains)
> diff -puN include/linux/hdreg.h~kill-HDIO_GETGEO_BIG_RAW include/linux/hdreg.h
> --- linux-2.6.0-test2-bk7/include/linux/hdreg.h~kill-HDIO_GETGEO_BIG_RAW 2003-08-08 23:53:47.021358088 +0200
> +++ linux-2.6.0-test2-bk7-root/include/linux/hdreg.h 2003-08-08 23:53:47.028357024 +0200
> @@ -395,14 +395,6 @@ struct hd_geometry {
> unsigned long start;
> };
>
> -/* BIG GEOMETRY - dying, used only by HDIO_GETGEO_BIG_RAW */
> -struct hd_big_geometry {
> - unsigned char heads;
> - unsigned char sectors;
> - unsigned int cylinders;
> - unsigned long start;
> -};
> -
> /* hd/ide ctl's that pass (arg) ptrs to user space are numbered 0x030n/0x031n */
> #define HDIO_GETGEO 0x0301 /* get device geometry */
> #define HDIO_GET_UNMASKINTR 0x0302 /* get current unmask setting */
> @@ -456,7 +448,7 @@ enum {
>
> /* hd/ide ctl's that pass (arg) ptrs to user space are numbered 0x033n/0x033n */
> /* 0x330 is reserved - used to be HDIO_GETGEO_BIG */
> -#define HDIO_GETGEO_BIG_RAW 0x0331 /* */
> +/* 0x331 is reserved - used to be HDIO_GETGEO_BIG_RAW */
>
> #define HDIO_SET_IDE_SCSI 0x0338
> #define HDIO_SET_SCSI_IDE 0x0339
>
> _
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>

Subject: Re: [PATCH] kill HDIO_GETGEO_BIG_RAW ioctl

On Sunday 10 of August 2003 22:29, you wrote:
> Besure to force all devices to LBA only because wrapping the cylinder
> count tends to make a mess.
>
> Cheers,
>
> --a

Ahhh... you mean not doing set_geometry() for LBA disks (we currently don't
do it only for LBA-48 disks) because for large disks the cylinder count wraps
and its not a nice thing, right?

cheers,

--bartlomiej

Subject: Re: [PATCH] kill HDIO_GETGEO_BIG_RAW ioctl

On Monday 11 of August 2003 01:24, you wrote:
> On Sunday 10 of August 2003 22:29, you wrote:
> > Besure to force all devices to LBA only because wrapping the cylinder
> > count tends to make a mess.

This should be fixed by a "[PATCH] disk geometry/capacity cleanups".
drive->cyl won't be recalculated et all and cyls number from a disk itself
will be used for set_geometry().

> Ahhh... you mean not doing set_geometry() for LBA disks (we currently don't
> do it only for LBA-48 disks) because for large disks the cylinder count
> wraps and its not a nice thing, right?

It still may be a good thing, cause we never fall-back to CHS addressing,
even for taskfile ioctl requests (or fix taskfile and allow fall-back?).

--bartlomiej

2003-08-11 12:30:22

by Andre Hedrick

[permalink] [raw]
Subject: Re: [PATCH] kill HDIO_GETGEO_BIG_RAW ioctl


28bit it is a free for all, and anything goes in linux because nobody is
willing to commit and force anything over 8.4GB to LBA only. People think
that "orphan sectors" are a bad thing. IFF one can only IO in LBA with
the exception of sectors at or below the 8.4GB limit then one never needs
to worry about rogue programs existing out in nowhere land.

Cheers,

--a

On Mon, 11 Aug 2003, Bartlomiej Zolnierkiewicz wrote:

> On Monday 11 of August 2003 01:24, you wrote:
> > On Sunday 10 of August 2003 22:29, you wrote:
> > > Besure to force all devices to LBA only because wrapping the cylinder
> > > count tends to make a mess.
>
> This should be fixed by a "[PATCH] disk geometry/capacity cleanups".
> drive->cyl won't be recalculated et all and cyls number from a disk itself
> will be used for set_geometry().
>
> > Ahhh... you mean not doing set_geometry() for LBA disks (we currently don't
> > do it only for LBA-48 disks) because for large disks the cylinder count
> > wraps and its not a nice thing, right?
>
> It still may be a good thing, cause we never fall-back to CHS addressing,
> even for taskfile ioctl requests (or fix taskfile and allow fall-back?).
>
> --bartlomiej
>

2003-08-11 12:46:52

by Jeff Garzik

[permalink] [raw]
Subject: Re: [PATCH] kill HDIO_GETGEO_BIG_RAW ioctl

Andre Hedrick wrote:
> 28bit it is a free for all, and anything goes in linux because nobody is
> willing to commit and force anything over 8.4GB to LBA only. People think
> that "orphan sectors" are a bad thing. IFF one can only IO in LBA with
> the exception of sectors at or below the 8.4GB limit then one never needs
> to worry about rogue programs existing out in nowhere land.


libata requires LBA support, and always uses LBA addressing <grin>

Jeff