[v6] add pvpanic mmio support

[Qemu-devel] [PATCH V6 5/6] hw/arm/virt: add pvpanic device in virt acpi table

Posted by Peng Hao 7 years, 2 months ago

add pvpanic device in virt acpi table, so when kenrel command line uses
acpi=force, kernel can get info from acpi table in aarch64.

Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
---
 hw/arm/virt-acpi-build.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
index 5785fb6..d126cee 100644
--- a/hw/arm/virt-acpi-build.c
+++ b/hw/arm/virt-acpi-build.c
@@ -61,6 +61,21 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
     }
 }
 
+static void acpi_dsdt_add_pvpanic(Aml *scope, const MemMapEntry *pvpanic_memmap)
+{
+    Aml *dev = aml_device("PANC");
+    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
+    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
+
+    Aml *crs = aml_resource_template();
+    aml_append(crs, aml_memory32_fixed(pvpanic_memmap->base,
+                                       pvpanic_memmap->size, AML_READ_WRITE));
+
+    aml_append(dev, aml_name_decl("_CRS", crs));
+
+    aml_append(scope, dev);
+}
+
 static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
                                            uint32_t uart_irq)
 {
@@ -770,6 +785,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
     acpi_dsdt_add_cpus(scope, vms->smp_cpus);
     acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
                        (irqmap[VIRT_UART] + ARM_SPI_BASE));
+    acpi_dsdt_add_pvpanic(scope, &memmap[VIRT_PVPANIC]);
     acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
     acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
     acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH V6 5/6] hw/arm/virt: add pvpanic device in virt acpi table

Posted by Andrew Jones 7 years, 2 months ago

On Mon, Nov 12, 2018 at 07:42:19PM +0800, Peng Hao wrote:
> add pvpanic device in virt acpi table, so when kenrel command line uses
> acpi=force, kernel can get info from acpi table in aarch64.
> 
> Signed-off-by: Peng Hao <peng.hao2@zte.com.cn>
> ---
>  hw/arm/virt-acpi-build.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/hw/arm/virt-acpi-build.c b/hw/arm/virt-acpi-build.c
> index 5785fb6..d126cee 100644
> --- a/hw/arm/virt-acpi-build.c
> +++ b/hw/arm/virt-acpi-build.c
> @@ -61,6 +61,21 @@ static void acpi_dsdt_add_cpus(Aml *scope, int smp_cpus)
>      }
>  }
>  
> +static void acpi_dsdt_add_pvpanic(Aml *scope, const MemMapEntry *pvpanic_memmap)
> +{
> +    Aml *dev = aml_device("PANC");

Shouldn't this be "PEVT" ("panic event"), like it is for x86?

> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));

Why add _UID? Also, I see x86 had some issues with not having _STA and
then having a _STA without the UI flag. It now has _STA=0xf
(PRESENT|ENABLED|UI|FUNCTIONING). I'm not saying we need to do that to,
but I'd like to know if it was considered and decided we don't need to.

