From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48816) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4qGK-0003ve-HQ for qemu-devel@nongnu.org; Mon, 23 May 2016 09:51:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b4qGF-0002gn-KT for qemu-devel@nongnu.org; Mon, 23 May 2016 09:51:16 -0400 Received: from mx1.redhat.com ([209.132.183.28]:41951) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4qGF-0002gf-Bq for qemu-devel@nongnu.org; Mon, 23 May 2016 09:51:11 -0400 References: <1463671442-15631-1-git-send-email-minyard@acm.org> <1463671442-15631-6-git-send-email-minyard@acm.org> <20160520115344.29b1a24d@nial.brq.redhat.com> <5740FD4B.5010205@acm.org> <5742C7D3.6090506@gmail.com> <574307FC.9000605@acm.org> From: Marcel Apfelbaum Message-ID: <57430ACA.7040900@redhat.com> Date: Mon, 23 May 2016 16:51:06 +0300 MIME-Version: 1.0 In-Reply-To: <574307FC.9000605@acm.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v5 5/6] acpi: Add IPMI table entries List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: minyard@acm.org, Igor Mammedov Cc: Paolo Bonzini , cminyard@mvista.com, qemu-devel@nongnu.org, "Michael S . Tsirkin" On 05/23/2016 04:39 PM, Corey Minyard wrote: > 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 was under impression that you are looking for a way to find the isa_bus. Since we have only a single isa_bus you can use object_resolve_path instead of adding the isa_bus to the pc machine. I didn't refer to the IPMI device, sorry if I was not clear enough. Thanks, Marcel > > 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)); > } > }