From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39214) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4HK6-0008Ky-CS for qemu-devel@nongnu.org; Sat, 21 May 2016 20:32:51 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1b4HK2-0003Gi-6f for qemu-devel@nongnu.org; Sat, 21 May 2016 20:32:49 -0400 Received: from mail-pa0-x241.google.com ([2607:f8b0:400e:c03::241]:33249) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1b4HK1-0003Ge-Vn for qemu-devel@nongnu.org; Sat, 21 May 2016 20:32:46 -0400 Received: by mail-pa0-x241.google.com with SMTP id f8so2486169pag.0 for ; Sat, 21 May 2016 17:32:45 -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> From: Corey Minyard Message-ID: <5740FD4B.5010205@acm.org> Date: Sat, 21 May 2016 19:28:59 -0500 MIME-Version: 1.0 In-Reply-To: <20160520115344.29b1a24d@nial.brq.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: Igor Mammedov Cc: "Michael S . Tsirkin" , Paolo Bonzini , qemu-devel@nongnu.org, cminyard@mvista.com 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 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? -corey