Re: [PATCH 2/2] drm/tiny: Add ofdrm for Open Firmware framebuffers

2022-05-18 Thread Mark Cave-Ayland

On 18/05/2022 19:30, Thomas Zimmermann wrote:


Open Firmware provides basic display output via the 'display' node.
DT platform code already provides a device that represents the node's
framebuffer. Add a DRM driver for the device. The display mode and
color format is pre-initialized by the system's firmware. Runtime
modesetting via DRM is not possible. The display is useful during
early boot stages or as error fallback.

Similar functionality is already provided by fbdev's offb driver,
which is insufficient for modern userspace. The old driver includes
support for BootX device tree, which can be found on old 32-bit
PowerPC Macintosh systems. If these are still in use, the
functionality can be added to ofdrm or implemented in a new
driver. As with simepldrm, the fbdev driver cannot be selected is
ofdrm is already enabled.

Two noteable points about the driver:

  * Reading the framebuffer aperture from the device tree is not
reliable on all systems. Ofdrm takes the heuristics and a comment
from offb to pick the correct range.

  * No resource management may be tied to the underlying PCI device.
Otherwise the handover to the native driver will fail with a resource
conflict. PCI management is therefore done as part of the platform
device's cleanup.

The driver has been tested on qemu's ppc64le emulation. The device
hand-over has been tested with bochs.


Thanks for working on this! Have you tried it on qemu-system-sparc and 
qemu-system-sparc64 at all? At least under QEMU I'd expect it to work for these 
platforms too, unless there is a particular dependency on PCI. A couple of comments 
inline below:



Signed-off-by: Thomas Zimmermann 
---
  MAINTAINERS   |   1 +
  drivers/gpu/drm/tiny/Kconfig  |  12 +
  drivers/gpu/drm/tiny/Makefile |   1 +
  drivers/gpu/drm/tiny/ofdrm.c  | 748 ++
  drivers/video/fbdev/Kconfig   |   1 +
  5 files changed, 763 insertions(+)
  create mode 100644 drivers/gpu/drm/tiny/ofdrm.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 43d833273ae9..090cbe1aa5e3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6395,6 +6395,7 @@ L:dri-de...@lists.freedesktop.org
  S:Maintained
  T:git git://anongit.freedesktop.org/drm/drm-misc
  F:drivers/gpu/drm/drm_aperture.c
+F: drivers/gpu/drm/tiny/ofdrm.c
  F:drivers/gpu/drm/tiny/simpledrm.c
  F:include/drm/drm_aperture.h
  
diff --git a/drivers/gpu/drm/tiny/Kconfig b/drivers/gpu/drm/tiny/Kconfig

index 627d637a1e7e..0bc54af42e7f 100644
--- a/drivers/gpu/drm/tiny/Kconfig
+++ b/drivers/gpu/drm/tiny/Kconfig
@@ -51,6 +51,18 @@ config DRM_GM12U320
 This is a KMS driver for projectors which use the GM12U320 chipset
 for video transfer over USB2/3, such as the Acer C120 mini projector.
  
+config DRM_OFDRM

+   tristate "Open Firmware display driver"
+   depends on DRM && MMU && PPC
+   select DRM_GEM_SHMEM_HELPER
+   select DRM_KMS_HELPER
+   help
+ DRM driver for Open Firmware framebuffers.
+
+ This driver assumes that the display hardware has been initialized
+ by the Open Firmware before the kernel boots. Scanout buffer, size,
+ and display format must be provided via device tree.
+
  config DRM_PANEL_MIPI_DBI
tristate "DRM support for MIPI DBI compatible panels"
depends on DRM && SPI
diff --git a/drivers/gpu/drm/tiny/Makefile b/drivers/gpu/drm/tiny/Makefile
index 1d9d6227e7ab..76dde89a044b 100644
--- a/drivers/gpu/drm/tiny/Makefile
+++ b/drivers/gpu/drm/tiny/Makefile
@@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ARCPGU)+= arcpgu.o
  obj-$(CONFIG_DRM_BOCHS)   += bochs.o
  obj-$(CONFIG_DRM_CIRRUS_QEMU) += cirrus.o
  obj-$(CONFIG_DRM_GM12U320)+= gm12u320.o
