[PATCH qemu] spapr: Force 32bit when resetting a core

[PATCH qemu] spapr: Force 32bit when resetting a core

[Alexey Kardashevskiy]

"PowerPC Processor binding to IEEE 1275" says in
"8.2.1. Initial Register Values" that the initial state is defined as
32bit so do it for both SLOF and VOF.

This should not cause behavioral change as SLOF switches to 64bit very
early anyway. As nothing enforces LE anywhere, this drops it for VOF.

The goal is to make VOF work with TCG as otherwise it barfs with
qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)

Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
---
 hw/ppc/spapr_cpu_core.c | 5 +++++
 hw/ppc/spapr_vof.c      | 2 --
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
index a57ba70a8781..a781e97f8d1d 100644
--- a/hw/ppc/spapr_cpu_core.c
+++ b/hw/ppc/spapr_cpu_core.c
@@ -37,6 +37,11 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
 
     cpu_reset(cs);
 
+    /*
+     * "PowerPC Processor binding to IEEE 1275" defines the initial MSR state
+     * as 32bit (MSR_SF=0) in "8.2.1. Initial Register Values".
+     */
+    env->msr &= ~(1ULL << MSR_SF);
     env->spr[SPR_HIOR] = 0;
 
     lpcr = env->spr[SPR_LPCR];
diff --git a/hw/ppc/spapr_vof.c b/hw/ppc/spapr_vof.c
index 40ce8fe0037c..a33f940c32bb 100644
--- a/hw/ppc/spapr_vof.c
+++ b/hw/ppc/spapr_vof.c
@@ -88,8 +88,6 @@ void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp)
     spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT,
                               stack_ptr, spapr->initrd_base,
                               spapr->initrd_size);
-    /* VOF is 32bit BE so enforce MSR here */
-    first_ppc_cpu->env.msr &= ~((1ULL << MSR_SF) | (1ULL << MSR_LE));
 
     /*
      * At this point the expected allocation map is:
-- 
2.30.2

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Cédric Le Goater]

On 1/7/22 08:24, Alexey Kardashevskiy wrote:
> "PowerPC Processor binding to IEEE 1275" says in
> "8.2.1. Initial Register Values" that the initial state is defined as
> 32bit so do it for both SLOF and VOF.
> 
> This should not cause behavioral change as SLOF switches to 64bit very
> early anyway. As nothing enforces LE anywhere, this drops it for VOF.
> 
> The goal is to make VOF work with TCG as otherwise it barfs with
> qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>   hw/ppc/spapr_cpu_core.c | 5 +++++
>   hw/ppc/spapr_vof.c      | 2 --
>   2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index a57ba70a8781..a781e97f8d1d 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -37,6 +37,11 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>   
>       cpu_reset(cs);
>   
> +    /*
> +     * "PowerPC Processor binding to IEEE 1275" defines the initial MSR state
> +     * as 32bit (MSR_SF=0) in "8.2.1. Initial Register Values".
> +     */

Indeed but this seems to contradict 8b9f2118ca40 ("ppc64: set MSR_SF bit").

Laurent, would you remember the reason for forcing 64bit ? It's been a while
since.

Thanks,

C.


