Re: [Qemu-devel] [PATCH 1/1] arm: virt: change GPIO trigger interrupt to pulse

[Shannon Zhao]

On 2016/2/26 23:42, Peter Maydell wrote:
> On 26 February 2016 at 15:28, Wei Huang <address@hidden> wrote:
>>
>>
>> On 02/26/2016 09:06 AM, Peter Maydell wrote:
>>> On 26 February 2016 at 14:54, Shannon Zhao <address@hidden> wrote:
>>>> On 2016/2/26 20:53, Peter Maydell wrote:
>>>>> I don't understand why a 500ms pulse is better than a short one.
>>>>>
>>>> Oh, I just pick a value which seems like a real latency for pressing a
>>>> button. What's your suggestion?
>>>
>>> I would prefer to avoid the pain of having a timer whose state
>>> needs to be migrated. It's unclear to me why a 500ms pulse
>>> will solve anything that an instantaneous pulse does not,
>>> so I'd like to better understand the problem first.
>>
>> The problem we found with pulse was: only the last state change in GPIO
>> is received by guest VM. In other words, with 0(L)->1(H)->0(L) or
>> 1(H)->0(L)->1(0), PL061 only sees the last state (0 and 1). I guess this
>> is because QEMU is non-preemptive. The solution is to have the following
>> steps:
>>   * qemu_set_irq(gpio_in, 1)
>>   * yeild to guest VM
>>   * qemu_set_irq(gpio_in, 0)
>>
>> Is there any way to do so in QEMU without using timer?
> 
> You have no guarantee that the guest VM will necessarily
> make any progress even with a 500ms pulse length, 
Yeah, if the guest is very busy! But it's true in the real world like
that we press a GPIO_KEY button, but guest is too busy that it can't
react to this then the guest can't shutdown.

> so I don't
> think increasing the length of the pulse is going to solidly
> fix this.
> 
This is not simply increasing the length. It just makes the guest not
blocked after the first qemu_set_irq so that the guest can detect the
button is pressed(i.e. get the value 1). Then after any time, call the
second qemu_set_irq then guest can detect the button is released.

You must be aware that here the purpose of qemu_set_irq is only to set
the value of gpio pin not directly set the interrupt controller. As for
injecting the interrupt, it's what the PL061 device does which detects
the pin value changed.

And for GPIO_KEY, it must detect 1 and 0 then it realizes it as a valid
input.

Have a look at the below function in drivers/input/keyboard/gpio_keys.c.

static void gpio_keys_gpio_report_event(struct gpio_button_data *bdata)
{
        const struct gpio_keys_button *button = bdata->button;
        struct input_dev *input = bdata->input;
        unsigned int type = button->type ?: EV_KEY;
        int state = (gpio_get_value_cansleep(button->gpio) ? 1 : 0) ^
button->active_low;

        if (type == EV_ABS) {
                if (state)
                        input_event(input, type, button->code, button->value);
        } else {
                input_event(input, type, button->code, !!state);
        }
        input_sync(input);
}


> As usual with any kind of interrupt you either need to
> trigger on an edge (and latch the trigger in the interrupt
> controller until the guest picks it up), or trigger on a
> level, and keep the level high until the guest acknowledges
> by writing back to the original device to tell it to drop
> the level.
> 
As said, here we only want change the some pin value and want guest to
get the value. The interrupt is triggered by PL061 device which just
notifies the guest that pin value is changed, please read.

Thanks,
-- 
Shannon