linux-kernel - Re: [PATCH 2/9] 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]

Message-ID: <49965768.7040102@goop.org>
Date:	Fri, 13 Feb 2009 21:32:24 -0800
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Yinghai Lu <yinghai@...nel.org>
CC:	Ingo Molnar <mingo@...e.hu>,
	the arch/x86 maintainers <x86@...nel.org>,
	Xen-devel <xen-devel@...ts.xensource.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Ian Campbell <Ian.Campbell@...rix.com>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>
Subject: Re: [PATCH 2/9] xen: hook io_apic read/write operations

Yinghai Lu wrote:
> Jeremy Fitzhardinge wrote:
>   
>> In Xen, writes to the IO APIC are paravirtualized via hypercalls, so
>> implement the appropriate operations.
>>
>> This version of the patch just hooks the io_apic read/write functions
>> directly, rather than introducing another layer of indirection.  The
>> xen_initial_domain() tests compile to 0 if CONFIG_XEN_DOM0 isn't set,
>> and are cheap if it is.
>>
>> (An alternative would be to add io_apic_ops, and point them to the Xen
>> implementation as needed.  HPA deemed this extra level of indirection to
>> be excessive.)
>>     
>
> that will be more clean.
>   

That was my thought too.


>> Signed-off-by: Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>
>> ---
>>  arch/x86/include/asm/io_apic.h |    6 ++++
>>  arch/x86/kernel/io_apic.c      |   32 ++++++++++++++++++++--
>>  arch/x86/xen/Makefile          |    3 +-
>>  arch/x86/xen/apic.c            |   57 ++++++++++++++++++++++++++++++++++++++++
>>  arch/x86/xen/enlighten.c       |    2 +
>>  arch/x86/xen/xen-ops.h         |    6 ++++
>>  6 files changed, 102 insertions(+), 4 deletions(-)
>>  create mode 100644 arch/x86/xen/apic.c
>>
>> diff --git a/arch/x86/include/asm/io_apic.h b/arch/x86/include/asm/io_apic.h
>> index 59cb4a1..20b543a 100644
>> --- a/arch/x86/include/asm/io_apic.h
>> +++ b/arch/x86/include/asm/io_apic.h
>> @@ -183,4 +183,10 @@ static inline void ioapic_init_mappings(void)	{ }
>>  static inline void probe_nr_irqs_gsi(void)	{ }
>>  #endif
>>  
>> +void xen_io_apic_init(void);
>> +unsigned int xen_io_apic_read(unsigned apic, unsigned reg);
>> +void xen_io_apic_write(unsigned int apic,
>> +		       unsigned int reg, unsigned int value);
>> +
>> +
>>  #endif /* _ASM_X86_IO_APIC_H */
>> diff --git a/arch/x86/kernel/io_apic.c b/arch/x86/kernel/io_apic.c
>> index 7248ca1..de0368a 100644
>> --- a/arch/x86/kernel/io_apic.c
>> +++ b/arch/x86/kernel/io_apic.c
>> @@ -62,6 +62,8 @@
>>  #include <asm/uv/uv_hub.h>
>>  #include <asm/uv/uv_irq.h>
>>  
>> +#include <asm/xen/hypervisor.h>
>> +
>>  #include <asm/genapic.h>
>>  
>>  #define __apicdebuginit(type) static type __init
>> @@ -399,14 +401,26 @@ static __attribute_const__ struct io_apic __iomem *io_apic_base(int idx)
>>  
>>  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);
>>     
>
> you may have _xen_io_apic_read(apic, reg);
> and xen_io_apic_read(apic, reg) will call xen_initial_domain internally.
>   

How would that work?  What would it return in the xen/non-xen states to 
indicate that the normal read operation should be performed?  I don't 
think it would be any clearer.

> or sth like
> extra if (io_apic_ops)
> 	io_apic->read()...
>   

I think if there were an _ops structure, it should just go though it 
unconditionally.

    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/