linux-kernel - RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking

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] [day] [month] [year] [list]

Message-ID: <715D42877B251141A38726ABF5CABF2C054568FD1E@pdsmsx503.ccr.corp.intel.com>
Date:	Tue, 19 May 2009 18:34:29 +0800
From:	"Han, Weidong" <weidong.han@...el.com>
To:	'Ingo Molnar' <mingo@...e.hu>
CC:	"Siddha, Suresh B" <suresh.b.siddha@...el.com>,
	"'dwmw2@...radead.org'" <dwmw2@...radead.org>,
	"'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>,
	"'iommu@...ts.linux-foundation.org'" 
	<iommu@...ts.linux-foundation.org>,
	"'kvm@...r.kernel.org'" <kvm@...r.kernel.org>
Subject: RE: [PATCH 2/2] Intel-IOMMU, intr-remap: source-id checking

Ingo Molnar wrote:
> * Han, Weidong <weidong.han@...el.com> wrote:
> 
>> Ingo Molnar wrote:
>>> * Han, Weidong <weidong.han@...el.com> wrote:
>>> 
>>>> Siddha, Suresh B wrote:
>>>>> On Wed, 2009-05-06 at 23:16 -0700, Han, Weidong wrote:
>>>>>> @@ -634,6 +694,44 @@ static int ir_parse_ioapic_scope(struct
>>>>>>  			       acpi_dmar_header *header, " 0x%Lx\n",
>>>>>>  			       scope->enumeration_id, drhd->address);
>>>>>> 
>>>>>> +			bus = pci_find_bus(drhd->segment, scope->bus);
>>>>>> +			path = (struct acpi_dmar_pci_path *)(scope + 1); +			count =
>>>>>> (scope->length - +				 sizeof(struct acpi_dmar_device_scope))
>>>>>> +				/ sizeof(struct acpi_dmar_pci_path);
>>>>>> +
>>>>>> +			while (count) {
>>>>>> +				if (pdev)
>>>>>> +					pci_dev_put(pdev);
>>>>>> +
>>>>>> +				if (!bus)
>>>>>> +					break;
>>>>>> +
>>>>>> +				pdev = pci_get_slot(bus,
>>>>>> +					PCI_DEVFN(path->dev, path->fn));
>>>>>> +				if (!pdev)
>>>>>> +					break;
>>>>> 
>>>>> ir_parse_ioapic_scope() happens very early in the boot. So, I
>>>>> don't think we can do the pci related discovery here.
>>>>> 
>>>> 
>>>> Thanks for your pointing it out. It should enable the source-id
>>>> checking for io-apic's after the pci subsystem is up. I will
>>>> change it.
>>> 
>>> Note, there's ways to do early PCI quirks too, check
>>> arch/x86/kernel/early-quirks.c. It's done by reading the PCI
>>> configuration space directly via a careful early-capable subset of
>>> the PCI config space APIs. 
>>> 
>>> But it's a method of last resort.
>>> 
>> 
>> Thanks for your reminder. It can use direct PCI access here as
>> follows. It's easy and clean. I think it's better than adding the
>> source-id checking for io-apic's after the pci subsystem is up. I
>> will send out updated patches after some tests.   
>> 
>> @@ -634,6 +695,24 @@ static int ir_parse_ioapic_scope(struct
>>                                acpi_dmar_header *header, " 0x%Lx\n",
>>                                scope->enumeration_id, drhd->address);
>> 
>> +                       bus = scope->bus;
>> +                       path = (struct acpi_dmar_pci_path *)(scope +
>> 1); +                       count = (scope->length -
>> +                                sizeof(struct
>> acpi_dmar_device_scope)) +                               /
>> sizeof(struct acpi_dmar_pci_path); + +                       while
>> (--count > 0) { +                               /* Access PCI
>> directly due to the PCI +                                * subsystem
>> isn't initialized yet. +                                */
>> +                               bus = read_pci_config_byte(bus,
>> path->dev, +                                       path->fn,
>> PCI_SECONDARY_BUS); +                               path++;
>> +                       }
>> +
>> +                       ir_ioapic[ir_ioapic_num].bus = bus;
>> +                       ir_ioapic[ir_ioapic_num].devfn =
>> +                                       PCI_DEVFN(path->dev,
>> path->fn); 
> 
> looks good IMO, beyond the obligatory comment-style nitpick [*] :-)
> Also, the function above seems to be way too large - please split it
> into a couple of natural helper functions.
> 
> Thanks,
> 
> 	Ingo
> 
> [*]
> 
> Please use the customary comment style:
> 
>   /*
>    * Comment .....
>    * ...... goes here:
>    */
> 
> specified in Documentation/CodingStyle.

I have sent out the updated patches. Thanks!

Regards,
Weidong--
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/