| 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
| ||
|
Message-ID: <87iqin166f.fsf@basil.nowhere.org>
Date: Tue, 23 Jun 2009 14:32:56 +0200
From: Andi Kleen <andi@...stfloor.org>
To: Len Brown <lenb@...nel.org>
Cc: sfi-devel@...plefirmware.org, linux-kernel@...r.kernel.org,
Feng Tang <feng.tang@...el.com>,
Len Brown <len.brown@...el.com>
Subject: Re: [PATCH 3/8] SFI: core support
Len Brown <lenb@...nel.org> writes:
> +static ulong __init sfi_early_find_syst(void)
> +{
> + unsigned long i;
> + char *pchar = (char *)SFI_SYST_SEARCH_BEGIN;
> +
> + for (i = 0; SFI_SYST_SEARCH_BEGIN + i < SFI_SYST_SEARCH_END; i += 16, pchar += 16) {
> + if (!strncmp(SFI_SIG_SYST, pchar, SFI_SIGNATURE_SIZE))
> + return SFI_SYST_SEARCH_BEGIN + i;
Such additional memory scans are always a bit risky, e.g. if there's
stray hardware there. Has it been verified that existing kernels
already scan this area?
> + mmapt = NULL;
> + for (i = 0; i < tbl_cnt; i++) {
> + if (!strncmp(SFI_SIG_MMAP, (char *)(u32)*pentry, 4)) {
> + mmapt = (struct sfi_table_simple *)(u32)*pentry;
> + break;
> + }
> + pentry++;
> + }
> + if (!mmapt)
> + return -1;
printk here?
> +
> + /* refer copy_e820_memory() */
> + num = SFI_GET_ENTRY_NUM(mmapt, sfi_mem_entry);
> + mentry = (struct sfi_mem_entry *)mmapt->pentry;
> + for (i = 0; i < num; i++) {
> + start = mentry->phy_start;
> + size = mentry->pages << PAGE_SHIFT;
> + end = start + size;
> +
> + if (start > end)
> + return -1;
> +
> + pr_debug(PREFIX "start = 0x%08x end = 0x%08x type = %d\n",
> + (u32)start, (u32)end, mentry->type);
> +
> + /* translate SFI mmap type to E820 map type */
> + switch (mentry->type) {
> + case EFI_CONVENTIONAL_MEMORY:
> + type = E820_RAM;
> + break;
> + case EFI_MEMORY_MAPPED_IO:
> + case EFI_UNUSABLE_MEMORY:
> + case EFI_RUNTIME_SERVICES_DATA:
> + mentry++;
Surely UNUSTABLE and RUNTIME_SERVICES should be added somewhere to the resources,
otherwise MMIO allocation might put something there.
> + }
> +
> + return 0;
> +}
> +
> +#ifdef CONFIG_X86_LOCAL_APIC
> +void __init mp_sfi_register_lapic_address(u64 address)
> +{
> + mp_lapic_addr = (unsigned long) address;
> +
> + set_fixmap_nocache(FIX_APIC_BASE, mp_lapic_addr);
> +
> + if (boot_cpu_physical_apicid == -1U)
> + boot_cpu_physical_apicid = read_apic_id();
> +
> + pr_info(PREFIX "Boot CPU = %d\n", boot_cpu_physical_apicid);
That's the same what ACPI does, isn't it? Some code sharing
would be good.
> +static int __init sfi_parse_cpus(struct sfi_table_header *table)
> +{
> + struct sfi_table_simple *sb;
> + struct sfi_cpu_table_entry *pentry;
> + int i;
> + int cpu_num;
> +
> + BUG_ON(!table);
Another useless BUG_ON. Some more below too.
> + sb = (struct sfi_table_simple *)table;
> +
> + cpu_num = SFI_GET_ENTRY_NUM(sb, sfi_cpu_table_entry);
> + pentry = (struct sfi_cpu_table_entry *) sb->pentry;
> +
> + for (i = 0; i < cpu_num; i++) {
> + mp_sfi_register_lapic(pentry->apicid);
> + pentry++;
> + }
Array overflow checking?
> + mp_ioapics[idx].apicver = 0;
> +#endif
> +
> + pr_info(PREFIX "IOAPIC[%d]: apic_id %d, version %d, address 0x%x\n",
> + idx, mp_ioapics[idx].apicid,
> + mp_ioapics[idx].apicver, (u32)mp_ioapics[idx].apicaddr);
> + /*
> + * Build basic GSI lookup table to facilitate gsi->io_apic lookups
> + * and to prevent reprogramming of IOAPIC pins (PCI GSIs).
> + */
> + mp_ioapic_routing[idx].apic_id = mp_ioapics[idx].apicid;
> + mp_ioapic_routing[idx].gsi_base = gsi_base;
> + mp_ioapic_routing[idx].gsi_end = gsi_base +
> + io_apic_get_redir_entries(idx);
> + gsi_base = mp_ioapic_routing[idx].gsi_end + 1;
> + pr_info(PREFIX "IOAPIC[%d]: apic_id %d, version %d, address 0x%x, "
> + "GSI %d-%d\n", idx, mp_ioapics[idx].apicid,
> + mp_ioapics[idx].apicver, (u32)mp_ioapics[idx].apicaddr,
> + mp_ioapic_routing[idx].gsi_base,
> + mp_ioapic_routing[idx].gsi_end);
That printk should be dependent on the runtime apic verbosity level?
> +
> +/*
> + * flag for whether using ioremap() to map the sfi tables, if yes
> + * each table only need be mapped once, otherwise each arch's
> + * early_ioremap and early_iounmap should be used each time a
> + * table is visited
> + */
> +static u32 sfi_tbl_permanant_mapped;
permanant is a typo?
> +
> +static void __iomem *sfi_map_memory(u32 phys, u32 size)
> +{
> + if (!phys || !size)
> + return NULL;
> +
> + if (sfi_tbl_permanant_mapped)
> + return ioremap((unsigned long)phys, size);
> + else
> + return arch_early_ioremap((unsigned long)phys, size);
> +}
imho it would be cleaner if the callers just called these functions
directly. Are the !phys !size checks really needed?
> +
> +void sfi_tb_install_table(u64 addr, u32 flags)
> +{
> + struct sfi_table_header *table;
> + u32 length;
> +
> + /* only map table header before knowing actual length */
> + table = sfi_map_memory(addr, sizeof(struct sfi_table_header));
> + if (!table)
> + return;
> +
> + length = table->length;
> + sfi_unmap_memory(table, sizeof(struct sfi_table_header));
> +
> + table = sfi_map_memory(addr, length);
> + if (!table)
> + return;
Since the mappings are always 4K you would only need to remap
if the size is > PAGE_SIZE
> +
> + if (sfi_tb_verify_checksum(table, length))
> + goto unmap_and_exit;
> +
> + /* Initialize sfi_tblist entry */
> + sfi_tblist.tables[sfi_tblist.count].flags = flags;
> + sfi_tblist.tables[sfi_tblist.count].address = addr;
> + sfi_tblist.tables[sfi_tblist.count].pointer = NULL;
> + memcpy(&sfi_tblist.tables[sfi_tblist.count].header,
> + table, sizeof(struct sfi_table_header));
To be honest I'm not sure why this list exists at all.
Is it that difficult to just rewalk the firmware supplied
table as needed?
-andi
--
ak@...ux.intel.com -- Speaking for myself only.
--
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