+obj-$(CONFIG_DRM_OFDRM)+= ofdrm.o
  obj-$(CONFIG_DRM_PANEL_MIPI_DBI)  += panel-mipi-dbi.o
  obj-$(CONFIG_DRM_SIMPLEDRM)   += simpledrm.o
  obj-$(CONFIG_TINYDRM_HX8357D) += hx8357d.o
diff --git a/drivers/gpu/drm/tiny/ofdrm.c b/drivers/gpu/drm/tiny/ofdrm.c
new file mode 100644
index ..aca715b36179
--- /dev/null
+++ b/drivers/gpu/drm/tiny/ofdrm.c
@@ -0,0 +1,748 @@
+// SPDX-License-Identifier: GPL-2.0-only
+
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+
+#define DRIVER_NAME"ofdrm"
+#define DRIVER_DESC"DRM driver for OF platform devices"
+#define DRIVER_DATE"20220501"
+#define DRIVER_MAJOR   1
+#define DRIVER_MINOR   0
+
+/*
+ * Assume a monitor resolution of 96 dpi to
+ * get a somewhat reasonable screen size.
+ */
+#define RES_MM(d)  \
+   (((d) * 254ul) / (96ul * 10ul))
+
+#define OFDRM_MODE(hd, vd) \
+   DRM_SIMPLE_MODE(hd, vd, RES_MM(hd), RES_MM(vd))
+
+/*
+ * Helpers for display nodes
+ */
+
+static int display_get_validated_int(struct drm_device *dev, const char *name, 

Shift overflow warnings in arch/powerpc/boot/addnote.c on 32-bit builds

2019-03-19 Thread Mark Cave-Ayland
Hi all,

Whilst building the latest git master on my G4 I noticed the following shift 
overflow
warnings in the build log for arch/powerpc/boot/addnote.c:


arch/powerpc/boot/addnote.c: In function ‘main’:
arch/powerpc/boot/addnote.c:75:47: warning: right shift count >= width of type
[-Wshift-count-overflow]
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~
arch/powerpc/boot/addnote.c:72:39: note: in definition of macro ‘PUT_16BE’
 #define PUT_16BE(off, v)(buf[off] = ((v) >> 8) & 0xff, \
   ^
arch/powerpc/boot/addnote.c:75:27: note: in expansion of macro ‘PUT_32BE’
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~~~
arch/powerpc/boot/addnote.c:94:50: note: in expansion of macro ‘PUT_64BE’
 #define PUT_64(off, v)  (e_data == ELFDATA2MSB ? PUT_64BE(off, v) : \
  ^~~~
arch/powerpc/boot/addnote.c:183:3: note: in expansion of macro ‘PUT_64’
   PUT_64(ph + PH_OFFSET, ns);
   ^~
arch/powerpc/boot/addnote.c:75:47: warning: right shift count >= width of type
[-Wshift-count-overflow]
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~
arch/powerpc/boot/addnote.c:73:23: note: in definition of macro ‘PUT_16BE’
 buf[(off) + 1] = (v) & 0xff)
   ^
arch/powerpc/boot/addnote.c:75:27: note: in expansion of macro ‘PUT_32BE’
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~~~
arch/powerpc/boot/addnote.c:94:50: note: in expansion of macro ‘PUT_64BE’
 #define PUT_64(off, v)  (e_data == ELFDATA2MSB ? PUT_64BE(off, v) : \
  ^~~~
arch/powerpc/boot/addnote.c:183:3: note: in expansion of macro ‘PUT_64’
   PUT_64(ph + PH_OFFSET, ns);
   ^~
arch/powerpc/boot/addnote.c:75:47: warning: right shift count >= width of type
[-Wshift-count-overflow]
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~
arch/powerpc/boot/addnote.c:72:39: note: in definition of macro ‘PUT_16BE’
 #define PUT_16BE(off, v)(buf[off] = ((v) >> 8) & 0xff, \
   ^
arch/powerpc/boot/addnote.c:75:27: note: in expansion of macro ‘PUT_32BE’
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~~~
arch/powerpc/boot/addnote.c:94:50: note: in expansion of macro ‘PUT_64BE’
 #define PUT_64(off, v)  (e_data == ELFDATA2MSB ? PUT_64BE(off, v) : \
  ^~~~
