From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50127) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4sCw-0007n8-HQ for qemu-devel@nongnu.org; Mon, 23 May 2016 11:55:55 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b4sCs-0008SV-5m for qemu-devel@nongnu.org; Mon, 23 May 2016 11:55:53 -0400 Received: from mail-pa0-x244.google.com ([2607:f8b0:400e:c03::244]:36824) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4sCr-0008SQ-RT for qemu-devel@nongnu.org; Mon, 23 May 2016 11:55:50 -0400 Received: by mail-pa0-x244.google.com with SMTP id eu11so1282622pad.3 for ; Mon, 23 May 2016 08:55:49 -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> <574307FC.9000605@acm.org> <57430ACA.7040900@redhat.com> From: Corey Minyard Message-ID: <57432801.5010801@acm.org> Date: Mon, 23 May 2016 10:55:45 -0500 MIME-Version: 1.0 In-Reply-To: <57430ACA.7040900@redhat.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 Apfelbaum , Igor Mammedov Cc: Paolo Bonzini , cminyard@mvista.com, qemu-devel@nongnu.org, "Michael S . Tsirkin" On 05/23/2016 08:51 AM, Marcel Apfelbaum wrote: > 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. > Got it, thanks. I was over thinking it. -corey > 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)); >> } >> } >