Skip to content

Commit

Permalink
WIP dde_linux: fix IRQ masking/unmasking
Browse files Browse the repository at this point in the history
Unmasking of a pending interrupt did not lead to immediate IRQ handler
execution in all cases.

Issue #5164
  • Loading branch information
chelmuth committed Mar 25, 2024
1 parent ac5ed43 commit fb00f4a
Show file tree
Hide file tree
Showing 12 changed files with 92 additions and 98 deletions.
2 changes: 2 additions & 0 deletions repos/dde_linux/src/include/lx_emul/irq.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ extern void * lx_emul_irq_task_struct;

int lx_emul_pending_irq(void);

void lx_emul_reset_pending_irq(unsigned int irq);

int lx_emul_irq_init(struct device_node *node, struct device_node *parent);

unsigned long lx_emul_irq_enable(void);
Expand Down
11 changes: 7 additions & 4 deletions repos/dde_linux/src/include/lx_kit/device.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
#include <util/list.h>
#include <util/xml_node.h>

#include <lx_kit/pending_irq.h>

namespace Lx_kit {
using namespace Genode;
Expand Down Expand Up @@ -65,17 +64,19 @@ class Lx_kit::Device : List<Device>::Element
using Index = Platform::Device::Irq::Index;

Index idx;
Pending_irq number;
unsigned number;
Io_signal_handler<Irq> handler;
bool masked { true };
bool occured { false };
bool pending { false };

This comment has been minimized.

Copy link
@skalk

skalk Mar 26, 2024

Member

@chelmuth: I know this existed before your change, but wouldn't it be even better to merge masked and occurred booleans into one state variable?

This comment has been minimized.

Copy link
@chelmuth

chelmuth Mar 26, 2024

Author Member

Are you referring to something like follows?

enum State {
  NO_IRQ, MASKED, PENDING, MASKED_PENDING
};

Note, we seem to require all four bool combinations in the code.

This comment has been minimized.

Copy link
@skalk

skalk Mar 26, 2024

Member

Yes, this is what I was thinking about.


Constructible<Platform::Device::Irq> session {};

Irq(Entrypoint & ep, unsigned idx, unsigned number);

void _handle();
void handle();
void mask();
void unmask(Platform::Device &);
void ack();
};

struct Io_port : List<Io_port>::Element
Expand Down Expand Up @@ -169,6 +170,8 @@ class Lx_kit::Device : List<Device>::Element
bool irq_unmask(unsigned irq);
void irq_mask(unsigned irq);
void irq_ack(unsigned irq);
int pending_irq();
void reset_pending(unsigned irq);

bool read_config(unsigned reg, unsigned len, unsigned *val);
bool write_config(unsigned reg, unsigned len, unsigned val);
Expand Down
1 change: 0 additions & 1 deletion repos/dde_linux/src/include/lx_kit/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@
#include <lx_kit/memory.h>
#include <lx_kit/scheduler.h>
#include <lx_kit/timeout.h>
#include <lx_kit/pending_irq.h>