arch/powerpc/boot/addnote.c:183:3: note: in expansion of macro ‘PUT_64’
   PUT_64(ph + PH_OFFSET, ns);
   ^~
arch/powerpc/boot/addnote.c:75:47: warning: right shift count >= width of type
[-Wshift-count-overflow]
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~
arch/powerpc/boot/addnote.c:73:23: note: in definition of macro ‘PUT_16BE’
 buf[(off) + 1] = (v) & 0xff)
   ^
arch/powerpc/boot/addnote.c:75:27: note: in expansion of macro ‘PUT_32BE’
 #define PUT_64BE(off, v)((PUT_32BE((off), (v) >> 32L), \
   ^~~~
arch/powerpc/boot/addnote.c:94:50: note: in expansion of macro ‘PUT_64BE’
 #define PUT_64(off, v)  (e_data == ELFDATA2MSB ? PUT_64BE(off, v) : \
  ^~~~
arch/powerpc/boot/addnote.c:183:3: note: in expansion of macro ‘PUT_64’
   PUT_64(ph + PH_OFFSET, ns);
   ^~
arch/powerpc/boot/addnote.c:85:73: warning: right shift count >= width of type
[-Wshift-count-overflow]
 #define PUT_64LE(off, v) (PUT_32LE((off), (v)), PUT_32LE((off) + 4, (v) >> 
32L))
 ^~
arch/powerpc/boot/addnote.c:82:39: note: in definition of macro ‘PUT_16LE’
 #define PUT_16LE(off, v) (buf[off] = (v) & 0xff, \
   ^
arch/powerpc/boot/addnote.c:85:49: note: in expansion of macro ‘PUT_32LE’
 #define PUT_64LE(off, v) (PUT_32LE((off), (v)), PUT_32LE((off) + 4, (v) >> 
32L))
 ^~~~
arch/powerpc/boot/addnote.c:95:5: note: in expansion of macro ‘PUT_64LE’
 PUT_64LE(off, v))
 ^~~~
arch/powerpc/boot/addnote.c:183:3: note: in expansion of macro ‘PUT_64’
   PUT_64(ph + PH_OFFSET, ns);
   ^~
arch/powerpc/boot/addnote.c:85:73: warning: right shift count >= width of type
[-Wshift-count-overflow]
 #define PUT_64LE(off, v) (PUT_32LE((off), (v)), PUT_32LE((off) + 4, (v) >> 
32L))
 ^~
arch/powerpc/boot/addnote.c:83:25: note: in definition of macro ‘PUT_16LE’
  buf[(off) + 1] = ((v) >> 8) & 0xff)
 ^
arch/powerpc/boot/addnote.c:85:49: note: in expansion of macro ‘PUT_32LE’
 #define PUT_64LE(off, v) 

Re: Mac Mini G4 hang on boot with git master

2019-03-17 Thread Mark Cave-Ayland
On 17/03/2019 16:25, christophe leroy wrote:

>> This was a weird one: bisecting directly from git master gave a nonsense 
>> result,
>> however by manually rebasing Michael's PR onto my last known good commit 
>> from master
>> I was able to finally pin it down to this commit:
>>
>>
>> 7a0d6955f3f7a4250da63d528bfff7a9c91b5725 is the first bad commit
>> commit 7a0d6955f3f7a4250da63d528bfff7a9c91b5725
>> Author: Christophe Leroy 
>> Date:   Thu Feb 21 10:37:55 2019 +
>>
>>  powerpc/6xx: Store PGDIR physical address in a SPRG
>>
>>  Use SPRN_SPRG2 to store the current thread PGDIR and
>>  avoid reading thread_struct.pgdir at every TLB miss.
>>
>>  Signed-off-by: Christophe Leroy 
>>  Signed-off-by: Michael Ellerman 
>>
>>
> 
> Hi,
> 
> The fix is there:
> 
> https://patchwork.ozlabs.org/patch/1053385/
> 
> Christophe

Hi Christophe,

Thank you! I've tried the patch here and have confirmed that it fixes the 
problem
with the hang on boot.


