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]
Message-ID: <20150608215625.39544c82@recife.lan>
Date:	Mon, 8 Jun 2015 21:56:25 -0300
From:	Mauro Carvalho Chehab <mchehab@....samsung.com>
To:	"Luis R. Rodriguez" <mcgrof@...not-panic.com>
Cc:	bp@...e.de, tomi.valkeinen@...com, bhelgaas@...gle.com,
	linux-media@...r.kernel.org, linux-rdma@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	"Luis R. Rodriguez" <mcgrof@...e.com>,
	Andy Walls <awalls@...metrocast.net>,
	Doug Ledford <dledford@...hat.com>,
	Andy Lutomirski <luto@...capital.net>,
	Suresh Siddha <sbsiddha@...il.com>,
	Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	Juergen Gross <jgross@...e.com>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	Dave Airlie <airlied@...hat.com>,
	Antonino Daplas <adaplas@...il.com>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Arnd Bergmann <arnd@...db.de>,
	"Michael S. Tsirkin" <mst@...hat.com>,
	Stefan Bader <stefan.bader@...onical.com>,
	Ville Syrjälä <syrjala@....fi>,
	Mel Gorman <mgorman@...e.de>, Vlastimil Babka <vbabka@...e.cz>,
	Davidlohr Bueso <dbueso@...e.de>, konrad.wilk@...cle.com,
	ville.syrjala@...ux.intel.com, david.vrabel@...rix.com,
	jbeulich@...e.com, toshi.kani@...com,
	Roger Pau Monné <roger.pau@...rix.com>,
	linux-fbdev@...r.kernel.org, ivtv-devel@...vdriver.org,
	xen-devel@...ts.xensource.com
