linux-kernel - Re: [Patch v4 07/16] x86, irq: Prefer assigned ID in APIC ID register for x86

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 10 Sep 2014 10:14:02 +0800
From:	Jiang Liu <jiang.liu@...ux.intel.com>
To:	Thomas Gleixner <tglx@...utronix.de>
CC:	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Randy Dunlap <rdunlap@...radead.org>,
	Yinghai Lu <yinghai@...nel.org>,
	Borislav Petkov <bp@...en8.de>,
	Grant Likely <grant.likely@...aro.org>, x86@...nel.org,
	Prarit Bhargava <prarit@...hat.com>,
	Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Tony Luck <tony.luck@...el.com>,
	Joerg Roedel <joro@...tes.org>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org,
	linux-acpi@...r.kernel.org,
	Sebastian Andrzej Siewior <sebastian@...akpoint.cc>,
	Ingo Molnar <mingo@...nel.org>
Subject: Re: [Patch v4 07/16] x86, irq: Prefer assigned ID in APIC ID register
 for x86_64



On 2014/9/9 19:04, Thomas Gleixner wrote:
> On Thu, 28 Aug 2014, Jiang Liu wrote:
> 
>> From: Yinghai Lu <yinghai@...nel.org>
>>
>> Perfer the assigned ID in APIC ID register for x86_64 if it's still
>> available.
>>
>> Signed-off-by: Yinghai Lu <yinghai@...nel.org>
>> Cc: Joerg Roedel <joro@...tes.org>
>> Cc: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
>> Cc: Sebastian Andrzej Siewior <sebastian@...akpoint.cc>
>> Signed-off-by: Jiang Liu <jiang.liu@...ux.intel.com>
>> ---
>>  arch/x86/kernel/apic/io_apic.c |   38 +++++++++++++++++++++++++++++++++-----
>>  1 file changed, 33 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/kernel/apic/io_apic.c b/arch/x86/kernel/apic/io_apic.c
>> index 3faf9599ff29..196d9c15fdec 100644
>> --- a/arch/x86/kernel/apic/io_apic.c
>> +++ b/arch/x86/kernel/apic/io_apic.c
>> @@ -3575,26 +3575,54 @@ static int __init io_apic_get_unique_id(int ioapic, int apic_id)
>>  	return apic_id;
>>  }
>>  
>> -static u8 __init io_apic_unique_id(u8 id)
>> +static u8 io_apic_unique_id(int idx, u8 id)
> 
> How is that change related to the changelog?
My bad, removal of __init marker belongs to next patch.

> 
>>  {
>>  	if ((boot_cpu_data.x86_vendor == X86_VENDOR_INTEL) &&
>>  	    !APIC_XAPIC(apic_version[boot_cpu_physical_apicid]))
>> -		return io_apic_get_unique_id(nr_ioapics, id);
>> +		return io_apic_get_unique_id(idx, id);
>>  	else
>>  		return id;
>>  }
>>  #else
>> -static u8 __init io_apic_unique_id(u8 id)
>> +static u8 io_apic_unique_id(int idx, u8 id)
>>  {
>>  	int i;
>> +	u8 new_id;
>> +	unsigned long flags;
>>  	DECLARE_BITMAP(used, 256);
>> +	union IO_APIC_reg_00 reg_00;
>>  
>>  	bitmap_zero(used, 256);
>>  	for_each_ioapic(i)
>>  		__set_bit(mpc_ioapic_id(i), used);
>>  	if (!test_bit(id, used))
>>  		return id;
>> -	return find_first_zero_bit(used, 256);
>> +
>> +	/* check register at first */
>> +	raw_spin_lock_irqsave(&ioapic_lock, flags);
>> +	reg_00.raw = io_apic_read(idx, 0);
>> +	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
>> +	new_id = reg_00.bits.ID;
>> +	if (!test_bit(new_id, used)) {
>> +		apic_printk(APIC_VERBOSE, KERN_INFO
>> +			"IOAPIC[%d]: Using reg apic_id %d instead of %d\n",
>> +			 idx, new_id, id);
>> +		return new_id;
>> +	}
>> +
>> +	new_id = find_first_zero_bit(used, 256);
>> +	reg_00.bits.ID = new_id;
>> +	raw_spin_lock_irqsave(&ioapic_lock, flags);
>> +	io_apic_write(idx, 0, reg_00.raw);
>> +	reg_00.raw = io_apic_read(idx, 0);
>> +	raw_spin_unlock_irqrestore(&ioapic_lock, flags);
>> +
>> +	/* Sanity check */
>> +	if (reg_00.bits.ID != new_id)
>> +		pr_warn("IOAPIC[%d]: Unable to change apic_id to %d!\n",
>> +			idx, new_id);
> 
> So we detect, that the ID could not be changed and we return it
> nevertheless?
Seems it deserves a BUG_ON here. We have tried the ID from ACPI table,
the ID from hardware register, they all fails. And we can't change the
hardware register too. Seems no way out here.
Regards!
Gerry
> 
> Thanks,
> 
> 	tglx
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/