ATB,

Mark.


Re: Mac Mini G4 hang on boot with git master

2019-03-17 Thread Mark Cave-Ayland
On 15/03/2019 13:37, Mark Cave-Ayland wrote:

> Hi all,
> 
> I've just done a git pull and rebuilt master on my Mac Mini G4 in order to 
> test
> Michael's merge of my KVM PR fix, and unfortunately my kernel now hangs on 
> boot :(
> 
> My last working git checkout was somewhere around the 5.0-rc stage, so I 
> suspect it's
> something that's been merged for 5.1.
> 
> The hang occurs just after the boot console is disabled which makes me wonder 
> if
> something is going wrong during PCI bus enumeration. Does anyone have an idea 
> as to
> what may be causing this? I can obviously bisect it down, but on slow 
> hardware it can
> take some time...

This was a weird one: bisecting directly from git master gave a nonsense result,
however by manually rebasing Michael's PR onto my last known good commit from 
master
I was able to finally pin it down to this commit:


7a0d6955f3f7a4250da63d528bfff7a9c91b5725 is the first bad commit
commit 7a0d6955f3f7a4250da63d528bfff7a9c91b5725
Author: Christophe Leroy 
Date:   Thu Feb 21 10:37:55 2019 +

powerpc/6xx: Store PGDIR physical address in a SPRG

Use SPRN_SPRG2 to store the current thread PGDIR and
avoid reading thread_struct.pgdir at every TLB miss.

Signed-off-by: Christophe Leroy 
Signed-off-by: Michael Ellerman 


ATB,

Mark.


Mac Mini G4 hang on boot with git master

2019-03-15 Thread Mark Cave-Ayland
Hi all,

I've just done a git pull and rebuilt master on my Mac Mini G4 in order to test
Michael's merge of my KVM PR fix, and unfortunately my kernel now hangs on boot 
:(

My last working git checkout was somewhere around the 5.0-rc stage, so I 
suspect it's
something that's been merged for 5.1.

The hang occurs just after the boot console is disabled which makes me wonder if
something is going wrong during PCI bus enumeration. Does anyone have an idea 
as to
what may be causing this? I can obviously bisect it down, but on slow hardware 
it can
take some time...


ATB,

Mark.


Re: [PATCH] powerpc: fix 32-bit KVM-PR lockup and panic with MacOS guest

2019-02-19 Thread Mark Cave-Ayland
On 19/02/2019 04:55, Michael Ellerman wrote:

> Mark Cave-Ayland  writes:
>> On 11/02/2019 00:30, Benjamin Herrenschmidt wrote:
>>
>>> On Fri, 2019-02-08 at 14:51 +, Mark Cave-Ayland wrote:
>>>>
>>>> Indeed, but there are still some questions to be asked here:
>>>>
>>>> 1) Why were these bits removed from the original bitmask in the first 
>>>> place without
>>>> it being documented in the commit message?
>>>>
>>>> 2) Is this the right fix? I'm told that MacOS guests already run without 
>>>> this patch
>>>> on a G5 under 64-bit KVM-PR which may suggest that this is a workaround 
>>>> for another
>>>> bug elsewhere in the 32-bit powerpc code.
>>>>
>>>>
>>>> If you think that these points don't matter, then I'm happy to resubmit 
>>>> the patch
>>>> as-is based upon your comments above.
>>>
>>> We should write a test case to verify that FE0/FE1 are properly
>>> preserved/context-switched etc... I bet if we accidentally wiped them,
>>> we wouldn't notice 99.9% of the time.
>>
>> Right I guess it's more likely to cause in issue in the KVM PR case because 
>> the guest
>> can alter the flags in a way that doesn't go through the normal process 
>> switch mechanism.
>>
>> The original patchset at
>> https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg98326.html 
>> does include
>> some tests in the first few patches, but AFAICT they are concerned with the 
>> contents
>> of the FP registers rather than the related MSRs.
> 
> fpu_preempt.c should be able to be adapted to also check the MSR bits.
> 
>> Who is the right person to ask about fixing issues related to context 
>> switching with
>> KVM PR?
> 
> KVM PR doesn't really have a maintainer TBH. Feel like volunteering? :)

