Re: [PATCH qemu] spapr: Force 32bit when resetting a core
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 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:0x6c04 rebuilt:0x6c00) 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 Thanks, C.
Re: [PATCH qemu] spapr: Force 32bit when resetting a core
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-powe...@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 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. > > signature.asc Description: PGP signature
Re: [PATCH qemu] spapr: Force 32bit when resetting a core
On 1/16/22 17:45, Peter Maydell wrote: On Fri, 7 Jan 2022 at 07:29, 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:0x6c04 rebuilt:0x6c00) 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
On Wed, 19 Jan 2022 at 04:03, Alexey Kardashevskiy wrote: > > > > On 1/17/22 03:45, Peter Maydell wrote: > > On Fri, 7 Jan 2022 at 07:29, 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:0x6c04 rebuilt:0x6c00) > > > > 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
On 1/17/22 03:45, Peter Maydell wrote: On Fri, 7 Jan 2022 at 07:29, 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:0x6c04 rebuilt:0x6c00) 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
[ 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
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: ---- >> 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 ffdf call-method color!: exception -21 ... call-method block-size: exception -21 >> call-method block-size failed with error ffdf call-method block-size: exception -21 >> call-method block-size failed with error ffdf 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 51 @ 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.00MHz revision : 2.2 (pvr 0039 0202) timebase : 1 platform : PowerMac model : PowerMac3,1 machine : PowerMac3,1 motherboard : PowerMac3,1 MacRISC MacRISC2 Power Macintosh detected as : 0 ((null)) pmac flags : 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
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-lviv...@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: ---- >> 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 ffdf call-method color!: exception -21 ... call-method block-size: exception -21 >> call-method block-size failed with error ffdf call-method block-size: exception -21 >> call-method block-size failed with error ffdf 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 namelength base( size ) system /dev/sda1 Apple_partition_map Apple 63 @ 1 ( 31.5k) Partition map /dev/sda2 Apple_HFS bootstrap 51 @ 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 Extra1 @ 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.00MHz revision: 2.2 (pvr 0039 0202) timebase : 1 platform: PowerMac model : PowerMac3,1 machine : PowerMac3,1 motherboard : PowerMac3,1 MacRISC MacRISC2 Power Macintosh detected as : 0 ((null)) pmac flags : 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
On Fri, 7 Jan 2022 at 07:29, 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:0x6c04 rebuilt:0x6c00) 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
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 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 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-lviv...@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-lviv...@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-de...@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
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-lviv...@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
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 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 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-lviv...@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-lviv...@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
On 1/7/22 14:39, Greg Kurz wrote: On Fri, 7 Jan 2022 23:19:03 +1100 David Gibson 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 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
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 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 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-lviv...@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 signature.asc Description: PGP signature
Re: [PATCH qemu] spapr: Force 32bit when resetting a core
On 08/01/2022 00:39, Greg Kurz wrote: On Fri, 7 Jan 2022 23:19:03 +1100 David Gibson 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 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-lviv...@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
On Fri, 7 Jan 2022 23:19:03 +1100 David Gibson 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 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 ? pgpqJVUnHyu5r.pgp Description: OpenPGP digital signature
Re: [PATCH qemu] spapr: Force 32bit when resetting a core
On Fri, Jan 07, 2022 at 12:57:47PM +0100, Greg Kurz wrote: > On Fri, 7 Jan 2022 18:24:23 +1100 > 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. > > 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 signature.asc Description: PGP signature
Re: [PATCH qemu] spapr: Force 32bit when resetting a core
On Fri, 7 Jan 2022 18:24:23 +1100 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. 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:0x6c04 rebuilt:0x6c00) > > Signed-off-by: Alexey Kardashevskiy > --- > 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
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:0x6c04 rebuilt:0x6c00)
Signed-off-by: Alexey Kardashevskiy
---
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:
