diff mbox

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

Message ID 1452696857-4091-1-git-send-email-shannon.zhao@linaro.org
State New
Headers show

Commit Message

Shannon Zhao Jan. 13, 2016, 2:54 p.m. UTC 
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(-)

-- 
2.1.0

Comments

Ard Biesheuvel Jan. 13, 2016, 3:04 p.m. UTC | #1 
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

>
Laszlo Ersek Jan. 13, 2016, 3:06 p.m. UTC | #2 
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>
Andrew Jones Jan. 13, 2016, 4:16 p.m. UTC | #3 
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
Laszlo Ersek Jan. 13, 2016, 4:26 p.m. UTC | #4 
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
Peter Maydell Jan. 15, 2016, 2:34 p.m. UTC | #5 
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
Shannon Zhao Jan. 15, 2016, 2:37 p.m. UTC | #6 
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
diff mbox

Patch

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);