Well I only have a 32-bit Mac Mini here which I'm using to help flush out bugs 
in
QEMU's emulation, so I can keep an occasional eye on the 32-bit side of things 
but as
it's a hobby project time is quite limited.

As/when time allows I'd be interested to figure out what MacOS 9 does that 
causes KVM
PR to bail, and if it's possible to run KVM PR on an SMP kernel but certainly 
I'd
need some help from the very knowledgable people on these lists.

>> I did add the original author's email address to my first few emails but have
>> had no response back :/
> 
> Cyril who wrote the original FPU patch has moved on to other things.

Ah okay then.


ATB,

Mark.


Re: [PATCH] powerpc: fix 32-bit KVM-PR lockup and panic with MacOS guest

2019-02-18 Thread Mark Cave-Ayland
On 19/02/2019 04:20, Michael Ellerman wrote:

Hi Michael,

> Mark Cave-Ayland  writes:
>> On 08/02/2019 14:45, Christophe Leroy wrote:
>>
>>> Le 08/02/2019 à 15:33, Mark Cave-Ayland a écrit :
>>>> Commit 8792468da5e1 "powerpc: Add the ability to save FPU without giving 
>>>> it up"
>>>
>>> Expected format for the above is:
>>>
>>> Commit 123456789abc ("text")
>>
>> Hi Christophe,
>>
>> Apologies - I'm fairly new at submitting kernel patches, but I can re-send 
>> it in the
>> correct format later if required.
>>
>>>> unexpectedly removed the MSR_FE0 and MSR_FE1 bits from the bitmask used to
>>>> update the MSR of the previous thread in __giveup_fpu() causing a KVM-PR 
>>>> MacOS
>>>> guest to lockup and panic the kernel.
> 
> Which kernel is panicking? The guest or the host?

It's the host kernel. As long as you occasionally tap a few keys to keep the 
screen
blanking disabled then you can see the panic on the physical console.

I've uploaded a photo I took during the bisection containing the panic when 
booting
MacOS X 10.2 under qemu-system-ppc to
https://www.ilande.co.uk/tmp/qemu/macmini-kvm.jpg in case you find it useful.

Given that it's really easy to recreate, let me know if you want me to do a git
pull/rebuild and/or if you need any debugging information as it's easy for me to
reproduce.

>>>> Reinstate these bits to the MSR bitmask to enable MacOS guests to run under
>>>> 32-bit KVM-PR once again without issue.
>>>>
>>>> Signed-off-by: Mark Cave-Ayland 
>>>
>>> Should include a Fixes: and a Cc to stable ?
>>>
>>> Fixes: 8792468da5e1 ("powerpc: Add the ability to save FPU without giving 
>>> it up")
>>> Cc: sta...@vger.kernel.org
>>
>> Indeed, but there are still some questions to be asked here:
>>
>> 1) Why were these bits removed from the original bitmask in the first place 
>> without
>> it being documented in the commit message?
> 
> It was almost certainly an accident.

Heh, okay :)

>> 2) Is this the right fix? I'm told that MacOS guests already run without 
>> this patch
>> on a G5 under 64-bit KVM-PR which may suggest that this is a workaround for 
>> another
>> bug elsewhere in the 32-bit powerpc code.
> 
> That's slightly worrying. It's hard to say without more detail on why
> the guest is crashing.
> 
> I think your patch looks OK based just on the fact that it restores the
> previous behaviour, so I'll pick it up and pass it through my usual
> testing. If nothing breaks I'll merge it.

That would be great! Does it need a CC to stable too? It would be great if this 
would
get picked up in the next set of Debian ports kernels, for example.


ATB,

Mark.


Re: [PATCH] powerpc: fix 32-bit KVM-PR lockup and panic with MacOS guest

2019-02-11 Thread Mark Cave-Ayland
On 11/02/2019 00:30, Benjamin Herrenschmidt wrote:

