linux-kernel - Re: [PATCH 15/16] floppy: separate the FDC's base address from its registers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 26 Feb 2020 18:36:52 +0300
From:   Denis Efremov <efremov@...ux.com>
To:     Willy Tarreau <w@....eu>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Jens Axboe <axboe@...nel.dk>, linux-block@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 15/16] floppy: separate the FDC's base address from its
 registers

> One place in the ARM code used to check if the port was equal to FD_DOR,
> this was changed to testing the register by applying a mask to the port,
> as was already done in the sparc code.
> 
> The sparc, m68k and parisc code could now be slightly cleaned up to
> benefit from the macro definitions above instead of the equivalent
> hard-coded values.

Just to note for future ref: the mask (7) can be introduced as define
during future clean up of these magic constants.

> 
> Signed-off-by: Willy Tarreau <w@....eu>
> ---
>  arch/arm/include/asm/floppy.h |  2 +-
>  drivers/block/floppy.c        |  9 ++++-----
>  include/uapi/linux/fdreg.h    | 18 +++++-------------
>  3 files changed, 10 insertions(+), 19 deletions(-)
> 
> diff --git a/arch/arm/include/asm/floppy.h b/arch/arm/include/asm/floppy.h
> index c665136..4e3fb71 100644
> --- a/arch/arm/include/asm/floppy.h
> +++ b/arch/arm/include/asm/floppy.h
> @@ -12,7 +12,7 @@
>  #define fd_outb(val,port)						\
>  	do {								\
>  		int new_val = (val);					\
> -		if ((port) == (u32)FD_DOR) {				\
> +		if ((port) & 7 == FD_DOR) {				\
>  			if (new_val & 0xf0)				\
>  				new_val = (new_val & 0x0c) |		\
>  					  floppy_selects[new_val & 3];	\
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index 250a451..4e43a7e 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -171,7 +171,6 @@ static int print_unex = 1;
>  #include <linux/kernel.h>
>  #include <linux/timer.h>
>  #include <linux/workqueue.h>
> -#define FDPATCHES
>  #include <linux/fdreg.h>
>  #include <linux/fd.h>
>  #include <linux/hdreg.h>
> @@ -594,14 +593,14 @@ static unsigned char fsector_t;	/* sector in track */
>  static unsigned char in_sector_offset;	/* offset within physical sector,
>  					 * expressed in units of 512 bytes */
>  
> -static inline unsigned char fdc_inb(int fdc, unsigned long addr)
> +static inline unsigned char fdc_inb(int fdc, int reg)
>  {
> -	return fd_inb(addr);
> +	return fd_inb(fdc_state[fdc].address + reg);
>  }
>  
> -static inline void fdc_outb(unsigned char value, int fdc, unsigned long addr)
> +static inline void fdc_outb(unsigned char value, int fdc, int reg)
>  {
> -	fd_outb(value, addr);
> +	fd_outb(value, fdc_state[fdc].address + reg);
>  }
>  
>  static inline bool drive_no_geom(int drive)
> diff --git a/include/uapi/linux/fdreg.h b/include/uapi/linux/fdreg.h
> index 5e2981d..1318881 100644
> --- a/include/uapi/linux/fdreg.h
> +++ b/include/uapi/linux/fdreg.h
> @@ -7,26 +7,18 @@
>   * Handbook", Sanches and Canton.
>   */
>  
> -#ifdef FDPATCHES
> -#define FD_IOPORT fdc_state[fdc].address
> -#else
> -/* It would be a lot saner just to force fdc_state[fdc].address to always
> -   be set ! FIXME */
> -#define FD_IOPORT 0x3f0

Again, just to note: FD_IOPORT (now removed), FDC1, FDC_BASE are pointing to
the same port 0x3f0 in many cases.
And at least in some cases used directly:
$ fgrep --include='*floppy*' -nrie '0x3f0' .
./arch/mips/include/asm/mach-generic/floppy.h:113:      return 0x3f0;
./arch/m68k/include/asm/floppy.h:124:     return 0x3f0;
./drivers/block/floppy.c:234:static unsigned short virtual_dma_port = 0x3f0;

> -#endif
> -
>  /* Fd controller regs. S&C, about page 340 */
> -#define FD_STATUS	(4 + FD_IOPORT )
> -#define FD_DATA		(5 + FD_IOPORT )
> +#define FD_STATUS	4
> +#define FD_DATA		5
>  
>  /* Digital Output Register */
> -#define FD_DOR		(2 + FD_IOPORT )
> +#define FD_DOR		2
>  
>  /* Digital Input Register (read) */
> -#define FD_DIR		(7 + FD_IOPORT )
> +#define FD_DIR		7
>  
>  /* Diskette Control Register (write)*/
> -#define FD_DCR		(7 + FD_IOPORT )
> +#define FD_DCR		7
>  
>  /* Bits of main status register */
>  #define STATUS_BUSYMASK	0x0F		/* drive busy mask */
> 

Denis