* [PATCH 2/2] OMAPDSS: of-dss: omap_dss_find_output_by_port_node() keep port refcount
From: Jyri Sarha @ 2015-08-06 18:41 UTC (permalink / raw)
To: linux-fbdev
The only user of dss_of_port_get_parent_device() function is
omap_dss_find_output_by_port_node() and it assumes the refcount of the
port parameter is not decremented by the call.
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
drivers/video/fbdev/omap2/dss/dss-of.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/dss/dss-of.c b/drivers/video/fbdev/omap2/dss/dss-of.c
index ab6ef16..43f999d 100644
--- a/drivers/video/fbdev/omap2/dss/dss-of.c
+++ b/drivers/video/fbdev/omap2/dss/dss-of.c
@@ -95,7 +95,7 @@ struct device_node *dss_of_port_get_parent_device(struct device_node *port)
if (!port)
return NULL;
- np = of_get_next_parent(port);
+ np = of_get_parent(port);
for (i = 0; i < 2 && np; ++i) {
struct property *prop;
--
1.9.1
^ permalink raw reply related
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Luis R. Rodriguez @ 2015-08-06 19:53 UTC (permalink / raw)
To: Toshi Kani
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <1434128306.11808.97.camel@misato.fc.hp.com>
On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com> wrote:
> On Fri, 2015-06-12 at 08:59 +0100, Jan Beulich wrote:
>> >>> On 12.06.15 at 01:23, <toshi.kani@hp.com> wrote:
>> > There are two usages on MTRRs:
>> > 1) MTRR entries set by firmware
>> > 2) MTRR entries set by OS drivers
>> >
>> > We can obsolete 2), but we have no control over 1). As UEFI firmwares
>> > also set this up, this usage will continue to stay. So, we should not
>> > get rid of the MTRR code that looks up the MTRR entries, while we have
>> > no need to modify them.
>> >
>> > Such MTRR entries provide safe guard to /dev/mem, which allows
>> > privileged user to access a range that may require UC mapping while
>> > the /dev/mem driver blindly maps it with WB. MTRRs converts WB to UC in
>> > such a case.
>>
>> But it wouldn't be impossible to simply read the MTRRs upon boot,
>> store the information, disable MTRRs, and correctly use PAT to
>> achieve the same effect (i.e. the "blindly maps" part of course
>> would need fixing).
>
> It could be done, but I do not see much benefit of doing it. One of the
> reasons platform vendors set MTRRs is so that a system won't hit a
> machine check when an OS bug leads an access with a wrong cache type.
>
> A machine check is hard to analyze and can be seen as a hardware issue by
> customers. Emulating MTRRs with PAT won't protect from such a bug.
That's seems like a fair and valid concern. This could only happen if
the OS would have code that would use MTRR, in the case of Linux we'll
soon be able to vet that this cannot happen. For those type of OSes...
could it be possible to negotiate or hint to the platform through an
attribute somehow that the OS has such capability to not use MTRR?
Then, only if this bit is set, the platform could then avoid such MTRR
settings, and if we have issues you can throw rocks at us.
Could this also be used to prevent SMIs with MTRRs?
Luis
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Luis R. Rodriguez @ 2015-08-06 19:55 UTC (permalink / raw)
To: Toshi Kani
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <CAB=NE6W3=SFTqabeD6gq7JCqFZ7+SBZh7Xa=RteO_8-3P7fbdw@mail.gmail.com>
On Thu, Aug 6, 2015 at 12:53 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> For those type of OSes...
> could it be possible to negotiate or hint to the platform through an
> attribute somehow that the OS has such capability to not use MTRR?
And if that's not possible how about a new platform setting that would
need to be set at the platform level to enable disabling this junk?
Then only folks who know what they are doing would enable it, and if
the customer set it, the issue would not be on the platform.
Luis
^ permalink raw reply
* Re: [PATCH v9 0/8] pci: add pci_iomap_wc() and pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-08-06 21:45 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Ingo Molnar, Luis R. Rodriguez, bp, arnd, luto, akpm, linux-pci,
linux-kernel, tomi.valkeinen, mst, toshi.kani, linux-fbdev,
xen-devel, benh
In-Reply-To: <20150722134348.GA10966@google.com>
On Wed, Jul 22, 2015 at 08:43:48AM -0500, Bjorn Helgaas wrote:
> Hi Ingo,
>
> On Wed, Jul 22, 2015 at 10:38:45AM +0200, Ingo Molnar wrote:
> >
> > * Bjorn Helgaas <bhelgaas@google.com> wrote:
> >
> > > > > > Let me know if these are OK or if there are any questions.
> > > > > >
> > > > > > [0] http://lkml.kernel.org/r/20150625204703.GC4898@pd.tnic
> > > > > > [1] http://lkml.kernel.org/r/20150707095012.GQ7021@wotan.suse.de
> > > > >
> > > > > Ingo,
> > > > >
> > > > > Just a friendly reminder. Let me know if there are any issues or questions.
> > > >
> > > > It would be nice to get an Acked-by from Bjorn for the PCI API bits.
> > >
> > > I think the actual code of pci_ioremap_wc() and pci_ioremap_wc_bar() is fine
> > > (although I might have named it pci_ioremap_bar_wc() for consistency).
> > >
> > > I declined to merge or ack them myself because they're obvious extensions of
> > > pci_ioremap() and pci_ioremap_bar(), and I would prefer that they be exported
> > > the same way, i.e., with EXPORT_SYMBOL(), not EXPORT_SYMBOL_GPL().
> >
> > Huh? AFAICS pci_ioremap_bar() has been a _GPL export for a long time:
> > ...
> > (ioremap_wc() is EXPORT_SYMBOL() mostly by accident, it's the odd one out.)
>
> You're right, I was mistaken about pci_ioremap_bar(). But I'm not
> convinced yet that ioremap_wc() is the odd one out. All the interfaces I
> found, with the exception of ioremap_uc() on x86 and pci_ioremap_bar(), are
> EXPORT_SYMBOL(), even the _wc and _wt flavors.
The documentation update I provided on EXPORT_SYMBOL_GPL() which is now
upstream, at *your request* specifically for this series, *acknowledges* these
differences in opinions about new features, but it also clarifies that
positions on EXPORT_SYMBOL_GPL() can be up to developers and maintainers then,
so long as its for *new features*.
So this can be a position to take, and the guidence is there now. You asked for
this.
I acknowledge a subtle topic we seem to have stumbled upon here is what happens
if there is old "related technology" APIs with EXPORT_SYMBPOL(). Let me
elaborate on that, in my effort to provide guidence to reflect some historical
baggage while being apolagetic for those who hold a position of exclusive use
for *any* new functionality with EXPORT_SYMBOL_GPL(), like myself.
You seem to be taking a position of not allowing patches in that express the
freedom to use EXPORT_SYMBOL() because "related technology APIs had used
EXPORT_SYMBOL()". This is rather unfair for a few reasons:
0) This assumes that folks who wrote some old ioremap() calls had a position to
green light proprietary drivers to use them and embrace the idea that
EXPORT_SYMBOL_GPL() is the whitelist. As discussed in *many places* this
cannot be a reasonably general assumption as it does a *huge disservice* to many
people who always have held the position over their code always to not be
usable by proprietary drivers, and in even some cases that EXPORT_SYMBOL_GPL()
is pointless and does more harm than good, to the point they want to throw
tables at you for trying to add EXPORT_SYMBOL_GPL() code. Please consider
this *seriously*, not doing so is unfair to them.
1) Regardless of 0) its unfair to developers who do not want to deal with bug
reports for new features *at all*. Now this is important: one reason to take a
position to use EXPORT_SYMBOL_GPL() for new features even on *related
technology* APIs can be that we *cannot* change older APIs, *even* if consensus
is gathered that we don't want bug reports for certain functionality or
features. That is, even if you think 0) is dodgy there can be a general
consensus and position by maintainers to not want bug reports on certain area
in the kernel. Also using your argument one could always negate this freedom
since EXPORT_SYMBOL() is historically spread out thorugh the entire kernel, so
one could make the argument that "related technology" APIs exist all over the
kernel, thereby denying any use of EXPORT_SYMBOL_GPL() everywhere. This is
really unfair for new developers who start contributing who *do not want*
proprietary drivers to make use of their own new features.
2) Even if you consider 0 and 1) dodgy... we have positions expressed from developers
and maintainers on PAT interfaces with consensus that we don't want to deal with bug
reports for new PAT interfaces for proprietary drivers.
> > Also, FWIIW: I personally got essentially zero feedback and help from proprietary
> > binary kernel module vendors in the past couple of years as x86 maintainer,
> > despite a fair chunk of kernel crashes reported on distro kernels occuring in
> > them...
> >
> > Based on that very negative experience, when we introduce something as complex and
> > as critical as new caching APIs, the last thing I want is to have obscure bugs in
> > binary modules I cannot fix in any reasonable fashion. So even if the parent APIs
> > of new APIs weren't already _GPL exports (as in this case), I'd export them as
> > _GPL in this case.
> >
> > > I think using EXPORT_SYMBOL_GPL to express individual political aims rather than
> > > as a hint about what might be derived work makes us look like zealots, and
> > > that's not my style.
> >
> > As far as I'm concerned it's a pure technological choice: I don't want to export
> > certain types of hard to fix and critical functionality to drivers that I cannot
> > then fix.
>
> That's a good argument that I hadn't heard before (or possibly it was there
> and I missed it).
Actually, that's likely the most common reason for these positions... so yes,
you missed it, but I don't blame you. Another strong reason is the strong
legal value over EXPORT_SYMBOL_GPL(). So folks in old camp 0) above may feel
now the need to be explicit due to the legal value of EXPORT_SYMBOL_GPL().
So let me re-iterate: camp 0) folks may have taken a slightly different
position these days. Also some folks who were maybe on the fence over
"related technology" positions may simply be fed up with proprietary drivers
in general, and they have the freedom to do so and if EXPORT_SYMBOL_GPL()
is a good technical stop-gap that's their choice.
> It would be stronger still if we could change the parent APIs similarly.
Sorry, that cannot happen. It is widely accepted that this was something we
would not do, in fact Linus has held a *strong* position that this would be
highly frowned upon. So think about this -- if you acknowledge that its
sensible for developers or maintainers to not want to deal with bug reports for
hard to fix functionality and we cannot change old APIs to EXPORT_SYMBOL_GPL()
then that in and of itself is a reason then for why some developers and
maintainers have taken the position to accept *new features* to go in with
EXPORT_SYMBOL_GPL(), as a compromise.
Those who do not want to deal with the implications of proprietary drivers only
have the option to make new features EXPORTS_SYMBOL_GPL(). Denying them this
means allowing a slew of crap reports for new features for all of us. It also
is denying anyone the sentiment or change of heart that perhaps enabling
proprietary drivers was a bad idea. Letting them use EXPORTS_SYMBOL_GPL()
enables them to express this sentiment.
> If a proprietary driver can't use pci_ioremap_wc() because
> it's exported _GPL, it's trivial to use ioremap_wc() directly.
That's under the assumption again that people who wrote ioremap_wc()
meant to enable such use. And again, the documentation today does
let folks add new *features* under EXPORT_SYMBOL_GPL() so if it makes
proprietary folks just do a bit more work, why not.
Luis
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Toshi Kani @ 2015-08-06 22:58 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <CAB=NE6W3=SFTqabeD6gq7JCqFZ7+SBZh7Xa=RteO_8-3P7fbdw@mail.gmail.com>
On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
> On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com> wrote:
> > On Fri, 2015-06-12 at 08:59 +0100, Jan Beulich wrote:
> > > > > > On 12.06.15 at 01:23, <toshi.kani@hp.com> wrote:
> > > > There are two usages on MTRRs:
> > > > 1) MTRR entries set by firmware
> > > > 2) MTRR entries set by OS drivers
> > > >
> > > > We can obsolete 2), but we have no control over 1). As UEFI
> > > > firmwares
> > > > also set this up, this usage will continue to stay. So, we should
> > > > not
> > > > get rid of the MTRR code that looks up the MTRR entries, while we
> > > > have
> > > > no need to modify them.
> > > >
> > > > Such MTRR entries provide safe guard to /dev/mem, which allows
> > > > privileged user to access a range that may require UC mapping while
> > > > the /dev/mem driver blindly maps it with WB. MTRRs converts WB to
> > > > UC in
> > > > such a case.
> > >
> > > But it wouldn't be impossible to simply read the MTRRs upon boot,
> > > store the information, disable MTRRs, and correctly use PAT to
> > > achieve the same effect (i.e. the "blindly maps" part of course
> > > would need fixing).
> >
> > It could be done, but I do not see much benefit of doing it. One of the
> > reasons platform vendors set MTRRs is so that a system won't hit a
> > machine check when an OS bug leads an access with a wrong cache type.
> >
> > A machine check is hard to analyze and can be seen as a hardware issue
> > by customers. Emulating MTRRs with PAT won't protect from such a bug.
>
> That's seems like a fair and valid concern. This could only happen if
> the OS would have code that would use MTRR, in the case of Linux we'll
> soon be able to vet that this cannot happen.
No, there is no OS support necessary to use MTRR. After firmware sets it
up, CPUs continue to use it without any OS support. I think the Linux
change you are referring is to obsolete legacy interfaces that modify the
MTRR setup. I agree that Linux should not modify MTRR.
> For those type of OSes...
> could it be possible to negotiate or hint to the platform through an
> attribute somehow that the OS has such capability to not use MTRR?
The OS can disable MTRR. However, this can also cause a problem in
firmware, which may rely on MTRR.
> Then, only if this bit is set, the platform could then avoid such MTRR
> settings, and if we have issues you can throw rocks at us.
> And if that's not possible how about a new platform setting that would
> need to be set at the platform level to enable disabling this junk?
> Then only folks who know what they are doing would enable it, and if
> the customer set it, the issue would not be on the platform.
> Could this also be used to prevent SMIs with MTRRs?
ACPI _OSI could be used for firmware to implement some OS-specific features,
but it may be too late for firmware to make major changes and is generally
useless unless OS requirements are described in a spec backed by logo
certification. SMIs are also used for platform management, such as fan
speed control.
Is there any issue for Linux to use MTRR set by firmware?
Thanks,
-Toshi
^ permalink raw reply
* Re: [PATCH] video: fbdev: atmel: fix warning for const return value
From: Sudip Mukherjee @ 2015-08-07 5:34 UTC (permalink / raw)
To: Rob Herring
Cc: Tomi Valkeinen, linux-kernel, linux-fbdev, Nicolas Ferre,
Jean-Christophe Plagniol-Villard
In-Reply-To: <1438011701-18423-1-git-send-email-robh@kernel.org>
On Mon, Jul 27, 2015 at 10:41:41AM -0500, Rob Herring wrote:
> A const on a return value is meaningless and generates a warning on some
> versions of gcc:
>
> drivers/video/fbdev/atmel_lcdfb.c:1003: warning: type qualifiers ignored on function return type
maybe this one should have been static instead of const.
regards
sudip
^ permalink raw reply
* Re: [PATCH 1/2] OMAPDSS: dss-of: Fix node refcount leak in omapdss_of_get_next_port()
From: Tomi Valkeinen @ 2015-08-07 10:38 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <2641bae47b15795f2a697e69c72ad1b7424d2178.1438885638.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 769 bytes --]
On 06/08/15 21:41, Jyri Sarha wrote:
> Signed-off-by: Jyri Sarha <jsarha@ti.com>
Please always fill in the patch description. In simplest cases it may be
the same as in the subject.
> ---
> drivers/video/fbdev/omap2/dss/dss-of.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/video/fbdev/omap2/dss/dss-of.c b/drivers/video/fbdev/omap2/dss/dss-of.c
> index 928ee63..ab6ef16 100644
> --- a/drivers/video/fbdev/omap2/dss/dss-of.c
> +++ b/drivers/video/fbdev/omap2/dss/dss-of.c
> @@ -60,6 +60,7 @@ omapdss_of_get_next_port(const struct device_node *parent,
> }
> prev = port;
> } while (of_node_cmp(port->name, "port") != 0);
> + of_node_put(ports);
I think a blank line is needed above of_node_put().
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] OMAPDSS: of-dss: omap_dss_find_output_by_port_node() keep port refcount
From: Tomi Valkeinen @ 2015-08-07 10:49 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1f1f0438904af54bcaadd6e1796dad06e7001172.1438885638.git.jsarha@ti.com>
[-- Attachment #1: Type: text/plain, Size: 1035 bytes --]
Hi,
On 06/08/15 21:41, Jyri Sarha wrote:
> The only user of dss_of_port_get_parent_device() function is
> omap_dss_find_output_by_port_node() and it assumes the refcount of the
> port parameter is not decremented by the call.
>
The subject of the patch should contain "dss-of", not "of-dss". Although
for both patches I think it's fine to use plain "OMAPDSS: " prefix. The
subject should also say "fix" or such.
The description above is kind of "detached". A patch description should
generally describe something in the lines of what the current behavior
is, what the problem is, and what the patch does. And the desc should be
independent of the subject.
This particular case is rather simple, and it's clear that
dss_of_port_get_parent_device() is not supposed to decrease the refcount
of the "port" parameter (as that kind of behavior is normally a special
case). And so it should be enough to say what the issue seen is and that
you fix the function to not decrement the parameter's refcount.
Tomi
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH v2 0/2] OMAPDSS: Fix of-node refcount issues
From: Jyri Sarha @ 2015-08-07 11:04 UTC (permalink / raw)
To: linux-fbdev
Changes since the first version:
- Fix commit descriptions and subject according to Tomi's comments
I found couple of refcounting issues related to OMAP DSS of-node
handling. Second patch should fix the "ERROR: Bad of_node_put() on
/encoder@0/ports/port@1" -problem.
In the long run it would make sense start using of_graph_*() functions
in OMAP DSS too. However the semantics of of_graph_*() functions are
quite different from omapdss_of_*() counterparts. So it better to just
fix these bugs first.
Jyri Sarha (2):
OMAPDSS: Fix node refcount leak in omapdss_of_get_next_port()
OMAPDSS: Fix omap_dss_find_output_by_port_node() port refcount
decrement
drivers/video/fbdev/omap2/dss/dss-of.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
--
1.9.1
^ permalink raw reply
* [PATCH v2 1/2] OMAPDSS: Fix node refcount leak in omapdss_of_get_next_port()
From: Jyri Sarha @ 2015-08-07 11:04 UTC (permalink / raw)
To: linux-fbdev
Fix node refcount leak in omapdss_of_get_next_port().
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
drivers/video/fbdev/omap2/dss/dss-of.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/video/fbdev/omap2/dss/dss-of.c b/drivers/video/fbdev/omap2/dss/dss-of.c
index 928ee63..ab6ef16 100644
--- a/drivers/video/fbdev/omap2/dss/dss-of.c
+++ b/drivers/video/fbdev/omap2/dss/dss-of.c
@@ -60,6 +60,7 @@ omapdss_of_get_next_port(const struct device_node *parent,
}
prev = port;
} while (of_node_cmp(port->name, "port") != 0);
+ of_node_put(ports);
}
return port;
--
1.9.1
^ permalink raw reply related
* [PATCH v2 2/2] OMAPDSS: Fix omap_dss_find_output_by_port_node() port refcount decrement
From: Jyri Sarha @ 2015-08-07 11:04 UTC (permalink / raw)
To: linux-fbdev
Fix omap_dss_find_output_by_port_node() port parameter refcount
decrementation. The only user of dss_of_port_get_parent_device()
function is omap_dss_find_output_by_port_node() and it assumes the
refcount of the port parameter is not decremented by the call.
Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
drivers/video/fbdev/omap2/dss/dss-of.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/omap2/dss/dss-of.c b/drivers/video/fbdev/omap2/dss/dss-of.c
index ab6ef16..43f999d 100644
--- a/drivers/video/fbdev/omap2/dss/dss-of.c
+++ b/drivers/video/fbdev/omap2/dss/dss-of.c
@@ -95,7 +95,7 @@ struct device_node *dss_of_port_get_parent_device(struct device_node *port)
if (!port)
return NULL;
- np = of_get_next_parent(port);
+ np = of_get_parent(port);
for (i = 0; i < 2 && np; ++i) {
struct property *prop;
--
1.9.1
^ permalink raw reply related
* [PATCH 1/5] staging: sm750fb: remove multiple blank line
From: Sudip Mukherjee @ 2015-08-07 12:16 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, devel, linux-fbdev, Sudip Mukherjee
Multiple blank lines are not kernel coding style and so checkpatch
complains. Remove them.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/staging/sm750fb/sm750.c | 26 --------------------------
1 file changed, 26 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index c7ab818..8990ff4 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -26,7 +26,6 @@
int smi_indent;
-
/*
* #ifdef __BIG_ENDIAN
* ssize_t lynxfb_ops_write(struct fb_info *info, const char __user *buf,
@@ -40,7 +39,6 @@ typedef void (*PROC_SPEC_SETUP)(struct lynx_share*, char *);
typedef int (*PROC_SPEC_MAP)(struct lynx_share*, struct pci_dev*);
typedef int (*PROC_SPEC_INITHW)(struct lynx_share*, struct pci_dev*);
-
/* common var for all device */
static int g_hwcursor = 1;
static int g_noaccel;
@@ -51,7 +49,6 @@ static char *g_settings;
static int g_dualview;
static char *g_option;
-
static const struct fb_videomode lynx750_ext[] = {
/* 1024x600-60 VESA [1.71:1] */
{NULL, 60, 1024, 600, 20423, 144, 40, 18, 1, 104, 3,
@@ -115,8 +112,6 @@ static const struct fb_videomode lynx750_ext[] = {
};
-
-
/* no hardware cursor supported under version 2.6.10, kernel bug */
static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
{
@@ -160,7 +155,6 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
cursor->setColor(cursor, fg, bg);
}
-
if (fbcursor->set & (FB_CUR_SETSHAPE | FB_CUR_SETIMAGE)) {
cursor->setData(cursor,
fbcursor->rop,
@@ -300,7 +294,6 @@ static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
struct lynxfb_crtc *crtc;
int ret;
-
if (!info)
return -EINVAL;
@@ -466,7 +459,6 @@ static int lynxfb_resume(struct pci_dev *pdev)
int ret;
-
ret = 0;
share = pci_get_drvdata(pdev);
@@ -478,7 +470,6 @@ static int lynxfb_resume(struct pci_dev *pdev)
return ret;
}
-
if (pdev->dev.power.power_state.event != PM_EVENT_FREEZE) {
pci_restore_state(pdev);
ret = pci_enable_device(pdev);
@@ -493,7 +484,6 @@ static int lynxfb_resume(struct pci_dev *pdev)
hw_sm750_inithw(share, pdev);
-
info = share->fbinfo[0];
if (info) {
@@ -518,7 +508,6 @@ static int lynxfb_resume(struct pci_dev *pdev)
fb_set_suspend(info, 0);
}
-
console_unlock();
return ret;
}
@@ -534,7 +523,6 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
int ret;
resource_size_t request;
-
par = info->par;
crtc = &par->crtc;
output = &par->output;
@@ -546,7 +534,6 @@ static int lynxfb_ops_check_var(struct fb_var_screeninfo *var,
var->yres,
var->bits_per_pixel);
-
switch (var->bits_per_pixel) {
case 8:
case 16:
@@ -617,7 +604,6 @@ exit:
return ret;
}
-
static int lynxfb_ops_setcolreg(unsigned regno,
unsigned red,
unsigned green,
@@ -652,7 +638,6 @@ static int lynxfb_ops_setcolreg(unsigned regno,
goto exit;
}
-
if (info->fix.visual = FB_VISUAL_TRUECOLOR && regno < 256) {
u32 val;
@@ -782,7 +767,6 @@ static struct fb_ops lynxfb_ops = {
.fb_cursor = lynxfb_ops_cursor,
};
-
static int lynxfb_set_fbinfo(struct fb_info *info, int index)
{
int i;
@@ -803,7 +787,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
"kernel HELPERS prepared vesa_modes",
};
-
static const char *fixId[2] = {
"sm750_fb1", "sm750_fb2",
};
@@ -824,7 +807,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
sm750fb_set_drv(par);
lynxfb_ops.fb_pan_display = lynxfb_ops_pan_display;
-
/* set current cursor variable and proc pointer,
* must be set after crtc member initialized */
crtc->cursor.offset = crtc->oScreen + crtc->vidmem_size - 1024;
@@ -841,7 +823,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
crtc->cursor.setData = hw_cursor_setData;
crtc->cursor.vstart = share->pvMem + crtc->cursor.offset;
-
crtc->cursor.share = share;
memset_io(crtc->cursor.vstart, 0, crtc->cursor.size);
if (!g_hwcursor) {
@@ -849,7 +830,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
crtc->cursor.disable(&crtc->cursor);
}
-
/* set info->fbops, must be set before fb_find_mode */
if (!share->accel_off) {
/* use 2d acceleration */
@@ -865,7 +845,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
g_fbmode[index] = g_fbmode[0];
}
-
for (i = 0; i < 3; i++) {
ret = fb_find_mode(var, info, g_fbmode[index],
@@ -935,7 +914,6 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
strlcpy(fix->id, fixId[index], sizeof(fix->id));
-
fix->smem_start = crtc->oScreen + share->vidmem_start;
pr_info("fix->smem_start = %lx\n", fix->smem_start);
/* according to mmap experiment from user space application,
@@ -998,7 +976,6 @@ static void sm750fb_setup(struct lynx_share *share, char *src)
#endif
int swap;
-
spec_share = container_of(share, struct sm750_share, share);
#ifdef CAP_EXPENSIION
exp_res = NULL;
@@ -1096,7 +1073,6 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
size_t spec_offset = 0;
int fbidx;
-
/* enable device */
if (pci_enable_device(pdev)) {
pr_err("can not enable device.\n");
@@ -1268,7 +1244,6 @@ static int __init lynxfb_setup(char *options)
int len;
char *opt, *tmp;
-
if (!options || !*options) {
pr_warn("no options.\n");
return 0;
@@ -1332,7 +1307,6 @@ static struct pci_driver lynxfb_driver = {
#endif
};
-
static int __init lynxfb_init(void)
{
char *option;
--
1.9.1
^ permalink raw reply related
* [PATCH 2/5] staging: sm750fb: space around operator
From: Sudip Mukherjee @ 2015-08-07 12:16 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, devel, linux-fbdev, Sudip Mukherjee
In-Reply-To: <1438949046-18765-1-git-send-email-sudipm.mukherjee@gmail.com>
Kernel coding style says to have a space around the operators.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
This patch will generate checkpatch warning about longline in 2 places,
those were not fixed to keep the readability. Will fix them later.
The warnings were already there, this patch has not introduced them.
drivers/staging/sm750fb/sm750.c | 30 +++++++++++++++++-------------
1 file changed, 17 insertions(+), 13 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index 8990ff4..c16b293 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -144,12 +144,12 @@ static int lynxfb_ops_cursor(struct fb_info *info, struct fb_cursor *fbcursor)
/* get the 16bit color of kernel means */
u16 fg, bg;
- fg = ((info->cmap.red[fbcursor->image.fg_color] & 0xf800))|
- ((info->cmap.green[fbcursor->image.fg_color] & 0xfc00) >> 5)|
+ fg = ((info->cmap.red[fbcursor->image.fg_color] & 0xf800)) |
+ ((info->cmap.green[fbcursor->image.fg_color] & 0xfc00) >> 5) |
((info->cmap.blue[fbcursor->image.fg_color] & 0xf800) >> 11);
- bg = ((info->cmap.red[fbcursor->image.bg_color] & 0xf800))|
- ((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5)|
+ bg = ((info->cmap.red[fbcursor->image.bg_color] & 0xf800)) |
+ ((info->cmap.green[fbcursor->image.bg_color] & 0xfc00) >> 5) |
((info->cmap.blue[fbcursor->image.bg_color] & 0xf800) >> 11);
cursor->setColor(cursor, fg, bg);
@@ -188,8 +188,9 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
pitch = info->fix.line_length;
Bpp = info->var.bits_per_pixel >> 3;
- color = (Bpp = 1)?region->color:((u32 *)info->pseudo_palette)[region->color];
- rop = (region->rop != ROP_COPY) ? HW_ROP2_XOR:HW_ROP2_COPY;
+ color = (Bpp = 1) ? region->color :
+ ((u32 *)info->pseudo_palette)[region->color];
+ rop = (region->rop != ROP_COPY) ? HW_ROP2_XOR : HW_ROP2_COPY;
/*
* If not use spin_lock,system will die if user load driver
@@ -278,7 +279,7 @@ _do_work:
spin_lock(&share->slock);
share->accel.de_imageblit(&share->accel,
- image->data, image->width>>3, 0,
+ image->data, image->width >> 3, 0,
base, pitch, Bpp,
image->dx, image->dy,
image->width, image->height,
@@ -684,7 +685,8 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
output = &par->output;
crtc = &par->crtc;
- crtc->vidmem_size = (share->dual)?share->vidmem_size>>1:share->vidmem_size;
+ crtc->vidmem_size = (share->dual) ? share->vidmem_size >> 1 :
+ share->vidmem_size;
/* setup crtc and output member */
spec_share->hwCursor = g_hwcursor;
@@ -701,10 +703,12 @@ static int sm750fb_set_drv(struct lynxfb_par *par)
output->proc_setMode = hw_sm750_output_setMode;
output->proc_checkMode = hw_sm750_output_checkMode;
- output->proc_setBLANK = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_setBLANK:hw_sm750_setBLANK;
+ output->proc_setBLANK = (share->revid = SM750LE_REVISION_ID) ?
+ hw_sm750le_setBLANK : hw_sm750_setBLANK;
output->clear = hw_sm750_output_clear;
/* chip specific phase */
- share->accel.de_wait = (share->revid = SM750LE_REVISION_ID)?hw_sm750le_deWait : hw_sm750_deWait;
+ share->accel.de_wait = (share->revid = SM750LE_REVISION_ID) ?
+ hw_sm750le_deWait : hw_sm750_deWait;
switch (spec_share->state.dataflow) {
case sm750_simul_pri:
output->paths = sm750_pnc;
@@ -814,7 +818,7 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
pr_info("crtc->cursor.mmio = %p\n", crtc->cursor.mmio);
crtc->cursor.maxH = crtc->cursor.maxW = 64;
- crtc->cursor.size = crtc->cursor.maxH*crtc->cursor.maxW*2/8;
+ crtc->cursor.size = crtc->cursor.maxH * crtc->cursor.maxW * 2 / 8;
crtc->cursor.disable = hw_cursor_disable;
crtc->cursor.enable = hw_cursor_enable;
crtc->cursor.setColor = hw_cursor_setColor;
@@ -896,13 +900,13 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
/* set info */
line_length = PADDING(crtc->line_pad,
- (var->xres_virtual * var->bits_per_pixel/8));
+ (var->xres_virtual * var->bits_per_pixel / 8));
info->pseudo_palette = &par->pseudo_palette[0];
info->screen_base = crtc->vScreen;
pr_debug("screen_base vaddr = %p\n", info->screen_base);
info->screen_size = line_length * var->yres_virtual;
- info->flags = FBINFO_FLAG_DEFAULT|0;
+ info->flags = FBINFO_FLAG_DEFAULT | 0;
/* set info->fix */
fix->type = FB_TYPE_PACKED_PIXELS;
--
1.9.1
^ permalink raw reply related
* [PATCH 3/5] staging: sm750fb: fix multiline comment
From: Sudip Mukherjee @ 2015-08-07 12:16 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, devel, linux-fbdev, Sudip Mukherjee
In-Reply-To: <1438949046-18765-1-git-send-email-sudipm.mukherjee@gmail.com>
Multiline comments are edited to be in the kernel coding style.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/staging/sm750fb/sm750.c | 63 +++++++++++++++++++++++++----------------
1 file changed, 39 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index c16b293..fa2183d 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -182,8 +182,10 @@ static void lynxfb_ops_fillrect(struct fb_info *info,
par = info->par;
share = par->share;
- /* each time 2d function begin to work,below three variable always need
- * be set, seems we can put them together in some place */
+ /*
+ * each time 2d function begin to work,below three variable always need
+ * be set, seems we can put them together in some place
+ */
base = par->crtc.oScreen;
pitch = info->fix.line_length;
Bpp = info->var.bits_per_pixel >> 3;
@@ -218,8 +220,10 @@ static void lynxfb_ops_copyarea(struct fb_info *info,
par = info->par;
share = par->share;
- /* each time 2d function begin to work,below three variable always need
- * be set, seems we can put them together in some place */
+ /*
+ * each time 2d function begin to work,below three variable always need
+ * be set, seems we can put them together in some place
+ */
base = par->crtc.oScreen;
pitch = info->fix.line_length;
Bpp = info->var.bits_per_pixel >> 3;
@@ -249,8 +253,10 @@ static void lynxfb_ops_imageblit(struct fb_info *info,
par = info->par;
share = par->share;
- /* each time 2d function begin to work,below three variable always need
- * be set, seems we can put them together in some place */
+ /*
+ * each time 2d function begin to work,below three variable always need
+ * be set, seems we can put them together in some place
+ */
base = par->crtc.oScreen;
pitch = info->fix.line_length;
Bpp = info->var.bits_per_pixel >> 3;
@@ -334,9 +340,10 @@ static int lynxfb_ops_set_par(struct fb_info *info)
fix->line_length = line_length;
pr_info("fix->line_length = %d\n", fix->line_length);
- /* var->red,green,blue,transp are need to be set by driver
+ /*
+ * var->red,green,blue,transp are need to be set by driver
* and these data should be set before setcolreg routine
- * */
+ */
switch (var->bits_per_pixel) {
case 8:
@@ -811,8 +818,10 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
sm750fb_set_drv(par);
lynxfb_ops.fb_pan_display = lynxfb_ops_pan_display;
- /* set current cursor variable and proc pointer,
- * must be set after crtc member initialized */
+ /*
+ * set current cursor variable and proc pointer,
+ * must be set after crtc member initialized
+ */
crtc->cursor.offset = crtc->oScreen + crtc->vidmem_size - 1024;
crtc->cursor.mmio = share->pvReg + 0x800f0 + (int)crtc->channel * 0x140;
@@ -920,12 +929,13 @@ static int lynxfb_set_fbinfo(struct fb_info *info, int index)
fix->smem_start = crtc->oScreen + share->vidmem_start;
pr_info("fix->smem_start = %lx\n", fix->smem_start);
- /* according to mmap experiment from user space application,
+ /*
+ * according to mmap experiment from user space application,
* fix->mmio_len should not larger than virtual size
* (xres_virtual x yres_virtual x ByPP)
* Below line maybe buggy when user mmap fb dev node and write
* data into the bound over virtual size
- * */
+ */
fix->smem_len = crtc->vidmem_size;
pr_info("fix->smem_len = %x\n", fix->smem_len);
info->screen_size = fix->smem_len;
@@ -1083,8 +1093,10 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
goto err_enable;
}
- /* though offset of share in sm750_share is 0,
- * we use this marcro as the same */
+ /*
+ * though offset of share in sm750_share is 0,
+ * we use this marcro as the same
+ */
spec_offset = offsetof(struct sm750_share, share);
spec_share = kzalloc(sizeof(*spec_share), GFP_KERNEL);
@@ -1108,10 +1120,12 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
spin_lock_init(&share->slock);
if (!share->accel_off) {
- /* hook deInit and 2d routines, notes that below hw_xxx
+ /*
+ * hook deInit and 2d routines, notes that below hw_xxx
* routine can work on most of lynx chips
* if some chip need specific function,
- * please hook it in smXXX_set_drv routine */
+ * please hook it in smXXX_set_drv routine
+ */
share->accel.de_init = hw_de_init;
share->accel.de_fillrect = hw_fillrect;
share->accel.de_copyarea = hw_copyarea;
@@ -1262,14 +1276,15 @@ static int __init lynxfb_setup(char *options)
tmp = g_settings;
- /* Notes:
- char * strsep(char **s,const char * ct);
- @s: the string to be searched
- @ct :the characters to search for
-
- strsep() updates @options to pointer after the first found token
- it also returns the pointer ahead the token.
- */
+ /*
+ * Notes:
+ * char * strsep(char **s,const char * ct);
+ * @s: the string to be searched
+ * @ct :the characters to search for
+ *
+ * strsep() updates @options to pointer after the first found token
+ * it also returns the pointer ahead the token.
+ */
while ((opt = strsep(&options, ":")) != NULL) {
/* options that mean for any lynx chips are configured here */
if (!strncmp(opt, "noaccel", strlen("noaccel")))
--
1.9.1
^ permalink raw reply related
* [PATCH 4/5] staging: sm750fb: rearrange code
From: Sudip Mukherjee @ 2015-08-07 12:16 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, devel, linux-fbdev, Sudip Mukherjee
In-Reply-To: <1438949046-18765-1-git-send-email-sudipm.mukherjee@gmail.com>
Rearrange the code to remove one exit label which also makes the code
less indented and more readable.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/staging/sm750fb/sm750.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index fa2183d..e7fad0f 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -261,22 +261,21 @@ static void lynxfb_ops_imageblit(struct fb_info *info,
pitch = info->fix.line_length;
Bpp = info->var.bits_per_pixel >> 3;
- if (image->depth = 1) {
- if (info->fix.visual = FB_VISUAL_TRUECOLOR ||
- info->fix.visual = FB_VISUAL_DIRECTCOLOR) {
- fgcol = ((u32 *)info->pseudo_palette)[image->fg_color];
- bgcol = ((u32 *)info->pseudo_palette)[image->bg_color];
- } else {
- fgcol = image->fg_color;
- bgcol = image->bg_color;
- }
- goto _do_work;
- }
/* TODO: Implement hardware acceleration for image->depth > 1 */
- cfb_imageblit(info, image);
- return;
+ if (image->depth != 1) {
+ cfb_imageblit(info, image);
+ return;
+ }
+
+ if (info->fix.visual = FB_VISUAL_TRUECOLOR ||
+ info->fix.visual = FB_VISUAL_DIRECTCOLOR) {
+ fgcol = ((u32 *)info->pseudo_palette)[image->fg_color];
+ bgcol = ((u32 *)info->pseudo_palette)[image->bg_color];
+ } else {
+ fgcol = image->fg_color;
+ bgcol = image->bg_color;
+ }
-_do_work:
/*
* If not use spin_lock, system will die if user load driver
* and immediately unload driver frequently (dual)
--
1.9.1
^ permalink raw reply related
* [PATCH 5/5] staging: sm750fb: simplify return
From: Sudip Mukherjee @ 2015-08-07 12:16 UTC (permalink / raw)
To: Greg Kroah-Hartman; +Cc: linux-kernel, devel, linux-fbdev, Sudip Mukherjee
In-Reply-To: <1438949046-18765-1-git-send-email-sudipm.mukherjee@gmail.com>
Lets return the return value directly instead of using a variable to
store the result.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
drivers/staging/sm750fb/sm750.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index e7fad0f..07f8afd 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -298,17 +298,13 @@ static int lynxfb_ops_pan_display(struct fb_var_screeninfo *var,
{
struct lynxfb_par *par;
struct lynxfb_crtc *crtc;
- int ret;
if (!info)
return -EINVAL;
- ret = 0;
par = info->par;
crtc = &par->crtc;
- ret = crtc->proc_panDisplay(crtc, var, info);
-
- return ret;
+ return crtc->proc_panDisplay(crtc, var, info);
}
static int lynxfb_ops_set_par(struct fb_info *info)
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND 1/3] staging: sm7xxfb: merge sm712fb with fbdev
From: Sudip Mukherjee @ 2015-08-07 13:13 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jonathan Corbet,
Greg Kroah-Hartman
Cc: linux-kernel, linux-fbdev, linux-doc, devel, Sudip Mukherjee
Now since all cleanups are done and the code is ready to be merged lets
move it out of staging into fbdev location.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
Hi Greg,
Resending the patch series as discussed with you. Patch generated
with -M, thought it will be easy for you to review.
drivers/staging/Kconfig | 2 --
drivers/staging/Makefile | 1 -
drivers/staging/sm7xxfb/Kconfig | 13 -------------
drivers/staging/sm7xxfb/Makefile | 1 -
drivers/staging/sm7xxfb/TODO | 12 ------------
drivers/video/fbdev/Kconfig | 14 ++++++++++++++
drivers/video/fbdev/Makefile | 1 +
drivers/{staging/sm7xxfb/sm7xx.h => video/fbdev/sm712.h} | 0
.../{staging/sm7xxfb/sm7xxfb.c => video/fbdev/sm712fb.c} | 2 +-
9 files changed, 16 insertions(+), 30 deletions(-)
delete mode 100644 drivers/staging/sm7xxfb/Kconfig
delete mode 100644 drivers/staging/sm7xxfb/Makefile
delete mode 100644 drivers/staging/sm7xxfb/TODO
rename drivers/{staging/sm7xxfb/sm7xx.h => video/fbdev/sm712.h} (100%)
rename drivers/{staging/sm7xxfb/sm7xxfb.c => video/fbdev/sm712fb.c} (99%)
diff --git a/drivers/staging/Kconfig b/drivers/staging/Kconfig
index 4f0b12d..39c5df6 100644
--- a/drivers/staging/Kconfig
+++ b/drivers/staging/Kconfig
@@ -56,8 +56,6 @@ source "drivers/staging/vt6656/Kconfig"
source "drivers/staging/iio/Kconfig"
-source "drivers/staging/sm7xxfb/Kconfig"
-
source "drivers/staging/sm750fb/Kconfig"
source "drivers/staging/xgifb/Kconfig"
diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 37149ca..018093d 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -22,7 +22,6 @@ obj-$(CONFIG_VT6655) += vt6655/
obj-$(CONFIG_VT6656) += vt6656/
obj-$(CONFIG_VME_BUS) += vme/
obj-$(CONFIG_IIO) += iio/
-obj-$(CONFIG_FB_SM7XX) += sm7xxfb/
obj-$(CONFIG_FB_SM750) += sm750fb/
obj-$(CONFIG_FB_XGI) += xgifb/
obj-$(CONFIG_USB_EMXX) += emxx_udc/
diff --git a/drivers/staging/sm7xxfb/Kconfig b/drivers/staging/sm7xxfb/Kconfig
deleted file mode 100644
index e2922ae..0000000
--- a/drivers/staging/sm7xxfb/Kconfig
+++ /dev/null
@@ -1,13 +0,0 @@
-config FB_SM7XX
- tristate "Silicon Motion SM7XX framebuffer support"
- depends on FB && PCI
- select FB_CFB_FILLRECT
- select FB_CFB_COPYAREA
- select FB_CFB_IMAGEBLIT
- help
- Frame buffer driver for the Silicon Motion SM710, SM712, SM721
- and SM722 chips.
-
- This driver is also available as a module. The module will be
- called sm7xxfb. If you want to compile it as a module, say M
- here and read <file:Documentation/kbuild/modules.txt>.
diff --git a/drivers/staging/sm7xxfb/Makefile b/drivers/staging/sm7xxfb/Makefile
deleted file mode 100644
index 48f471c..0000000
--- a/drivers/staging/sm7xxfb/Makefile
+++ /dev/null
@@ -1 +0,0 @@
-obj-$(CONFIG_FB_SM7XX) += sm7xxfb.o
diff --git a/drivers/staging/sm7xxfb/TODO b/drivers/staging/sm7xxfb/TODO
deleted file mode 100644
index 7cb0b24..0000000
--- a/drivers/staging/sm7xxfb/TODO
+++ /dev/null
@@ -1,12 +0,0 @@
-TODO:
-- Dual head support
-- 2D acceleration support
-- use kernel coding style
-- refine the code and remove unused code
-- move it to drivers/video/fbdev/sm7xxfb.c
-
-Please send any patches to
- Greg Kroah-Hartman <greg@kroah.com>
- Sudip Mukherjee <sudipm.mukherjee@gmail.com>
- Teddy Wang <teddy.wang@siliconmotion.com>
- Sudip Mukherjee <sudip@vectorindia.org>
diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
index 2d98de5..2307909 100644
--- a/drivers/video/fbdev/Kconfig
+++ b/drivers/video/fbdev/Kconfig
@@ -2475,3 +2475,17 @@ config FB_SSD1307
help
This driver implements support for the Solomon SSD1307
OLED controller over I2C.
+
+config FB_SM712
+ tristate "Silicon Motion SM712 framebuffer support"
+ depends on FB && PCI
+ select FB_CFB_FILLRECT
+ select FB_CFB_COPYAREA
+ select FB_CFB_IMAGEBLIT
+ help
+ Frame buffer driver for the Silicon Motion SM710, SM712, SM721
+ and SM722 chips.
+
+ This driver is also available as a module. The module will be
+ called sm712fb. If you want to compile it as a module, say M
+ here and read <file:Documentation/kbuild/modules.txt>.
diff --git a/drivers/video/fbdev/Makefile b/drivers/video/fbdev/Makefile
index cecea50..50ed1b4 100644
--- a/drivers/video/fbdev/Makefile
+++ b/drivers/video/fbdev/Makefile
@@ -131,6 +131,7 @@ obj-$(CONFIG_FB_JZ4740) += jz4740_fb.o
obj-$(CONFIG_FB_PUV3_UNIGFX) += fb-puv3.o
obj-$(CONFIG_FB_HYPERV) += hyperv_fb.o
obj-$(CONFIG_FB_OPENCORES) += ocfb.o
+obj-$(CONFIG_FB_SM712) += sm712fb.o
# Platform or fallback drivers go here
obj-$(CONFIG_FB_UVESA) += uvesafb.o
diff --git a/drivers/staging/sm7xxfb/sm7xx.h b/drivers/video/fbdev/sm712.h
similarity index 100%
rename from drivers/staging/sm7xxfb/sm7xx.h
rename to drivers/video/fbdev/sm712.h
diff --git a/drivers/staging/sm7xxfb/sm7xxfb.c b/drivers/video/fbdev/sm712fb.c
similarity index 99%
rename from drivers/staging/sm7xxfb/sm7xxfb.c
rename to drivers/video/fbdev/sm712fb.c
index 07c2199..629bfa2 100644
--- a/drivers/staging/sm7xxfb/sm7xxfb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -32,7 +32,7 @@
#include <linux/pm.h>
#endif
-#include "sm7xx.h"
+#include "sm712.h"
/*
* Private structure
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND 2/3] Documentation/fb: add documentation for sm712fb
From: Sudip Mukherjee @ 2015-08-07 13:13 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jonathan Corbet,
Greg Kroah-Hartman
Cc: linux-kernel, linux-fbdev, linux-doc, devel, Sudip Mukherjee
In-Reply-To: <1438952475-22151-1-git-send-email-sudipm.mukherjee@gmail.com>
Create the documentation for SM712. Mention all the supported modes and
how to use.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
Documentation/fb/sm712fb.txt | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
create mode 100644 Documentation/fb/sm712fb.txt
diff --git a/Documentation/fb/sm712fb.txt b/Documentation/fb/sm712fb.txt
new file mode 100644
index 0000000..c388442
--- /dev/null
+++ b/Documentation/fb/sm712fb.txt
@@ -0,0 +1,31 @@
+What is sm712fb?
+========+
+This is a graphics framebuffer driver for Silicon Motion SM712 based processors.
+
+How to use it?
+=======
+
+Switching modes is done using the video=sm712fb:... boot parameter.
+
+If you want, for example, enable a resolution of 1280x1024x24bpp you should
+pass to the kernel this command line: "video=sm712fb:0x31B".
+
+You should not compile-in vesafb.
+
+Currently supported video modes are:
+
+[Graphic modes]
+
+bpp | 640x480 800x600 1024x768 1280x1024
+----+--------------------------------------------
+ 8 | 0x301 0x303 0x305 0x307
+ 16 | 0x311 0x314 0x317 0x31A
+ 24 | 0x312 0x315 0x318 0x31B
+
+Missing Features
+========
+(alias TODO list)
+
+ * 2D acceleratrion
+ * dual-head support
--
1.9.1
^ permalink raw reply related
* [PATCH RESEND 3/3] MAINTAINERS: update maintainers list
From: Sudip Mukherjee @ 2015-08-07 13:13 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, Jonathan Corbet,
Greg Kroah-Hartman
Cc: linux-kernel, linux-fbdev, linux-doc, devel, Sudip Mukherjee
In-Reply-To: <1438952475-22151-1-git-send-email-sudipm.mukherjee@gmail.com>
Now since sm712fb has moved out of staging update the maintainers list
accordingly.
Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
---
MAINTAINERS | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index 9289ecb..ba6c537 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9324,6 +9324,15 @@ S: Maintained
F: drivers/media/i2c/ov2659.c
F: include/media/ov2659.h
+SILICON MOTION SM712 FRAME BUFFER DRIVER
+M: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
+M: Teddy Wang <teddy.wang@siliconmotion.com>
+M: Sudip Mukherjee <sudip@vectorindia.org>
+L: linux-fbdev@vger.kernel.org
+S: Maintained
+F: drivers/video/fbdev/sm712*
+F: Documentation/fb/sm712fb.txt
+
SIS 190 ETHERNET DRIVER
M: Francois Romieu <romieu@fr.zoreil.com>
L: netdev@vger.kernel.org
@@ -9744,14 +9753,6 @@ L: linux-wireless@vger.kernel.org
S: Maintained
F: drivers/staging/rtl8723au/
-STAGING - SILICON MOTION SM7XX FRAME BUFFER DRIVER
-M: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
-M: Teddy Wang <teddy.wang@siliconmotion.com>
-M: Sudip Mukherjee <sudip@vectorindia.org>
-L: linux-fbdev@vger.kernel.org
-S: Maintained
-F: drivers/staging/sm7xxfb/
-
STAGING - SILICON MOTION SM750 FRAME BUFFER DRIVER
M: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
M: Teddy Wang <teddy.wang@siliconmotion.com>
--
1.9.1
^ permalink raw reply related
* [PATCH] drm: Remove two-level menu in Kconfig
From: Thierry Reding @ 2015-08-07 16:10 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen; +Cc: linux-fbdev, dri-devel
From: Thierry Reding <treding@nvidia.com>
The Direct Rendering Manager Kconfig option is already a separate menu,
so remove the extra level to make it easier to navigate.
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
drivers/video/Kconfig | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 8bf495ffb020..e0606c01e8ac 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -22,9 +22,7 @@ source "drivers/gpu/vga/Kconfig"
source "drivers/gpu/host1x/Kconfig"
source "drivers/gpu/ipu-v3/Kconfig"
-menu "Direct Rendering Manager"
source "drivers/gpu/drm/Kconfig"
-endmenu
menu "Frame buffer Devices"
source "drivers/video/fbdev/Kconfig"
--
2.4.5
^ permalink raw reply related
* [PATCH v3 0/9] Use correctly the Xen memory terminologies
From: Julien Grall @ 2015-08-07 16:34 UTC (permalink / raw)
To: xen-devel
Cc: linux-fbdev, x86, netdev, H. Peter Anvin, Jiri Slaby,
Thomas Gleixner, Russell King, linux-scsi, Tomi Valkeinen,
stefano.stabellini, Ingo Molnar, linux-input,
Jean-Christophe Plagniol-Villard, ian.campbell,
Konrad Rzeszutek Wilk, James E.J. Bottomley, Boris Ostrovsky,
linux-arm-kernel, Juergen Gross, Wei Liu, Greg Kroah-Hartman,
Dmitry Torokhov, linux-kernel, Julien Grall,
David Vrabel <david.v>
Hi all,
This patch series aims to use the memory terminologies described in
include/xen/mm.h [1] for Linux xen code.
The differences from v2 is minor but I resent it because my 64K series depends
on this series.
Linux is using mistakenly MFN when GFN is meant, I suspect this is because the
first support of Xen was for PV. This has brought some misimplementation
of memory helpers on ARM and make the developper confused about the expected
behavior.
For instance, with pfn_to_mfn, we expect to get a MFN based on the name.
Although, if we look at the implementation on x86, it's returning a GFN.
Most of the callers are also using it this way.
The first 2 patches of this series is ARM related in order to remove
PV specific helpers which should not be used and fixing the implementation of
pfn_to_mfn.
The rest of the series is here rename most of the usage in the common code
of MFN to GFN. I also took the opportunity to replace most of the call to
pfn_to_gfn in the common code by page_to_gfn avoid construction such
as pfn_to_gfn(page_to_pfn(...).
Note the one xen-blkfront will be dropped by 64K series [2], I can include
here if necessary.
Major changes in v3:
- More typoes
- Rename page_to_gfn to xen_page_to_gfn to avoid confusing with the
KVM function gfn_to_page
Major changes in v2:
- Use bfn rather than dfn for the DMA address
- Re-introduced pfn_to_mfn for PV guests only
- Typoes
For all the changes see in each patch.
This series is based on xentip for-linus-4.3 branch. A branch with all the
patches can be found here:
git://xenbits.xen.org/people/julieng/linux-arm.git branch page-renaming-v3
It has been boot tested on ARM64 and ARM32 and only built for x86.
I would be happy if someone can give a try on x86 as I don't have a x86
box setup with Xen.
Sincerely yours,
[1] http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;hç58ed14f390342513405dd766e874934573e6cb
[2] https://lkml.org/lkml/2015/7/9/628
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "James E.J. Bottomley" <JBottomley@odin.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: linux-api@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-scsi@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: x86@kernel.org
Julien Grall (9):
arm/xen: Remove helpers which are PV specific
xen: Make clear that swiotlb and biomerge are dealing with DMA address
arm/xen: implement correctly pfn_to_mfn
xen: Use correctly the Xen memory terminologies
xen/tmem: Use xen_page_to_gfn rather than pfn_to_gfn
video/xen-fbfront: Further s/MFN/GFN clean-up
hvc/xen: Further s/MFN/GFN clean-up
xen/privcmd: Further s/MFN/GFN/ clean-up
xen/xenbus: Rename the variable xen_store_mfn to xen_store_gfn
arch/arm/include/asm/xen/page.h | 44 ++++++++++++++++-----------------
arch/arm/xen/enlighten.c | 18 +++++++-------
arch/arm/xen/mm.c | 4 +--
arch/x86/include/asm/xen/page.h | 35 +++++++++++++++++++++-----
arch/x86/xen/mmu.c | 32 ++++++++++++------------
arch/x86/xen/smp.c | 2 +-
drivers/block/xen-blkfront.c | 6 ++---
drivers/input/misc/xen-kbdfront.c | 4 +--
drivers/net/xen-netback/netback.c | 4 +--
drivers/net/xen-netfront.c | 12 +++++----
drivers/scsi/xen-scsifront.c | 10 ++++----
drivers/tty/hvc/hvc_xen.c | 18 ++++++--------
drivers/video/fbdev/xen-fbfront.c | 20 +++++++--------
drivers/xen/balloon.c | 2 +-
drivers/xen/biomerge.c | 6 ++---
drivers/xen/events/events_base.c | 2 +-
drivers/xen/events/events_fifo.c | 4 +--
drivers/xen/gntalloc.c | 3 ++-
drivers/xen/manage.c | 2 +-
drivers/xen/privcmd.c | 44 ++++++++++++++++-----------------
drivers/xen/swiotlb-xen.c | 16 ++++++------
drivers/xen/tmem.c | 21 ++++++----------
drivers/xen/xenbus/xenbus_client.c | 2 +-
drivers/xen/xenbus/xenbus_dev_backend.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 16 ++++++------
drivers/xen/xlate_mmu.c | 18 +++++++-------
include/uapi/xen/privcmd.h | 4 +++
include/xen/page.h | 4 +--
include/xen/xen-ops.h | 10 ++++----
29 files changed, 191 insertions(+), 174 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH v3 4/9] xen: Use correctly the Xen memory terminologies
From: Julien Grall @ 2015-08-07 16:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1438965282-19103-1-git-send-email-julien.grall@citrix.com>
Based on include/xen/mm.h [1], Linux is mistakenly using MFN when GFN
is meant, I suspect this is because the first support for Xen was for
PV. This resulted in some misimplementation of helpers on ARM and
confused developers about the expected behavior.
For instance, with pfn_to_mfn, we expect to get an MFN based on the name.
Although, if we look at the implementation on x86, it's returning a GFN.
For clarity and avoid new confusion, replace any reference to mfn with
gfn in any helpers used by PV drivers. The x86 code will still keep some
reference of pfn_to_mfn but exclusively for PV (a BUG_ON has been added
to ensure this). No changes as been made in the hypercall field, even
though they may be invalid, in order to keep the same as the defintion
in xen repo.
Note that page_to_mfn has been renamed to xen_page_to_gfn to avoid a
name to close to the KVM function gfn_to_page.
Take also the opportunity to simplify simple construction such
as pfn_to_mfn(page_to_pfn(page)) into xen_page_to_gfn. More complex clean up
will come in follow-up patches.
[1] http://xenbits.xen.org/gitweb/?p=xen.git;a=commitdiff;hç58ed14f390342513405dd766e874934573e6cb
Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Russell King <linux@arm.linux.org.uk>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: "James E.J. Bottomley" <JBottomley@odin.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Jiri Slaby <jslaby@suse.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-input@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-scsi@vger.kernel.org
Cc: linuxppc-dev@lists.ozlabs.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Note that I've re-introduced in v2 mfn_to_pfn & co only for x86 PV code.
The helpers contain a BUG_ON to ensure that it's never called for
auto-translated guests. I did as best as my can to determine whether
mfn or gfn helpers should be used. Although, I haven't tried to boot
it.
It may be possible to do further cleanup in the mmu.c where I found
some check to auto-translated. I'm not sure why given that the pvmmu
callback are only used for non-auto translated guest.
Changes in v3:
- Add Stefano's reviewed-by (except for the x86 bits)
- Add Wei (netback) and Dmitry's (input) acked-by
- Keep the VIRT <-> MACHINE macro in the same order as before
in arch/x86/include/asm/xen/page.h
- Rename page_to_gfn to xen_page_to_gfn to avoid confusion with
the KVM function gfn_to_page.
- Typoes in the commit title
Changes in v2:
- Give directly the URL to the commit rather than the commit ID
- xenstored_local_init: keep the cast to void *
- Typoes
- Keep pfn_to_mfn for x86 and PV-only. The *mfn* helpers are
used in arch/x86/xen for enlighten.c, mmu.c, p2m.c, setup.c,
smp.c and mm.c
---
arch/arm/include/asm/xen/page.h | 13 +++++++------
arch/x86/include/asm/xen/page.h | 31 +++++++++++++++++++++++++------
arch/x86/xen/smp.c | 2 +-
drivers/block/xen-blkfront.c | 6 +++---
drivers/input/misc/xen-kbdfront.c | 4 ++--
drivers/net/xen-netback/netback.c | 4 ++--
drivers/net/xen-netfront.c | 12 +++++++-----
drivers/scsi/xen-scsifront.c | 10 +++++-----
drivers/tty/hvc/hvc_xen.c | 5 +++--
drivers/video/fbdev/xen-fbfront.c | 4 ++--
drivers/xen/balloon.c | 2 +-
drivers/xen/events/events_base.c | 2 +-
drivers/xen/events/events_fifo.c | 4 ++--
drivers/xen/gntalloc.c | 3 ++-
drivers/xen/manage.c | 2 +-
drivers/xen/tmem.c | 4 ++--
drivers/xen/xenbus/xenbus_client.c | 2 +-
drivers/xen/xenbus/xenbus_dev_backend.c | 2 +-
drivers/xen/xenbus/xenbus_probe.c | 8 +++-----
include/xen/page.h | 4 ++--
20 files changed, 73 insertions(+), 51 deletions(-)
diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
index 911d62b..1279563 100644
--- a/arch/arm/include/asm/xen/page.h
+++ b/arch/arm/include/asm/xen/page.h
@@ -34,14 +34,15 @@ typedef struct xpaddr {
unsigned long __pfn_to_mfn(unsigned long pfn);
extern struct rb_root phys_to_mach;
-static inline unsigned long pfn_to_mfn(unsigned long pfn)
+/* Pseudo-physical <-> Guest conversion */
+static inline unsigned long pfn_to_gfn(unsigned long pfn)
{
return pfn;
}
-static inline unsigned long mfn_to_pfn(unsigned long mfn)
+static inline unsigned long gfn_to_pfn(unsigned long gfn)
{
- return mfn;
+ return gfn;
}
/* Pseudo-physical <-> BUS conversion */
@@ -65,9 +66,9 @@ static inline unsigned long bfn_to_pfn(unsigned long bfn)
#define bfn_to_local_pfn(bfn) bfn_to_pfn(bfn)
-/* VIRT <-> MACHINE conversion */
-#define virt_to_mfn(v) (pfn_to_mfn(virt_to_pfn(v)))
-#define mfn_to_virt(m) (__va(mfn_to_pfn(m) << PAGE_SHIFT))
+/* VIRT <-> GUEST conversion */
+#define virt_to_gfn(v) (pfn_to_gfn(virt_to_pfn(v)))
+#define gfn_to_virt(m) (__va(gfn_to_pfn(m) << PAGE_SHIFT))
/* Only used in PV code. But ARM guests are always HVM. */
static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
diff --git a/arch/x86/include/asm/xen/page.h b/arch/x86/include/asm/xen/page.h
index 724bffb..0b762f6 100644
--- a/arch/x86/include/asm/xen/page.h
+++ b/arch/x86/include/asm/xen/page.h
@@ -101,8 +101,7 @@ static inline unsigned long pfn_to_mfn(unsigned long pfn)
{
unsigned long mfn;
- if (xen_feature(XENFEAT_auto_translated_physmap))
- return pfn;
+ BUG_ON(xen_feature(XENFEAT_auto_translated_physmap));
mfn = __pfn_to_mfn(pfn);
@@ -147,8 +146,7 @@ static inline unsigned long mfn_to_pfn(unsigned long mfn)
{
unsigned long pfn;
- if (xen_feature(XENFEAT_auto_translated_physmap))
- return mfn;
+ BUG_ON(xen_feature(XENFEAT_auto_translated_physmap));
pfn = mfn_to_pfn_no_overrides(mfn);
if (__pfn_to_mfn(pfn) != mfn)
@@ -176,9 +174,26 @@ static inline xpaddr_t machine_to_phys(xmaddr_t machine)
return XPADDR(PFN_PHYS(mfn_to_pfn(PFN_DOWN(machine.maddr))) | offset);
}
+/* Pseudo-physical <-> Guest conversion */
+static inline unsigned long pfn_to_gfn(unsigned long pfn)
+{
+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ return pfn;
+ else
+ return pfn_to_mfn(pfn);
+}
+
+static inline unsigned long gfn_to_pfn(unsigned long gfn)
+{
+ if (xen_feature(XENFEAT_auto_translated_physmap))
+ return gfn;
+ else
+ return mfn_to_pfn(gfn);
+}
+
/* Pseudo-physical <-> Bus conversion */
-#define pfn_to_bfn(pfn) pfn_to_mfn(pfn)
-#define bfn_to_pfn(bfn) mfn_to_pfn(bfn)
+#define pfn_to_bfn(pfn) pfn_to_gfn(pfn)
+#define bfn_to_pfn(bfn) gfn_to_pfn(bfn)
/*
* We detect special mappings in one of two ways:
@@ -219,6 +234,10 @@ static inline unsigned long bfn_to_local_pfn(unsigned long mfn)
#define virt_to_mfn(v) (pfn_to_mfn(virt_to_pfn(v)))
#define mfn_to_virt(m) (__va(mfn_to_pfn(m) << PAGE_SHIFT))
+/* VIRT <-> GUEST conversion */
+#define virt_to_gfn(v) (pfn_to_gfn(virt_to_pfn(v)))
+#define gfn_to_virt(g) (__va(gfn_to_pfn(g) << PAGE_SHIFT))
+
static inline unsigned long pte_mfn(pte_t pte)
{
return (pte.pte & PTE_PFN_MASK) >> PAGE_SHIFT;
diff --git a/arch/x86/xen/smp.c b/arch/x86/xen/smp.c
index 8648438..1e0931b 100644
--- a/arch/x86/xen/smp.c
+++ b/arch/x86/xen/smp.c
@@ -429,7 +429,7 @@ cpu_initialize_context(unsigned int cpu, struct task_struct *idle)
}
#endif
ctxt->user_regs.esp = idle->thread.sp0 - sizeof(struct pt_regs);
- ctxt->ctrlreg[3] = xen_pfn_to_cr3(virt_to_mfn(swapper_pg_dir));
+ ctxt->ctrlreg[3] = xen_pfn_to_cr3(virt_to_gfn(swapper_pg_dir));
if (HYPERVISOR_vcpu_op(VCPUOP_initialise, cpu, ctxt))
BUG();
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 5b45ee5..ebb3624 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -249,7 +249,7 @@ static struct grant *get_grant(grant_ref_t *gref_head,
struct blkfront_info *info)
{
struct grant *gnt_list_entry;
- unsigned long buffer_mfn;
+ unsigned long buffer_gfn;
BUG_ON(list_empty(&info->grants));
gnt_list_entry = list_first_entry(&info->grants, struct grant,
@@ -268,10 +268,10 @@ static struct grant *get_grant(grant_ref_t *gref_head,
BUG_ON(!pfn);
gnt_list_entry->pfn = pfn;
}
- buffer_mfn = pfn_to_mfn(gnt_list_entry->pfn);
+ buffer_gfn = pfn_to_gfn(gnt_list_entry->pfn);
gnttab_grant_foreign_access_ref(gnt_list_entry->gref,
info->xbdev->otherend_id,
- buffer_mfn, 0);
+ buffer_gfn, 0);
return gnt_list_entry;
}
diff --git a/drivers/input/misc/xen-kbdfront.c b/drivers/input/misc/xen-kbdfront.c
index 95599e4..23d0549 100644
--- a/drivers/input/misc/xen-kbdfront.c
+++ b/drivers/input/misc/xen-kbdfront.c
@@ -232,7 +232,7 @@ static int xenkbd_connect_backend(struct xenbus_device *dev,
struct xenbus_transaction xbt;
ret = gnttab_grant_foreign_access(dev->otherend_id,
- virt_to_mfn(info->page), 0);
+ virt_to_gfn(info->page), 0);
if (ret < 0)
return ret;
info->gref = ret;
@@ -255,7 +255,7 @@ static int xenkbd_connect_backend(struct xenbus_device *dev,
goto error_irqh;
}
ret = xenbus_printf(xbt, dev->nodename, "page-ref", "%lu",
- virt_to_mfn(info->page));
+ virt_to_gfn(info->page));
if (ret)
goto error_xenbus;
ret = xenbus_printf(xbt, dev->nodename, "page-gref", "%u", info->gref);
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 880d0d6..8293374 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -314,7 +314,7 @@ static void xenvif_gop_frag_copy(struct xenvif_queue *queue, struct sk_buff *skb
} else {
copy_gop->source.domid = DOMID_SELF;
copy_gop->source.u.gmfn - virt_to_mfn(page_address(page));
+ virt_to_gfn(page_address(page));
}
copy_gop->source.offset = offset;
@@ -1284,7 +1284,7 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
queue->tx_copy_ops[*copy_ops].source.offset = txreq.offset;
queue->tx_copy_ops[*copy_ops].dest.u.gmfn - virt_to_mfn(skb->data);
+ virt_to_gfn(skb->data);
queue->tx_copy_ops[*copy_ops].dest.domid = DOMID_SELF;
queue->tx_copy_ops[*copy_ops].dest.offset offset_in_page(skb->data);
diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index f948c46..47f791e 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -291,7 +291,7 @@ static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
struct sk_buff *skb;
unsigned short id;
grant_ref_t ref;
- unsigned long pfn;
+ unsigned long gfn;
struct xen_netif_rx_request *req;
skb = xennet_alloc_one_rx_buffer(queue);
@@ -307,12 +307,12 @@ static void xennet_alloc_rx_buffers(struct netfront_queue *queue)
BUG_ON((signed short)ref < 0);
queue->grant_rx_ref[id] = ref;
- pfn = page_to_pfn(skb_frag_page(&skb_shinfo(skb)->frags[0]));
+ gfn = xen_page_to_gfn(skb_frag_page(&skb_shinfo(skb)->frags[0]));
req = RING_GET_REQUEST(&queue->rx, req_prod);
gnttab_grant_foreign_access_ref(ref,
queue->info->xbdev->otherend_id,
- pfn_to_mfn(pfn),
+ gfn,
0);
req->id = id;
@@ -430,8 +430,10 @@ static struct xen_netif_tx_request *xennet_make_one_txreq(
ref = gnttab_claim_grant_reference(&queue->gref_tx_head);
BUG_ON((signed short)ref < 0);
- gnttab_grant_foreign_access_ref(ref, queue->info->xbdev->otherend_id,
- page_to_mfn(page), GNTMAP_readonly);
+ gnttab_grant_foreign_access_ref(ref,
+ queue->info->xbdev->otherend_id,
+ xen_page_to_gfn(page),
+ GNTMAP_readonly);
queue->tx_skbs[id].skb = skb;
queue->grant_tx_page[id] = page;
diff --git a/drivers/scsi/xen-scsifront.c b/drivers/scsi/xen-scsifront.c
index fad22ca..9dc8687 100644
--- a/drivers/scsi/xen-scsifront.c
+++ b/drivers/scsi/xen-scsifront.c
@@ -377,7 +377,6 @@ static int map_data_for_request(struct vscsifrnt_info *info,
unsigned int data_len = scsi_bufflen(sc);
unsigned int data_grants = 0, seg_grants = 0;
struct scatterlist *sg;
- unsigned long mfn;
struct scsiif_request_segment *seg;
ring_req->nr_segments = 0;
@@ -420,9 +419,9 @@ static int map_data_for_request(struct vscsifrnt_info *info,
ref = gnttab_claim_grant_reference(&gref_head);
BUG_ON(ref = -ENOSPC);
- mfn = pfn_to_mfn(page_to_pfn(page));
gnttab_grant_foreign_access_ref(ref,
- info->dev->otherend_id, mfn, 1);
+ info->dev->otherend_id,
+ xen_page_to_gfn(page), 1);
shadow->gref[ref_cnt] = ref;
ring_req->seg[ref_cnt].gref = ref;
ring_req->seg[ref_cnt].offset = (uint16_t)off;
@@ -454,9 +453,10 @@ static int map_data_for_request(struct vscsifrnt_info *info,
ref = gnttab_claim_grant_reference(&gref_head);
BUG_ON(ref = -ENOSPC);
- mfn = pfn_to_mfn(page_to_pfn(page));
gnttab_grant_foreign_access_ref(ref,
- info->dev->otherend_id, mfn, grant_ro);
+ info->dev->otherend_id,
+ xen_page_to_gfn(page),
+ grant_ro);
shadow->gref[ref_cnt] = ref;
seg->gref = ref;
diff --git a/drivers/tty/hvc/hvc_xen.c b/drivers/tty/hvc/hvc_xen.c
index a9d837f..efe5124 100644
--- a/drivers/tty/hvc/hvc_xen.c
+++ b/drivers/tty/hvc/hvc_xen.c
@@ -265,7 +265,8 @@ static int xen_pv_console_init(void)
return 0;
}
info->evtchn = xen_start_info->console.domU.evtchn;
- info->intf = mfn_to_virt(xen_start_info->console.domU.mfn);
+ /* GFN = MFN for PV guest */
+ info->intf = gfn_to_virt(xen_start_info->console.domU.mfn);
info->vtermno = HVC_COOKIE;
spin_lock(&xencons_lock);
@@ -390,7 +391,7 @@ static int xencons_connect_backend(struct xenbus_device *dev,
if (IS_ERR(info->hvc))
return PTR_ERR(info->hvc);
if (xen_pv_domain())
- mfn = virt_to_mfn(info->intf);
+ mfn = virt_to_gfn(info->intf);
else
mfn = __pa(info->intf) >> PAGE_SHIFT;
ret = gnttab_alloc_grant_references(1, &gref_head);
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 09dc447..25e3cce 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -539,7 +539,7 @@ static int xenfb_remove(struct xenbus_device *dev)
static unsigned long vmalloc_to_mfn(void *address)
{
- return pfn_to_mfn(vmalloc_to_pfn(address));
+ return pfn_to_gfn(vmalloc_to_pfn(address));
}
static void xenfb_init_shared_page(struct xenfb_info *info,
@@ -586,7 +586,7 @@ static int xenfb_connect_backend(struct xenbus_device *dev,
goto unbind_irq;
}
ret = xenbus_printf(xbt, dev->nodename, "page-ref", "%lu",
- virt_to_mfn(info->page));
+ virt_to_gfn(info->page));
if (ret)
goto error_xenbus;
ret = xenbus_printf(xbt, dev->nodename, "event-channel", "%u",
diff --git a/drivers/xen/balloon.c b/drivers/xen/balloon.c
index fd93369..9734649 100644
--- a/drivers/xen/balloon.c
+++ b/drivers/xen/balloon.c
@@ -441,7 +441,7 @@ static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
/* Update direct mapping, invalidate P2M, and add to balloon. */
for (i = 0; i < nr_pages; i++) {
pfn = frame_list[i];
- frame_list[i] = pfn_to_mfn(pfn);
+ frame_list[i] = pfn_to_gfn(pfn);
page = pfn_to_page(pfn);
#ifdef CONFIG_XEN_HAVE_PVMMU
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index ed620e5..c49bb7a 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -1688,7 +1688,7 @@ void __init xen_init_IRQ(void)
struct physdev_pirq_eoi_gmfn eoi_gmfn;
pirq_eoi_map = (void *)__get_free_page(GFP_KERNEL|__GFP_ZERO);
- eoi_gmfn.gmfn = virt_to_mfn(pirq_eoi_map);
+ eoi_gmfn.gmfn = virt_to_gfn(pirq_eoi_map);
rc = HYPERVISOR_physdev_op(PHYSDEVOP_pirq_eoi_gmfn_v2, &eoi_gmfn);
/* TODO: No PVH support for PIRQ EOI */
if (rc != 0) {
diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index ed673e1..1d4baf5 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -111,7 +111,7 @@ static int init_control_block(int cpu,
for (i = 0; i < EVTCHN_FIFO_MAX_QUEUES; i++)
q->head[i] = 0;
- init_control.control_gfn = virt_to_mfn(control_block);
+ init_control.control_gfn = virt_to_gfn(control_block);
init_control.offset = 0;
init_control.vcpu = cpu;
@@ -167,7 +167,7 @@ static int evtchn_fifo_setup(struct irq_info *info)
/* Mask all events in this page before adding it. */
init_array_page(array_page);
- expand_array.array_gfn = virt_to_mfn(array_page);
+ expand_array.array_gfn = virt_to_gfn(array_page);
ret = HYPERVISOR_event_channel_op(EVTCHNOP_expand_array, &expand_array);
if (ret < 0)
diff --git a/drivers/xen/gntalloc.c b/drivers/xen/gntalloc.c
index e53fe19..14370df 100644
--- a/drivers/xen/gntalloc.c
+++ b/drivers/xen/gntalloc.c
@@ -142,7 +142,8 @@ static int add_grefs(struct ioctl_gntalloc_alloc_gref *op,
/* Grant foreign access to the page. */
rc = gnttab_grant_foreign_access(op->domid,
- pfn_to_mfn(page_to_pfn(gref->page)), readonly);
+ xen_page_to_gfn(gref->page),
+ readonly);
if (rc < 0)
goto undo;
gref_ids[i] = gref->gref_id = rc;
diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c
index d10effe..e12bd36 100644
--- a/drivers/xen/manage.c
+++ b/drivers/xen/manage.c
@@ -80,7 +80,7 @@ static int xen_suspend(void *data)
* is resuming in a new domain.
*/
si->cancelled = HYPERVISOR_suspend(xen_pv_domain()
- ? virt_to_mfn(xen_start_info)
+ ? virt_to_gfn(xen_start_info)
: 0);
xen_arch_post_suspend(si->cancelled);
diff --git a/drivers/xen/tmem.c b/drivers/xen/tmem.c
index 239738f..28c97ff 100644
--- a/drivers/xen/tmem.c
+++ b/drivers/xen/tmem.c
@@ -131,7 +131,7 @@ static int xen_tmem_new_pool(struct tmem_pool_uuid uuid,
static int xen_tmem_put_page(u32 pool_id, struct tmem_oid oid,
u32 index, unsigned long pfn)
{
- unsigned long gmfn = xen_pv_domain() ? pfn_to_mfn(pfn) : pfn;
+ unsigned long gmfn = pfn_to_gfn(pfn);
return xen_tmem_op(TMEM_PUT_PAGE, pool_id, oid, index,
gmfn, 0, 0, 0);
@@ -140,7 +140,7 @@ static int xen_tmem_put_page(u32 pool_id, struct tmem_oid oid,
static int xen_tmem_get_page(u32 pool_id, struct tmem_oid oid,
u32 index, unsigned long pfn)
{
- unsigned long gmfn = xen_pv_domain() ? pfn_to_mfn(pfn) : pfn;
+ unsigned long gmfn = pfn_to_gfn(pfn);
return xen_tmem_op(TMEM_GET_PAGE, pool_id, oid, index,
gmfn, 0, 0, 0);
diff --git a/drivers/xen/xenbus/xenbus_client.c b/drivers/xen/xenbus/xenbus_client.c
index 9ad3272..daa267a 100644
--- a/drivers/xen/xenbus/xenbus_client.c
+++ b/drivers/xen/xenbus/xenbus_client.c
@@ -380,7 +380,7 @@ int xenbus_grant_ring(struct xenbus_device *dev, void *vaddr,
for (i = 0; i < nr_pages; i++) {
err = gnttab_grant_foreign_access(dev->otherend_id,
- virt_to_mfn(vaddr), 0);
+ virt_to_gfn(vaddr), 0);
if (err < 0) {
xenbus_dev_fatal(dev, err,
"granting access to ring page");
diff --git a/drivers/xen/xenbus/xenbus_dev_backend.c b/drivers/xen/xenbus/xenbus_dev_backend.c
index b17707e..ee6d9ef 100644
--- a/drivers/xen/xenbus/xenbus_dev_backend.c
+++ b/drivers/xen/xenbus/xenbus_dev_backend.c
@@ -49,7 +49,7 @@ static long xenbus_alloc(domid_t domid)
goto out_err;
gnttab_grant_foreign_access_ref(GNTTAB_RESERVED_XENSTORE, domid,
- virt_to_mfn(xen_store_interface), 0 /* writable */);
+ virt_to_gfn(xen_store_interface), 0 /* writable */);
arg.dom = DOMID_SELF;
arg.remote_dom = domid;
diff --git a/drivers/xen/xenbus/xenbus_probe.c b/drivers/xen/xenbus/xenbus_probe.c
index 4308fb3..b3870f4 100644
--- a/drivers/xen/xenbus/xenbus_probe.c
+++ b/drivers/xen/xenbus/xenbus_probe.c
@@ -711,9 +711,7 @@ static int __init xenstored_local_init(void)
if (!page)
goto out_err;
- xen_store_mfn = xen_start_info->store_mfn - pfn_to_mfn(virt_to_phys((void *)page) >>
- PAGE_SHIFT);
+ xen_store_mfn = xen_start_info->store_mfn = virt_to_gfn((void *)page);
/* Next allocate a local port which xenstored can bind to */
alloc_unbound.dom = DOMID_SELF;
@@ -787,12 +785,12 @@ static int __init xenbus_init(void)
err = xenstored_local_init();
if (err)
goto out_error;
- xen_store_interface = mfn_to_virt(xen_store_mfn);
+ xen_store_interface = gfn_to_virt(xen_store_mfn);
break;
case XS_PV:
xen_store_evtchn = xen_start_info->store_evtchn;
xen_store_mfn = xen_start_info->store_mfn;
- xen_store_interface = mfn_to_virt(xen_store_mfn);
+ xen_store_interface = gfn_to_virt(xen_store_mfn);
break;
case XS_HVM:
err = hvm_get_parameter(HVM_PARAM_STORE_EVTCHN, &v);
diff --git a/include/xen/page.h b/include/xen/page.h
index c5ed20b..f202992 100644
--- a/include/xen/page.h
+++ b/include/xen/page.h
@@ -3,9 +3,9 @@
#include <asm/xen/page.h>
-static inline unsigned long page_to_mfn(struct page *page)
+static inline unsigned long xen_page_to_gfn(struct page *page)
{
- return pfn_to_mfn(page_to_pfn(page));
+ return pfn_to_gfn(page_to_pfn(page));
}
struct xen_memory_region {
--
2.1.4
^ permalink raw reply related
* [PATCH v3 6/9] video/xen-fbfront: Further s/MFN/GFN clean-up
From: Julien Grall @ 2015-08-07 16:34 UTC (permalink / raw)
To: xen-devel
Cc: ian.campbell, stefano.stabellini, linux-kernel, Julien Grall,
David Vrabel, Jean-Christophe Plagniol-Villard, Tomi Valkeinen,
linux-fbdev
In-Reply-To: <1438965282-19103-1-git-send-email-julien.grall@citrix.com>
The PV driver xen-fbfront is only dealing with GFN and not MFN. Rename
all the occurence of MFN to GFN.
Also take the opportunity to replace to usage of pfn_to_gfn by
xen_page_to_gfn.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
---
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: linux-fbdev@vger.kernel.org
Changes in v3:
- Typoes in the commit message
- Renamed page_to_gfn to xen_page_to_gfn
Changes in v2:
- Add David's reviewed-by
---
drivers/video/fbdev/xen-fbfront.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/video/fbdev/xen-fbfront.c b/drivers/video/fbdev/xen-fbfront.c
index 25e3cce..0567d51 100644
--- a/drivers/video/fbdev/xen-fbfront.c
+++ b/drivers/video/fbdev/xen-fbfront.c
@@ -46,7 +46,7 @@ struct xenfb_info {
int nr_pages;
int irq;
struct xenfb_page *page;
- unsigned long *mfns;
+ unsigned long *gfns;
int update_wanted; /* XENFB_TYPE_UPDATE wanted */
int feature_resize; /* XENFB_TYPE_RESIZE ok */
struct xenfb_resize resize; /* protected by resize_lock */
@@ -402,8 +402,8 @@ static int xenfb_probe(struct xenbus_device *dev,
info->nr_pages = (fb_size + PAGE_SIZE - 1) >> PAGE_SHIFT;
- info->mfns = vmalloc(sizeof(unsigned long) * info->nr_pages);
- if (!info->mfns)
+ info->gfns = vmalloc(sizeof(unsigned long) * info->nr_pages);
+ if (!info->gfns)
goto error_nomem;
/* set up shared page */
@@ -530,29 +530,29 @@ static int xenfb_remove(struct xenbus_device *dev)
framebuffer_release(info->fb_info);
}
free_page((unsigned long)info->page);
- vfree(info->mfns);
+ vfree(info->gfns);
vfree(info->fb);
kfree(info);
return 0;
}
-static unsigned long vmalloc_to_mfn(void *address)
+static unsigned long vmalloc_to_gfn(void *address)
{
- return pfn_to_gfn(vmalloc_to_pfn(address));
+ return xen_page_to_gfn(vmalloc_to_page(address));
}
static void xenfb_init_shared_page(struct xenfb_info *info,
struct fb_info *fb_info)
{
int i;
- int epd = PAGE_SIZE / sizeof(info->mfns[0]);
+ int epd = PAGE_SIZE / sizeof(info->gfns[0]);
for (i = 0; i < info->nr_pages; i++)
- info->mfns[i] = vmalloc_to_mfn(info->fb + i * PAGE_SIZE);
+ info->gfns[i] = vmalloc_to_gfn(info->fb + i * PAGE_SIZE);
for (i = 0; i * epd < info->nr_pages; i++)
- info->page->pd[i] = vmalloc_to_mfn(&info->mfns[i * epd]);
+ info->page->pd[i] = vmalloc_to_gfn(&info->gfns[i * epd]);
info->page->width = fb_info->var.xres;
info->page->height = fb_info->var.yres;
--
2.1.4
^ permalink raw reply related
* [BUG, RFC] Different bitfields in documentation and driver
From: Petr Cvek @ 2015-08-07 19:14 UTC (permalink / raw)
To: linux-arm-kernel
Hello,
It seems that PXA2xx framebuffer has switched bitfields in nonstd definition (when using overlay).
This:
http://lxr.free-electrons.com/source/drivers/video/fbdev/pxafb.h#L80
versus this:
http://lxr.free-electrons.com/source/Documentation/fb/pxafb.txt#L128
Should be this changed in documentation or in driver? I was able to find only one use (in form of little outdated mplayer patch) and it fills both bitfields with zero (so potential change in driver will not break it).
P.S. Lack of overlay2's use is probably caused by errata "E24: LCD: Enabling Overlay 2 for YUV420 hangs LCD controller.", which is not workarounded in this driver. It should be mentioned in documentation and/or code, because the only way to start the LCD again is a full PXA SoC reset.
Petr Cvek
^ permalink raw reply
* Re: [Xen-devel] RIP MTRR - status update for upcoming v4.2
From: Luis R. Rodriguez @ 2015-08-07 20:25 UTC (permalink / raw)
To: Toshi Kani
Cc: Jan Beulich, Andy Lutomirski, Bjorn Helgaas, Jej B, X86 ML,
Andrew Morton, Ville Syrjälä, Julia Lawall, xen-devel,
Dave Airlie, Ville Syrjälä, Juergen Gross,
Borislav Petkov, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org, linux-media,
linux-pci@vger.kernel.org
In-Reply-To: <1438901893.3109.72.camel@hp.com>
On Thu, Aug 6, 2015 at 3:58 PM, Toshi Kani <toshi.kani@hp.com> wrote:
> On Thu, 2015-08-06 at 12:53 -0700, Luis R. Rodriguez wrote:
>> On Fri, Jun 12, 2015 at 9:58 AM, Toshi Kani <toshi.kani@hp.com> wrote:
>> > On Fri, 2015-06-12 at 08:59 +0100, Jan Beulich wrote:
>> > > > > > On 12.06.15 at 01:23, <toshi.kani@hp.com> wrote:
>> > > > There are two usages on MTRRs:
>> > > > 1) MTRR entries set by firmware
>> > > > 2) MTRR entries set by OS drivers
>> > > >
>> > > > We can obsolete 2), but we have no control over 1). As UEFI
>> > > > firmwares
>> > > > also set this up, this usage will continue to stay. So, we should
>> > > > not
>> > > > get rid of the MTRR code that looks up the MTRR entries, while we
>> > > > have
>> > > > no need to modify them.
>> > > >
>> > > > Such MTRR entries provide safe guard to /dev/mem, which allows
>> > > > privileged user to access a range that may require UC mapping while
>> > > > the /dev/mem driver blindly maps it with WB. MTRRs converts WB to
>> > > > UC in
>> > > > such a case.
>> > >
>> > > But it wouldn't be impossible to simply read the MTRRs upon boot,
>> > > store the information, disable MTRRs, and correctly use PAT to
>> > > achieve the same effect (i.e. the "blindly maps" part of course
>> > > would need fixing).
>> >
>> > It could be done, but I do not see much benefit of doing it. One of the
>> > reasons platform vendors set MTRRs is so that a system won't hit a
>> > machine check when an OS bug leads an access with a wrong cache type.
>> >
>> > A machine check is hard to analyze and can be seen as a hardware issue
>> > by customers. Emulating MTRRs with PAT won't protect from such a bug.
>>
>> That's seems like a fair and valid concern. This could only happen if
>> the OS would have code that would use MTRR, in the case of Linux we'll
>> soon be able to vet that this cannot happen.
>
> No, there is no OS support necessary to use MTRR. After firmware sets it
> up, CPUs continue to use it without any OS support. I think the Linux
> change you are referring is to obsolete legacy interfaces that modify the
> MTRR setup. I agree that Linux should not modify MTRR.
Its a bit more than that though. Since you agree that the OS can live
without MTRR code I was hoping to then see if we can fold out PAT
Linux code from under the MTRR dependency on Linux and make PAT a
first class citizen, maybe at least for x86-64. Right now you can only
get PAT support on Linux if you have MTRR code, but I'd like to see if
instead we can rip MTRR code out completely under its own Kconfig and
let it start rotting away.
Code-wise the only issue I saw was that PAT code also relies on
mtrr_type_lookup(), see pat_x_mtrr_type(), but other than this I found
no other obvious issues.
Platform firmware and SMIs seems to be the only other possible issue.
More on this below.
>> For those type of OSes...
>> could it be possible to negotiate or hint to the platform through an
>> attribute somehow that the OS has such capability to not use MTRR?
>
> The OS can disable MTRR. However, this can also cause a problem in
> firmware, which may rely on MTRR.
Can you describe what type of issues we could expect ? I tend to care
more about this for 64-bit systems so if 32-bit platforms would be
more of the ones which could cause an issue would restricting
disabling MTRR only for 64-bit help?
>> Then, only if this bit is set, the platform could then avoid such MTRR
>> settings, and if we have issues you can throw rocks at us.
>
>> And if that's not possible how about a new platform setting that would
>> need to be set at the platform level to enable disabling this junk?
>> Then only folks who know what they are doing would enable it, and if
>> the customer set it, the issue would not be on the platform.
>
>> Could this also be used to prevent SMIs with MTRRs?
>
> ACPI _OSI could be used for firmware to implement some OS-specific features,
> but it may be too late for firmware to make major changes and is generally
> useless unless OS requirements are described in a spec backed by logo
> certification.
I see.. So there are no guarantees that platform firmware would not
expect OS MTRR support.
> SMIs are also used for platform management, such as fan
> speed control.
And its conceivable that some devices, or the platform itself, may
trigger SMIs to have the platform firmware poke with MTRRs?
> Is there any issue for Linux to use MTRR set by firmware?
Even though we don't have the Kconfig option right now to disable MTRR
cod explicitly I'll note that there are a few other cases that could
flip Linux to note use MTRR:
a) Some BIOSes could let MTRR get disabled
b) As of Xen 4.4, the hypervisor disables X86_FEATURE_MTRR which
disables MTRR on Linux
If these environments can exist it'd be good to understand possible
issues that could creep up as a result of the OS not having MTRR
enabled. If this is a reasonable thing for x86-64 I was hoping we
could just let users opt-in to a similar build configuration through
the OS by letting PAT not depend on MTRR.
Luis
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox