linux-kernel - Re: [PATCH 3/8] SFI: core 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: <20090623163246.1cdf755e@feng-desktop>
Date:	Tue, 23 Jun 2009 16:32:46 +0800
From:	Feng Tang <feng.tang@...el.com>
To:	Ingo Molnar <mingo@...e.hu>
CC:	Len Brown <lenb@...nel.org>, "H. Peter Anvin" <hpa@...or.com>,
	"Thomas Gleixner" <tglx@...utronix.de>,
	Yinghai Lu <yinghai@...nel.org>,
	"Andrew Morton" <akpm@...ux-foundation.org>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	"sfi-devel@...plefirmware.org" <sfi-devel@...plefirmware.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"Brown, Len" <len.brown@...el.com>
Subject: Re: [PATCH 3/8] SFI: core support

On Tue, 23 Jun 2009 15:56:43 +0800
Ingo Molnar <mingo@...e.hu> wrote:


> >
> > +#ifdef CONFIG_X86_LOCAL_APIC
> > +static u64 sfi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
> > +#endif
> 
> if SFI adds a 'depends on X86_LOCAL_APIC' the ugly #ifdef can be
> dropped.

When Len designed the SFI spec, he considered the possibility of being used by
multiple archs, so we chose not to add a x86 dependency, though adding these
#ifdef does make code ugly :P

> > +{
> > +     unsigned long i;
> 
> ... like here.
> 
> > +     char *pchar = (char *)SFI_SYST_SEARCH_BEGIN;
> > +
> > +     for (i = 0; SFI_SYST_SEARCH_BEGIN + i < SFI_SYST_SEARCH_END;
> > i += 16, pchar += 16) {
> 
> What does the magic constant '16' mean here?

My bad not puting clear comments here, the SFI spec defines SYST table starts
at a 16-byte boundary


> > +
> > +static int __init sfi_parse_apic(struct sfi_table_header *table)
> > +{
> > +     struct sfi_table_simple *sb;
> > +     struct sfi_apic_table_entry *pentry;
> > +     int i, num;
> > +
> > +     BUG_ON(!table);
> 
> Same as comment above - is this case anticipated? If yes, is a crash
> the best answer?

Yes, usually table won't be NULL

> > +
> > +#define SFI_ACPI_TABLE       1
> 
> In general, nice stuff - basically SFI is cleanly implemented ACPI
> tables without any of the run-code-in-acpi-tables complications,
> right?

Thanks for the comments, I really got inspired :). The expectation for SFI
is to be able to run cleanly with CONFIG_ACPI=n, and it works fine on some
intel platform.

Thanks,
Feng

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