On Sat, Sep 3, 2011 at 9:41 PM, Anthony Liguori <anth...@codemonkey.ws> wrote:
> On 09/03/2011 04:10 PM, Blue Swirl wrote:
>>
>> On Sat, Sep 3, 2011 at 8:07 PM, Anthony Liguori<anth...@codemonkey.ws>
>> 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.
Pretty similar to what you propose below except the magical sharing.
> 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.
OK, this should work.
>> 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
>
