[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] AHCI: Masking of IRQs actually masks them
From: |
Alexander Graf |
Subject: |
Re: [Qemu-devel] [PATCH] AHCI: Masking of IRQs actually masks them |
Date: |
Thu, 9 Feb 2012 15:52:36 +0100 |
On 09.02.2012, at 15:41, Kevin Wolf wrote:
> Am 30.01.2012 23:29, schrieb Alexander Graf:
>> When masking IRQ lines, we should actually mask them out and not declare
>> them active anymore. Once we mask them in again, they are allowed to trigger
>> again.
>>
>> Signed-off-by: Alexander Graf <address@hidden>
>> ---
>> hw/ide/ahci.c | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/hw/ide/ahci.c b/hw/ide/ahci.c
>> index c2c168d..f8e9eb4 100644
>> --- a/hw/ide/ahci.c
>> +++ b/hw/ide/ahci.c
>> @@ -146,6 +146,7 @@ static void ahci_check_irq(AHCIState *s)
>>
>> DPRINTF(-1, "check irq %#x\n", s->control_regs.irqstatus);
>>
>> + s->control_regs.irqstatus = 0;
>> for (i = 0; i < s->ports; i++) {
>> AHCIPortRegs *pr = &s->dev[i].port_regs;
>> if (pr->irq_stat & pr->irq_mask) {
>
> Is this an independent bug fix?
No, it's the same thing. Without this we only OR new interrupts into irqstatus.
This way we also allow clearing of them when the producer vanishes / gets
masked.
>
>> @@ -216,6 +217,7 @@ static void ahci_port_write(AHCIState *s, int port, int
>> offset, uint32_t val)
>> break;
>> case PORT_IRQ_STAT:
>> pr->irq_stat &= ~val;
>> + ahci_check_irq(s);
>> break;
>> case PORT_IRQ_MASK:
>> pr->irq_mask = val & 0xfdc000ff;
>
> Makes some sense, but isn't really about masking interrupts either?
> (From the commit message I would have expected that you touch PORT_IRQ_MASK)
There are multiple layers of IRQ lines, wired to each other, with each layer
being able to mask specific bits. We didn't catch the part where irq_stat was
losing a bit, either by masking or by explicit unsetting.
Alex