linux-kernel - Re: [PATCH 11/25] xen: Xen SMP guest support

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: <462FA1CC.5010002@goop.org>
Date:	Wed, 25 Apr 2007 11:45:32 -0700
From:	Jeremy Fitzhardinge <jeremy@...p.org>
To:	Andi Kleen <ak@...e.de>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	virtualization@...ts.osdl.org, lkml <linux-kernel@...r.kernel.org>,
	Benjamin LaHaise <bcrl@...ck.org>,
	Ingo Molnar <mingo@...hat.com>
Subject: Re: [PATCH 11/25] xen: Xen SMP guest support

Andi Kleen wrote:
>   
>>  #ifdef CONFIG_X86_LOCAL_APIC
>>  static unsigned long xen_apic_read(unsigned long reg)
>>     
>
> Can't you just make them all NULL? Crashing here should be correct.
>   

No.  The APIC setup code does at least one unconditional read to get the
APIC version before deciding the APIC is disabled/not present.  In my
testing that's the only access to the APIC which happens under Xen, but
it wouldn't surprise me if there were other stray accesses with
different configurations (like the P4_THERMAL stuff).

>> +#ifdef CONFIG_SMP
>> +static const struct smp_ops xen_smp_ops __initdata = {
>>     
>
> Why __initdata? 
>   

It gets copied into the real smp_ops early on, so it isn't needed after
that.

>> +/* VCPUs are single-cored, and have no siblings */
>> +static void set_cpu_sibling_map(int cpu)
>>     
>
> Can you put this somewhere generic and use it everywhere? Don't want
> duplication of this code.
>   

OK.

> Strange check?
>   

Er, yes, that does look odd.

>> +
>> + * We need to reload %cr3 since the page tables may be going
>> + * away from under us..
>> + */
>> +static inline void leave_mm (unsigned long cpu)
>> +{
>> +#ifdef CONFIG_SMP
>> +	if (per_cpu(cpu_tlbstate, cpu).state == TLBSTATE_OK)
>> +		BUG();
>> +	cpu_clear(cpu, per_cpu(cpu_tlbstate, cpu).active_mm->cpu_vm_mask);
>> +#endif
>> +	load_cr3(swapper_pg_dir);
>>  }
>>     
>
> Should be out of line
>   

OK.

    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/