[PATCH] xen/video: Set EFI framebuffer to WC by default

[PATCH] xen/video: Set EFI framebuffer to WC by default

[Ross Lagerwall]

Set the EFI framebuffer to write-combining by default. This makes
booting somewhat faster, but more importantly avoids tripping the
watchdog. In particular, before on my test machine, each frame redraw
would take around 80ms, which can trip the 5s watchdog when constructing
dom0, since it outputs something like 60 lines without processing
pending softirqs.

Both Linux and FreeBSD map the EFI framebuffer as write-combining by
default, so I assume (hope) that this is a safe change to make.

Also add documentation for the vesa-mtrr parameter.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 docs/misc/xen-command-line.markdown | 14 ++++++++++++++
 xen/drivers/video/vesa.c            | 12 +++++++++++-
 2 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index aa684c0..e5e3bd6 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -1373,6 +1373,20 @@ cache-warming. 1ms (1000) has been measured as a good value.
 ### vesa-mtrr
 > `= <integer>`
 
+> Default: `3` when using the EFI framebuffer, `0` otherwise.
+
+Specify the access mode to the VGA framebuffer.
+
+`0` does not change the access mode.
+
+`1` uses uncached mappings.
+
+`2` uses write-back mappings.
+
+`3` uses write-combining mappings.
+
+`4` uses write-through mappings.
+
 ### vesa-ram
 > `= <integer>`
 
diff --git a/xen/drivers/video/vesa.c b/xen/drivers/video/vesa.c
index 575db62..abab068 100644
--- a/xen/drivers/video/vesa.c
+++ b/xen/drivers/video/vesa.c
@@ -142,7 +142,13 @@ void __init vesa_init(void)
 #include <asm/mtrr.h>
 
 static unsigned int vesa_mtrr;
-integer_param("vesa-mtrr", vesa_mtrr);
+static __initdata bool_t vesa_mtrr_set;
+static void __init parse_vesa_mtrr(const char * s)
+{
+    vesa_mtrr = simple_strtoull(s, NULL, 0);
+    vesa_mtrr_set = 1;
+}
+custom_param("vesa-mtrr", parse_vesa_mtrr);
 
 void __init vesa_mtrr_init(void)
 {
@@ -152,6 +158,10 @@ void __init vesa_mtrr_init(void)
     unsigned int size_total;
     int rc, type;
 
+    /* Default to write-combining for the EFI framebuffer. */
+    if ( !vesa_mtrr_set && vga_console_info.video_type == XEN_VGATYPE_EFI_LFB )
+        vesa_mtrr = 3;
+
     if ( !lfb || (vesa_mtrr == 0) || (vesa_mtrr >= ARRAY_SIZE(mtrr_types)) )
         return;
 
-- 
2.1.0

Re: [PATCH] xen/video: Set EFI framebuffer to WC by default

[Jan Beulich]

>>> On 11.06.15 at 14:09, <ross.lagerwall@citrix.com> wrote:
> Set the EFI framebuffer to write-combining by default. This makes
> booting somewhat faster, but more importantly avoids tripping the
> watchdog. In particular, before on my test machine, each frame redraw
> would take around 80ms, which can trip the 5s watchdog when constructing
> dom0, since it outputs something like 60 lines without processing
> pending softirqs.

That would need fixing then. What are those 60 lines?

> Both Linux and FreeBSD map the EFI framebuffer as write-combining by
> default, so I assume (hope) that this is a safe change to make.

No, an unaware Dom0 OS may not work correctly when the frame
buffer is WC. It also might come as a surprise to the Dom0 OS that
there is a WC range in one of the MTRRs where none would be
expected. Plus - why for EFI only?

> --- a/xen/drivers/video/vesa.c
> +++ b/xen/drivers/video/vesa.c
> @@ -142,7 +142,13 @@ void __init vesa_init(void)
>  #include <asm/mtrr.h>
>  
>  static unsigned int vesa_mtrr;
> -integer_param("vesa-mtrr", vesa_mtrr);
> +static __initdata bool_t vesa_mtrr_set;
> +static void __init parse_vesa_mtrr(const char * s)
> +{
> +    vesa_mtrr = simple_strtoull(s, NULL, 0);
> +    vesa_mtrr_set = 1;
> +}
> +custom_param("vesa-mtrr", parse_vesa_mtrr);

I don't think any of this would be really needed if you simply
initialized vesa_mtrr to e.g. ~0.

Jan

Re: [PATCH] xen/video: Set EFI framebuffer to WC by default

[Ross Lagerwall]

On 06/11/2015 01:35 PM, Jan Beulich wrote:
>>>> On 11.06.15 at 14:09, <ross.lagerwall@citrix.com> wrote:
>> Set the EFI framebuffer to write-combining by default. This makes
>> booting somewhat faster, but more importantly avoids tripping the
>> watchdog. In particular, before on my test machine, each frame redraw
>> would take around 80ms, which can trip the 5s watchdog when constructing
>> dom0, since it outputs something like 60 lines without processing
>> pending softirqs.
>
> That would need fixing then. What are those 60 lines?

I think it's the lines from "Testing NMI watchdog on all CPUs: ok" to 
"Scrubbing Free RAM".

>
>> Both Linux and FreeBSD map the EFI framebuffer as write-combining by
>> default, so I assume (hope) that this is a safe change to make.
>
> No, an unaware Dom0 OS may not work correctly when the frame
> buffer is WC. It also might come as a surprise to the Dom0 OS that
> there is a WC range in one of the MTRRs where none would be
> expected. Plus - why for EFI only?

Because that's what other OSes did, but if it's not going to work... I 
can send a patch which we're currently using which sticks in a few 
process_pending_softirqs() in strategic places.

-- 
Ross Lagerwall

Re: [PATCH] xen/video: Set EFI framebuffer to WC by default

[Jan Beulich]

>>> On 11.06.15 at 15:03, <ross.lagerwall@citrix.com> wrote:
> On 06/11/2015 01:35 PM, Jan Beulich wrote:
>>>>> On 11.06.15 at 14:09, <ross.lagerwall@citrix.com> wrote:
>>> Both Linux and FreeBSD map the EFI framebuffer as write-combining by
>>> default, so I assume (hope) that this is a safe change to make.
>>
>> No, an unaware Dom0 OS may not work correctly when the frame
>> buffer is WC. It also might come as a surprise to the Dom0 OS that
>> there is a WC range in one of the MTRRs where none would be
>> expected. Plus - why for EFI only?
> 
> Because that's what other OSes did, but if it's not going to work... I 
> can send a patch which we're currently using which sticks in a few 
> process_pending_softirqs() in strategic places.

Yes please (quite a few of them seem to be debug level ones only
anyway, i.e. not usually a problem on production systems).

Jan