On 10/22/2012 11:23 AM, Liu Ping Fan wrote:
> Use local lock to protect e1000. When calling the system function,
> dropping the fine lock before acquiring the big lock. This will
> introduce broken device state, which need extra effort to fix.
>
> Signed-off-by: Liu Ping Fan <[email protected]>
> ---
> hw/e1000.c | 24 +++++++++++++++++++++++-
> 1 files changed, 23 insertions(+), 1 deletions(-)
>
> diff --git a/hw/e1000.c b/hw/e1000.c
> index ae8a6c5..5eddab5 100644
> --- a/hw/e1000.c
> +++ b/hw/e1000.c
> @@ -85,6 +85,7 @@ typedef struct E1000State_st {
> NICConf conf;
> MemoryRegion mmio;
> MemoryRegion io;
> + QemuMutex e1000_lock;
Can call it 'lock'.
>
> uint32_t mac_reg[0x8000];
> uint16_t phy_reg[0x20];
> @@ -223,13 +224,27 @@ static const uint32_t mac_reg_init[] = {
> static void
> set_interrupt_cause(E1000State *s, int index, uint32_t val)
> {
> + QemuThread *t;
> +
> if (val && (E1000_DEVID >= E1000_DEV_ID_82547EI_MOBILE)) {
> /* Only for 8257x */
> val |= E1000_ICR_INT_ASSERTED;
> }
> s->mac_reg[ICR] = val;
> s->mac_reg[ICS] = val;
> - qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) != 0);
> +
> + t = pthread_getspecific(qemu_thread_key);
> + if (t->context_type == 1) {
> + qemu_mutex_unlock(&s->e1000_lock);
> + qemu_mutex_lock_iothread();
> + }
> + if (DEVICE(s)->state < DEV_STATE_STOPPING) {
> + qemu_set_irq(s->dev.irq[0], (s->mac_reg[IMS] & s->mac_reg[ICR]) !=
> 0);
> + }
> + if (t->context_type == 1) {
> + qemu_mutex_unlock_iothread();
> + qemu_mutex_lock(&s->e1000_lock);
> + }
> }
This is way too complicated for device model authors. There's no way to
get it correct.
If mmio dispatch needs to call a non-thread-safe subsystem, it must
acquire the big lock:
Something like
e1000_mmio_read()
{
if (index < NREADOPS && macreg_readops[index]){
macreg_lockops[index].lock(s);
ret = macreg_readops[index](s, index);
macreg_lockops[index].unlock(s);
}
DBGOUT(UNKNOWN, "MMIO unknown read addr=0x%08x\n", index<<2);
}
Where .lock() either locks just the local lock, or both locks. As
subsystems are converted to be thread safe, we can remove this.
--
error compiling committee.c: too many arguments to function
