| Message ID | 20221213125218.39868-3-philmd@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | hw/ppc: Remove tswap() calls | expand |
On Tue, 13 Dec 2022 at 12:52, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > The tswap64() calls introduced in commit 4be21d561d ("pseries: > savevm support for pseries machine") are used to store the HTAB > in the migration stream (see savevm_htab_handlers) and are in > big-endian format. I think they're reading the run-time spapr->htab data structure (some of which is stuck onto the wire as a stream-of-bytes buffer and some of which is not). But either way, it's a target-endian data structure, because the code in hw/ppc/spapr_softmmu.c which reads and writes entries in it is using ldq_p() and stq_p(), and the current in-tree version of these macros is doing a "read host 64-bit and convert to/from target endianness wih tswap64". > #define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2)) > -#define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) > -#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) > -#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY)) > -#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY)) > +#define HPTE_VALID(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) > +#define HPTE_DIRTY(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) > +#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= cpu_to_be64(~HPTE64_V_HPTE_DIRTY)) > +#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= cpu_to_be64(HPTE64_V_HPTE_DIRTY)) This means we now have one file that's accessing this data structure as "this is target-endian", and one file that's accessing it as "this is big-endian". It happens that that ends up meaning the same thing because PPC is always TARGET_BIG_ENDIAN, but it seems a bit inconsistent. We should decide whether we're thinking of the data structure as target-endian or big-endian and change all the accessors appropriately (or none of them -- currently we're completely consistent about treating it as "target endian", I think). I also think that "cast a pointer into a byte-array to uint64_t* and then dereference it" is less preferable than using ldq_p() and stq_p(), but that's arguably a separate thing. thanks -- PMM
On 12/13/22 10:51, Peter Maydell wrote: > On Tue, 13 Dec 2022 at 12:52, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: >> >> The tswap64() calls introduced in commit 4be21d561d ("pseries: >> savevm support for pseries machine") are used to store the HTAB >> in the migration stream (see savevm_htab_handlers) and are in >> big-endian format. > > I think they're reading the run-time spapr->htab data structure > (some of which is stuck onto the wire as a stream-of-bytes buffer > and some of which is not). But either way, it's a target-endian > data structure, because the code in hw/ppc/spapr_softmmu.c which > reads and writes entries in it is using ldq_p() and stq_p(), > and the current in-tree version of these macros is doing a > "read host 64-bit and convert to/from target endianness wih tswap64". > >> #define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2)) >> -#define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) >> -#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) >> -#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY)) >> -#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY)) >> +#define HPTE_VALID(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) >> +#define HPTE_DIRTY(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) >> +#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= cpu_to_be64(~HPTE64_V_HPTE_DIRTY)) >> +#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= cpu_to_be64(HPTE64_V_HPTE_DIRTY)) > > This means we now have one file that's accessing this data structure > as "this is target-endian", and one file that's accessing it as > "this is big-endian". It happens that that ends up meaning the same > thing because PPC is always TARGET_BIG_ENDIAN, but it seems a bit > inconsistent. > > We should decide whether we're thinking of the data structure > as target-endian or big-endian and change all the accessors > appropriately (or none of them -- currently we're completely > consistent about treating it as "target endian", I think). Yes, most if not all accesses are being handled as "target endian", even though the target is always big endian. IIUC the idea behind Phil's cleanups is exactly to replace uses of "target-something" if the endianess of the host is irrelevant, which is the case for ppc64. We would then change the semantics of the code gradually to make it consistent again. However, I don't feel comfortable acking this patch alone since 4be21d561d is from David and I don't know if there's a great design behind the use of tswap64() to manipulate the hpte. David, would you care to comment? Daniel > > I also think that "cast a pointer into a byte-array to uint64_t* > and then dereference it" is less preferable than using ldq_p() > and stq_p(), but that's arguably a separate thing. > > thanks > -- PMM
On Fri, 16 Dec 2022 at 19:11, Daniel Henrique Barboza <danielhb413@gmail.com> wrote: > > > > On 12/13/22 10:51, Peter Maydell wrote: > > On Tue, 13 Dec 2022 at 12:52, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > >> > >> The tswap64() calls introduced in commit 4be21d561d ("pseries: > >> savevm support for pseries machine") are used to store the HTAB > >> in the migration stream (see savevm_htab_handlers) and are in > >> big-endian format. > > > > I think they're reading the run-time spapr->htab data structure > > (some of which is stuck onto the wire as a stream-of-bytes buffer > > and some of which is not). But either way, it's a target-endian > > data structure, because the code in hw/ppc/spapr_softmmu.c which > > reads and writes entries in it is using ldq_p() and stq_p(), > > and the current in-tree version of these macros is doing a > > "read host 64-bit and convert to/from target endianness wih tswap64". > > > >> #define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2)) > >> -#define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) > >> -#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) > >> -#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY)) > >> -#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY)) > >> +#define HPTE_VALID(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) > >> +#define HPTE_DIRTY(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) > >> +#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= cpu_to_be64(~HPTE64_V_HPTE_DIRTY)) > >> +#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= cpu_to_be64(HPTE64_V_HPTE_DIRTY)) > > > > This means we now have one file that's accessing this data structure > > as "this is target-endian", and one file that's accessing it as > > "this is big-endian". It happens that that ends up meaning the same > > thing because PPC is always TARGET_BIG_ENDIAN, but it seems a bit > > inconsistent. > > > > We should decide whether we're thinking of the data structure > > as target-endian or big-endian and change all the accessors > > appropriately (or none of them -- currently we're completely > > consistent about treating it as "target endian", I think). > > Yes, most if not all accesses are being handled as "target endian", even > though the target is always big endian. > > IIUC the idea behind Phil's cleanups is exactly to replace uses of > "target-something" if the endianess of the host is irrelevant, which > is the case for ppc64. We would then change the semantics of the code > gradually to make it consistent again. I would be happier if we just did all the functions that read and write this byte array at once -- there are not many of them. thanks -- PMM
On Fri, Dec 16, 2022 at 09:39:19PM +0000, Peter Maydell wrote: > On Fri, 16 Dec 2022 at 19:11, Daniel Henrique Barboza > <danielhb413@gmail.com> wrote: > > > > > > > > On 12/13/22 10:51, Peter Maydell wrote: > > > On Tue, 13 Dec 2022 at 12:52, Philippe Mathieu-Daudé <philmd@linaro.org> wrote: > > >> > > >> The tswap64() calls introduced in commit 4be21d561d ("pseries: > > >> savevm support for pseries machine") are used to store the HTAB > > >> in the migration stream (see savevm_htab_handlers) and are in > > >> big-endian format. > > > > > > I think they're reading the run-time spapr->htab data structure > > > (some of which is stuck onto the wire as a stream-of-bytes buffer > > > and some of which is not). But either way, it's a target-endian > > > data structure, because the code in hw/ppc/spapr_softmmu.c which > > > reads and writes entries in it is using ldq_p() and stq_p(), > > > and the current in-tree version of these macros is doing a > > > "read host 64-bit and convert to/from target endianness wih tswap64". > > > > > >> #define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2)) > > >> -#define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) > > >> -#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) > > >> -#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY)) > > >> -#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY)) > > >> +#define HPTE_VALID(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) > > >> +#define HPTE_DIRTY(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) > > >> +#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= cpu_to_be64(~HPTE64_V_HPTE_DIRTY)) > > >> +#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= cpu_to_be64(HPTE64_V_HPTE_DIRTY)) > > > > > > This means we now have one file that's accessing this data structure > > > as "this is target-endian", and one file that's accessing it as > > > "this is big-endian". It happens that that ends up meaning the same > > > thing because PPC is always TARGET_BIG_ENDIAN, but it seems a bit > > > inconsistent. > > > > > > We should decide whether we're thinking of the data structure > > > as target-endian or big-endian and change all the accessors > > > appropriately (or none of them -- currently we're completely > > > consistent about treating it as "target endian", I think). > > > > Yes, most if not all accesses are being handled as "target endian", even > > though the target is always big endian. So "target is always big endian" is pretty misleading for POWER. We always define "TARGET_BIG_ENDIAN" in qemu, but for at least 10 years the CPUs have been capable of running in either big endian or little endian mode (selected at runtime). Some variants can choose endianness on a per-page basis. Since the creation of the ISA it's had "byte reversed" load and store instructions that let it use little endian for specific memory accesses. Really the whole notion of an ISA having an "endianness" doesn't make a lot of sense - it's an individual load or store to memory that has an endianness which can depend on a bunch of factors. When these macros were created, an ISA nearly always used the same endianness, but that's not really true any more - not just for POWER, but for a bunch of targets. So from that point of view, I think getting rid of tswap() - particularly one that has compile time semantics, rather than behaviour which can depend on cpu mode/state is a good idea. I believe that even when running in little-endian mode, the hash page tables are encoded in big-endian, so I think the proposed change makes sense. > > IIUC the idea behind Phil's cleanups is exactly to replace uses of > > "target-something" if the endianess of the host is irrelevant, which > > is the case for ppc64. We would then change the semantics of the code > > gradually to make it consistent again. > > I would be happier if we just did all the functions that read and > write this byte array at once -- there are not many of them. > > thanks > -- PMM >
On Mon, 19 Dec 2022 at 06:35, David Gibson <david@gibson.dropbear.id.au> wrote: > > On Fri, Dec 16, 2022 at 09:39:19PM +0000, Peter Maydell wrote: > > On Fri, 16 Dec 2022 at 19:11, Daniel Henrique Barboza > > <danielhb413@gmail.com> wrote: > > > > > > > > > > > > On 12/13/22 10:51, Peter Maydell wrote: > > > Yes, most if not all accesses are being handled as "target endian", even > > > though the target is always big endian. > > So "target is always big endian" is pretty misleading for POWER. We > always define "TARGET_BIG_ENDIAN" in qemu, but for at least 10 years > the CPUs have been capable of running in either big endian or little > endian mode (selected at runtime). Some variants can choose > endianness on a per-page basis. Since the creation of the ISA it's > had "byte reversed" load and store instructions that let it use little > endian for specific memory accesses. Yeah, this is like Arm (and for the purposes of this thread I meant essentially "TARGET_BIG_ENDIAN is always defined"). > Really the whole notion of an ISA having an "endianness" doesn't make > a lot of sense - it's an individual load or store to memory that has > an endianness which can depend on a bunch of factors. When these > macros were created, an ISA nearly always used the same endianness, > but that's not really true any more - not just for POWER, but for a > bunch of targets. So from that point of view, I think getting rid of > tswap() - particularly one that has compile time semantics, rather > than behaviour which can depend on cpu mode/state is a good idea. I tend to think of the TARGET_BIG_ENDIAN/not setting as being something like "CPU bus endianness". At least for Arm, when you put the CPU into BE mode it pretty much means "the CPU byteswaps the data when it comes in/out", AIUI. > I believe that even when running in little-endian mode, the hash page > tables are encoded in big-endian, so I think the proposed change makes > sense. OK. I still think we should consistently change all the places that are accessing this data structure, though, not just half of them. thanks -- PMM
On Mon, Dec 19, 2022 at 10:39:40AM +0000, Peter Maydell wrote: > On Mon, 19 Dec 2022 at 06:35, David Gibson <david@gibson.dropbear.id.au> wrote: > > > > On Fri, Dec 16, 2022 at 09:39:19PM +0000, Peter Maydell wrote: > > > On Fri, 16 Dec 2022 at 19:11, Daniel Henrique Barboza > > > <danielhb413@gmail.com> wrote: > > > > > > > > > > > > > > > > On 12/13/22 10:51, Peter Maydell wrote: > > > > Yes, most if not all accesses are being handled as "target endian", even > > > > though the target is always big endian. > > > > So "target is always big endian" is pretty misleading for POWER. We > > always define "TARGET_BIG_ENDIAN" in qemu, but for at least 10 years > > the CPUs have been capable of running in either big endian or little > > endian mode (selected at runtime). Some variants can choose > > endianness on a per-page basis. Since the creation of the ISA it's > > had "byte reversed" load and store instructions that let it use little > > endian for specific memory accesses. > > Yeah, this is like Arm (and for the purposes of this thread > I meant essentially "TARGET_BIG_ENDIAN is always defined"). Ok. > > Really the whole notion of an ISA having an "endianness" doesn't make > > a lot of sense - it's an individual load or store to memory that has > > an endianness which can depend on a bunch of factors. When these > > macros were created, an ISA nearly always used the same endianness, > > but that's not really true any more - not just for POWER, but for a > > bunch of targets. So from that point of view, I think getting rid of > > tswap() - particularly one that has compile time semantics, rather > > than behaviour which can depend on cpu mode/state is a good idea. > > I tend to think of the TARGET_BIG_ENDIAN/not setting as being > something like "CPU bus endianness". At least for Arm, when you > put the CPU into BE mode it pretty much means "the CPU byteswaps > the data when it comes in/out", AIUI. Hmm, I guess. We're not really modelling down to the level of bus byte lanes, though, so I'm not really convinced that's a meaningful definition in the context of qemu. > > I believe that even when running in little-endian mode, the hash page > > tables are encoded in big-endian, so I think the proposed change makes > > sense. > > OK. I still think we should consistently change all the places that are > accessing this data structure, though, not just half of them. Yes, that makes sense. Although what exactly constitutes "this data structure" is a bit complex here. If we mean just the spapr specific "external HPT", then there are only a few more references to it. If we mean all instances of a powerpc hashed page table, then there are a bunch more in the cpu target code.
On Wed, 21 Dec 2022 at 01:35, David Gibson <david@gibson.dropbear.id.au> wrote: > On Mon, Dec 19, 2022 at 10:39:40AM +0000, Peter Maydell wrote: > > OK. I still think we should consistently change all the places that are > > accessing this data structure, though, not just half of them. > > Yes, that makes sense. Although what exactly constitutes "this data > structure" is a bit complex here. If we mean just the spapr specific > "external HPT", then there are only a few more references to it. If > we mean all instances of a powerpc hashed page table, then there are a > bunch more in the cpu target code. I had in mind "places where we write this specific array of bytes spapr->htab". thanks -- PMM
On 12/21/22 13:33, Peter Maydell wrote: > On Wed, 21 Dec 2022 at 01:35, David Gibson <david@gibson.dropbear.id.au> wrote: >> On Mon, Dec 19, 2022 at 10:39:40AM +0000, Peter Maydell wrote: >>> OK. I still think we should consistently change all the places that are >>> accessing this data structure, though, not just half of them. >> >> Yes, that makes sense. Although what exactly constitutes "this data >> structure" is a bit complex here. If we mean just the spapr specific >> "external HPT", then there are only a few more references to it. If >> we mean all instances of a powerpc hashed page table, then there are a >> bunch more in the cpu target code. > > I had in mind "places where we write this specific array of bytes > spapr->htab". spapr_store_hpte() seems to be the most annoying part. It is used by hcalls h_enter, h_remove, h_protect. Reworking the interface to present pte0/pte1 as BE variables means reworking the whole hw/ppc/spapr_softmmu.c file. That's feasible but not a small task since the changes will root down in the target hash mmu code which is shared by all platforms ... :/ spapr_hpte_set_c() are spapr_hpte_set_r() are of a different kind. C.
On Wed, 21 Dec 2022 at 16:03, Cédric Le Goater <clg@kaod.org> wrote: > > On 12/21/22 13:33, Peter Maydell wrote: > > On Wed, 21 Dec 2022 at 01:35, David Gibson <david@gibson.dropbear.id.au> wrote: > >> On Mon, Dec 19, 2022 at 10:39:40AM +0000, Peter Maydell wrote: > >>> OK. I still think we should consistently change all the places that are > >>> accessing this data structure, though, not just half of them. > >> > >> Yes, that makes sense. Although what exactly constitutes "this data > >> structure" is a bit complex here. If we mean just the spapr specific > >> "external HPT", then there are only a few more references to it. If > >> we mean all instances of a powerpc hashed page table, then there are a > >> bunch more in the cpu target code. > > > > I had in mind "places where we write this specific array of bytes > > spapr->htab". > > > spapr_store_hpte() seems to be the most annoying part. It is used > by hcalls h_enter, h_remove, h_protect. Reworking the interface > to present pte0/pte1 as BE variables means reworking the whole > hw/ppc/spapr_softmmu.c file. That's feasible but not a small task > since the changes will root down in the target hash mmu code which > is shared by all platforms ... :/ Don't you just need to change spapr_store_hpte() to use stq_be_p() instead of stq_p() ? > spapr_hpte_set_c() are spapr_hpte_set_r() are of a different kind. That code seems to suggest we already implicitly assume that spapr->htab fields have a given endianness... thanks -- PMM
On Wed, Dec 21, 2022 at 10:15:28PM +0000, Peter Maydell wrote: > On Wed, 21 Dec 2022 at 16:03, Cédric Le Goater <clg@kaod.org> wrote: > > > > On 12/21/22 13:33, Peter Maydell wrote: > > > On Wed, 21 Dec 2022 at 01:35, David Gibson <david@gibson.dropbear.id.au> wrote: > > >> On Mon, Dec 19, 2022 at 10:39:40AM +0000, Peter Maydell wrote: > > >>> OK. I still think we should consistently change all the places that are > > >>> accessing this data structure, though, not just half of them. > > >> > > >> Yes, that makes sense. Although what exactly constitutes "this data > > >> structure" is a bit complex here. If we mean just the spapr specific > > >> "external HPT", then there are only a few more references to it. If > > >> we mean all instances of a powerpc hashed page table, then there are a > > >> bunch more in the cpu target code. > > > > > > I had in mind "places where we write this specific array of bytes > > > spapr->htab". Seems a reasonable amount to tackle for now. > > spapr_store_hpte() seems to be the most annoying part. It is used > > by hcalls h_enter, h_remove, h_protect. Reworking the interface > > to present pte0/pte1 as BE variables means reworking the whole > > hw/ppc/spapr_softmmu.c file. That's feasible but not a small task > > since the changes will root down in the target hash mmu code which > > is shared by all platforms ... :/ > > Don't you just need to change spapr_store_hpte() to use stq_be_p() > instead of stq_p() ? I think Peter is right. The values passed to the function are "host endian" (really, they don't have an endianness since they'll be in registers). > > spapr_hpte_set_c() are spapr_hpte_set_r() are of a different kind. > > That code seems to suggest we already implicitly assume that > spapr->htab fields have a given endianness... Yes, we absolutely do. We rely on the HPTE always being big-endian.
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 66b414d2e9..8b1d5689d2 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -39,6 +39,7 @@ #include "sysemu/reset.h" #include "sysemu/runstate.h" #include "qemu/log.h" +#include "qemu/bswap.h" #include "hw/fw-path-provider.h" #include "elf.h" #include "net/net.h" @@ -1357,10 +1358,10 @@ static bool spapr_get_pate(PPCVirtualHypervisor *vhyp, PowerPCCPU *cpu, } #define HPTE(_table, _i) (void *)(((uint64_t *)(_table)) + ((_i) * 2)) -#define HPTE_VALID(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) -#define HPTE_DIRTY(_hpte) (tswap64(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) -#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= tswap64(~HPTE64_V_HPTE_DIRTY)) -#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= tswap64(HPTE64_V_HPTE_DIRTY)) +#define HPTE_VALID(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_VALID) +#define HPTE_DIRTY(_hpte) (be64_to_cpu(*((uint64_t *)(_hpte))) & HPTE64_V_HPTE_DIRTY) +#define CLEAN_HPTE(_hpte) ((*(uint64_t *)(_hpte)) &= cpu_to_be64(~HPTE64_V_HPTE_DIRTY)) +#define DIRTY_HPTE(_hpte) ((*(uint64_t *)(_hpte)) |= cpu_to_be64(HPTE64_V_HPTE_DIRTY)) /* * Get the fd to access the kernel htab, re-opening it if necessary
The tswap64() calls introduced in commit 4be21d561d ("pseries: savevm support for pseries machine") are used to store the HTAB in the migration stream (see savevm_htab_handlers) and are in big-endian format. Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org> --- hw/ppc/spapr.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)