From: Cao jin <caoj.fnst@cn.fujitsu.com>
To: Borislav Petkov <bp@alien8.de>
Cc: <x86@kernel.org>, <linux-pm@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <linux-acpi@vger.kernel.org>,
<rjw@rjwysocki.net>, <len.brown@intel.com>, <pavel@ucw.cz>,
<tglx@linutronix.de>, <mingo@redhat.com>, <hpa@zytor.com>
Subject: Re: [RFC PATCH 1/2] x86/acpi: Improve code readablity of early madt processing
Date: Mon, 16 Mar 2020 17:20:54 +0800 [thread overview]
Message-ID: <553ef3e0-0270-5631-b7fe-7fa1d48a5dc1@cn.fujitsu.com> (raw)
In-Reply-To: <8b19ed2f-2470-c522-cc47-f615c615be20@cn.fujitsu.com>
Hello BP,
Does the explanation make sense to you?
BTW, also test it on i386, boots fine.
--
Sincerely,
Cao jin
On 2/25/20 3:02 PM, Cao jin wrote:
> On 2/24/20 9:21 PM, Borislav Petkov wrote:
>> On Thu, Jan 23, 2020 at 09:41:43AM +0800, Cao jin wrote:
>>> Current processing logic is confusing.
>>>
>>> Return value of early_acpi_parse_madt_lapic_addr_ovr() indicates error(< 0),
>>> parsed entry number(>= 0).
>>
>> You mean, the count of table entries parsed successfully?
>
> Yes, 0 for no override sub-table.
>
>>
>>> So, it makes no sense to initialize acpi_lapic & smp_found_config
>>> seeing no override entry, instead, initialize them seeing MADT.
>>
>> Err, that logical conclusion is not really clear to me - pls try
>> again with more detail. I kinda see what you mean by looking at
>> acpi_process_madt() but before I commit a change like that, I better
>> have the warm and fuzzy feeling that it is correct and properly
>> explained in its commit message.
>>
>
> My understanding of early_acpi_process_madt(): mainly for getting APIC
> register base address(acpi_lapic_addr) from MADT, then process it via
> register_lapic_address(). acpi_lapic_addr could be got from one of
> following 2 places:
>
> 1. MADT header (32-bit address, always exist)
> 2. MADT sub-table: Local APIC Address Override (64-bit address,
> optional, high priority and use it if present)
>
> So the making-sense logic to me goes like:
>
> 1. get (32-bit) acpi_lapic_addr from MADT header.
> 2. check if there is MADT override structure & get 64-bit
> acpi_lapic_addr if present.
> 3. register_lapic_address(acpi_lapic_addr);
>
> Then, it looks weird to me putting register_lapic_address() into
> early_acpi_parse_madt_lapic_addr_ovr(), the result is not wrong, but the
> code logic is hard for newbie. (these 2 functions both does more than
> its name tells, register_lapic_address() also get boot cpu APIC ID &
> version.)
>
> Variable acpi_lapic and its counterpart smp_found_config from MPS
> indicate whether it is SMP system, right? The following code:
>
>
> error = early_acpi_parse_madt_lapic_addr_ovr();
> if (!error) {
> acpi_lapic = 1;
> smp_found_config = 1;
> }
>
> means setting them when there is no override sub-table, so why can't
> moving the setting operation out? Another issue: if there *is* override
> sub-table, don't set those two?
>
next prev parent reply other threads:[~2020-03-16 9:21 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-23 1:41 [RFC PATCH 0/2] x86/boot: early ACPI MADT processing cleanup Cao jin
2020-01-23 1:41 ` [RFC PATCH 1/2] x86/acpi: Improve code readablity of early madt processing Cao jin
2020-02-24 13:21 ` Borislav Petkov
2020-02-25 7:02 ` Cao jin
2020-03-16 9:20 ` Cao jin [this message]
2020-01-23 1:41 ` [RFC PATCH 2/2] x86/acpi: Cleanup acpi_process_madt() Cao jin
2020-02-19 7:32 ` [RFC PATCH 0/2] x86/boot: early ACPI MADT processing cleanup Cao jin
2020-03-27 12:30 ` Cao jin
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=553ef3e0-0270-5631-b7fe-7fa1d48a5dc1@cn.fujitsu.com \
--to=caoj.fnst@cn.fujitsu.com \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=len.brown@intel.com \
--cc=linux-acpi@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=pavel@ucw.cz \
--cc=rjw@rjwysocki.net \
--cc=tglx@linutronix.de \
--cc=x86@kernel.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).