[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000
From: |
Avi Kivity |
Subject: |
Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000 |
Date: |
Mon, 22 Oct 2012 12:37:56 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
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 <address@hidden>
> ---
> 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
- [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, (continued)
- [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Liu Ping Fan, 2012/10/22
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Jan Kiszka, 2012/10/22
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Peter Maydell, 2012/10/22
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, liu ping fan, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Paolo Bonzini, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Peter Maydell, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Jan Kiszka, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Paolo Bonzini, 2012/10/23
- Re: [Qemu-devel] [patch v4 08/16] QemuThread: make QemuThread as tls to store extra info, Peter Maydell, 2012/10/23
[Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Liu Ping Fan, 2012/10/22
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000,
Avi Kivity <=
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/23
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, liu ping fan, 2012/10/24
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/24
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Avi Kivity, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, Jan Kiszka, 2012/10/25
- Re: [Qemu-devel] [patch v4 12/16] e1000: apply fine lock on e1000, liu ping fan, 2012/10/29