> +    env->msr &= ~(1ULL << MSR_SF);
>       env->spr[SPR_HIOR] = 0;
>   
>       lpcr = env->spr[SPR_LPCR];
> diff --git a/hw/ppc/spapr_vof.c b/hw/ppc/spapr_vof.c
> index 40ce8fe0037c..a33f940c32bb 100644
> --- a/hw/ppc/spapr_vof.c
> +++ b/hw/ppc/spapr_vof.c
> @@ -88,8 +88,6 @@ void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp)
>       spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT,
>                                 stack_ptr, spapr->initrd_base,
>                                 spapr->initrd_size);
> -    /* VOF is 32bit BE so enforce MSR here */
> -    first_ppc_cpu->env.msr &= ~((1ULL << MSR_SF) | (1ULL << MSR_LE));
>   
>       /*
>        * At this point the expected allocation map is:
> 

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Greg Kurz]

On Fri, 7 Jan 2022 18:24:23 +1100
Alexey Kardashevskiy <aik@ozlabs.ru> wrote:

> "PowerPC Processor binding to IEEE 1275" says in
> "8.2.1. Initial Register Values" that the initial state is defined as
> 32bit so do it for both SLOF and VOF.
> 
> This should not cause behavioral change as SLOF switches to 64bit very
> early anyway. 

Only one CPU goes through SLOF. What about the other ones, including
hot plugged CPUs ?

> As nothing enforces LE anywhere, this drops it for VOF.
> 
> The goal is to make VOF work with TCG as otherwise it barfs with
> qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)
> 
> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
> ---
>  hw/ppc/spapr_cpu_core.c | 5 +++++
>  hw/ppc/spapr_vof.c      | 2 --
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index a57ba70a8781..a781e97f8d1d 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -37,6 +37,11 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>  
>      cpu_reset(cs);
>  
> +    /*
> +     * "PowerPC Processor binding to IEEE 1275" defines the initial MSR state
> +     * as 32bit (MSR_SF=0) in "8.2.1. Initial Register Values".
> +     */
> +    env->msr &= ~(1ULL << MSR_SF);
>      env->spr[SPR_HIOR] = 0;
>  
>      lpcr = env->spr[SPR_LPCR];
> diff --git a/hw/ppc/spapr_vof.c b/hw/ppc/spapr_vof.c
> index 40ce8fe0037c..a33f940c32bb 100644
> --- a/hw/ppc/spapr_vof.c
> +++ b/hw/ppc/spapr_vof.c
> @@ -88,8 +88,6 @@ void spapr_vof_reset(SpaprMachineState *spapr, void *fdt, Error **errp)
>      spapr_cpu_set_entry_state(first_ppc_cpu, SPAPR_ENTRY_POINT,
>                                stack_ptr, spapr->initrd_base,
>                                spapr->initrd_size);
> -    /* VOF is 32bit BE so enforce MSR here */
> -    first_ppc_cpu->env.msr &= ~((1ULL << MSR_SF) | (1ULL << MSR_LE));
>  
>      /*
>       * At this point the expected allocation map is:

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 798 bytes --]

On Fri, Jan 07, 2022 at 12:57:47PM +0100, Greg Kurz wrote:
> On Fri, 7 Jan 2022 18:24:23 +1100
> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> 
> > "PowerPC Processor binding to IEEE 1275" says in
> > "8.2.1. Initial Register Values" that the initial state is defined as
> > 32bit so do it for both SLOF and VOF.
> > 
> > This should not cause behavioral change as SLOF switches to 64bit very
> > early anyway. 
> 
> Only one CPU goes through SLOF. What about the other ones, including
> hot plugged CPUs ?

Those will be started by the start-cpu RTAS call which has its own
semantics.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Greg Kurz]

[-- Attachment #1: Type: text/plain, Size: 1012 bytes --]

On Fri, 7 Jan 2022 23:19:03 +1100
David Gibson <david@gibson.dropbear.id.au> wrote:

> On Fri, Jan 07, 2022 at 12:57:47PM +0100, Greg Kurz wrote:
> > On Fri, 7 Jan 2022 18:24:23 +1100
> > Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > 
> > > "PowerPC Processor binding to IEEE 1275" says in
> > > "8.2.1. Initial Register Values" that the initial state is defined as
> > > 32bit so do it for both SLOF and VOF.
> > > 
> > > This should not cause behavioral change as SLOF switches to 64bit very
> > > early anyway. 
> > 
> > Only one CPU goes through SLOF. What about the other ones, including
> > hot plugged CPUs ?
> 
> Those will be started by the start-cpu RTAS call which has its own
> semantics.
> 

Ah indeed, there's code in linux/arch/powerpc/kernel/head_64.S to switch
secondaries to 64bit... but then, as noted by Cedric, ppc_cpu_reset(),
which is called earlier sets MSR_SF but the changelog of commit 8b9f2118ca40
doesn't provide much details on the motivation. Any idea ?

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Alexey Kardashevskiy]

On 08/01/2022 00:39, Greg Kurz wrote:
> On Fri, 7 Jan 2022 23:19:03 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
>> On Fri, Jan 07, 2022 at 12:57:47PM +0100, Greg Kurz wrote:
>>> On Fri, 7 Jan 2022 18:24:23 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>>> "PowerPC Processor binding to IEEE 1275" says in
>>>> "8.2.1. Initial Register Values" that the initial state is defined as
>>>> 32bit so do it for both SLOF and VOF.
>>>>
>>>> This should not cause behavioral change as SLOF switches to 64bit very
>>>> early anyway.
>>>
>>> Only one CPU goes through SLOF. What about the other ones, including
>>> hot plugged CPUs ?
>>
>> Those will be started by the start-cpu RTAS call which has its own
>> semantics.
>>
> 
> Ah indeed, there's code in linux/arch/powerpc/kernel/head_64.S to switch
> secondaries to 64bit... but then, as noted by Cedric, ppc_cpu_reset(),
> which is called earlier sets MSR_SF but the changelog of commit 8b9f2118ca40
> doesn't provide much details on the motivation. Any idea ?

https://patchwork.kernel.org/project/qemu-devel/patch/1458121432-2855-1-git-send-email-lvivier@redhat.com/

this is probably it:

===
Reset is properly defined as an exception (0x100). For exceptions, the
970MP user manual for example says:

4.5 Exception Definitions
When an exception/interrupt is taken, all bits in the MSR are set to
‘0’, with the following exceptions:
• Exceptions always set MSR[SF] to ‘1’.
===

but it looks like the above is about emulation bare metal 970 rather 
than pseries VCPU so that quote does not apply to spapr.

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2567 bytes --]

On Mon, Jan 10, 2022 at 01:52:06PM +1100, Alexey Kardashevskiy wrote:
> 
> 
> On 08/01/2022 00:39, Greg Kurz wrote:
> > On Fri, 7 Jan 2022 23:19:03 +1100
> > David Gibson <david@gibson.dropbear.id.au> wrote:
> > 
> > > On Fri, Jan 07, 2022 at 12:57:47PM +0100, Greg Kurz wrote:
> > > > On Fri, 7 Jan 2022 18:24:23 +1100
> > > > Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> > > > 
> > > > > "PowerPC Processor binding to IEEE 1275" says in
> > > > > "8.2.1. Initial Register Values" that the initial state is defined as
> > > > > 32bit so do it for both SLOF and VOF.
> > > > > 
> > > > > This should not cause behavioral change as SLOF switches to 64bit very
> > > > > early anyway.
> > > > 
> > > > Only one CPU goes through SLOF. What about the other ones, including
> > > > hot plugged CPUs ?
> > > 
> > > Those will be started by the start-cpu RTAS call which has its own
> > > semantics.
> > > 
> > 
> > Ah indeed, there's code in linux/arch/powerpc/kernel/head_64.S to switch
> > secondaries to 64bit... but then, as noted by Cedric, ppc_cpu_reset(),
> > which is called earlier sets MSR_SF but the changelog of commit 8b9f2118ca40
> > doesn't provide much details on the motivation. Any idea ?
> 
> https://patchwork.kernel.org/project/qemu-devel/patch/1458121432-2855-1-git-send-email-lvivier@redhat.com/
> 
> this is probably it:
> 
> ===
> Reset is properly defined as an exception (0x100). For exceptions, the
> 970MP user manual for example says:
> 
> 4.5 Exception Definitions
> When an exception/interrupt is taken, all bits in the MSR are set to
> ‘0’, with the following exceptions:
> • Exceptions always set MSR[SF] to ‘1’.
> ===
> 
> but it looks like the above is about emulation bare metal 970 rather than
> pseries VCPU so that quote does not apply to spapr.

PAPR is rather confusing on the topic (looking at PAPR+ 2.10).
Initially it says:

"When a processor thread exits the RTAS stopped state, it must begin
execution in real mode, with the MSR in the same state as from a
system reset interrupt (except for the MSRHV bit which is on if not
running under a hypervisor and off if running under a hypervisor)"

But further down it has a table of how all the MSR bits are supposed
to be set by start-cpu, and it looks like that might not match the
0x100 conditions in some cases.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Cédric Le Goater]

On 1/7/22 14:39, Greg Kurz wrote:
> On Fri, 7 Jan 2022 23:19:03 +1100
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
>> On Fri, Jan 07, 2022 at 12:57:47PM +0100, Greg Kurz wrote:
>>> On Fri, 7 Jan 2022 18:24:23 +1100
>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>>> "PowerPC Processor binding to IEEE 1275" says in
>>>> "8.2.1. Initial Register Values" that the initial state is defined as
>>>> 32bit so do it for both SLOF and VOF.
>>>>
>>>> This should not cause behavioral change as SLOF switches to 64bit very
>>>> early anyway.
>>>
>>> Only one CPU goes through SLOF. What about the other ones, including
>>> hot plugged CPUs ?
>>
>> Those will be started by the start-cpu RTAS call which has its own
>> semantics.
>>
> 
> Ah indeed, there's code in linux/arch/powerpc/kernel/head_64.S to switch
> secondaries to 64bit... but then, as noted by Cedric, ppc_cpu_reset(),
> which is called earlier sets MSR_SF but the changelog of commit 8b9f2118ca40
> doesn't provide much details on the motivation. Any idea ?

I found some reference to the commit here but it doesn't seem
to be the root cause :

  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1723914

Thanks,

C.

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Cédric Le Goater]

On 1/10/22 03:52, Alexey Kardashevskiy wrote:
> 
> 
> On 08/01/2022 00:39, Greg Kurz wrote:
>> On Fri, 7 Jan 2022 23:19:03 +1100
>> David Gibson <david@gibson.dropbear.id.au> wrote:
>>
>>> On Fri, Jan 07, 2022 at 12:57:47PM +0100, Greg Kurz wrote:
>>>> On Fri, 7 Jan 2022 18:24:23 +1100
>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>
>>>>> "PowerPC Processor binding to IEEE 1275" says in
>>>>> "8.2.1. Initial Register Values" that the initial state is defined as
>>>>> 32bit so do it for both SLOF and VOF.
>>>>>
>>>>> This should not cause behavioral change as SLOF switches to 64bit very
>>>>> early anyway.
>>>>
>>>> Only one CPU goes through SLOF. What about the other ones, including
>>>> hot plugged CPUs ?
>>>
>>> Those will be started by the start-cpu RTAS call which has its own
>>> semantics.
>>>
>>
>> Ah indeed, there's code in linux/arch/powerpc/kernel/head_64.S to switch
>> secondaries to 64bit... but then, as noted by Cedric, ppc_cpu_reset(),
>> which is called earlier sets MSR_SF but the changelog of commit 8b9f2118ca40
>> doesn't provide much details on the motivation. Any idea ?
> 
> https://patchwork.kernel.org/project/qemu-devel/patch/1458121432-2855-1-git-send-email-lvivier@redhat.com/
> 
> this is probably it:
> 
> ===
> Reset is properly defined as an exception (0x100). For exceptions, the
> 970MP user manual for example says:
> 
> 4.5 Exception Definitions
> When an exception/interrupt is taken, all bits in the MSR are set to
> ‘0’, with the following exceptions:
> • Exceptions always set MSR[SF] to ‘1’.
> ===
> 
> but it looks like the above is about emulation bare metal 970 rather than pseries VCPU so that quote does not apply to spapr.

Yes, more info here :

   https://patchwork.kernel.org/project/qemu-devel/patch/1458121432-2855-1-git-send-email-lvivier@redhat.com/

mac99+970 only boots with a 64bit kernel. 32bit are not supported because
of the use of the rfi instruction which was removed in v2.01. 32bit user
space is supported though.

However I was not able to build a disk with a compatible boot partition
for OpenBIOS. The above support only applies for kernel loaded in memory.
May be Mark knows how to do this ?

Anyhow, I didn't see any regression on PAPR with this patch, TCG or KVM.

Thanks,

C.

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Mark Cave-Ayland]

On 14/01/2022 14:12, Cédric Le Goater wrote:

> Yes, more info here :
> 
> https://patchwork.kernel.org/project/qemu-devel/patch/1458121432-2855-1-git-send-email-lvivier@redhat.com/ 
> 
> mac99+970 only boots with a 64bit kernel. 32bit are not supported because
> of the use of the rfi instruction which was removed in v2.01. 32bit user
> space is supported though.
> 
> However I was not able to build a disk with a compatible boot partition
> for OpenBIOS. The above support only applies for kernel loaded in memory.
> May be Mark knows how to do this ?

The Mac machines generally require a HFS filesystem for booting with OpenBIOS: it's a 
bit convoluted, but some instructions for grub can be found at 
https://wiki.gentoo.org/wiki/GRUB_on_Open_Firmware_(PowerPC).

I can certainly help out if you get stuck. If this is a more obscure combination then 
is it worth adding a tiny image for use with avocado?


ATB,

Mark.

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 3484 bytes --]

On Fri, 14 Jan 2022, Cédric Le Goater wrote:
> On 1/10/22 03:52, Alexey Kardashevskiy wrote:
>> On 08/01/2022 00:39, Greg Kurz wrote:
>>> On Fri, 7 Jan 2022 23:19:03 +1100
>>> David Gibson <david@gibson.dropbear.id.au> wrote:
>>> 
>>>> On Fri, Jan 07, 2022 at 12:57:47PM +0100, Greg Kurz wrote:
>>>>> On Fri, 7 Jan 2022 18:24:23 +1100
>>>>> Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>>> 
>>>>>> "PowerPC Processor binding to IEEE 1275" says in
>>>>>> "8.2.1. Initial Register Values" that the initial state is defined as
>>>>>> 32bit so do it for both SLOF and VOF.
>>>>>> 
>>>>>> This should not cause behavioral change as SLOF switches to 64bit very
>>>>>> early anyway.
>>>>> 
>>>>> Only one CPU goes through SLOF. What about the other ones, including
>>>>> hot plugged CPUs ?
>>>> 
>>>> Those will be started by the start-cpu RTAS call which has its own
>>>> semantics.
>>>> 
>>> 
>>> Ah indeed, there's code in linux/arch/powerpc/kernel/head_64.S to switch
>>> secondaries to 64bit... but then, as noted by Cedric, ppc_cpu_reset(),
>>> which is called earlier sets MSR_SF but the changelog of commit 
>>> 8b9f2118ca40
>>> doesn't provide much details on the motivation. Any idea ?
>> 
>> https://patchwork.kernel.org/project/qemu-devel/patch/1458121432-2855-1-git-send-email-lvivier@redhat.com/
>> 
>> this is probably it:
>> 
>> ===
>> Reset is properly defined as an exception (0x100). For exceptions, the
>> 970MP user manual for example says:
>> 
>> 4.5 Exception Definitions
>> When an exception/interrupt is taken, all bits in the MSR are set to
>> ‘0’, with the following exceptions:
>> • Exceptions always set MSR[SF] to ‘1’.
>> ===
>> 
>> but it looks like the above is about emulation bare metal 970 rather than 
>> pseries VCPU so that quote does not apply to spapr.
>
> Yes, more info here :
>
>  https://patchwork.kernel.org/project/qemu-devel/patch/1458121432-2855-1-git-send-email-lvivier@redhat.com/
>
> mac99+970 only boots with a 64bit kernel. 32bit are not supported because
> of the use of the rfi instruction which was removed in v2.01. 32bit user
> space is supported though.
>
> However I was not able to build a disk with a compatible boot partition
> for OpenBIOS. The above support only applies for kernel loaded in memory.

Not sure it's related or helpful but I've managed to install Adélie Linux 
(which still has official support for PPC Macs) on qemu-system-ppc64 -M 
mac99,via=pmu a while ago. Here's a thread about my experiences with 
Adélie Linux:

https://lists-old.adelielinux.org/hyperkitty/list/adelie-devel@lists.adelielinux.org/thread/CNWIYZCFN7XDBSDCZDVIUE3SXE2EX6YF/index.html

but that mentions qemu-system-ppc and ppc32 version so this was before the 
ppc64 install which may be a newer Adélie version that I had no such 
problem starting. Ar least I did not need the patched OpenBIOS for ppc64. 
I think that version worked with qemu-system-ppc64 and could just install 
it following the manual install described in the Adélie docs:

https://git.adelielinux.org/adelie/docs/-/wikis/Quick-Start-Guides/Installation

To boot the installed system I had to type "boot hd:2,\grub" as it did not 
find the boot partition for some reason (maybe because not preserving 
nvram variables).

The installed disk has these partitions:
1: Apple partition map
2. boot (HFS)
3: root (ext4)

This was a while ago so I don't remember the details but I think it worked 
for the G5 mac99 without much problem.

Regards,
BALATON Zoltan

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Peter Maydell]

On Fri, 7 Jan 2022 at 07:29, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
> "PowerPC Processor binding to IEEE 1275" says in
> "8.2.1. Initial Register Values" that the initial state is defined as
> 32bit so do it for both SLOF and VOF.
>
> This should not cause behavioral change as SLOF switches to 64bit very
> early anyway. As nothing enforces LE anywhere, this drops it for VOF.
>
> The goal is to make VOF work with TCG as otherwise it barfs with
> qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)

If you get this assert it means that something is changing
the CPU state and not calling the function to recalculate
the hflags (which are basically caching part of the CPU state).
So I don't know whether this patch is correct or not in updating
MSR bits, but in any case I think it is only masking the
missing-hflags-update that is causing the assertion...

-- PMM

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Cédric Le Goater]

On 1/15/22 15:21, Mark Cave-Ayland wrote:
> On 14/01/2022 14:12, Cédric Le Goater wrote:
> 
>> Yes, more info here :
>>
>> https://patchwork.kernel.org/project/qemu-devel/patch/1458121432-2855-1-git-send-email-lvivier@redhat.com/
>> mac99+970 only boots with a 64bit kernel. 32bit are not supported because
>> of the use of the rfi instruction which was removed in v2.01. 32bit user
>> space is supported though.
>>
>> However I was not able to build a disk with a compatible boot partition
>> for OpenBIOS. The above support only applies for kernel loaded in memory.
>> May be Mark knows how to do this ?
> 
> The Mac machines generally require a HFS filesystem for booting with OpenBIOS: it's a bit convoluted, but some instructions for grub can be found at https://wiki.gentoo.org/wiki/GRUB_on_Open_Firmware_(PowerPC).

Initially, I installed a debian11 ppc64 on a QEMU mac99/970 machine.
Something went wrong with the bootloader at installation and I was
stuck with memory boot. I didn't manage to restore a decent boot
setup even after that.

I stole the HFS partition from a real G5 and after some manual tweaks
on the disk, I got it working.

There are a few harmless errors :
     
     >> =============================================================
     >> OpenBIOS 1.1 [Jan 15 2022 14:23]
     >> Configuration device id QEMU version 1 machine id 3
     >> CPUs: 1
     >> Memory: 1024M
     >> UUID: 00000000-0000-0000-0000-000000000000
     >> CPU type PowerPC,970
     milliseconds isn't unique.
     Welcome to OpenBIOS v1.1 built on Jan 15 2022 14:23
     Trying hd:,\\:tbxi...
     >> switching to new context:
     call-method color!: exception -21
     >> call-method color! failed with error ffffffdf
     call-method color!: exception -21
     
     ...
     call-method block-size: exception -21
     >> call-method block-size failed with error ffffffdf
     call-method block-size: exception -21
     >> call-method block-size failed with error ffffffdf
     

I guess the initial problem is in the debian installer. I will report.

> I can certainly help out if you get stuck. If this is a more obscure combination 
> then is it worth adding a tiny image for use with avocado?

No. It is really a standard setup :

     root@vm02:~# mac-fdisk -l /dev/sda
     /dev/sda
             #                    type name                length   base    ( size )  system
     /dev/sda1     Apple_partition_map Apple                   63 @ 1       ( 31.5k)  Partition map
     /dev/sda2               Apple_HFS bootstrap           500001 @ 64      (244.1M)  HFS
     /dev/sda3         Apple_UNIX_SVR2 untitled           39634766 @ 500065  ( 18.9G)  Linux native
     /dev/sda4         Apple_UNIX_SVR2 swap               1808208 @ 40134831 (882.9M)  Linux swap
     /dev/sda5              Apple_Free Extra                    1 @ 41943039 (  0.5k)  Free space
     
     Block size=512, Number of Blocks=41943040
     DeviceType=0x0, DeviceId=0x0
     
     root@vm02:~# cat /proc/cpuinfo
     processor	: 0
     cpu		: PPC970, altivec supported
     clock		: 900.000000MHz
     revision	: 2.2 (pvr 0039 0202)
     
     timebase	: 100000000
     platform	: PowerMac
     model		: PowerMac3,1
     machine		: PowerMac3,1
     motherboard	: PowerMac3,1 MacRISC MacRISC2 Power Macintosh
     detected as	: 0 ((null))
     pmac flags	: 00000000
     pmac-generation	: NewWorld
     root@vm02:~# uname -a
     Linux vm02 5.15.0-2-powerpc64 #1 SMP Debian 5.15.5-2 (2021-12-18) ppc64 GNU/Linux
  

Thanks,

C.

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Mark Cave-Ayland]

On 17/01/2022 14:52, Cédric Le Goater wrote:

> Initially, I installed a debian11 ppc64 on a QEMU mac99/970 machine.
> Something went wrong with the bootloader at installation and I was
> stuck with memory boot. I didn't manage to restore a decent boot
> setup even after that.

Interesting. I had a similar issue using the debian ports images on mac99/ppc32: 
everything went well all up until the bootloader installation which failed. When I 
looked at the installer logs IIRC there was a kernel panic somewhere in the hfs 
module which I figured is likely an emulation bug somewhere.

> I stole the HFS partition from a real G5 and after some manual tweaks
> on the disk, I got it working.
> 
> There are a few harmless errors :
>      >> =============================================================
>      >> OpenBIOS 1.1 [Jan 15 2022 14:23]
>      >> Configuration device id QEMU version 1 machine id 3
>      >> CPUs: 1
>      >> Memory: 1024M
>      >> UUID: 00000000-0000-0000-0000-000000000000
>      >> CPU type PowerPC,970
>      milliseconds isn't unique.
>      Welcome to OpenBIOS v1.1 built on Jan 15 2022 14:23
>      Trying hd:,\\:tbxi...
>      >> switching to new context:
>      call-method color!: exception -21
>      >> call-method color! failed with error ffffffdf
>      call-method color!: exception -21
>      ...
>      call-method block-size: exception -21
>      >> call-method block-size failed with error ffffffdf
>      call-method block-size: exception -21
>      >> call-method block-size failed with error ffffffdf
> 
> I guess the initial problem is in the debian installer. I will report.
> 
>> I can certainly help out if you get stuck. If this is a more obscure combination 
>> then is it worth adding a tiny image for use with avocado?
> 
> No. It is really a standard setup :
> 
>      root@vm02:~# mac-fdisk -l /dev/sda
>      /dev/sda
>              #                    type name                length   base    ( size )  
> system
>      /dev/sda1     Apple_partition_map Apple                   63 @ 1       ( 31.5k)  
> Partition map
>      /dev/sda2               Apple_HFS bootstrap           500001 @ 64      (244.1M)  
> HFS
>      /dev/sda3         Apple_UNIX_SVR2 untitled           39634766 @ 500065  ( 
> 18.9G)  Linux native
>      /dev/sda4         Apple_UNIX_SVR2 swap               1808208 @ 40134831 
> (882.9M)  Linux swap
>      /dev/sda5              Apple_Free Extra                    1 @ 41943039 (  
> 0.5k)  Free space
>      Block size=512, Number of Blocks=41943040
>      DeviceType=0x0, DeviceId=0x0
>      root@vm02:~# cat /proc/cpuinfo
>      processor    : 0
>      cpu        : PPC970, altivec supported
>      clock        : 900.000000MHz
>      revision    : 2.2 (pvr 0039 0202)
>      timebase    : 100000000
>      platform    : PowerMac
>      model        : PowerMac3,1
>      machine        : PowerMac3,1
>      motherboard    : PowerMac3,1 MacRISC MacRISC2 Power Macintosh
>      detected as    : 0 ((null))
>      pmac flags    : 00000000
>      pmac-generation    : NewWorld
>      root@vm02:~# uname -a
>      Linux vm02 5.15.0-2-powerpc64 #1 SMP Debian 5.15.5-2 (2021-12-18) ppc64 GNU/Linux

Fantastic! Glad everything is all working for you :)


ATB,

Mark.

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Cédric Le Goater]

[ Adding Fred ]

On 1/18/22 09:30, Mark Cave-Ayland wrote:
> On 17/01/2022 14:52, Cédric Le Goater wrote:
> 
>> Initially, I installed a debian11 ppc64 on a QEMU mac99/970 machine.
>> Something went wrong with the bootloader at installation and I was
>> stuck with memory boot. I didn't manage to restore a decent boot
>> setup even after that.
> 
> Interesting. I had a similar issue using the debian ports images on mac99/ppc32: everything went well all up until the bootloader installation which failed. When I looked at the installer logs IIRC there was a kernel panic somewhere in the hfs module which I figured is likely an emulation bug somewhere.

Is that a known issue ? I guess these install configs are not often
tested.

Thanks,

C.

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Alexey Kardashevskiy]

On 1/17/22 03:45, Peter Maydell wrote:
> On Fri, 7 Jan 2022 at 07:29, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>> "PowerPC Processor binding to IEEE 1275" says in
>> "8.2.1. Initial Register Values" that the initial state is defined as
>> 32bit so do it for both SLOF and VOF.
>>
>> This should not cause behavioral change as SLOF switches to 64bit very
>> early anyway. As nothing enforces LE anywhere, this drops it for VOF.
>>
>> The goal is to make VOF work with TCG as otherwise it barfs with
>> qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)
> 
> If you get this assert it means that something is changing
> the CPU state and not calling the function to recalculate
> the hflags (which are basically caching part of the CPU state).
> So I don't know whether this patch is correct or not in updating
> MSR bits, but in any case I think it is only masking the
> missing-hflags-update that is causing the assertion...


If we emulate a bare metal machine, then most likely we want MSR_SF 
(==64bit) set. But this particular case is paravirtual pseries/spapr and 
it requires special handling so spapr_reset_vcpu() seems to be the right 
place.


> 
> -- PMM

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Peter Maydell]

On Wed, 19 Jan 2022 at 04:03, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>
>
>
> On 1/17/22 03:45, Peter Maydell wrote:
> > On Fri, 7 Jan 2022 at 07:29, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
> >>
> >> "PowerPC Processor binding to IEEE 1275" says in
> >> "8.2.1. Initial Register Values" that the initial state is defined as
> >> 32bit so do it for both SLOF and VOF.
> >>
> >> This should not cause behavioral change as SLOF switches to 64bit very
> >> early anyway. As nothing enforces LE anywhere, this drops it for VOF.
> >>
> >> The goal is to make VOF work with TCG as otherwise it barfs with
> >> qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)
> >
> > If you get this assert it means that something is changing
> > the CPU state and not calling the function to recalculate
> > the hflags (which are basically caching part of the CPU state).
> > So I don't know whether this patch is correct or not in updating
> > MSR bits, but in any case I think it is only masking the
> > missing-hflags-update that is causing the assertion...
>
>
> If we emulate a bare metal machine, then most likely we want MSR_SF
> (==64bit) set. But this particular case is paravirtual pseries/spapr and
> it requires special handling so spapr_reset_vcpu() seems to be the right
> place.

That may be so, but my point remains: if you are getting hflags
mismatch asserts then something is failing to recalculate the
hflags after updating CPU state.

-- PMM

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Cédric Le Goater]

On 1/16/22 17:45, Peter Maydell wrote:
> On Fri, 7 Jan 2022 at 07:29, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>
>> "PowerPC Processor binding to IEEE 1275" says in
>> "8.2.1. Initial Register Values" that the initial state is defined as
>> 32bit so do it for both SLOF and VOF.
>>
>> This should not cause behavioral change as SLOF switches to 64bit very
>> early anyway. As nothing enforces LE anywhere, this drops it for VOF.
>>
>> The goal is to make VOF work with TCG as otherwise it barfs with
>> qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)
> 
> If you get this assert it means that something is changing
> the CPU state and not calling the function to recalculate
> the hflags (which are basically caching part of the CPU state).
> So I don't know whether this patch is correct or not in updating
> MSR bits, but in any case I think it is only masking the
> missing-hflags-update that is causing the assertion...


yes. Currently, spapr_vof_reset() is called from the pseries
machine reset handler and modifies brutally the MSR without
calling hreg_compute_hflags()to update the hflags. Hence
the warning.

The proposal moves the MSR update under the pseries CPU reset
handler: spapr_reset_vcpu() where it should be. spapr_reset_vcpu()
also updates the LPCR register and calls hreg_compute_hflags()
when done.

The question we all had was why it was set to 64bit initially
by commit 8b9f2118ca40 which seems to be in contradiction with
the PAPR specs saying the CPUs should start in 32bit mode.
It is not clear but I didn't see any regression on pseries or
on the macbook machine using 970 CPUs. I think we are fine.


Thanks,

C.

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Frédéric Bonnard]

[-- Attachment #1: Type: text/plain, Size: 1347 bytes --]

Hi,
I mainly focus on 'ppc64el' on which Debian 11 installs well.
I'm pretty sure I've not tried Debian 11 on 'ppc64' (i.e. be) with qemu.
Even less on 'powerpc' (i.e. 32b).
Now I know that powerpc has a different bootloader installation process
compared to ppc64* in Debian, but it's converging recently at the Debian
installer level.

Could you guys send details to debian-powerpc@lists.debian.org so that
me or others (more 'powerpc' and 'ppc64' dev/users) can have a look ?

F.


On Tue, 18 Jan 2022 10:12:46 +0100 Cédric Le Goater <clg@kaod.org> wrote:
> [ Adding Fred ]
> 
> On 1/18/22 09:30, Mark Cave-Ayland wrote:
> > On 17/01/2022 14:52, Cédric Le Goater wrote:
> > 
> >> Initially, I installed a debian11 ppc64 on a QEMU mac99/970 machine.
> >> Something went wrong with the bootloader at installation and I was
> >> stuck with memory boot. I didn't manage to restore a decent boot
> >> setup even after that.
> > 
> > Interesting. I had a similar issue using the debian ports images on mac99/ppc32: everything went well all up until the bootloader installation which failed. When I looked at the installer logs IIRC there was a kernel panic somewhere in the hfs module which I figured is likely an emulation bug somewhere.
> 
> Is that a known issue ? I guess these install configs are not often
> tested.
> 
> Thanks,
> 
> C.
> 
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH qemu] spapr: Force 32bit when resetting a core

[Cédric Le Goater]

On 1/19/22 12:05, Cédric Le Goater wrote:
> On 1/16/22 17:45, Peter Maydell wrote:
>> On Fri, 7 Jan 2022 at 07:29, Alexey Kardashevskiy <aik@ozlabs.ru> wrote:
>>>
>>> "PowerPC Processor binding to IEEE 1275" says in
>>> "8.2.1. Initial Register Values" that the initial state is defined as
>>> 32bit so do it for both SLOF and VOF.
>>>
>>> This should not cause behavioral change as SLOF switches to 64bit very
>>> early anyway. As nothing enforces LE anywhere, this drops it for VOF.
>>>
>>> The goal is to make VOF work with TCG as otherwise it barfs with
>>> qemu: fatal: TCG hflags mismatch (current:0x6c000004 rebuilt:0x6c000000)
>>
>> If you get this assert it means that something is changing
>> the CPU state and not calling the function to recalculate
>> the hflags (which are basically caching part of the CPU state).
>> So I don't know whether this patch is correct or not in updating
>> MSR bits, but in any case I think it is only masking the
>> missing-hflags-update that is causing the assertion...
> 
> 
> yes. Currently, spapr_vof_reset() is called from the pseries
> machine reset handler and modifies brutally the MSR without
> calling hreg_compute_hflags()to update the hflags. Hence
> the warning.
> 
> The proposal moves the MSR update under the pseries CPU reset
> handler: spapr_reset_vcpu() where it should be. spapr_reset_vcpu()
> also updates the LPCR register and calls hreg_compute_hflags()
> when done.
> 
> The question we all had was why it was set to 64bit initially
> by commit 8b9f2118ca40 which seems to be in contradiction with
> the PAPR specs saying the CPUs should start in 32bit mode.
> It is not clear but I didn't see any regression on pseries or
> on the macbook machine using 970 CPUs. I think we are fine.

With that said,

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.