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
| ||
|
Date: Mon, 9 Jul 2012 14:32:30 +0200 From: Joerg Roedel <joerg.roedel@....com> To: Ingo Molnar <mingo@...nel.org> CC: <iommu@...ts.linux-foundation.org>, <linux-kernel@...r.kernel.org>, <x86@...nel.org>, Yinghai Lu <yinghai@...nel.org>, Suresh Siddha <suresh.b.siddha@...el.com> Subject: Re: [PATCH 03/28] x86/irq: Use irq_remap specific print_IO_APIC paths only on Intel On Fri, Jul 06, 2012 at 04:00:37PM +0200, Ingo Molnar wrote: > ( A proper abstraction would stick it all into some sort PCI > driver alike structure, including enumeration, initialization, > debugging and other non-core details. ) This is certainly a multi-cycle process to make sure we do not break everything from one release to another. But cleaning that 4000+ lines monster up is a good thing, I agree with that. > So the way this could work in a cleaner fashion is to > encapsulate the logic even more. Today we have a per irq_desc > irq_cfg data descriptor, but there's still global knowledge in > actual vector allocation such as create_irq() or > msi_compose_msg(). Patterns like: > > if (irq_remapped(cfg)) { > compose_remapped_msi_msg(pdev, irq, dest, msg, hpet_id); > return err; > } > > if (x2apic_enabled()) > msg->address_hi = MSI_ADDR_BASE_HI | > MSI_ADDR_EXT_DEST_ID(dest); > else > msg->address_hi = MSI_ADDR_BASE_HI; > > all all signs of insufficient abstraction. Why was this code merged when you are unhappy with it? About the irq_remapped() thing: to abstract this properly it has to be a per-irq abstraction, because in the MSI case some interrupts may be remapped and others may be not. In the AMD IOMMU case for example, the MSI interrupt of the IOMMU device itself is not remapped but all others are. So function pointers in 'struct irq_cfg' come into mind for that. What do you think about that? > More importantly, all the silly open-coded if (irq_remapping_enabled) > checks should be eliminated from core x86 code. IRQ remapping > should be either be an irq_chip detail or should live in a > separate layer. I'll start with factoring out these irq_remapping_enabled and post the results so that we can agree on the right direction. I gained some experience with this code while factoring out all the VT-d internals from it in one of the last cycles. > So before extending all this please get this into shape. What about Patches 1 and 2? Any comments on these or are they fine? Kind Regards, Joerg -- AMD Operating System Research Center Advanced Micro Devices GmbH Einsteinring 24 85609 Dornach General Managers: Alberto Bozzo Registration: Dornach, Landkr. Muenchen; Registerger. Muenchen, HRB Nr. 43632 -- 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/
Powered by blists - more mailing lists