From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:46244) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4q4i-0007Ms-L1 for qemu-devel@nongnu.org; Mon, 23 May 2016 09:39:17 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b4q4e-0008Uq-CU for qemu-devel@nongnu.org; Mon, 23 May 2016 09:39:15 -0400 Received: from mail-pa0-x234.google.com ([2607:f8b0:400e:c03::234]:36729) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4q4e-0008Ue-1Y for qemu-devel@nongnu.org; Mon, 23 May 2016 09:39:12 -0400 Received: by mail-pa0-x234.google.com with SMTP id bt5so62497461pac.3 for ; Mon, 23 May 2016 06:39:11 -0700 (PDT) Sender: Corey Minyard Reply-To: minyard@acm.org 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> From: Corey Minyard Message-ID: <574307FC.9000605@acm.org> Date: Mon, 23 May 2016 08:39:08 -0500 MIME-Version: 1.0 In-Reply-To: <5742C7D3.6090506@gmail.com> 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: marcel@redhat.com, Igor Mammedov Cc: Paolo Bonzini , cminyard@mvista.com, qemu-devel@nongnu.org, "Michael S . Tsirkin" 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)); } }