Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On 29 October 2015 at 14:27, Shannon Zhao wrote: > ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced > platforms(like ARM). It uses GPIO pin to trigger an event to the guest. > For QEMU, here we add PL061 GPIO controller and use PIN 3 for > system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and > memory hotplug. > > This patchset adds system_powerdown support on ARM through both ACPI and > DT ways. It adds a GPIO controller(here is PL061) in machine virt and > uses GPIO-singled event for ACPI while gpio-keys for DT. It can be > fetched from [1] and has been tested for the guests starting by ACPI or > DT while guests use systemd or acpid. > Changes since v1: > * rewrite GPIO Connection Descriptor (Michael) > > Shannon Zhao (8): > hw/arm/virt: Add a GPIO controller > hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table > hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table > hw/acpi/aml-build: Add GPIO Connection Descriptor > hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection > hw/arm/virt-acpi-build: Add _E03 for Power Button > hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3 > hw/arm/virt: Add gpio-keys node for Poweroff using DT Thanks; this patchset looks generally good to me (I had a few minor remarks about patch 8). I would appreciate review from folks familiar with the ACPI spec for patches 2, 3, 4, 5 and 6, though. -- PMM
Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On Fri, 27 Nov 2015 17:18:06 + Peter Maydell wrote: > On 29 October 2015 at 14:27, Shannon Zhao wrote: > > ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced > > platforms(like ARM). It uses GPIO pin to trigger an event to the guest. > > For QEMU, here we add PL061 GPIO controller and use PIN 3 for > > system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and > > memory hotplug. > > > > This patchset adds system_powerdown support on ARM through both ACPI and > > DT ways. It adds a GPIO controller(here is PL061) in machine virt and > > uses GPIO-singled event for ACPI while gpio-keys for DT. It can be > > fetched from [1] and has been tested for the guests starting by ACPI or > > DT while guests use systemd or acpid. > > > Changes since v1: > > * rewrite GPIO Connection Descriptor (Michael) > > > > Shannon Zhao (8): > > hw/arm/virt: Add a GPIO controller > > hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table > > hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table > > hw/acpi/aml-build: Add GPIO Connection Descriptor > > hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection > > hw/arm/virt-acpi-build: Add _E03 for Power Button > > hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3 > > hw/arm/virt: Add gpio-keys node for Poweroff using DT > > Thanks; this patchset looks generally good to me (I had a few > minor remarks about patch 8). I would appreciate review from > folks familiar with the ACPI spec for patches 2, 3, 4, 5 and 6, though. I'll do it. > > -- PMM >
Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On 10/29/2015 09:27 AM, Shannon Zhao wrote:
> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
> platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
> For QEMU, here we add PL061 GPIO controller and use PIN 3 for
> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
> memory hotplug.
>
> This patchset adds system_powerdown support on ARM through both ACPI and
> DT ways. It adds a GPIO controller(here is PL061) in machine virt and
> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
> fetched from [1] and has been tested for the guests starting by ACPI or
> DT while guests use systemd or acpid.
>
> a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support
> amba device[2], it could use PL061 directly without modification to its
> kernel driver code. In addition, we should use ACPI to start VM,
> referring to below script. QEMU_EFI.fd can be fetched from [3].
Hi Shannon,
Thanks for re-sending it. This is a desired feature because we don't
want to rely on tricks (such as guest-agent) for external VM power
management. I have tested V1 recently by back-porting to my in-house
kernel; it worked well.
I will help review this new version.
Thanks,
-Wei
>
> qemu-system-aarch64 \
> -smp 1 -m 1024 -M virt \
> -cpu cortex-a57 -nographic \
> -monitor telnet::,server,nowait \
> -bios QEMU_EFI.fd \
> -kernel Image \
> -initrd guestfs.cpio.gz \
> -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram
> earlycon=pl011,0x900 rw acpi=force"
>
> b) DT way. Start vm as usual.
>
> qemu-system-aarch64 \
> -smp 1 -m 1024 -M virt \
> -cpu cortex-a57 -nographic \
> -monitor telnet::,server,nowait \
> -kernel Image \
> -initrd guestfs.cpio.gz \
> -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram
> earlycon=pl011,0x900 rw"
>
> Guest internal setup:
> If your guest FS uses systemd, you should check file
> /lib/udev/rules.d/70-power-switch.rules and add the following line in it
> if it doesn't exist.
>
> SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform",
> ATTRS{keys}=="116", TAG+="power-switch"
>
> If your guest FS uses acpid, you should check it has
> /etc/acpi/powerbtn.sh and /etc/acpi/events/powerbtn. Refer to [4] for
> the setup of acpid.
>
> Thanks,
> Shannon
>
> [1]
> https://git.linaro.org/people/shannon.zhao/qemu.git/shortlog/refs/heads/PowerButton_v2
> [2] https://lkml.org/lkml/2015/9/30/392
> [3] http://people.linaro.org/~shannon.zhao/ACPI_ARM/QEMU_EFI.fd
> [4] https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI/GPIOPowerButton
>
> Changes since v1:
> * rewrite GPIO Connection Descriptor (Michael)
>
> Shannon Zhao (8):
> hw/arm/virt: Add a GPIO controller
> hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
> hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
> hw/acpi/aml-build: Add GPIO Connection Descriptor
> hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
> hw/arm/virt-acpi-build: Add _E03 for Power Button
> hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
> hw/arm/virt: Add gpio-keys node for Poweroff using DT
>
> hw/acpi/aml-build.c | 79
> +
> hw/arm/virt-acpi-build.c| 45 ++
> hw/arm/virt.c | 60 ++
> include/hw/acpi/aml-build.h | 26 +++
> include/hw/arm/virt.h | 1 +
> 5 files changed, 211 insertions(+)
>
Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On 2015/10/30 2:17, Wei Huang wrote: > On 10/29/2015 09:27 AM, Shannon Zhao wrote: >> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced >> platforms(like ARM). It uses GPIO pin to trigger an event to the guest. >> For QEMU, here we add PL061 GPIO controller and use PIN 3 for >> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and >> memory hotplug. >> >> This patchset adds system_powerdown support on ARM through both ACPI and >> DT ways. It adds a GPIO controller(here is PL061) in machine virt and >> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be >> fetched from [1] and has been tested for the guests starting by ACPI or >> DT while guests use systemd or acpid. >> >> a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support >> amba device[2], it could use PL061 directly without modification to its >> kernel driver code. In addition, we should use ACPI to start VM, >> referring to below script. QEMU_EFI.fd can be fetched from [3]. > > Hi Shannon, > > Thanks for re-sending it. This is a desired feature because we don't > want to rely on tricks (such as guest-agent) for external VM power > management. I have tested V1 recently by back-porting to my in-house > kernel; it worked well. > > I will help review this new version. > Thanks for your help:) -- Shannon
Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On 10/29/2015 09:27 AM, Shannon Zhao wrote:
> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
> platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
> For QEMU, here we add PL061 GPIO controller and use PIN 3 for
> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
> memory hotplug.
>
> This patchset adds system_powerdown support on ARM through both ACPI and
> DT ways. It adds a GPIO controller(here is PL061) in machine virt and
> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
> fetched from [1] and has been tested for the guests starting by ACPI or
> DT while guests use systemd or acpid.
>
> a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support
> amba device[2], it could use PL061 directly without modification to its
> kernel driver code. In addition, we should use ACPI to start VM,
> referring to below script. QEMU_EFI.fd can be fetched from [3].
>
> qemu-system-aarch64 \
> -smp 1 -m 1024 -M virt \
> -cpu cortex-a57 -nographic \
> -monitor telnet::,server,nowait \
> -bios QEMU_EFI.fd \
> -kernel Image \
> -initrd guestfs.cpio.gz \
> -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram
> earlycon=pl011,0x900 rw acpi=force"
>
> b) DT way. Start vm as usual.
>
> qemu-system-aarch64 \
> -smp 1 -m 1024 -M virt \
> -cpu cortex-a57 -nographic \
> -monitor telnet::,server,nowait \
> -kernel Image \
> -initrd guestfs.cpio.gz \
> -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram
> earlycon=pl011,0x900 rw"
>
> Guest internal setup:
> If your guest FS uses systemd, you should check file
> /lib/udev/rules.d/70-power-switch.rules and add the following line in it
> if it doesn't exist.
>
> SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform",
> ATTRS{keys}=="116", TAG+="power-switch"
>
> If your guest FS uses acpid, you should check it has
> /etc/acpi/powerbtn.sh and /etc/acpi/events/powerbtn. Refer to [4] for
> the setup of acpid.
>
> Thanks,
> Shannon
>
> [1]
> https://git.linaro.org/people/shannon.zhao/qemu.git/shortlog/refs/heads/PowerButton_v2
> [2] https://lkml.org/lkml/2015/9/30/392
> [3] http://people.linaro.org/~shannon.zhao/ACPI_ARM/QEMU_EFI.fd
> [4] https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI/GPIOPowerButton
Compared with v1, this version doesn't work with system_powerdown
command. In my setup, the testing environment is exactly same for both
v1 and v2. So something changed in v2 caused system_powerdown fail to work.
-Wei
>
> Changes since v1:
> * rewrite GPIO Connection Descriptor (Michael)
>
> Shannon Zhao (8):
> hw/arm/virt: Add a GPIO controller
> hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
> hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
> hw/acpi/aml-build: Add GPIO Connection Descriptor
> hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
> hw/arm/virt-acpi-build: Add _E03 for Power Button
> hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
> hw/arm/virt: Add gpio-keys node for Poweroff using DT
>
> hw/acpi/aml-build.c | 79
> +
> hw/arm/virt-acpi-build.c| 45 ++
> hw/arm/virt.c | 60 ++
> include/hw/acpi/aml-build.h | 26 +++
> include/hw/arm/virt.h | 1 +
> 5 files changed, 211 insertions(+)
>
Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On 11/10/2015 11:38 AM, Wei Huang wrote:
>
>
> On 10/29/2015 09:27 AM, Shannon Zhao wrote:
>> ACPI SPEC 5.0 defines GPIO-signaled ACPI Events for Hardware-reduced
>> platforms(like ARM). It uses GPIO pin to trigger an event to the guest.
>> For QEMU, here we add PL061 GPIO controller and use PIN 3 for
>> system_powerdown, reserving PIN 0, 1, 2 for PCI hotplug, CPU hotplug and
>> memory hotplug.
>>
>> This patchset adds system_powerdown support on ARM through both ACPI and
>> DT ways. It adds a GPIO controller(here is PL061) in machine virt and
>> uses GPIO-singled event for ACPI while gpio-keys for DT. It can be
>> fetched from [1] and has been tested for the guests starting by ACPI or
>> DT while guests use systemd or acpid.
>>
>> a) ACPI way. Since Graeme send a patchset to make ACPI on ARM64 support
>> amba device[2], it could use PL061 directly without modification to its
>> kernel driver code. In addition, we should use ACPI to start VM,
>> referring to below script. QEMU_EFI.fd can be fetched from [3].
>>
>> qemu-system-aarch64 \
>> -smp 1 -m 1024 -M virt \
>> -cpu cortex-a57 -nographic \
>> -monitor telnet::,server,nowait \
>> -bios QEMU_EFI.fd \
>> -kernel Image \
>> -initrd guestfs.cpio.gz \
>> -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram
>> earlycon=pl011,0x900 rw acpi=force"
>>
>> b) DT way. Start vm as usual.
>>
>> qemu-system-aarch64 \
>> -smp 1 -m 1024 -M virt \
>> -cpu cortex-a57 -nographic \
>> -monitor telnet::,server,nowait \
>> -kernel Image \
>> -initrd guestfs.cpio.gz \
>> -append "rdinit=/sbin/init console=ttyAMA0 mem=512M root=/dev/ram
>> earlycon=pl011,0x900 rw"
>>
>> Guest internal setup:
>> If your guest FS uses systemd, you should check file
>> /lib/udev/rules.d/70-power-switch.rules and add the following line in it
>> if it doesn't exist.
>>
>> SUBSYSTEM=="input", KERNEL=="event*", SUBSYSTEMS=="platform",
>> ATTRS{keys}=="116", TAG+="power-switch"
>>
>> If your guest FS uses acpid, you should check it has
>> /etc/acpi/powerbtn.sh and /etc/acpi/events/powerbtn. Refer to [4] for
>> the setup of acpid.
>>
>> Thanks,
>> Shannon
>>
>> [1]
>> https://git.linaro.org/people/shannon.zhao/qemu.git/shortlog/refs/heads/PowerButton_v2
>> [2] https://lkml.org/lkml/2015/9/30/392
>> [3] http://people.linaro.org/~shannon.zhao/ACPI_ARM/QEMU_EFI.fd
>> [4] https://wiki.linaro.org/LEG/Engineering/Kernel/ACPI/GPIOPowerButton
>
> Compared with v1, this version doesn't work with system_powerdown
> command. In my setup, the testing environment is exactly same for both
> v1 and v2. So something changed in v2 caused system_powerdown fail to work.
>
I found this was caused by the change of "_HID" name for GPIO device. It
was changed from "LNRO0009" (v1) to "ARMH0061" (v2), which doesn't match
with my stock guest kernel PL061 driver. After changing the guest
kernel, it is working again. So:
Tested-by: Wei Huang
I will provide reviews to each individual patches.
> -Wei
>
>>
>> Changes since v1:
>> * rewrite GPIO Connection Descriptor (Michael)
>>
>> Shannon Zhao (8):
>> hw/arm/virt: Add a GPIO controller
>> hw/arm/virt-acpi-build: Add GPIO controller in ACPI DSDT table
>> hw/arm/virt-acpi-build: Add power button device in ACPI DSDT table
>> hw/acpi/aml-build: Add GPIO Connection Descriptor
>> hw/acpi/aml-build: Add a wrapper for GPIO Interrupt Connection
>> hw/arm/virt-acpi-build: Add _E03 for Power Button
>> hw/arm/virt: Add QEMU powerdown notifier and hook it to GPIO Pin 3
>> hw/arm/virt: Add gpio-keys node for Poweroff using DT
>>
>> hw/acpi/aml-build.c | 79
>> +
>> hw/arm/virt-acpi-build.c| 45 ++
>> hw/arm/virt.c | 60 ++
>> include/hw/acpi/aml-build.h | 26 +++
>> include/hw/arm/virt.h | 1 +
>> 5 files changed, 211 insertions(+)
>>
>
Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On 2015/11/11 4:56, Wei Huang wrote: > >> >> Compared with v1, this version doesn't work with system_powerdown >> command. In my setup, the testing environment is exactly same for both >> v1 and v2. So something changed in v2 caused system_powerdown fail to work. >> > > I found this was caused by the change of "_HID" name for GPIO device. It > was changed from "LNRO0009" (v1) to "ARMH0061" (v2), which doesn't match > with my stock guest kernel PL061 driver. After changing the guest > kernel, it is working again. So: > Hi Wei, Thanks very much for your help. The reason why I change the _HID is based on the _HID 0f PL011 which is ARMH0011. About the _HID of ARM company's devices, I have to say that I didn't see which _HID they should be at any public place. I heard(maybe it's not correct) there is a _HID list of ARM devices. If so, I think ARM should publish them in public, otherwise the only thing we can do is to guess or refer to existing _HID. BTW, I think you could use this kernel patches[1] which adds ACPI support for AMBA device and drop the previous one. [1] https://lkml.org/lkml/2015/9/30/392 > Tested-by: Wei Huang > > I will provide reviews to each individual patches. > Thanks again. :) -- Shannon
Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On 11 November 2015 at 01:29, Shannon Zhao wrote: > On 2015/11/11 4:56, Wei Huang wrote: >> I found this was caused by the change of "_HID" name for GPIO device. It >> was changed from "LNRO0009" (v1) to "ARMH0061" (v2), which doesn't match >> with my stock guest kernel PL061 driver. After changing the guest >> kernel, it is working again. So: > Thanks very much for your help. The reason why I change the _HID is > based on the _HID 0f PL011 which is ARMH0011. About the _HID of ARM > company's devices, I have to say that I didn't see which _HID they > should be at any public place. I heard(maybe it's not correct) there is > a _HID list of ARM devices. If so, I think ARM should publish them in > public, otherwise the only thing we can do is to guess or refer to > existing _HID. Please confirm for definite the right _HID values; don't guess them. If you say you don't know the right HID values and mark a patchset as RFC we can go and try to find out the right answers. If you just put guesses into a patchset we could easily end up committing the patch with the wrong info... thanks -- PMM
Re: [Qemu-devel] [PATCH v2 0/8] Add system_powerdown support on ARM for ACPI and DT
On 2015/11/11 16:36, Peter Maydell wrote: > On 11 November 2015 at 01:29, Shannon Zhao wrote: >> On 2015/11/11 4:56, Wei Huang wrote: >>> I found this was caused by the change of "_HID" name for GPIO device. It >>> was changed from "LNRO0009" (v1) to "ARMH0061" (v2), which doesn't match >>> with my stock guest kernel PL061 driver. After changing the guest >>> kernel, it is working again. So: > >> Thanks very much for your help. The reason why I change the _HID is >> based on the _HID 0f PL011 which is ARMH0011. About the _HID of ARM >> company's devices, I have to say that I didn't see which _HID they >> should be at any public place. I heard(maybe it's not correct) there is >> a _HID list of ARM devices. If so, I think ARM should publish them in >> public, otherwise the only thing we can do is to guess or refer to >> existing _HID. > > Please confirm for definite the right _HID values; don't guess > them. If you say you don't know the right HID values and mark > a patchset as RFC we can go and try to find out the right > answers. If you just put guesses into a patchset we could > easily end up committing the patch with the wrong info... > Of course I will try to find the right _HID and tried before. But what I want to say is why ARM doesn't publish these _HIDs if there is really a list( I don't think this is a secret). And if there is not a list, ARM should create one since these devices belong to ARM. Thanks, -- Shannon
