From: Igor Mammedov <imammedo@redhat.com>
To: Marian Posteuca <posteuca@mutex.one>
Cc: Peter Maydell <peter.maydell@linaro.org>,
Eduardo Habkost <ehabkost@redhat.com>,
Ben Warren <ben@skyportsystems.com>,
"Michael S. Tsirkin" <mst@redhat.com>,
Richard Henderson <richard.henderson@linaro.org>,
qemu-devel@nongnu.org, Dongjiu Geng <gengdongjiu@huawei.com>,
Shannon Zhao <shannon.zhaosl@gmail.com>,
Xiang Zheng <zhengxiang9@huawei.com>,
qemu-arm@nongnu.org, Paolo Bonzini <pbonzini@redhat.com>,
Xiao Guangrong <xiaoguangrong.eric@gmail.com>
Subject: Re: [PATCH v2] acpi: Permit OEM ID and OEM table ID fields to be changed
Date: Tue, 29 Dec 2020 12:31:18 +0100 [thread overview]
Message-ID: <20201229123118.64e1a41f@redhat.com> (raw)
In-Reply-To: <878s9onq41.fsf@mutex.one>
On Wed, 23 Dec 2020 23:56:30 +0200
Marian Posteuca <posteuca@mutex.one> wrote:
> Thanks for the thorough review.
>
> Igor Mammedov <imammedo@redhat.com> writes:
> > On Tue, 22 Dec 2020 13:33:53 +0200
> > Marian Posteuca <posteuca@mutex.one> wrote:
> >
> > I see defaults are now initialized in pcmc->oem_[table_]id fields,
> > and sometimes used from there, so question is why
> > do we need use_sig_oem and keeping old code
> >
> > if (oem_id) {
> > strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
> > } else {
> > memcpy(h->oem_id, ACPI_BUILD_APPNAME6, 6);
> > }
> >
> > if ()) {
> > strncpy((char *)h->oem_table_id, oem_table_id, sizeof(h->oem_table_id));
> > } else {
> > memcpy(h->oem_table_id, ACPI_BUILD_APPNAME4, 4);
> > memcpy(h->oem_table_id + 4, sig, 4);
> > }
> > I'd rather drop 'else' branches altogether and simplify to something like this
> >
> > g_assert(oem_id);
> > strncpy((char *)h->oem_id, oem_id, sizeof h->oem_id);
> > g_assert(oem_table_id)
> > strncpy((char *)h->oem_table_id, oem_table_id, sizeof(h->oem_table_id));
> > + padding
> >
> > and make sure ids are properly propagated everywhere.
> >
>
> I'm not sure if I understood this point correctly. You want to remove the appending
> of the sig part to the oem_table_id field, and just use whatever is
> passed by the caller for oem_table_id?
yes, according to spec unique oem_table_id helps only in distinguishing different
pieces of DSDT/SSDT tables, for other tables it doesn't make any sense to make it unique.
and this matches what real machines do.
prev parent reply other threads:[~2020-12-29 11:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-22 11:33 [PATCH v2] acpi: Permit OEM ID and OEM table ID fields to be changed Marian Posteuca
2020-12-22 12:43 ` Michael Tokarev
2020-12-22 15:39 ` Marian Posteuca
2020-12-22 22:46 ` Paolo Bonzini
2020-12-27 23:12 ` Marian Posteuca
2020-12-23 10:54 ` Igor Mammedov
2020-12-23 21:56 ` Marian Posteuca
2020-12-29 11:31 ` Igor Mammedov [this message]
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=20201229123118.64e1a41f@redhat.com \
--to=imammedo@redhat.com \
--cc=ben@skyportsystems.com \
--cc=ehabkost@redhat.com \
--cc=gengdongjiu@huawei.com \
--cc=mst@redhat.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=posteuca@mutex.one \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=shannon.zhaosl@gmail.com \
--cc=xiaoguangrong.eric@gmail.com \
--cc=zhengxiang9@huawei.com \
/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).