Re: [PATCH v22 5/9] ACPI: Record the Generic Error Status Block address

[Philippe Mathieu-Daudé]

On 1/17/20 11:47 AM, gengdongjiu wrote:
> On 2020/1/17 15:39, Philippe Mathieu-Daudé wrote:
> > >          table_offsets = g_array_new(false, true /* clear */,
> > >                                            sizeof(uint32_t));
> > > @@ -831,7 +832,9 @@ void virt_acpi_build(VirtMachineState *vms, AcpiBuildTables 
> > > *tables)
> > >        acpi_add_table(table_offsets, tables_blob);
> > >        build_spcr(tables_blob, tables->linker, vms);
> > >    -    if (vms->ras) {
> > > +    acpi_ged_state = ACPI_GED(object_resolve_path_type("", TYPE_ACPI_GED,
> > > +                                                       NULL));
> >
> > Testing vms->ras first is cheaper than calling object_resolve_path_type(). 
> > Since some people are spending lot of time to reduce VM boot time, it might be 
> > worth considering.
> Thanks Philippe's comments.
>
> Do you think it should be written to below[1]? right?
>
> [1]:
> if (vms->ras && acpi_ged_state)

No, as:

  if (vms->ras) {
    AcpiGedState *acpi_ged_state;

    acpi_ged_state = ACPI_GED(object_resolve_path_type("", 
TYPE_ACPI_GED, NULL));
    if (acpi_ged_state) {
      acpi_add_table(table_offsets, tables_blob);
      ...

Maybe I'm wrong and there is not much impact in VM boot time here, 
reviews welcomed :)

> > > +    if (acpi_ged_state &&  vms->ras) {
> > >            acpi_add_table(table_offsets, tables_blob);
> > >            build_ghes_error_table(tables->hardware_errors, tables->linker);
> > >            acpi_build_hest(tables_blob, tables->hardware_errors,
> > > @@ -925,6 +928,7 @@ void virt_acpi_setup(VirtMachineState *vms)
> > >    {