Re: [PATCH v8 01/13] x86/apic: Construct a selector for the interrupt delivery mode

From	Dou Liyang <douly.fnst@cn.fujitsu.com>
Newsgroups	linux.kernel
Subject	Re: [PATCH v8 01/13] x86/apic: Construct a selector for the interrupt delivery mode
Date	2017-09-13 05:50 +0200
Message-ID	<up6Qx-20h-3@gated-at.bofh.it> (permalink)
References	(2 earlier) <umFB7-hz-5@gated-at.bofh.it> <umWsh-35p-15@gated-at.bofh.it> <umXy1-3Mj-3@gated-at.bofh.it> <uoIbv-2Sd-5@gated-at.bofh.it> <up5KR-1mw-3@gated-at.bofh.it>
Organization	linux.* mail to news gateway

Show all headers | View raw

Hi Baoquan,

At 09/13/2017 10:30 AM, Baoquan He wrote:
> Hi dou,
>
> On 09/12/17 at 09:20am, Dou Liyang wrote:
>> I thought again and again, I would not change this check logic.
>>
>> Because actually, we have three possibilities:
>>
>>   1. ACPI on chip
>>   2. 82489DX
>>   3. no APIC
>>
>> lapic_is_integrated() is used to check the APIC's type which is
>> APIC on chip or 82489DX. It has a prerequisite, we should avoid
>> the third possibility(no APIC) first, which is decided by
>> boot_cpu_has(X86_FEATURE_APIC) and smp_found_config. So, the original
>> logic:
>>
>> if (!boot_cpu_has(X86_FEATURE_APIC) && !smp_found_config)
>
> I won't insist that the logic need be changed. From the test result, the
> patchset works very well with notsc specified. And the whole patchset
> looks not risky. Maybe the patch putting acpi_early_init() earlier can
> be posted independently and involve other ARCHes maintainer to review.
>

Yes,  I will send it as an independent patch, and Cc other ARCH
maintainers

> About the code logic, I think the confusion comes from the unclear
> condition check. E.g the above case, you said it's used to check
> discrete apic, in fact !boot_cpu_has(X86_FEATURE_APIC) could means 3
> cases:
> 1) discrete apic
> 2) no apic
> 3) integrated apic but disabled by bios.

Indeed

>
> See, that's why it's confusing, the condition of judgement is not
> adequate. I don't know why the code contributer wanted to check discrete
> apic case with it.
>
> Anyway, after discussion, it's clear to me now. And the code works well.
> So it's up to you to change it or not. Except of this place, the whole
> patchset looks good.

Thank you very much for your review and test.


Thanks,
	dou.
>
> Thanks
> Baoquan
>
>>
>> ...is not just for 82489DX, but also for no APIC.
>>
>> It looks more correct and understandable than us.
>>
>> I am sorry my comments were wrong, and misled us. I will modify it
>> in my next version.
>>
>> BTW, How about your test result, is this series OK?
>>
>> Thanks,
>> 	dou.
>>
>>
>
>
>

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

Re: [PATCH v8 01/13] x86/apic: Construct a selector for the interrupt  delivery mode Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-09-12 03:30 +0200
  Re: [PATCH v8 01/13] x86/apic: Construct a selector for the  interrupt delivery mode Baoquan He <bhe@redhat.com> - 2017-09-12 10:10 +0200
  Re: [PATCH v8 01/13] x86/apic: Construct a selector for the  interrupt delivery mode Baoquan He <bhe@redhat.com> - 2017-09-13 04:40 +0200
    Re: [PATCH v8 01/13] x86/apic: Construct a selector for the interrupt  delivery mode Dou Liyang <douly.fnst@cn.fujitsu.com> - 2017-09-13 05:50 +0200

csiph-web