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

[<peng.hao2@zte.com.cn>]

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