> +
> +    Aml *crs = aml_resource_template();
> +    aml_append(crs, aml_memory32_fixed(pvpanic_memmap->base,
> +                                       pvpanic_memmap->size, AML_READ_WRITE));
> +
> +    aml_append(dev, aml_name_decl("_CRS", crs));
> +
> +    aml_append(scope, dev);
> +}
> +
>  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>                                             uint32_t uart_irq)
>  {
> @@ -770,6 +785,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>      acpi_dsdt_add_cpus(scope, vms->smp_cpus);
>      acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
> +    acpi_dsdt_add_pvpanic(scope, &memmap[VIRT_PVPANIC]);
>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
>      acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> -- 
> 1.8.3.1
> 
> 

Thanks,
drew

Re: [Qemu-devel][PATCH V6 5/6] hw/arm/virt: add pvpanic device invirt acpi table

Posted by peng.hao2@zte.com.cn 7 years, 2 months ago

>> add pvpanic device in virt acpi table, so when kenrel command line uses
>> acpi=force, kernel can get info from acpi table in aarch64.

[...]

>>
>> +static void acpi_dsdt_add_pvpanic(Aml *scope, const MemMapEntry *pvpanic_memmap)
>> +{
>> +    Aml *dev = aml_device("PANC");
>
>Shouldn't this be "PEVT" ("panic event"), like it is for x86?
>
yeah, I will change it.
>> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>
>Why add _UID? Also, I see x86 had some issues with not having _STA and
>then having a _STA without the UI flag. It now has _STA=0xf
>(PRESENT|ENABLED|UI|FUNCTIONING). I'm not saying we need to do that to,
>but I'd like to know if it was considered and decided we don't need to.
>
The way the kernel code processes _STA is that if _STA is not found, the default 
setting is (PRESENT|ENABLED|UI|FUNCTIONING). So I think it is not necessary
 to add it. It is only parsed in the pvpanic driver. 
for _UID, I think it is used for device index. I just fill it with 0 because there is 
only one pvpanic device.

by the way, How to get the value of ACPI conveniently? how dou you get the 
value of _STA?

Thanks.
>> +
>> +    Aml *crs = aml_resource_template();
>> +    aml_append(crs, aml_memory32_fixed(pvpanic_memmap->base,
>> +                                       pvpanic_memmap->size, AML_READ_WRITE));
>> +
>> +    aml_append(dev, aml_name_decl("_CRS", crs));
>> +
>> +    aml_append(scope, dev);
>> +}
>> +
>>  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
>>                                             uint32_t uart_irq)
>>  {
>> @@ -770,6 +785,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
>>      acpi_dsdt_add_cpus(scope, vms->smp_cpus);
>>      acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
>>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
>> +    acpi_dsdt_add_pvpanic(scope, &memmap[VIRT_PVPANIC]);
>>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
>>      acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
>>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
>> --
>> 1.8.3.1
>>
>>
>
>Thanks,
>drew

Re: [Qemu-devel] [PATCH V6 5/6] hw/arm/virt: add pvpanic device invirt acpi table

Posted by Andrew Jones 7 years, 2 months ago

On Fri, Nov 16, 2018 at 09:45:42AM +0800, peng.hao2@zte.com.cn wrote:
> >> add pvpanic device in virt acpi table, so when kenrel command line uses
> >> acpi=force, kernel can get info from acpi table in aarch64.
> 
> [...]
> 
> >>
> >> +static void acpi_dsdt_add_pvpanic(Aml *scope, const MemMapEntry *pvpanic_memmap)
> >> +{
> >> +    Aml *dev = aml_device("PANC");
> >
> >Shouldn't this be "PEVT" ("panic event"), like it is for x86?
> >
> yeah, I will change it.
> >> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
> >> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
> >
> >Why add _UID? Also, I see x86 had some issues with not having _STA and
> >then having a _STA without the UI flag. It now has _STA=0xf
> >(PRESENT|ENABLED|UI|FUNCTIONING). I'm not saying we need to do that to,
> >but I'd like to know if it was considered and decided we don't need to.
> >
> The way the kernel code processes _STA is that if _STA is not found, the default 
> setting is (PRESENT|ENABLED|UI|FUNCTIONING). So I think it is not necessary
>  to add it. It is only parsed in the pvpanic driver. 

Good enough for me.

> for _UID, I think it is used for device index. I just fill it with 0 because there is 
> only one pvpanic device.

OK, but I'm still not sure it's necessary.

> 
> by the way, How to get the value of ACPI conveniently? how dou you get the 
> value of _STA?

Not sure what you mean here. From where do you want to check the value?
If you mean from guest userspace, then there's nothing convenient that I
know of. You'll have to disassemble the tables you extract from sysfs,
afaik.

Thanks,
drew

> 
> Thanks.
> >> +
> >> +    Aml *crs = aml_resource_template();
> >> +    aml_append(crs, aml_memory32_fixed(pvpanic_memmap->base,
> >> +                                       pvpanic_memmap->size, AML_READ_WRITE));
> >> +
> >> +    aml_append(dev, aml_name_decl("_CRS", crs));
> >> +
> >> +    aml_append(scope, dev);
> >> +}
> >> +
> >>  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> >>                                             uint32_t uart_irq)
> >>  {
> >> @@ -770,6 +785,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >>      acpi_dsdt_add_cpus(scope, vms->smp_cpus);
> >>      acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
> >>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
> >> +    acpi_dsdt_add_pvpanic(scope, &memmap[VIRT_PVPANIC]);
> >>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
> >>      acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
> >>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> >> --
> >> 1.8.3.1
> >>
> >>
> >
> >Thanks,
> >drew

Re: [Qemu-devel][PATCH V6 5/6] hw/arm/virt: add pvpanic deviceinvirt acpi table

Posted by peng.hao2@zte.com.cn 7 years, 2 months ago

>> >> add pvpanic device in virt acpi table, so when kenrel command line uses
>> >> acpi=force, kernel can get info from acpi table in aarch64.
>>
>> [...]
>>
>> >>
>> >> +static void acpi_dsdt_add_pvpanic(Aml *scope, const MemMapEntry *pvpanic_memmap)
>> >> +{
>> >> +    Aml *dev = aml_device("PANC");
>> >
>> >Shouldn't this be "PEVT" ("panic event"), like it is for x86?
>> >
>> yeah, I will change it.
>> >> +    aml_append(dev, aml_name_decl("_HID", aml_string("QEMU0001")));
>> >> +    aml_append(dev, aml_name_decl("_UID", aml_int(0)));
>> >
>> >Why add _UID? Also, I see x86 had some issues with not having _STA and
>> >then having a _STA without the UI flag. It now has _STA=0xf
>> >(PRESENT|ENABLED|UI|FUNCTIONING). I'm not saying we need to do that to,
>> >but I'd like to know if it was considered and decided we don't need to.
>> >
>> The way the kernel code processes _STA is that if _STA is not found, the default
>> setting is (PRESENT|ENABLED|UI|FUNCTIONING). So I think it is not necessary
>>  to add it. It is only parsed in the pvpanic driver.
>
>Good enough for me.
>
>> for _UID, I think it is used for device index. I just fill it with 0 because there is
>> only one pvpanic device.
>
>OK, but I'm still not sure it's necessary.
>
you can see other devices in virt-acpi-build.c. _UID is used for device index.

>>
>> by the way, How to get the value of ACPI conveniently? how dou you get the
>> value of _STA?
>
>Not sure what you mean here. From where do you want to check the value?
>If you mean from guest userspace, then there's nothing convenient that I
>know of. You'll have to disassemble the tables you extract from sysfs,
>afaik.
>
yeah ,I just want to know this.
thanks
>Thanks,
>drew

>
> Thanks.
> >> +
> >> +    Aml *crs = aml_resource_template();
> >> +    aml_append(crs, aml_memory32_fixed(pvpanic_memmap->base,
> >> +                                       pvpanic_memmap->size, AML_READ_WRITE));
> >> +
> >> +    aml_append(dev, aml_name_decl("_CRS", crs));
> >> +
> >> +    aml_append(scope, dev);
> >> +}
> >> +
> >>  static void acpi_dsdt_add_uart(Aml *scope, const MemMapEntry *uart_memmap,
> >>                                             uint32_t uart_irq)
> >>  {
> >> @@ -770,6 +785,7 @@ build_dsdt(GArray *table_data, BIOSLinker *linker, VirtMachineState *vms)
> >>      acpi_dsdt_add_cpus(scope, vms->smp_cpus);
> >>      acpi_dsdt_add_uart(scope, &memmap[VIRT_UART],
> >>                         (irqmap[VIRT_UART] + ARM_SPI_BASE));
> >> +    acpi_dsdt_add_pvpanic(scope, &memmap[VIRT_PVPANIC]);
> >>      acpi_dsdt_add_flash(scope, &memmap[VIRT_FLASH]);
> >>      acpi_dsdt_add_fw_cfg(scope, &memmap[VIRT_FW_CFG]);
> >>      acpi_dsdt_add_virtio(scope, &memmap[VIRT_MMIO],
> >> --
> >> 1.8.3.1
> >>
> >>
> >
> >Thanks,
> >drew