From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:36001) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4lny-00005U-PN for qemu-devel@nongnu.org; Mon, 23 May 2016 05:05:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b4lnu-0002lF-Vj for qemu-devel@nongnu.org; Mon, 23 May 2016 05:05:41 -0400 Received: from mail-lf0-x243.google.com ([2a00:1450:4010:c07::243]:34107) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4lnu-0002kP-Kj for qemu-devel@nongnu.org; Mon, 23 May 2016 05:05:38 -0400 Received: by mail-lf0-x243.google.com with SMTP id 65so1960413lfq.1 for ; Mon, 23 May 2016 02:05:30 -0700 (PDT) Reply-To: marcel@redhat.com 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> From: Marcel Apfelbaum Message-ID: <5742C7D3.6090506@gmail.com> Date: Mon, 23 May 2016 12:05:23 +0300 MIME-Version: 1.0 In-Reply-To: <5740FD4B.5010205@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/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 >