namespace Lx_kit {

Expand Down
34 changes: 0 additions & 34 deletions repos/dde_linux/src/include/lx_kit/pending_irq.h

This file was deleted.

11 changes: 1 addition & 10 deletions repos/dde_linux/src/include/lx_kit/scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
#include <util/fifo.h>
#include <util/list.h>
#include <lx_kit/task.h>
#include <lx_kit/pending_irq.h>

namespace Lx_kit {
class Scheduler;
Expand All @@ -45,8 +44,6 @@ class Lx_kit::Scheduler

void _execute();

Genode::Fifo<Lx_kit::Pending_irq> _pending_irqs { };

public:

Task & current();
Expand All @@ -62,13 +59,7 @@ class Lx_kit::Scheduler

void execute();

void unblock_irq_handler(Pending_irq &);
template <typename FN> void pending_irq(FN const &fn)
{
_pending_irqs.dequeue([&] (Pending_irq const &pirq) {
fn(pirq.value); });
}

void unblock_irq_handler();
void unblock_time_handler();

Task & task(void * t);
Expand Down
13 changes: 11 additions & 2 deletions repos/dde_linux/src/lib/lx_emul/irq.cc
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,17 @@ extern "C" int lx_emul_pending_irq()
{
int pending_irq = -1;

Lx_kit::env().scheduler.pending_irq([&] (unsigned int irq) {
pending_irq = (int)irq; });
Lx_kit::env().devices.for_each([&] (Lx_kit::Device & d) {
if (pending_irq == -1)
pending_irq = d.pending_irq();
});

return pending_irq;
}


extern "C" void lx_emul_reset_pending_irq(unsigned int irq)
{
Lx_kit::env().devices.for_each([&] (Lx_kit::Device & d) {
d.reset_pending(irq); });
}
2 changes: 1 addition & 1 deletion repos/dde_linux/src/lib/lx_emul/pci.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ lx_emul_pci_for_each_device(void * bus, lx_emul_add_device_callback_t fn)
env().devices.for_each([&] (Device & d) {
unsigned irq = 0;
d.for_each_irq([&] (Device::Irq & i) {
if (!irq) irq = i.number.value; });
if (!irq) irq = i.number; });

d.for_pci_config([&] (Device::Pci_config & cfg) {
fn(bus, num++, d.name().string(), cfg.vendor_id, cfg.device_id,
Expand Down
9 changes: 3 additions & 6 deletions repos/dde_linux/src/lib/lx_emul/pin.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace {

Lx_kit::Env &_env;

Lx_kit::Pending_irq _pending_irq { 0 };
unsigned _pending_irq = 0;

public:

Expand All @@ -40,9 +40,9 @@ namespace {

void trigger_irq(Number number)
{
_pending_irq.value = number.value;
_pending_irq = number.value;

_env.scheduler.unblock_irq_handler(_pending_irq);
_env.scheduler.unblock_irq_handler();
_env.scheduler.schedule();
}
};
Expand Down Expand Up @@ -223,9 +223,6 @@ extern "C" void lx_emul_pin_control(char const *pin_name, bool enabled)
}


extern "C" void lx_emul_backtrace(void);


extern "C" int lx_emul_pin_sense(char const *pin_name)
{
bool result = false;
Expand Down
12 changes: 8 additions & 4 deletions repos/dde_linux/src/lib/lx_emul/spec/x86/irqchip.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ static void dde_irq_mask(struct irq_data *d)
}


static void dde_irq_ack(struct irq_data *d)
{
lx_emul_reset_pending_irq(d->hwirq);
}


int lx_emul_irq_init(struct device_node *node, struct device_node *parent)
{
return 0;
Expand All @@ -41,7 +47,7 @@ struct irq_chip dde_irqchip_data_chip = {
.irq_mask = dde_irq_mask,
.irq_disable = dde_irq_mask,
.irq_unmask = dde_irq_unmask,
.irq_mask_ack = dde_irq_mask,
.irq_ack = dde_irq_ack,
};


Expand All @@ -54,7 +60,6 @@ int lx_emul_irq_task_function(void * data)
lx_emul_task_schedule(true);

for (;;) {

irq = lx_emul_pending_irq();
if (irq == -1)
break;
Expand All @@ -64,8 +69,7 @@ int lx_emul_irq_task_function(void * data)

if (!irq) {
ack_bad_irq(irq);
WARN_ONCE(true, "Unexpected interrupt %d received!\n",
irq);
WARN_ONCE(true, "Unexpected interrupt %d received!\n", irq);
} else {
generic_handle_irq(irq);
lx_emul_irq_eoi(irq);
Expand Down
10 changes: 1 addition & 9 deletions repos/dde_linux/src/lib/lx_emul/virt/irq.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,4 @@ extern "C" void lx_emul_irq_mask(unsigned int ) { }
extern "C" void lx_emul_irq_eoi(unsigned int ) { }


extern "C" int lx_emul_pending_irq()
{
int pending_irq = -1;

Lx_kit::env().scheduler.pending_irq([&] (unsigned int irq) {
pending_irq = (int)irq; });

return pending_irq;
}
extern "C" int lx_emul_pending_irq() { return -1; }
81 changes: 57 additions & 24 deletions repos/dde_linux/src/lib/lx_kit/device.cc
Original file line number Diff line number Diff line change
Expand Up @@ -38,25 +38,43 @@ bool Device::Io_port::match(uint16_t addr)
}


/****************
** Device::Irq**
****************/
/*****************
** Device::Irq **
*****************/

void Device::Irq::_handle()
{
handle();
pending = true;

env().scheduler.unblock_irq_handler();
env().scheduler.schedule();
}


void Device::Irq::handle()
void Device::Irq::ack()
{
occured = true;
if (session.constructed())
session->ack();
}

if (masked)
return;

env().scheduler.unblock_irq_handler(number);
void Device::Irq::mask()
{
masked = true;
}


void Device::Irq::unmask(Platform::Device &dev)
{
if (!session.constructed()) {
session.construct(dev, idx);
session->sigh_omit_initial_signal(handler);
session->ack();
}

masked = false;

env().scheduler.unblock_irq_handler();
}


Expand Down Expand Up @@ -139,25 +157,42 @@ void * Device::io_mem_local_addr(addr_t phys_addr, size_t size)
}


int Device::pending_irq()
{
if (!_pdev.constructed())
return -1;

int result = -1;

for_each_irq([&] (Irq & irq) {
if (result == -1 && !irq.masked && irq.pending)
result = irq.number;
});

return result;
}


void Device::reset_pending(unsigned number)
{
for_each_irq([&] (Irq & irq) {
if (irq.number == number)
irq.pending = false;
});
}


bool Device::irq_unmask(unsigned number)
{
bool ret = false;

for_each_irq([&] (Irq & irq) {
if (irq.number.value != number)
if (irq.number != number)
return;

ret = true;
enable();

if (!irq.session.constructed()) {
irq.session.construct(*_pdev, irq.idx);
irq.session->sigh_omit_initial_signal(irq.handler);
irq.session->ack();
}

irq.masked = false;
if (irq.occured) irq.handle();
irq.unmask(*_pdev);
});

return ret;
Expand All @@ -170,8 +205,7 @@ void Device::irq_mask(unsigned number)
return;

for_each_irq([&] (Irq & irq) {
if (irq.number.value == number) irq.masked = true; });

if (irq.number == number) irq.mask(); });
}


Expand All @@ -181,10 +215,9 @@ void Device::irq_ack(unsigned number)
return;

for_each_irq([&] (Irq & irq) {
if (irq.number.value != number || !irq.occured || !irq.session.constructed())
if (irq.number != number)
return;
irq.occured = false;
irq.session->ack();
irq.ack();
});
}

Expand Down
4 changes: 1 addition & 3 deletions repos/dde_linux/src/lib/lx_kit/scheduler.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,8 @@ void Scheduler::remove(Task & task)
}


void Scheduler::unblock_irq_handler(Pending_irq &pirq)
void Scheduler::unblock_irq_handler()
{
_pending_irqs.enqueue(pirq);

for (Task * t = _present_list.first(); t; t = t->next()) {
if (t->type() == Task::IRQ_HANDLER) t->unblock();
}
Expand Down

0 comments on commit fb00f4a

Please sign in to comment.