qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Igor Mammedov <imammedo@redhat.com>
To: Corey Minyard <minyard@acm.org>
Cc: "Michael S . Tsirkin" <mst@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	qemu-devel@nongnu.org, cminyard@mvista.com
Subject: Re: [Qemu-devel] [PATCH v5 5/6] acpi: Add IPMI table entries
Date: Mon, 23 May 2016 15:15:00 +0200	[thread overview]
Message-ID: <20160523151500.7dde2483@nial.brq.redhat.com> (raw)
In-Reply-To: <5742FAB8.2060207@acm.org>

On Mon, 23 May 2016 07:42:32 -0500
Corey Minyard <minyard@acm.org> wrote:

> On 05/23/2016 05:01 AM, Igor Mammedov wrote:
> > On Sat, 21 May 2016 19:28:59 -0500
> > Corey Minyard <minyard@acm.org> 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.  
> > on linux these methods are evaluated by ACPICA core and named constant
> > is equivalent to a method without arguments that returns constant value.
> >
> > It might be worth to investigate why it doesn't work.  
> 
> I just tried this again and it did work.  I'm not sure why it didn't work
> before, if it was a change in Linux or my error.
> 
> However, the latest IPMI spec has the following text:
> 
> Note: _IFT and _SRV, following, have been reserved in ACPI 3.0 as names for
> control methods defined for SPMI
> 
> Just because it works in Linux doesn't mean it will work on other OSes.
> Wouldn't it be safer to use a method here?
I think it should be safe to use named constant instead of method.
Spec often uses terms method/object interchangeably for argument-less
methods and we use it to a make simpler/smaller AML bytecode.
So far there weren't any issues caused by it either on linux/windows guests.

PS:
ACPI6.0 spec uses named object instead of method for _IFT as an example.


> -corey
> 
> >> -corey  
> 

  reply	other threads:[~2016-05-23 13:15 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-19 15:23 [Qemu-devel] [PATCH v5 0/6] Add IPMI to firmware tables minyard
2016-05-19 15:23 ` [Qemu-devel] [PATCH v5 1/6] ipmi: rework the fwinfo to be fetched from the interface minyard
2016-05-19 15:23 ` [Qemu-devel] [PATCH v5 2/6] pc: Postpone SMBIOS table installation to post machine init minyard
2016-05-20  8:59   ` Igor Mammedov
2016-05-19 15:23 ` [Qemu-devel] [PATCH v5 3/6] smbios: Move table build tools into an include file minyard
2016-05-19 15:24 ` [Qemu-devel] [PATCH v5 4/6] ipmi: Add SMBIOS table entry minyard
2016-05-20  9:11   ` Igor Mammedov
2016-05-19 15:24 ` [Qemu-devel] [PATCH v5 5/6] acpi: Add IPMI table entries minyard
2016-05-20  9:53   ` Igor Mammedov
2016-05-22  0:28     ` Corey Minyard
2016-05-23  9:05       ` Marcel Apfelbaum
2016-05-23 13:39         ` Corey Minyard
2016-05-23 13:51           ` Marcel Apfelbaum
2016-05-23 15:55             ` Corey Minyard
2016-05-23 10:01       ` Igor Mammedov
2016-05-23 12:42         ` Corey Minyard
2016-05-23 13:15           ` Igor Mammedov [this message]
2016-05-23 18:06     ` Paolo Bonzini
2016-05-23 19:25       ` Corey Minyard
2016-05-24 10:03         ` Paolo Bonzini
2016-05-24  7:17       ` Igor Mammedov
2016-05-24 10:03         ` Paolo Bonzini

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160523151500.7dde2483@nial.brq.redhat.com \
    --to=imammedo@redhat.com \
    --cc=cminyard@mvista.com \
    --cc=minyard@acm.org \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).