[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH for-5.0] hw/ppc/ppc440_uc.c: Remove incorrect iothread lockin
From: |
David Gibson |
Subject: |
Re: [PATCH for-5.0] hw/ppc/ppc440_uc.c: Remove incorrect iothread locking from dcr_write_pcie() |
Date: |
Tue, 31 Mar 2020 10:41:12 +1100 |
On Mon, Mar 30, 2020 at 01:52:28PM +0100, Peter Maydell wrote:
> In dcr_write_pcie() we take the iothread lock around a call to
> pcie_host_mmcfg_udpate(). This is an incorrect attempt to deal with
> the bug fixed in commit 235352ee6e73d7716, where we were not taking
> the iothread lock before calling device dcr read/write functions.
> (It's not sufficient locking, because although the other cases in the
> switch statement won't assert, there is no locking which prevents
> multiple guest CPUs from trying to access the PPC460EXPCIEState
> struct at the same time and corrupting data.)
>
> Unfortunately with commit 235352ee6e73d7716 we are now trying
> to recursively take the iothread lock, which will assert:
>
> $ qemu-system-ppc -M sam460ex --display none
> **
>
> ERROR:/home/petmay01/linaro/qemu-from-laptop/qemu/cpus.c:1830:qemu_mutex_lock_iothread_impl:
> assertion failed: (!qemu_mutex_iothread_locked())
> Aborted (core dumped)
>
> Remove the locking within dcr_write_pcie().
>
> Fixes: 235352ee6e73d7716
> Signed-off-by: Peter Maydell <address@hidden>
> ---
> I did a grep of hw/ppc and didn't see anything else that was doing
> its own locking inside a dcr read/write fn.
> ---
Applied to ppc-for-5.0, thanks.
> hw/ppc/ppc440_uc.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/hw/ppc/ppc440_uc.c b/hw/ppc/ppc440_uc.c
> index d5ea962249f..b30e093cbb0 100644
> --- a/hw/ppc/ppc440_uc.c
> +++ b/hw/ppc/ppc440_uc.c
> @@ -13,7 +13,6 @@
> #include "qemu/error-report.h"
> #include "qapi/error.h"
> #include "qemu/log.h"
> -#include "qemu/main-loop.h"
> #include "qemu/module.h"
> #include "cpu.h"
> #include "hw/irq.h"
> @@ -1183,9 +1182,7 @@ static void dcr_write_pcie(void *opaque, int dcrn,
> uint32_t val)
> case PEGPL_CFGMSK:
> s->cfg_mask = val;
> size = ~(val & 0xfffffffe) + 1;
> - qemu_mutex_lock_iothread();
> pcie_host_mmcfg_update(PCIE_HOST_BRIDGE(s), val & 1, s->cfg_base,
> size);
> - qemu_mutex_unlock_iothread();
> break;
> case PEGPL_MSGBAH:
> s->msg_base = ((uint64_t)val << 32) | (s->msg_base & 0xffffffff);
--
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