qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path


From: Anthony Liguori
Subject: Re: [Qemu-devel] [PATCH] pc: Clean up PIC-to-APIC IRQ path
Date: Sat, 03 Sep 2011 16:41:53 -0500
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.17) Gecko/20110516 Lightning/1.0b2 Thunderbird/3.1.10

On 09/03/2011 04:10 PM, Blue Swirl wrote:
On Sat, Sep 3, 2011 at 8:07 PM, Anthony Liguori<address@hidden>  wrote:
On 09/01/2011 12:58 AM, Avi Kivity wrote:

On 08/31/2011 07:59 PM, Blue Swirl wrote:


That makes it impossible to migrate level-triggered irq lines. Or at
least,
the receiver has to remember the state, instead of (or in addition
to) the
sender.

Both ends probably need to remember the state. That should work
without any multiphase restores and transient suppressors.

State should always correspond to real hardware state - a flip flop or
capacitor. Input is not state, it is input.

It might be also possible to introduce stateful signal lines which
save and restore their state, then the receiving end could check what
is the current level. However, if you consider that the devices may be
restored in random order, if the IRQ line device happens to be
restored later, the receiver would still get wrong information. Adding
priorities could solve this, but I think stateless IRQs are the only
sane way.

I agree that irqs should be stateless, since they don't have any memory
associated.

In Real Life, you can tie a single bit multiple registers together with
boolean logic to form an output pin.  This is essentially computed state.
  If we wanted to model a stateless pin, we would need to do something like:

struct Serial {
   uint8_t thr;
   uint8_t lsr;
};

static bool serial_get_irq(Serial *s) {
   return (s->thr&  THRE) | (s->lsr&  LSRE);
}

static void serial_write(Serial *s, uint64_t addr, uint8_t value)
{
   switch (addr) {
   case THR:
      bool old_irq = serial_get_irq(s);
      s->thr = value;
      if (!old_irq&&  serial_get_irq(s)) {
          notify_edge_change(s);
      }
   ...
}

static void serial_init(Serial *s)
{
    register_pin(s, serial_get_irq);
}

Obviously, this is pretty sucky.  This is what we do today but we don't have
a way to query irq value which is wrong.  You could fix that by adding the
get function but that's not terribly fun.  A better way:

struct Serial {
    Pin irq;
    uint8_t thr;
    uint8_t lsr;
};

static void serial_update_irq(Serial *s)
{
   pin_set_level(&s->irq, (s->thr&  THRE) | (s->lsr&  LSRE));
}

static void serial_write(Serial *s) {
   switch (addr) {
   case THR:
      s->thr = value;
      serial_update_irq(s);
   ...
}

This results in much nicer code.  The edge handling can be done in generic
code which will make things more robust overall.

I'm sorry but I don't see a huge difference, could you elaborate?

The main difference is whether the Pin is capable of determine if there was a level change on its own. It can only do this is if knows the current level which implies that its holding state.


Maybe if the internal state of Pin is magically shared between the
endpoint devices (like typedef bool *Pin) and the devices somehow
still save Pin state as part of their own despite the duplication,

I'm somewhat confused by what you mean here.

If you have two devices that have a connection, one has an output pin and one has an input pin. This would look like this:

struct Serial {
   Pin irq; // output pin
};

struct PIIX3 {
   Pin *in[16]; // input pins
};

As part of connecting devices, you'd basically do:

PIIX3 piix3;
Serial serial;

piix3.in[4] = &serial.irq;

serial.irq setting it pin level doesn't do anything to piix3. piix3 has to explicitly read the pin state for its behavior to influence anything.

Here's the flow with taking migration into account:

1) PIIX3 maintains some type of state, performs action (A) whenever in[3] changes its state based on an edge change notifier.

2) During migration, PIIX3 has its state saved as does Serial. Pin is part of Serial so it also has its state saved.

3) During restore, PIIX3 has its state restored, as does Serial, and Pin. Action (A) is not invoked because notifiers are not fired when a device is not realized. Restore happens before a device is realized.

So the scenario you're concerned about doesn't happen and it doesn't require anything funky.

this could work. Restoring Pins first and then devices is a sort of
priority scheme.

There is no priority. But devices have an explicit realize event and in general, shouldn't react to other devices until realize happens. You need this behavior to support construction properly.

Regards,

Anthony Liguori



reply via email to

[Prev in Thread] Current Thread [Next in Thread]