Message ID | 20200322192258.14039-1-peter.maydell@linaro.org |
---|---|
State | Superseded |
Headers | show |
Series | [for-5.0] hw/ppc: Take QEMU lock when calling ppc_dcr_read/write() | expand |
On Sun, Mar 22, 2020 at 07:22:58PM +0000, Peter Maydell wrote: > The ppc_dcr_read() and ppc_dcr_write() functions call into callbacks > in device code, so we need to hold the QEMU iothread lock while > calling them. This is the case already for the callsites in > kvmppc_handle_dcr_read/write(), but we must also take the lock when > calling the helpers from TCG. > > This fixes a bug where attempting to initialise the PPC405EP > SDRAM will cause an assertion when sdram_map_bcr() attempts > to remap memory regions. > > Reported-by: Amit Lazar <abasarlaz@hotmail.com> > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Applied to ppc-for-5.0. > --- > Amit reported this bug via IRC. > > target/ppc/timebase_helper.c | 40 +++++++++++++++++++++++------------- > 1 file changed, 26 insertions(+), 14 deletions(-) > > diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c > index 703bd9ed18b..d16360ab667 100644 > --- a/target/ppc/timebase_helper.c > +++ b/target/ppc/timebase_helper.c > @@ -21,6 +21,7 @@ > #include "exec/helper-proto.h" > #include "exec/exec-all.h" > #include "qemu/log.h" > +#include "qemu/main-loop.h" > > /*****************************************************************************/ > /* SPR accesses */ > @@ -167,13 +168,19 @@ target_ulong helper_load_dcr(CPUPPCState *env, target_ulong dcrn) > raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, > POWERPC_EXCP_INVAL | > POWERPC_EXCP_INVAL_INVAL, GETPC()); > - } else if (unlikely(ppc_dcr_read(env->dcr_env, > - (uint32_t)dcrn, &val) != 0)) { > - qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n", > - (uint32_t)dcrn, (uint32_t)dcrn); > - raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, > - POWERPC_EXCP_INVAL | > - POWERPC_EXCP_PRIV_REG, GETPC()); > + } else { > + int ret; > + > + qemu_mutex_lock_iothread(); > + ret = ppc_dcr_read(env->dcr_env, (uint32_t)dcrn, &val); > + qemu_mutex_unlock_iothread(); > + if (unlikely(ret != 0)) { > + qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n", > + (uint32_t)dcrn, (uint32_t)dcrn); > + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, > + POWERPC_EXCP_INVAL | > + POWERPC_EXCP_PRIV_REG, GETPC()); > + } > } > return val; > } > @@ -185,12 +192,17 @@ void helper_store_dcr(CPUPPCState *env, target_ulong dcrn, target_ulong val) > raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, > POWERPC_EXCP_INVAL | > POWERPC_EXCP_INVAL_INVAL, GETPC()); > - } else if (unlikely(ppc_dcr_write(env->dcr_env, (uint32_t)dcrn, > - (uint32_t)val) != 0)) { > - qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d %03x\n", > - (uint32_t)dcrn, (uint32_t)dcrn); > - raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, > - POWERPC_EXCP_INVAL | > - POWERPC_EXCP_PRIV_REG, GETPC()); > + } else { > + int ret; > + qemu_mutex_lock_iothread(); > + ret = ppc_dcr_write(env->dcr_env, (uint32_t)dcrn, (uint32_t)val); > + qemu_mutex_unlock_iothread(); > + if (unlikely(ret != 0)) { > + qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d %03x\n", > + (uint32_t)dcrn, (uint32_t)dcrn); > + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, > + POWERPC_EXCP_INVAL | > + POWERPC_EXCP_PRIV_REG, GETPC()); > + } > } > } -- 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
diff --git a/target/ppc/timebase_helper.c b/target/ppc/timebase_helper.c index 703bd9ed18b..d16360ab667 100644 --- a/target/ppc/timebase_helper.c +++ b/target/ppc/timebase_helper.c @@ -21,6 +21,7 @@ #include "exec/helper-proto.h" #include "exec/exec-all.h" #include "qemu/log.h" +#include "qemu/main-loop.h" /*****************************************************************************/ /* SPR accesses */ @@ -167,13 +168,19 @@ target_ulong helper_load_dcr(CPUPPCState *env, target_ulong dcrn) raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL, GETPC()); - } else if (unlikely(ppc_dcr_read(env->dcr_env, - (uint32_t)dcrn, &val) != 0)) { - qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n", - (uint32_t)dcrn, (uint32_t)dcrn); - raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, - POWERPC_EXCP_INVAL | - POWERPC_EXCP_PRIV_REG, GETPC()); + } else { + int ret; + + qemu_mutex_lock_iothread(); + ret = ppc_dcr_read(env->dcr_env, (uint32_t)dcrn, &val); + qemu_mutex_unlock_iothread(); + if (unlikely(ret != 0)) { + qemu_log_mask(LOG_GUEST_ERROR, "DCR read error %d %03x\n", + (uint32_t)dcrn, (uint32_t)dcrn); + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, + POWERPC_EXCP_INVAL | + POWERPC_EXCP_PRIV_REG, GETPC()); + } } return val; } @@ -185,12 +192,17 @@ void helper_store_dcr(CPUPPCState *env, target_ulong dcrn, target_ulong val) raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL, GETPC()); - } else if (unlikely(ppc_dcr_write(env->dcr_env, (uint32_t)dcrn, - (uint32_t)val) != 0)) { - qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d %03x\n", - (uint32_t)dcrn, (uint32_t)dcrn); - raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, - POWERPC_EXCP_INVAL | - POWERPC_EXCP_PRIV_REG, GETPC()); + } else { + int ret; + qemu_mutex_lock_iothread(); + ret = ppc_dcr_write(env->dcr_env, (uint32_t)dcrn, (uint32_t)val); + qemu_mutex_unlock_iothread(); + if (unlikely(ret != 0)) { + qemu_log_mask(LOG_GUEST_ERROR, "DCR write error %d %03x\n", + (uint32_t)dcrn, (uint32_t)dcrn); + raise_exception_err_ra(env, POWERPC_EXCP_PROGRAM, + POWERPC_EXCP_INVAL | + POWERPC_EXCP_PRIV_REG, GETPC()); + } } }
The ppc_dcr_read() and ppc_dcr_write() functions call into callbacks in device code, so we need to hold the QEMU iothread lock while calling them. This is the case already for the callsites in kvmppc_handle_dcr_read/write(), but we must also take the lock when calling the helpers from TCG. This fixes a bug where attempting to initialise the PPC405EP SDRAM will cause an assertion when sdram_map_bcr() attempts to remap memory regions. Reported-by: Amit Lazar <abasarlaz@hotmail.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- Amit reported this bug via IRC. target/ppc/timebase_helper.c | 40 +++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 14 deletions(-) -- 2.20.1