> On Fri, 2019-02-08 at 14:51 +0000, Mark Cave-Ayland wrote:
>>
>> Indeed, but there are still some questions to be asked here:
>>
>> 1) Why were these bits removed from the original bitmask in the first place 
>> without
>> it being documented in the commit message?
>>
>> 2) Is this the right fix? I'm told that MacOS guests already run without 
>> this patch
>> on a G5 under 64-bit KVM-PR which may suggest that this is a workaround for 
>> another
>> bug elsewhere in the 32-bit powerpc code.
>>
>>
>> If you think that these points don't matter, then I'm happy to resubmit the 
>> patch
>> as-is based upon your comments above.
> 
> We should write a test case to verify that FE0/FE1 are properly
> preserved/context-switched etc... I bet if we accidentally wiped them,
> we wouldn't notice 99.9% of the time.

Right I guess it's more likely to cause in issue in the KVM PR case because the 
guest
can alter the flags in a way that doesn't go through the normal process switch 
mechanism.

The original patchset at
https://www.mail-archive.com/linuxppc-dev@lists.ozlabs.org/msg98326.html does 
include
some tests in the first few patches, but AFAICT they are concerned with the 
contents
of the FP registers rather than the related MSRs.

Who is the right person to ask about fixing issues related to context switching 
with
KVM PR? I did add the original author's email address to my first few emails 
but have
had no response back :/


ATB,

Mark.


Re: [PATCH] powerpc: fix 32-bit KVM-PR lockup and panic with MacOS guest

2019-02-08 Thread Mark Cave-Ayland
On 08/02/2019 14:45, Christophe Leroy wrote:

> Le 08/02/2019 à 15:33, Mark Cave-Ayland a écrit :
>> Commit 8792468da5e1 "powerpc: Add the ability to save FPU without giving it 
>> up"
> 
> Expected format for the above is:
> 
> Commit 123456789abc ("text")

Hi Christophe,

Apologies - I'm fairly new at submitting kernel patches, but I can re-send it 
in the
correct format later if required.

>> unexpectedly removed the MSR_FE0 and MSR_FE1 bits from the bitmask used to
>> update the MSR of the previous thread in __giveup_fpu() causing a KVM-PR 
>> MacOS
>> guest to lockup and panic the kernel.
>>
>> Reinstate these bits to the MSR bitmask to enable MacOS guests to run under
>> 32-bit KVM-PR once again without issue.
>>
>> Signed-off-by: Mark Cave-Ayland 
> 
> Should include a Fixes: and a Cc to stable ?
> 
> Fixes: 8792468da5e1 ("powerpc: Add the ability to save FPU without giving it 
> up")
> Cc: sta...@vger.kernel.org

Indeed, but there are still some questions to be asked here:

1) Why were these bits removed from the original bitmask in the first place 
without
it being documented in the commit message?

2) Is this the right fix? I'm told that MacOS guests already run without this 
patch
on a G5 under 64-bit KVM-PR which may suggest that this is a workaround for 
another
bug elsewhere in the 32-bit powerpc code.


If you think that these points don't matter, then I'm happy to resubmit the 
patch
as-is based upon your comments above.


ATB,

Mark.


[PATCH] powerpc: fix 32-bit KVM-PR lockup and panic with MacOS guest

2019-02-08 Thread Mark Cave-Ayland
Commit 8792468da5e1 "powerpc: Add the ability to save FPU without giving it up"
unexpectedly removed the MSR_FE0 and MSR_FE1 bits from the bitmask used to
update the MSR of the previous thread in __giveup_fpu() causing a KVM-PR MacOS
guest to lockup and panic the kernel.

Reinstate these bits to the MSR bitmask to enable MacOS guests to run under
32-bit KVM-PR once again without issue.

Signed-off-by: Mark Cave-Ayland 
---
 arch/powerpc/kernel/process.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index ce393df243aa..71bad4b6f80d 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -176,7 +176,7 @@ static void __giveup_fpu(struct task_struct *tsk)
 
save_fpu(tsk);
msr = tsk->thread.regs->msr;
-   msr &= ~MSR_FP;
+   msr &= ~(MSR_FP|MSR_FE0|MSR_FE1);
 #ifdef CONFIG_VSX
if (cpu_has_feature(CPU_FTR_VSX))
msr &= ~MSR_VSX;
-- 
2.11.0



MSR_FE mask change broke KVM PR MacOS guest

2019-01-25 Thread Mark Cave-Ayland
Hi all,

