Re: [Qemu-devel] [PATCH v5 5/6] acpi: Add IPMI table entries

[Marcel Apfelbaum <marcel.apfelbaum@gmail.com>]

On 05/22/2016 03:28 AM, Corey Minyard wrote:
> Thanks for all the comments.  I didn't know about stubs, as
> there's nothing that currently uses it in hw directory, but
> it's easy enough to add.  I did have two comment below:
>
> On 05/20/2016 04:53 AM, Igor Mammedov wrote:
>> On Thu, 19 May 2016 10:24:01 -0500
>> minyard@acm.org wrote:
> .
> .
> .
>>
>> +    aml_append(dev, aml_name_decl("_STR", aml_string("ipmi_%s",
>> +                                                     info->interface_name)));
>> +    aml_append(dev, aml_name_decl("_UID", aml_int(info->uuid)));
>> +    aml_append(dev, aml_name_decl("_CRS", aml_ipmi_crs(info, resource)));
>> +
>> +    /*
>> +     * The spec seems to require these to be methods.  All the examples
>> +     * show them this way and it doesn't seem to work if they are not.
>> +     */
>> +    method = aml_method("_IFT", 0, AML_NOTSERIALIZED);
>> +    aml_append(method, aml_return(aml_int(info->interface_type)));
>> +    aml_append(dev, method);
>> +    method = aml_method("_SRV", 0, AML_NOTSERIALIZED);
>> +    aml_append(method, aml_return(aml_int(version)));
>> +    aml_append(dev, method);
>> replace these methods with aml_name_decl() as they do not contain any logic
>> except of returning static value.
>
> I'm not sure why, but what you ask doesn't work.  These have to be
> methods, and that is show by the IPMI spec, as the comment above
> these says.
>
> .
> .
> .
>
>> @@ -2011,7 +2015,7 @@ build_dsdt(GArray *table_data, GArray *linker,
>>           build_hpet_aml(dsdt);
>>           build_piix4_pm(dsdt);one
>>           build_piix4_isa_bridge(dsdt);
>> -        build_isa_devices_aml(dsdt);
>> +        build_isa_devices_aml(dsdt, pcms->isa_bus);
>> I'm not sure about adding 'isa_bus' field to PCMachineState,
>> it might be better to find a ISA bus object internally in
>> build_isa_devices_aml() and assert if found more than one,
>> since code assumes that there is only one anyway.
>

I agree.

> I don't see a clean way to find the ISA bus object.  Well, I guess
> you can scan the PCI bus for an ISA bus object, is that what you
> mean?
>

You can run a QOM query. Please look for object_resolve_path function.
Please let me know if you have difficulties with it.

Thanks,
Marcel



> -corey
>