Subject: Re: [PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT
 disabled

Em Mon, 08 Jun 2015 17:20:20 -0700
"Luis R. Rodriguez" <mcgrof@...not-panic.com> escreveu:

> From: "Luis R. Rodriguez" <mcgrof@...e.com>
> 
> We are burrying direct access to MTRR code support on
> x86 in order to take advantage of PAT. In the future we
> also want to make the default behaviour of ioremap_nocache()
> to use strong UC, use of mtrr_add() on those systems
> would make write-combining void.
> 
> In order to help both enable us to later make strong
> UC default and in order to phase out direct MTRR access
> code port the driver over to arch_phys_wc_add() and
> annotate that the device driver requires systems to
> boot with PAT disabled, with the nopat kernel parameter.
> 
> This is a worthy comprmise given that the hardware is
> really rare these days, and perhaps only some lost souls
> in some third world country are expected to be using this
> feature of the device driver.
> 
> Acked-by: Andy Walls <awalls@...metrocast.net>
> Cc: Andy Walls <awalls@...metrocast.net>
> Cc: Doug Ledford <dledford@...hat.com>
> Cc: Mauro Carvalho Chehab <mchehab@....samsung.com>

Provided that you fix the issues below:
Acked-by: Mauro Carvalho Chehab <mchehab@....samsung.com>

> Cc: Andy Lutomirski <luto@...capital.net>
> Cc: Suresh Siddha <sbsiddha@...il.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Juergen Gross <jgross@...e.com>
> Cc: Daniel Vetter <daniel.vetter@...ll.ch>
> Cc: Dave Airlie <airlied@...hat.com>
> Cc: Bjorn Helgaas <bhelgaas@...gle.com>
> Cc: Antonino Daplas <adaplas@...il.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@...com>
> Cc: Dave Hansen <dave.hansen@...ux.intel.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Michael S. Tsirkin <mst@...hat.com>
> Cc: Stefan Bader <stefan.bader@...onical.com>
> Cc: Ville Syrjälä <syrjala@....fi>
> Cc: Mel Gorman <mgorman@...e.de>
> Cc: Vlastimil Babka <vbabka@...e.cz>
> Cc: Borislav Petkov <bp@...e.de>
> Cc: Davidlohr Bueso <dbueso@...e.de>
> Cc: konrad.wilk@...cle.com
> Cc: ville.syrjala@...ux.intel.com
> Cc: david.vrabel@...rix.com
> Cc: jbeulich@...e.com
> Cc: toshi.kani@...com
> Cc: Roger Pau Monné <roger.pau@...rix.com>
> Cc: linux-fbdev@...r.kernel.org
> Cc: linux-kernel@...r.kernel.org
> Cc: ivtv-devel@...vdriver.org
> Cc: linux-media@...r.kernel.org
> Cc: xen-devel@...ts.xensource.com
> Signed-off-by: Luis R. Rodriguez <mcgrof@...e.com>
> ---
>  drivers/media/pci/ivtv/Kconfig  |  3 +++
>  drivers/media/pci/ivtv/ivtvfb.c | 58 ++++++++++++++++-------------------------
>  2 files changed, 26 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
> index dd6ee57e..b2a7f88 100644
> --- a/drivers/media/pci/ivtv/Kconfig
> +++ b/drivers/media/pci/ivtv/Kconfig
> @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
>  	  This is used in the Hauppauge PVR-350 card. There is a driver
>  	  homepage at <http://www.ivtvdriver.org>.
>  
> +	  If you have this hardware you will need to boot with PAT disabled
> +	  on your x86 systems, use the nopat kernel parameter.
> +

Hmm... FB_IVTV is not hardware... it is framebuffer support for IVTV.
It is optional to use FB API for the video output port of this board,
instead of using V4L2 API.

I would say, instead, something like: 

	"In order to use this module, you will need to boot with PAT disabled
	  on x86 systems, using the nopat kernel parameter."

>  	  To compile this driver as a module, choose M here: the
>  	  module will be called ivtvfb.
> diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
> index 9ff1230..7685ae3 100644
> --- a/drivers/media/pci/ivtv/ivtvfb.c
> +++ b/drivers/media/pci/ivtv/ivtvfb.c
> @@ -44,8 +44,8 @@
>  #include <linux/ivtvfb.h>
>  #include <linux/slab.h>
>  
> -#ifdef CONFIG_MTRR
> -#include <asm/mtrr.h>
> +#ifdef CONFIG_X86_64
> +#include <asm/pat.h>
>  #endif
>  
>  #include "ivtv-driver.h"
> @@ -155,12 +155,11 @@ struct osd_info {
>  	/* Buffer size */
>  	u32 video_buffer_size;
>  
> -#ifdef CONFIG_MTRR
>  	/* video_base rounded down as required by hardware MTRRs */
>  	unsigned long fb_start_aligned_physaddr;
>  	/* video_base rounded up as required by hardware MTRRs */
>  	unsigned long fb_end_aligned_physaddr;
> -#endif
> +	int wc_cookie;
>  
>  	/* Store the buffer offset */
>  	int set_osd_coords_x;
> @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
>  static int ivtvfb_init_io(struct ivtv *itv)
>  {
>  	struct osd_info *oi = itv->osd_info;
> +	/* Find the largest power of two that maps the whole buffer */
> +	int size_shift = 31;
>  
>  	mutex_lock(&itv->serialize_lock);
>  	if (ivtv_init_on_first_open(itv)) {
> @@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
>  			oi->video_pbase, oi->video_vbase,
>  			oi->video_buffer_size / 1024);
>  
> -#ifdef CONFIG_MTRR
> -	{
> -		/* Find the largest power of two that maps the whole buffer */
> -		int size_shift = 31;
> -
> -		while (!(oi->video_buffer_size & (1 << size_shift))) {
> -			size_shift--;
> -		}
> -		size_shift++;
> -		oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
> -		oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
> -		oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> -		oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> -		if (mtrr_add(oi->fb_start_aligned_physaddr,
> -			oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
> -			     MTRR_TYPE_WRCOMB, 1) < 0) {
> -			IVTVFB_INFO("disabled mttr\n");
> -			oi->fb_start_aligned_physaddr = 0;
> -			oi->fb_end_aligned_physaddr = 0;
> -		}
> -	}
> -#endif
> -
> +	while (!(oi->video_buffer_size & (1 << size_shift)))
> +		size_shift--;
> +	size_shift++;
> +	oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
> +	oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
> +	oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
> +	oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
> +	oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
> +					 oi->fb_end_aligned_physaddr -
> +					 oi->fb_start_aligned_physaddr);
>  	/* Blank the entire osd. */
>  	memset_io(oi->video_vbase, 0, oi->video_buffer_size);
>  
> @@ -1172,14 +1160,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
>  
>  	/* Release pseudo palette */
>  	kfree(oi->ivtvfb_info.pseudo_palette);
> -
> -#ifdef CONFIG_MTRR
> -	if (oi->fb_end_aligned_physaddr) {
> -		mtrr_del(-1, oi->fb_start_aligned_physaddr,
> -			oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
> -	}
> -#endif
> -
> +	arch_phys_wc_del(oi->wc_cookie);
>  	kfree(oi);
>  	itv->osd_info = NULL;
>  }
> @@ -1284,6 +1265,13 @@ static int __init ivtvfb_init(void)
>  	int registered = 0;
>  	int err;
>  
> +#ifdef CONFIG_X86_64
> +	if (WARN(pat_enabled(),
> +		 "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
> +		return EINVAL;

Errors are always negative. So: 
		return -EINVAL

Or, perhaps, -ENODEV.

> +	}
> +#endif
> +
>  	if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
>  		printk(KERN_ERR "ivtvfb:  ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
>  		     IVTV_MAX_CARDS - 1);
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