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

[peng.hao2]

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