linux-kernel - Re: [PATCH 02/18] xen: hook io

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:	Sat, 09 May 2009 08:40:56 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Ingo Molnar <mingo@...e.hu>
CC:	the arch/x86 maintainers <x86@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Xen-devel <xen-devel@...ts.xensource.com>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>
Subject: Re: [PATCH 02/18] xen: hook io_apic read/write operations

Ingo Molnar wrote:
> * Ingo Molnar <mingo@...e.hu> wrote:
>
>   
>> * Jeremy Fitzhardinge <jeremy@...p.org> wrote:
>>
>>     
>>> --- a/arch/x86/kernel/apic/io_apic.c
>>> +++ b/arch/x86/kernel/apic/io_apic.c
>>> @@ -62,8 +62,10 @@
>>>  #include <asm/uv/uv_hub.h>
>>>  #include <asm/uv/uv_irq.h>
>>>  
>>> +#include <asm/xen/hypervisor.h>
>>>  #include <asm/apic.h>
>>>  
>>> +
>>>  #define __apicdebuginit(type) static type __init
>>>  
>>>  /*
>>> @@ -407,14 +409,26 @@ static inline void io_apic_eoi(unsigned int apic, unsigned int vector)
>>>  
>>>  static inline unsigned int io_apic_read(unsigned int apic, unsigned int reg)
>>>  {
>>> -	struct io_apic __iomem *io_apic = io_apic_base(apic);
>>> +	struct io_apic __iomem *io_apic;
>>> +
>>> +	if (xen_initial_domain())
>>> +		return xen_io_apic_read(apic, reg);
>>>       
>> hm, any reason why we dont want to create a 'struct io_apic' 
>> driver abstraction instead of spreading xen_initial_domain() 
>> checks all around the code?
>>     

My initial patch did that, and I'm happy to revive it.  But HPA 
preferred this approach, arguing against introducing another layer of 
abstraction for the sake of one user.

> And on a higher level, i still dont see why you dont do the whole 
> Xen thing under an irqchip. There should be no extra crappy checks 
> in native code.
>   

Hm, every time you see this code, you always have this quasi-Pavlovian 
response.  You say "use an irqchip".  I say:

    * We already use irqchip
    * but most of the interesting IO apic accesses (routing) are not
      done via the irqchip interface
    * so irqchip doesn't help

And then you don't reply.  And then you raise it again.

I would *always* prefer to hook into an interface like irqchip rather 
than gouge into the code, but I really think that irqchip isn't that 
interface.  If you have a more specific suggestion or proposal I'll 
happily follow it up, but repeating "you should use an irqchip" isn't 
getting anywhere.

To reiterate:

    * irq_chip is all about interrupt delivery, masking, acking, etc
    * these Xen dom0 apic changes are all about interrupt routing
    * irq_chip doesn't cover routing


    J
--
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/