[Qemu-devel] [PATCH v2] ARM: Virt: Don't generate RTC ACPI device when using UEFI

[Qemu-devel] [PATCH v2] ARM: Virt: Don't generate RTC ACPI device when using UEFI

[Shannon Zhao]

When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
While UEFI can use libfdt to disable the RTC device node in the DTB that
it passes to the OS, it cannot modify AML. Therefore, we won't generate
the RTC ACPI device at all when using UEFI.

Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
---
v2: just totally don't generate the RTC ACPI device when using UEFI
---
 hw/arm/virt-acpi-build.c | 19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 0caf5ce..ac568a3 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -94,23 +94,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
     aml_append(scope, dev);
 }
 
-static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
-                                          uint32_t rtc_irq)
-{
-    Aml *dev = aml_device("RTC0");
-    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
-    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
-
-    Aml *crs = aml_resource_template();
-    aml_append(crs, aml_memory32_fixed(rtc_memmap->base,
-                                       rtc_memmap->size, AML_READ_WRITE));
-    aml_append(crs,
-               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
-                             AML_EXCLUSIVE, &rtc_irq, 1));
-    aml_append(dev, aml_name_decl("_CRS", crs));
-    aml_append(scope, dev);
-}
-
 static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
 {
     Aml *dev, *crs;
@@ -575,8 +558,6 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
     acpi_dsdt_add_cpus(scope, guest_info->smp_cpus);
     acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
                        (irqmap[VIRT_UART] + ARM_SPI_BASE));