Referencing my bug report over on the kvm-ppc list at
https://www.spinics.net/lists/kvm-ppc/msg14971.html, I've been experiencing a 
hard
lockup and panic on a G4 Mac Mini when trying run MacOS X under KVM PR which 
I've
bisected down to the following commit:


$ git bisect bad
8792468da5e12e77e76e1edf081acf0392abb331 is the first bad commit
commit 8792468da5e12e77e76e1edf081acf0392abb331
Author: Cyril Bur 
Date:   Mon Feb 29 17:53:49 2016 +1100

powerpc: Add the ability to save FPU without giving it up

This patch adds the ability to be able to save the FPU registers to the
thread struct without giving up (disabling the facility) next time the
process returns to userspace.

This patch optimises the thread copy path (as a result of a fork() or
clone()) so that the parent thread can return to userspace with hot
registers avoiding a possibly pointless reload of FPU register state.

Signed-off-by: Cyril Bur 
Signed-off-by: Michael Ellerman 


Working through the changes which can be found at
https://github.com/torvalds/linux/commit/8792468da5e12e77e76e1edf081acf0392abb331,
I've discovered that the lockup is being caused by the removal of the the 
MSR_FE0 and
MSR_FE1 masks from the previous task during giveup_fpu().

Before this patch:


_GLOBAL(giveup_fpu)
mfmsr   r5
ori r5,r5,MSR_FP
#ifdef CONFIG_VSX
BEGIN_FTR_SECTION
orisr5,r5,MSR_VSX@h
END_FTR_SECTION_IFSET(CPU_FTR_VSX)
#endif
SYNC_601
ISYNC_601
MTMSRD(r5)  /* enable use of fpu now */
SYNC_601
isync
PPC_LCMPI   0,r3,0
beqlr-  /* if no previous owner, done */
addir3,r3,THREAD/* want THREAD of task */
PPC_LL  r6,THREAD_FPSAVEAREA(r3)
PPC_LL  r5,PT_REGS(r3)
PPC_LCMPI   0,r6,0
bne 2f
addir6,r3,THREAD_FPSTATE
2:  PPC_LCMPI   0,r5,0
SAVE_32FPVSRS(0, R4, R6)
mffsfr0
stfdfr0,FPSTATE_FPSCR(r6)
beq 1f
PPC_LL  r4,_MSR-STACK_FRAME_OVERHEAD(r5)
li  r3,MSR_FP|MSR_FE0|MSR_FE1

^
#ifdef CONFIG_VSX
BEGIN_FTR_SECTION
orisr3,r3,MSR_VSX@h
END_FTR_SECTION_IFSET(CPU_FTR_VSX)
#endif
andcr4,r4,r3/* disable FP for previous task */


and after the patch:

void __giveup_fpu(struct task_struct *tsk)
{
save_fpu(tsk);
tsk->thread.regs->msr &= ~MSR_FP;
 ^^^

#ifdef CONFIG_VSX
if (cpu_has_feature(CPU_FTR_VSX))
tsk->thread.regs->msr &= ~MSR_VSX;
#endif
}


Reinstating the MSR_FE0 and MSR_FE1 bitmasks fixes the issue allowing MacOS X 
to boot
without incident again:


diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 14c09d25de98..f9ac44621dda 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -136,7 +136,7 @@ EXPORT_SYMBOL(__msr_check_and_clear);
 void __giveup_fpu(struct task_struct *tsk)
 {
save_fpu(tsk);
-   tsk->thread.regs->msr &= ~MSR_FP;
+   tsk->thread.regs->msr &= ~(MSR_FP|MSR_FE0|MSR_FE1);
 #ifdef CONFIG_VSX
if (cpu_has_feature(CPU_FTR_VSX))
tsk->thread.regs->msr &= ~MSR_VSX;


Would the above be accepted as a patch? The commit message for the patch above 
fails
to mention why the MSR_FE* constants were removed from the MSR bitmask, so I'm 
not
sure whether this was deliberate? Certainly running with the above patch 
applied, I
haven't noticed anything obviously broken. And could this patch cause any 
ill-effects
on 64-bit PPC CPUs?


ATB,

Mark.