linux-kernel - Re: ioremap_uc() followed by set_memory_wc()

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: <CALCETrUFtEMYh8i00ke0f939=17bAQxMDOBZMn_3yk3Nz1AnFA@mail.gmail.com>
Date:	Wed, 15 Apr 2015 16:42:43 -0700
From:	Andy Lutomirski <luto@...capital.net>
To:	Andy Walls <awalls@...metrocast.net>
Cc:	"Luis R. Rodriguez" <mcgrof@...e.com>,
	Toshi Kani <toshi.kani@...com>,
	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	Hal Rosenstock <hal.rosenstock@...il.com>,
	Sean Hefty <sean.hefty@...el.com>,
	Suresh Siddha <sbsiddha@...il.com>,
	Rickard Strandqvist <rickard_strandqvist@...ctrumdigital.se>,
	Mike Marciniszyn <mike.marciniszyn@...el.com>,
	Roland Dreier <roland@...estorage.com>,
	Juergen Gross <jgross@...e.com>,
	Mauro Carvalho Chehab <mchehab@....samsung.com>,
	Borislav Petkov <bp@...e.de>, Mel Gorman <mgorman@...e.de>,
	Vlastimil Babka <vbabka@...e.cz>,
	Davidlohr Bueso <dbueso@...e.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Jean-Christophe Plagniol-Villard <plagnioj@...osoft.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ville Syrjälä <syrjala@....fi>,
	Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
	linux-media@...r.kernel.org, X86 ML <x86@...nel.org>
Subject: Re: ioremap_uc() followed by set_memory_wc() - burrying MTRR

On Wed, Apr 15, 2015 at 3:38 PM, Andy Walls <awalls@...metrocast.net> wrote:
> Hi All,
>
> On Mon, 2015-04-13 at 19:49 +0200, Luis R. Rodriguez wrote:
> [snip]
>>  I only saw a few drivers using overlapping ioremap*()
>> calls though on my MTRR review and they are all old devices so likely mostly
>> used on non-PAT systems, but there might be other corner cases elsewhere.
>>
>> Lets recap, as I see it we have a few options with all this in mind on our
>> quest to bury MTRR (and later make strong UC default):
>>
>> 1) Let drivers do their own get_vm_area() calls as you note and handle the
>>    cut and splicing of ioremap areas
>>
>> 2) Provide an API to split and splice ioremap ranges
>>
>> 3) Try to avoid these types of situations and let drivers simply try to
>>    work towards a proper clean non-overlapping different ioremap() ranges
>>
>> Let me know if you think of others but please keep in mind the UC- to strong
>> UC converstion we want to do later as well. We have ruled out using
>> set_memor_wc() now.
>>
>> I prefer option 3), its technically possible but only for *new* drivers
>> and we'd need either some hard work to split the ioremap() areas or provide
>> a a set of *transient* APIs as I had originally proposed to phase / hope for
>> final transition. There are 3 drivers to address:
>>
>> [snip]
>
> Just some background for folks:
> The ivtv driver supports cards that perform Standard Definition PAL,
> NTSC, and SECAM TV capture + hardware MPEG-2 encoding and MPEG-2
> decoding + TV output.
>
> Of the supported cards only *one* card type, the PVR-350 based on the
> CX23415 chip, can perform the MPEG-2 or YUV video decoding and output,
> with an OSD overlay.  PVR-350's are legacy PCI cards that have been end
> of life since 2088 or earlier.  Despite that, there are still used units
> available on Amazon and eBay.
>
> The ivtvfb driver module is an *optional* companion driver module that
> provides a framebuffer interface for the user to output an X display, FB
> console, or whatever to a standard definition analog PAL, NTSC, or SECAM
> TV or VCR.  It does this by co-opting the OSD overlay of the video
> decoding output engine and having it take up the whole screen.
>
>
>
>> c) ivtv: the driver does not have the PCI space mapped out separately, and
>> in fact it actually does not do the math for the framebuffer, instead it lets
>> the device's own CPU do that and assume where its at, see
>> ivtvfb_get_framebuffer() and CX2341X_OSD_GET_FRAMEBUFFER, it has a get
>> but not a setter. Its not clear if the firmware would make a split easy.
>
> The CX2341[56]'s firmware + hardware machine are notorious for bugs and
> being hard to work with.  It would be difficult to determine with any
> certainty that the firmware returned predictable OSD framebuffer
> addresses from one user's system to the next.
>
>
>> We'd need ioremap_ucminus() here too and __arch_phys_wc_add().
>
> Yes.
>
> As a driver writer/maintainer, IMO the name ioremap_ucminus() is jargon,
> without a clear meaning from the name, and maybe too x86 PAT specific?
> The pat.txt file under Documentation didn't explain what UC- meant; I
> had to go searching old LKML emails to understand it was a unchached
> region that could be overridden with write combine regions.
>
> To me names along, these lines would be better:
>    ioremap_nocache_weak(), or
>    ioremap_nocache_mutable(), or
>    ioremap_nocache()  (with ioremap_nocache_strong() or something for
> the UC version)
>

IMO the right solution would be to avoid ioremapping the whole bar at
startup.  Instead ioremap pieces once the driver learns what they are.
This wouldn't have any of these problems -- you'd ioremap() register
regions and you'd ioremap_wc() the framebuffer once you find it.  If
there are regions of unknown purpose, just don't map them all.

Would this be feasible?

--Andy
--
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/