-    acpi_dsdt_add_rtc(scope, &memmap[VIRT_RTC],
-                      (irqmap[VIRT_RTC] + ARM_SPI_BASE));
     acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
     acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
                     (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
-- 
2.1.0

Re: [Qemu-devel] [PATCH v2] ARM: Virt: Don't generate RTC ACPI device when using UEFI

[Ard Biesheuvel]

On 13 January 2016 at 15:54, Shannon Zhao <shannon.zhao@linaro.org> wrote:
> When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
> While UEFI can use libfdt to disable the RTC device node in the DTB that
> it passes to the OS, it cannot modify AML. Therefore, we won't generate
> the RTC ACPI device at all when using UEFI.
>
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> v2: just totally don't generate the RTC ACPI device when using UEFI
> ---
>  hw/arm/virt-acpi-build.c | 19 -------------------
>  1 file changed, 19 deletions(-)
>
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 0caf5ce..ac568a3 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -94,23 +94,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>      aml_append(scope, dev);
>  }
>
> -static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> -                                          uint32_t rtc_irq)
> -{
> -    Aml *dev = aml_device("RTC0");
> -    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> -    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> -
> -    Aml *crs = aml_resource_template();
> -    aml_append(crs, aml_memory32_fixed(rtc_memmap->base,
> -                                       rtc_memmap->size, AML_READ_WRITE));
> -    aml_append(crs,
> -               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                             AML_EXCLUSIVE, &rtc_irq, 1));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -    aml_append(scope, dev);
> -}
> -
>  static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  {
>      Aml *dev, *crs;
> @@ -575,8 +558,6 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>      acpi_dsdt_add_cpus(scope, guest_info->smp_cpus);
>      acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
> -    acpi_dsdt_add_rtc(scope, &memmap[VIRT_RTC],
> -                      (irqmap[VIRT_RTC] + ARM_SPI_BASE));
>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>                      (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
> --
> 2.1.0
>

Re: [Qemu-devel] [PATCH v2] ARM: Virt: Don't generate RTC ACPI device when using UEFI

[Laszlo Ersek]

On 01/13/16 15:54, Shannon Zhao wrote:
> When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
> While UEFI can use libfdt to disable the RTC device node in the DTB that
> it passes to the OS, it cannot modify AML. Therefore, we won't generate
> the RTC ACPI device at all when using UEFI.
> 
> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
> ---
> v2: just totally don't generate the RTC ACPI device when using UEFI
> ---
>  hw/arm/virt-acpi-build.c | 19 -------------------
>  1 file changed, 19 deletions(-)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 0caf5ce..ac568a3 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -94,23 +94,6 @@ static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>      aml_append(scope, dev);
>  }
>  
> -static void acpi_dsdt_add_rtc(Aml *scope, const MemMapEntry *rtc_memmap,
> -                                          uint32_t rtc_irq)
> -{
> -    Aml *dev = aml_device("RTC0");
> -    aml_append(dev, aml_name_decl("_HID", aml_string("LNRO0013")));
> -    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> -
> -    Aml *crs = aml_resource_template();
> -    aml_append(crs, aml_memory32_fixed(rtc_memmap->base,
> -                                       rtc_memmap->size, AML_READ_WRITE));
> -    aml_append(crs,
> -               aml_interrupt(AML_CONSUMER, AML_LEVEL, AML_ACTIVE_HIGH,
> -                             AML_EXCLUSIVE, &rtc_irq, 1));
> -    aml_append(dev, aml_name_decl("_CRS", crs));
> -    aml_append(scope, dev);
> -}
> -
>  static void acpi_dsdt_add_flash(Aml *scope, const MemMapEntry *flash_memmap)
>  {
>      Aml *dev, *crs;
> @@ -575,8 +558,6 @@ build_dsdt(GArray *table_data, GArray *linker, VirtGuestInfo *guest_info)
>      acpi_dsdt_add_cpus(scope, guest_info->smp_cpus);
>      acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
> -    acpi_dsdt_add_rtc(scope, &memmap[VIRT_RTC],
> -                      (irqmap[VIRT_RTC] + ARM_SPI_BASE));
>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>                      (irqmap[VIRT_MMIO] + ARM_SPI_BASE), NUM_VIRTIO_TRANSPORTS);
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Re: [Qemu-devel] [PATCH v2] ARM: Virt: Don't generate RTC ACPI device when using UEFI

[Andrew Jones]

On Wed, Jan 13, 2016 at 10:54:17PM +0800, Shannon Zhao wrote:
> When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
> While UEFI can use libfdt to disable the RTC device node in the DTB that
> it passes to the OS, it cannot modify AML. Therefore, we won't generate
> the RTC ACPI device at all when using UEFI.

I think a condensed comment similar to this commit message would be nice
in build_dsdt, or somewhere. We have a policy for mach-virt of generating
ACPI for everything we generate DT. It'd be good if we documented all the
divergences in order to avoid confusion. I won't insist though.

Thanks,
drew

Re: [Qemu-devel] [PATCH v2] ARM: Virt: Don't generate RTC ACPI device when using UEFI

[Laszlo Ersek]

On 01/13/16 17:16, Andrew Jones wrote:
> On Wed, Jan 13, 2016 at 10:54:17PM +0800, Shannon Zhao wrote:
>> When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
>> While UEFI can use libfdt to disable the RTC device node in the DTB that
>> it passes to the OS, it cannot modify AML. Therefore, we won't generate
>> the RTC ACPI device at all when using UEFI.
> 
> I think a condensed comment similar to this commit message would be nice
> in build_dsdt, or somewhere. We have a policy for mach-virt of generating
> ACPI for everything we generate DT. It'd be good if we documented all the
> divergences in order to avoid confusion. [...]

Good idea!

Thanks
Laszlo

Re: [Qemu-devel] [PATCH v2] ARM: Virt: Don't generate RTC ACPI device when using UEFI

[Peter Maydell]

On 13 January 2016 at 16:26, Laszlo Ersek <lersek@redhat.com> wrote:
> On 01/13/16 17:16, Andrew Jones wrote:
>> On Wed, Jan 13, 2016 at 10:54:17PM +0800, Shannon Zhao wrote:
>>> When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
>>> While UEFI can use libfdt to disable the RTC device node in the DTB that
>>> it passes to the OS, it cannot modify AML. Therefore, we won't generate
>>> the RTC ACPI device at all when using UEFI.
>>
>> I think a condensed comment similar to this commit message would be nice
>> in build_dsdt, or somewhere. We have a policy for mach-virt of generating
>> ACPI for everything we generate DT. It'd be good if we documented all the
>> divergences in order to avoid confusion. [...]
>
> Good idea!

I'm happy with the code in this patch, so you can add my
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Shannon, I'm assuming you're planning a v3 with the comment that
Andrew and Laszlo suggest.

thanks
-- PMM

Re: [Qemu-devel] [PATCH v2] ARM: Virt: Don't generate RTC ACPI device when using UEFI

[Shannon Zhao]

On 2016/1/15 22:34, Peter Maydell wrote:
> On 13 January 2016 at 16:26, Laszlo Ersek<lersek@redhat.com>  wrote:
>> >On 01/13/16 17:16, Andrew Jones wrote:
>>> >>On Wed, Jan 13, 2016 at 10:54:17PM +0800, Shannon Zhao wrote:
>>>> >>>When booting the VM with UEFI, UEFI takes ownership of the RTC hardware.
>>>> >>>While UEFI can use libfdt to disable the RTC device node in the DTB that
>>>> >>>it passes to the OS, it cannot modify AML. Therefore, we won't generate
>>>> >>>the RTC ACPI device at all when using UEFI.
>>> >>
>>> >>I think a condensed comment similar to this commit message would be nice
>>> >>in build_dsdt, or somewhere. We have a policy for mach-virt of generating
>>> >>ACPI for everything we generate DT. It'd be good if we documented all the
>>> >>divergences in order to avoid confusion. [...]
>> >
>> >Good idea!
> I'm happy with the code in this patch, so you can add my
> Reviewed-by: Peter Maydell<peter.maydell@linaro.org>
>
Sorry, missed this. I've sent the v3.

> Shannon, I'm assuming you're planning a v3 with the comment that
> Andrew and Laszlo suggest.

-- 
Shannon