| Message ID | 1454005340-15682-1-git-send-email-wei@redhat.com |
|---|---|
| State | New |
| Headers | show |
Hi， This makes ACPI work well but makes DT not work. The reason is systemd or acpid open /dev/input/event0 failed. So the interrupt could be injected and could see under /proc/interrupts but guest doesn't have any action. I'll investigate why it opens failed later. 2016年1月29日星期五，Wei Huang <wei@redhat.com> 写道： > When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot > request will succeed; but the following shutdown/reboot requests > fail to trigger VMs to react. Notice that in mach-virt machine > model GPIO is defined as edge-triggered and active-high in ACPI. > This patch changes the behavior of powerdown notifier from PULLUP > to PULSE. It solves the problem described above (i.e. reboot > continues to work). > > Signed-off-by: Wei Huang <wei@redhat.com <javascript:;>> > --- > hw/arm/virt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 05f9087..b5468a9 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -546,7 +546,7 @@ static DeviceState *pl061_dev; > static void virt_powerdown_req(Notifier *n, void *opaque) > { > /* use gpio Pin 3 for power button event */ > - qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); > + qemu_irq_pulse(qdev_get_gpio_in(pl061_dev, 3)); > } > > static Notifier virt_system_powerdown_notifier = { > -- > 1.8.3.1 > >
On 01/29/2016 04:10 AM, Shannon Zhao wrote: > Hi， > > This makes ACPI work well but makes DT not work. The reason is systemd or > acpid open /dev/input/event0 failed. So the interrupt could be injected and > could see under /proc/interrupts but guest doesn't have any action. I'll > investigate why it opens failed later. That is interesting. Could you try it with the following? This reverses the order to down-up and worked on ACPI case. qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); Thanks, -Wei > > 2016年1月29日星期五，Wei Huang <wei@redhat.com> 写道： > >> When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot >> request will succeed; but the following shutdown/reboot requests >> fail to trigger VMs to react. Notice that in mach-virt machine >> model GPIO is defined as edge-triggered and active-high in ACPI. >> This patch changes the behavior of powerdown notifier from PULLUP >> to PULSE. It solves the problem described above (i.e. reboot >> continues to work). >> >> Signed-off-by: Wei Huang <wei@redhat.com <javascript:;>> >> --- >> hw/arm/virt.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >> index 05f9087..b5468a9 100644 >> --- a/hw/arm/virt.c >> +++ b/hw/arm/virt.c >> @@ -546,7 +546,7 @@ static DeviceState *pl061_dev; >> static void virt_powerdown_req(Notifier *n, void *opaque) >> { >> /* use gpio Pin 3 for power button event */ >> - qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >> + qemu_irq_pulse(qdev_get_gpio_in(pl061_dev, 3)); >> } >> >> static Notifier virt_system_powerdown_notifier = { >> -- >> 1.8.3.1 >> >> >
On 2016/1/29 22:35, Wei Huang wrote: > > > On 01/29/2016 04:10 AM, Shannon Zhao wrote: >> Hi， >> >> This makes ACPI work well but makes DT not work. The reason is systemd or >> acpid open /dev/input/event0 failed. So the interrupt could be injected and >> could see under /proc/interrupts but guest doesn't have any action. I'll >> investigate why it opens failed later. > > That is interesting. Could you try it with the following? This reverses > the order to down-up and worked on ACPI case. > Yeah, that's very weird. > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); > I'll try this tomorrow. But even if this works, it's still weird. > Thanks, > -Wei > >> >> 2016年1月29日星期五，Wei Huang <wei@redhat.com> 写道： >> >>> When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot >>> request will succeed; but the following shutdown/reboot requests >>> fail to trigger VMs to react. Notice that in mach-virt machine >>> model GPIO is defined as edge-triggered and active-high in ACPI. >>> This patch changes the behavior of powerdown notifier from PULLUP >>> to PULSE. It solves the problem described above (i.e. reboot >>> continues to work). >>> >>> Signed-off-by: Wei Huang <wei@redhat.com <javascript:;>> >>> --- >>> hw/arm/virt.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>> index 05f9087..b5468a9 100644 >>> --- a/hw/arm/virt.c >>> +++ b/hw/arm/virt.c >>> @@ -546,7 +546,7 @@ static DeviceState *pl061_dev; >>> static void virt_powerdown_req(Notifier *n, void *opaque) >>> { >>> /* use gpio Pin 3 for power button event */ >>> - qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >>> + qemu_irq_pulse(qdev_get_gpio_in(pl061_dev, 3)); >>> } >>> >>> static Notifier virt_system_powerdown_notifier = { >>> -- >>> 1.8.3.1 >>> >>> >>
On 01/29/2016 08:46 AM, Shannon Zhao wrote: > > > On 2016/1/29 22:35, Wei Huang wrote: >> >> >> On 01/29/2016 04:10 AM, Shannon Zhao wrote: >>> Hi， >>> >>> This makes ACPI work well but makes DT not work. The reason is >>> systemd or >>> acpid open /dev/input/event0 failed. So the interrupt could be >>> injected and >>> could see under /proc/interrupts but guest doesn't have any action. I'll >>> investigate why it opens failed later. >> >> That is interesting. Could you try it with the following? This reverses >> the order to down-up and worked on ACPI case. >> > Yeah, that's very weird. > >> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); >> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >> > I'll try this tomorrow. But even if this works, it's still weird. To reproduce this case, do the following steps using current upstream qemu: create vm => reboot vm (succeed) => reboot or shutdown vm (fail). Apparently the last interrupt wasn't received correctly. -Wei > >> Thanks, >> -Wei >> >>> >>> 2016年1月29日星期五，Wei Huang <wei@redhat.com> 写道： >>> >>>> When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot >>>> request will succeed; but the following shutdown/reboot requests >>>> fail to trigger VMs to react. Notice that in mach-virt machine >>>> model GPIO is defined as edge-triggered and active-high in ACPI. >>>> This patch changes the behavior of powerdown notifier from PULLUP >>>> to PULSE. It solves the problem described above (i.e. reboot >>>> continues to work). >>>> >>>> Signed-off-by: Wei Huang <wei@redhat.com <javascript:;>> >>>> --- >>>> hw/arm/virt.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c >>>> index 05f9087..b5468a9 100644 >>>> --- a/hw/arm/virt.c >>>> +++ b/hw/arm/virt.c >>>> @@ -546,7 +546,7 @@ static DeviceState *pl061_dev; >>>> static void virt_powerdown_req(Notifier *n, void *opaque) >>>> { >>>> /* use gpio Pin 3 for power button event */ >>>> - qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >>>> + qemu_irq_pulse(qdev_get_gpio_in(pl061_dev, 3)); >>>> } >>>> >>>> static Notifier virt_system_powerdown_notifier = { >>>> -- >>>> 1.8.3.1 >>>> >>>> >>> >
On 29 January 2016 at 14:46, Shannon Zhao <shannon.zhao@linaro.org> wrote: > On 2016/1/29 22:35, Wei Huang wrote: >> On 01/29/2016 04:10 AM, Shannon Zhao wrote: >>> This makes ACPI work well but makes DT not work. The reason is systemd or >>> acpid open /dev/input/event0 failed. So the interrupt could be injected >>> and >>> could see under /proc/interrupts but guest doesn't have any action. I'll >>> investigate why it opens failed later. >> >> >> That is interesting. Could you try it with the following? This reverses >> the order to down-up and worked on ACPI case. >> > Yeah, that's very weird. > >> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); >> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >> > I'll try this tomorrow. But even if this works, it's still weird. I wonder if we should be asserting the GPIO pin in the powerdown-request hook and then deasserting it on system reset somewhere... thanks -- PMM
On 01/29/2016 08:50 AM, Peter Maydell wrote: > On 29 January 2016 at 14:46, Shannon Zhao <shannon.zhao@linaro.org> wrote: >> On 2016/1/29 22:35, Wei Huang wrote: >>> On 01/29/2016 04:10 AM, Shannon Zhao wrote: >>>> This makes ACPI work well but makes DT not work. The reason is systemd or >>>> acpid open /dev/input/event0 failed. So the interrupt could be injected >>>> and >>>> could see under /proc/interrupts but guest doesn't have any action. I'll >>>> investigate why it opens failed later. >>> >>> >>> That is interesting. Could you try it with the following? This reverses >>> the order to down-up and worked on ACPI case. >>> >> Yeah, that's very weird. >> >>> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); >>> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >>> >> I'll try this tomorrow. But even if this works, it's still weird. > > I wonder if we should be asserting the GPIO pin in the powerdown-request > hook and then deasserting it on system reset somewhere... This is another possibility. We can try to reset the pl061 state by hooking up with dc->reset and see what happens. > > thanks > -- PMM >
On 2016/1/29 22:50, Wei Huang wrote: > > On 01/29/2016 08:46 AM, Shannon Zhao wrote: >> > >> > >> >On 2016/1/29 22:35, Wei Huang wrote: >>> >> >>> >> >>> >>On 01/29/2016 04:10 AM, Shannon Zhao wrote: >>>> >>>Hi， >>>> >>> >>>> >>>This makes ACPI work well but makes DT not work. The reason is >>>> >>>systemd or >>>> >>>acpid open /dev/input/event0 failed. So the interrupt could be >>>> >>>injected and >>>> >>>could see under /proc/interrupts but guest doesn't have any action. I'll >>>> >>>investigate why it opens failed later. >>> >> >>> >>That is interesting. Could you try it with the following? This reverses >>> >>the order to down-up and worked on ACPI case. >>> >> >> >Yeah, that's very weird. >> > >>> >>qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); >>> >>qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >>> >> >> >I'll try this tomorrow. But even if this works, it's still weird. > To reproduce this case, do the following steps using current upstream > qemu: create vm => reboot vm (succeed) => reboot or shutdown vm (fail). > Apparently the last interrupt wasn't received correctly. Yes, I reproduce this today. Let's clarify current state. Firstly, for ACPI it should use qemu_irq_pulse since we make the GPIO pin edge-triggered. And for DT, it uses gpio-key which is also edge-triggered that we could get from output of guest /proc/interrupts. Secondly, current upstream qemu with your patch makes second reboot works when using ACPI. But first shutdown/reboot doesn't works when using DT since the systemd or acpid open /dev/input/event0 failed. This is what I'm surprised. Wei, what userspace program your guest uses? systemd or acpid? Could you please try to use DT to test your patch? And see if there is a same result with me.(I know Redhat kernel uses ACPI by default, so you could append acpi=off to switch to DT) Thanks,
On 29 January 2016 at 15:13, Wei Huang <wei@redhat.com> wrote: > > > On 01/29/2016 08:50 AM, Peter Maydell wrote: >> I wonder if we should be asserting the GPIO pin in the powerdown-request >> hook and then deasserting it on system reset somewhere... > > This is another possibility. We can try to reset the pl061 state by > hooking up with dc->reset and see what happens. Ah, yes, PL061 hasn't been updated to implement reset. That is almost certainly your problem. thanks -- PMM
On 2016/1/29 22:35, Wei Huang wrote: > > On 01/29/2016 04:10 AM, Shannon Zhao wrote: >> > Hi， >> > >> > This makes ACPI work well but makes DT not work. The reason is systemd or >> > acpid open /dev/input/event0 failed. To correct, systemd or acpid open /dev/input/event0 successfully but it waits for events and when we input "system_powerdown", it doesn't get the event. > So the interrupt could be injected and >> > could see under /proc/interrupts but guest doesn't have any action. I'll >> > investigate why it opens failed later. > That is interesting. Could you try it with the following? This reverses > the order to down-up and worked on ACPI case. > > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); If we don't input "system_powerdown" during guest booting(before device driver load), current QEMU with above codes could work for DT. But if we input "system_powerdown" during guest booting, guest has no action when we input "system_powerdown" again after guest booting completely. I add dc->reset for pl061 but get the same result. Below is the output of acpid: ./acpid -l -n -d input layer /dev/input/event0 (gpio-keys) opened successfully, fd 4 inotify fd: 5 inotify wd: 1 RTNETLINK1 answers: No such file or directory acpid: error talking to the kernel via netlink netlink opened successfully acpid: starting up with netlink and the input layer parsing conf file /etc/acpi/events/powerbtn acpid: 1 rule loaded acpid: waiting for events: event logging is on It will stuck here even if we input "system_powerdown". Below is the right output when we input "system_powerdown". ./acpid -l -n -d input layer /dev/input/event0 (gpio-keys) opened successfully, fd 4 inotify fd: 5 inotify wd: 1 RTNETLINK1 answers: No such file or directory acpid: error talking to the kernel via netlink netlink opened successfully acpid: starting up with netlink and the input layer parsing conf file /etc/acpi/events/powerbtn acpid: 1 rule loaded acpid: waiting for events: event logging is on acpid: received input layer event "button/power PBTN 00000080 00000000" acpid: rule from /etc/acpi/events/powerbtn matched acpid: executing action "/etc/acpi/powerbtn.sh" Thanks,
On Fri, 29 Jan 2016 09:13:15 -0600 Wei Huang <wei@redhat.com> wrote: > On 01/29/2016 08:50 AM, Peter Maydell wrote: > > On 29 January 2016 at 14:46, Shannon Zhao <shannon.zhao@linaro.org> wrote: > >> On 2016/1/29 22:35, Wei Huang wrote: > >>> On 01/29/2016 04:10 AM, Shannon Zhao wrote: > >>>> This makes ACPI work well but makes DT not work. The reason is systemd or > >>>> acpid open /dev/input/event0 failed. So the interrupt could be injected > >>>> and > >>>> could see under /proc/interrupts but guest doesn't have any action. I'll > >>>> investigate why it opens failed later. > >>> > >>> > >>> That is interesting. Could you try it with the following? This reverses > >>> the order to down-up and worked on ACPI case. > >>> > >> Yeah, that's very weird. > >> > >>> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); > >>> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); > >>> > >> I'll try this tomorrow. But even if this works, it's still weird. > > > > I wonder if we should be asserting the GPIO pin in the powerdown-request > > hook and then deasserting it on system reset somewhere... > > This is another possibility. We can try to reset the pl061 state by > hooking up with dc->reset and see what happens. I think that's what we do on x86. > > > > > thanks > > -- PMM > > >
On 02/01/2016 04:17 AM, Igor Mammedov wrote: > On Fri, 29 Jan 2016 09:13:15 -0600 > Wei Huang <wei@redhat.com> wrote: > >> On 01/29/2016 08:50 AM, Peter Maydell wrote: >>> On 29 January 2016 at 14:46, Shannon Zhao <shannon.zhao@linaro.org> wrote: >>>> On 2016/1/29 22:35, Wei Huang wrote: >>>>> On 01/29/2016 04:10 AM, Shannon Zhao wrote: >>>>>> This makes ACPI work well but makes DT not work. The reason is systemd or >>>>>> acpid open /dev/input/event0 failed. So the interrupt could be injected >>>>>> and >>>>>> could see under /proc/interrupts but guest doesn't have any action. I'll >>>>>> investigate why it opens failed later. >>>>> >>>>> >>>>> That is interesting. Could you try it with the following? This reverses >>>>> the order to down-up and worked on ACPI case. >>>>> >>>> Yeah, that's very weird. >>>> >>>>> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0); >>>>> qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >>>>> >>>> I'll try this tomorrow. But even if this works, it's still weird. >>> >>> I wonder if we should be asserting the GPIO pin in the powerdown-request >>> hook and then deasserting it on system reset somewhere... >> >> This is another possibility. We can try to reset the pl061 state by >> hooking up with dc->reset and see what happens. > I think that's what we do on x86. So the device state is wrong after reset. I just sent out a fix for it, which was verified on ACPI code path. Basically the old_in_data is stale after reboot, causing QEMU to believe that it isn't necessary to raise up IRQ again for the second reboot/shutdown (see s->old_in_data ^ s->data in pl061.c file). Shannon: could you double check from your side with my patch? -Wei > >> >>> >>> thanks >>> -- PMM >>> >> >
28.01.2016 21:22, Wei Huang wrote: > When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot > request will succeed; but the following shutdown/reboot requests > fail to trigger VMs to react. Notice that in mach-virt machine > model GPIO is defined as edge-triggered and active-high in ACPI. > This patch changes the behavior of powerdown notifier from PULLUP > to PULSE. It solves the problem described above (i.e. reboot > continues to work). So, what's the outcome of this? :) Thanks, /mjt > diff --git a/hw/arm/virt.c b/hw/arm/virt.c > index 05f9087..b5468a9 100644 > --- a/hw/arm/virt.c > +++ b/hw/arm/virt.c > @@ -546,7 +546,7 @@ static DeviceState *pl061_dev; > static void virt_powerdown_req(Notifier *n, void *opaque) > { > /* use gpio Pin 3 for power button event */ > - qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); > + qemu_irq_pulse(qdev_get_gpio_in(pl061_dev, 3)); > } > > static Notifier virt_system_powerdown_notifier = { >
On 3 February 2016 at 07:15, Michael Tokarev <mjt@tls.msk.ru> wrote: > 28.01.2016 21:22, Wei Huang wrote: >> When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot >> request will succeed; but the following shutdown/reboot requests >> fail to trigger VMs to react. Notice that in mach-virt machine >> model GPIO is defined as edge-triggered and active-high in ACPI. >> This patch changes the behavior of powerdown notifier from PULLUP >> to PULSE. It solves the problem described above (i.e. reboot >> continues to work). > > So, what's the outcome of this? :) This patch is definitely wrong. The patch to fix up the gpio reset stuff is definitely the right idea. Whether it fixes the reported failure or some further change is also needed is currently unclear. thanks -- PMM
On 2/3/16 04:46, Peter Maydell wrote: > On 3 February 2016 at 07:15, Michael Tokarev <mjt@tls.msk.ru> wrote: >> 28.01.2016 21:22, Wei Huang wrote: >>> When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot >>> request will succeed; but the following shutdown/reboot requests >>> fail to trigger VMs to react. Notice that in mach-virt machine >>> model GPIO is defined as edge-triggered and active-high in ACPI. >>> This patch changes the behavior of powerdown notifier from PULLUP >>> to PULSE. It solves the problem described above (i.e. reboot >>> continues to work). >> >> So, what's the outcome of this? :) > > This patch is definitely wrong. The patch to fix up the > gpio reset stuff is definitely the right idea. Whether it > fixes the reported failure or some further change is also > needed is currently unclear. I will NAK this one for now. Please see V2 patch, which is necessary. In the meanwhile, I think there is a problem with pulling-up only in current implementation. Let me debug Shannon's DT problem first. > > thanks > -- PMM >
On 2016/2/4 0:01, Wei Huang wrote: > > On 2/3/16 04:46, Peter Maydell wrote: >> > On 3 February 2016 at 07:15, Michael Tokarev <mjt@tls.msk.ru> wrote: >>> >> 28.01.2016 21:22, Wei Huang wrote: >>>> >>> When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot >>>> >>> request will succeed; but the following shutdown/reboot requests >>>> >>> fail to trigger VMs to react. Notice that in mach-virt machine >>>> >>> model GPIO is defined as edge-triggered and active-high in ACPI. >>>> >>> This patch changes the behavior of powerdown notifier from PULLUP >>>> >>> to PULSE. It solves the problem described above (i.e. reboot >>>> >>> continues to work). >>> >> >>> >> So, what's the outcome of this? :) >> > >> > This patch is definitely wrong. The patch to fix up the >> > gpio reset stuff is definitely the right idea. Whether it >> > fixes the reported failure or some further change is also >> > needed is currently unclear. > I will NAK this one for now. Please see V2 patch, which is necessary. In > the meanwhile, I think there is a problem with pulling-up only in > current implementation. Let me debug Shannon's DT problem first. > Hi Wei, The reason of DT problem is that when we use qemu_irq_pulse(i.e qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0);), it will inject the GPIO interrupt until it executes qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0) because the qemu main thread is serialized and then guest will get the button value as zero, so it's failed to report the input event. See gpio_keys_gpio_report_event in drivers/input/keyboard/gpio_keys.c int state = gpio_get_value_cansleep(button->gpio); The state is always zero. The solution I think would be making the each qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1 or 0) cloud inject an interrupt to guest. Thanks,
On 02/03/2016 07:44 PM, Shannon Zhao wrote: > > > On 2016/2/4 0:01, Wei Huang wrote: >> >> On 2/3/16 04:46, Peter Maydell wrote: >>>> On 3 February 2016 at 07:15, Michael Tokarev <mjt@tls.msk.ru> wrote: >>>>>> 28.01.2016 21:22, Wei Huang wrote: >>>>>>>> When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot >>>>>>>> request will succeed; but the following shutdown/reboot requests >>>>>>>> fail to trigger VMs to react. Notice that in mach-virt machine >>>>>>>> model GPIO is defined as edge-triggered and active-high in ACPI. >>>>>>>> This patch changes the behavior of powerdown notifier from PULLUP >>>>>>>> to PULSE. It solves the problem described above (i.e. reboot >>>>>>>> continues to work). >>>>>> >>>>>> So, what's the outcome of this? :) >>>> >>>> This patch is definitely wrong. The patch to fix up the >>>> gpio reset stuff is definitely the right idea. Whether it >>>> fixes the reported failure or some further change is also >>>> needed is currently unclear. >> I will NAK this one for now. Please see V2 patch, which is necessary. In >> the meanwhile, I think there is a problem with pulling-up only in >> current implementation. Let me debug Shannon's DT problem first. >> > Hi Wei, > > The reason of DT problem is that when we use qemu_irq_pulse(i.e > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0);), it will inject the > GPIO interrupt until it executes > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0) because the qemu main > thread is serialized and then guest will get the button value as zero, > so it's failed to report the input event. > > See gpio_keys_gpio_report_event > in drivers/input/keyboard/gpio_keys.c > int state = gpio_get_value_cansleep(button->gpio); > > The state is always zero. I reversed the order of edge pulling. The state is 1 according to printk inside gpio_keys driver. However the reboot still failed with two reboots (1 very early, 1 later). > > The solution I think would be making the each > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1 or 0) cloud inject an > interrupt to guest. > > Thanks, >
On 2016/2/4 14:10, Wei Huang wrote: > > On 02/03/2016 07:44 PM, Shannon Zhao wrote: >> > >> > >> > On 2016/2/4 0:01, Wei Huang wrote: >>> >> >>> >> On 2/3/16 04:46, Peter Maydell wrote: >>>>> >>>> On 3 February 2016 at 07:15, Michael Tokarev <mjt@tls.msk.ru> wrote: >>>>>>> >>>>>> 28.01.2016 21:22, Wei Huang wrote: >>>>>>>>> >>>>>>>> When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot >>>>>>>>> >>>>>>>> request will succeed; but the following shutdown/reboot requests >>>>>>>>> >>>>>>>> fail to trigger VMs to react. Notice that in mach-virt machine >>>>>>>>> >>>>>>>> model GPIO is defined as edge-triggered and active-high in ACPI. >>>>>>>>> >>>>>>>> This patch changes the behavior of powerdown notifier from PULLUP >>>>>>>>> >>>>>>>> to PULSE. It solves the problem described above (i.e. reboot >>>>>>>>> >>>>>>>> continues to work). >>>>>>> >>>>>> >>>>>>> >>>>>> So, what's the outcome of this? :) >>>>> >>>> >>>>> >>>> This patch is definitely wrong. The patch to fix up the >>>>> >>>> gpio reset stuff is definitely the right idea. Whether it >>>>> >>>> fixes the reported failure or some further change is also >>>>> >>>> needed is currently unclear. >>> >> I will NAK this one for now. Please see V2 patch, which is necessary. In >>> >> the meanwhile, I think there is a problem with pulling-up only in >>> >> current implementation. Let me debug Shannon's DT problem first. >>> >> >> > Hi Wei, >> > >> > The reason of DT problem is that when we use qemu_irq_pulse(i.e >> > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); >> > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0);), it will inject the >> > GPIO interrupt until it executes >> > qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0) because the qemu main >> > thread is serialized and then guest will get the button value as zero, >> > so it's failed to report the input event. >> > >> > See gpio_keys_gpio_report_event >> > in drivers/input/keyboard/gpio_keys.c >> > int state = gpio_get_value_cansleep(button->gpio); >> > >> > The state is always zero. > I reversed the order of edge pulling. The state is 1 according to printk > inside gpio_keys driver. However the reboot still failed with two > reboots (1 very early, 1 later). > Because to make the input work, it should call input_event twice I think. input_event(input, type, button->code, 1) means the button pressed input_event(input, type, button->code, 0) means the button released But We only see guest entering gpio_keys_gpio_report_event once. My original purpose is like below: call qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1) to make guest execute input_event(input, type, button->code, 1) call qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 0) to make guest execute input_event(input, type, button->code, 0). But even though it calls qemu_set_irq twice, it only calls pl061_update once in qemu.
diff --git a/hw/arm/virt.c b/hw/arm/virt.c index 05f9087..b5468a9 100644 --- a/hw/arm/virt.c +++ b/hw/arm/virt.c @@ -546,7 +546,7 @@ static DeviceState *pl061_dev; static void virt_powerdown_req(Notifier *n, void *opaque) { /* use gpio Pin 3 for power button event */ - qemu_set_irq(qdev_get_gpio_in(pl061_dev, 3), 1); + qemu_irq_pulse(qdev_get_gpio_in(pl061_dev, 3)); } static Notifier virt_system_powerdown_notifier = {
When QEMU is hook'ed up with libvirt/virsh, the first ACPI reboot request will succeed; but the following shutdown/reboot requests fail to trigger VMs to react. Notice that in mach-virt machine model GPIO is defined as edge-triggered and active-high in ACPI. This patch changes the behavior of powerdown notifier from PULLUP to PULSE. It solves the problem described above (i.e. reboot continues to work). Signed-off-by: Wei Huang <wei@redhat.com> --- hw/arm/virt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)