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

[Corey Minyard <minyard@acm.org>]

On 05/23/2016 04:05 AM, Marcel Apfelbaum wrote:
> 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
>

Thanks for that info.

I looked at that, and it works for a single object, but I don't see a way to
make this work if there is more than one IPMI device defined.

I changed to the following code, which seems to work ok.  You pass in the
ISA bus (or the SMBus in the I2C case).

void build_acpi_ipmi_devices(Aml *scope, BusState *bus)
{
     BusChild *kid;

     QTAILQ_FOREACH(kid, &bus->children,  sibling) {
         DeviceState *dev = kid->child;
         Object *obj = object_dynamic_cast(OBJECT(dev), 
TYPE_IPMI_INTERFACE);
         IPMIInterface *ii;
         IPMIInterfaceClass *iic;
         IPMIFwInfo info;

         if (!obj) {
             continue;
         }

         ii = IPMI_INTERFACE(obj);
         iic = IPMI_INTERFACE_GET_CLASS(obj);
         memset(&info, 0, sizeof(info));
         iic->get_fwinfo(ii, &info);
         aml_append(scope, acpi_ipmi_device(&info));
     }
}