I'll backport the fix. Thanks.
K.
On 01/10/2009 17:47, "Mark Johnson" <johnson.nh@xxxxxxxxx> wrote:
> FYI. I'm seeing the same error with this patch that we saw
> in unstable.
>
> tboot.c: In function `tboot_copy_memory':
> tboot.c:77: warning: 'map_addr' might be used uninitialized in this function
> gmake[5]: *** [tboot.o] Error 1
>
>
> MRJ
>
>
> On Thu, Oct 1, 2009 at 11:25 AM, Xen patchbot-3.4-testing
> <patchbot-3.4-testing@xxxxxxxxxxxxxxxxxxx> wrote:
>> # HG changeset patch
>> # User Keir Fraser <keir.fraser@xxxxxxxxxx>
>> # Date 1254410050 -3600
>> # Node ID efcb14260292a686740cee8239aae0a503097eb8
>> # Parent 1764b0cba2e9dcf1684d5c82975302277ba62064
>> tboot: fix tboot memory mapping for 32b
>>
>> This patch used fixmap to get TXT heap base/size and SINIT base/size
>> from TXT pub config registers (whose address starts from 0xfed20000),
>> and get DMAR table copy from TXT heap (whose address may start from
>> 0x7d520000) for tboot, instead of using map_pages_to_xen(), which will
>> cause panic on x86_32.
>>
>> Signed-off-by: Shane Wang <shane.wang@xxxxxxxxx>
>> xen-unstable changeset: 20242:bcb6b95b30b1
>> xen-unstable date: Tue Sep 22 08:36:40 2009 +0100
>> ---
>> xen/arch/x86/tboot.c | 94
>> ++++++++++++++++++++++++-------------------
>> xen/include/asm-x86/fixmap.h | 1
>> 2 files changed, 54 insertions(+), 41 deletions(-)
>>
>> diff -r 1764b0cba2e9 -r efcb14260292 xen/arch/x86/tboot.c
>> --- a/xen/arch/x86/tboot.c Thu Oct 01 16:13:03 2009 +0100
>> +++ b/xen/arch/x86/tboot.c Thu Oct 01 16:14:10 2009 +0100
>> @@ -70,12 +70,29 @@ typedef struct __packed {
>> uint32_t vtd_dmars_off;
>> } sinit_mle_data_t;
>>
>> +static void tboot_copy_memory(unsigned char *va, uint32_t size,
>> + unsigned long pa)
>> +{
>> + uint32_t map_base;
>> + unsigned long map_addr;
>> + int i;
>> +
>> + map_base = 0;
>> + for (i = 0; i < size; i++) {
>> + if ( map_base != PFN_DOWN(pa + i) ) {
>> + map_base = PFN_DOWN(pa + i);
>> + set_fixmap(FIX_TBOOT_MAP_ADDRESS, map_base << PAGE_SHIFT);
>> + map_addr = (unsigned long)fix_to_virt(FIX_TBOOT_MAP_ADDRESS);
>> + }
>> + *(va + i) = *(unsigned char *)(map_addr + pa + i
>> + - (map_base << PAGE_SHIFT));
>> + }
>> +}
>> +
>> void __init tboot_probe(void)
>> {
>> tboot_shared_t *tboot_shared;
>> unsigned long p_tboot_shared;
>> - uint32_t map_base, map_size;
>> - unsigned long map_addr;
>>
>> /* Look for valid page-aligned address for shared page. */
>> p_tboot_shared = simple_strtoul(opt_tboot, NULL, 0);
>> @@ -108,26 +125,17 @@ void __init tboot_probe(void)
>> /* these will be needed by tboot_protect_mem_regions() and/or
>> tboot_parse_dmar_table(), so get them now */
>>
>> - map_base = PFN_DOWN(TXT_PUB_CONFIG_REGS_BASE);
>> - map_size = PFN_UP(NR_TXT_CONFIG_PAGES * PAGE_SIZE);
>> - map_addr = (unsigned long)__va(map_base << PAGE_SHIFT);
>> - if ( map_pages_to_xen(map_addr, map_base, map_size, __PAGE_HYPERVISOR) )
>> - return;
>> -
>> + txt_heap_base = txt_heap_size = sinit_base = sinit_size = 0;
>> /* TXT Heap */
>> - txt_heap_base =
>> - *(uint64_t *)__va(TXT_PUB_CONFIG_REGS_BASE + TXTCR_HEAP_BASE);
>> - txt_heap_size =
>> - *(uint64_t *)__va(TXT_PUB_CONFIG_REGS_BASE + TXTCR_HEAP_SIZE);
>> -
>> + tboot_copy_memory((unsigned char *)&txt_heap_base,
>> sizeof(txt_heap_base),
>> + TXT_PUB_CONFIG_REGS_BASE + TXTCR_HEAP_BASE);
>> + tboot_copy_memory((unsigned char *)&txt_heap_size,
>> sizeof(txt_heap_size),
>> + TXT_PUB_CONFIG_REGS_BASE + TXTCR_HEAP_SIZE);
>> /* SINIT */
>> - sinit_base =
>> - *(uint64_t *)__va(TXT_PUB_CONFIG_REGS_BASE + TXTCR_SINIT_BASE);
>> - sinit_size =
>> - *(uint64_t *)__va(TXT_PUB_CONFIG_REGS_BASE + TXTCR_SINIT_SIZE);
>> -
>> - destroy_xen_mappings((unsigned long)__va(map_base << PAGE_SHIFT),
>> - (unsigned long)__va((map_base + map_size) <<
>> PAGE_SHIFT));
>> + tboot_copy_memory((unsigned char *)&sinit_base, sizeof(sinit_base),
>> + TXT_PUB_CONFIG_REGS_BASE + TXTCR_SINIT_BASE);
>> + tboot_copy_memory((unsigned char *)&sinit_size, sizeof(sinit_size),
>> + TXT_PUB_CONFIG_REGS_BASE + TXTCR_SINIT_SIZE);
>> }
>>
>> /* definitions from xen/drivers/passthrough/vtd/iommu.h
>> @@ -380,11 +388,14 @@ int __init tboot_protect_mem_regions(voi
>>
>> int __init tboot_parse_dmar_table(acpi_table_handler dmar_handler)
>> {
>> - uint32_t map_base, map_size;
>> - unsigned long map_vaddr;
>> - void *heap_ptr;
>> struct acpi_table_header *dmar_table;
>> int rc;
>> + uint64_t size;
>> + uint32_t dmar_table_length;
>> + unsigned long pa;
>> + sinit_mle_data_t sinit_mle_data;
>> + unsigned char *dmar_table_raw;
>> +
>>
>> if ( !tboot_in_measured_env() )
>> return acpi_table_parse(ACPI_SIG_DMAR, dmar_handler);
>> @@ -396,32 +407,33 @@ int __init tboot_parse_dmar_table(acpi_t
>> return 1;
>>
>> /* map TXT heap into Xen addr space */
>> - map_base = PFN_DOWN(txt_heap_base);
>> - map_size = PFN_UP(txt_heap_size);
>> - map_vaddr = (unsigned long)__va(map_base << PAGE_SHIFT);
>> - if ( map_pages_to_xen(map_vaddr, map_base, map_size, __PAGE_HYPERVISOR)
>> )
>> - return 1;
>>
>> /* walk heap to SinitMleData */
>> - heap_ptr = __va(txt_heap_base);
>> + pa = txt_heap_base;
>> /* skip BiosData */
>> - heap_ptr += *(uint64_t *)heap_ptr;
>> + tboot_copy_memory((unsigned char *)&size, sizeof(size), pa);
>> + pa += size;
>> /* skip OsMleData */
>> - heap_ptr += *(uint64_t *)heap_ptr;
>> + tboot_copy_memory((unsigned char *)&size, sizeof(size), pa);
>> + pa += size;
>> /* skip OsSinitData */
>> - heap_ptr += *(uint64_t *)heap_ptr;
>> + tboot_copy_memory((unsigned char *)&size, sizeof(size), pa);
>> + pa += size;
>> /* now points to SinitMleDataSize; set to SinitMleData */
>> - heap_ptr += sizeof(uint64_t);
>> + pa += sizeof(uint64_t);
>> + tboot_copy_memory((unsigned char *)&sinit_mle_data,
>> sizeof(sinit_mle_data),
>> + pa);
>> /* get addr of DMAR table */
>> - dmar_table = (struct acpi_table_header *)(heap_ptr +
>> - ((sinit_mle_data_t *)heap_ptr)->vtd_dmars_off -
>> sizeof(uint64_t));
>> -
>> + pa += sinit_mle_data.vtd_dmars_off - sizeof(uint64_t);
>> + tboot_copy_memory((unsigned char *)&dmar_table_length,
>> + sizeof(dmar_table_length),
>> + pa + sizeof(char) * ACPI_NAME_SIZE);
>> + dmar_table_raw = xmalloc_array(unsigned char, dmar_table_length);
>> + tboot_copy_memory(dmar_table_raw, dmar_table_length, pa);
>> + dmar_table = (struct acpi_table_header *)dmar_table_raw;
>> rc = dmar_handler(dmar_table);
>> -
>> - destroy_xen_mappings(
>> - (unsigned long)__va(map_base << PAGE_SHIFT),
>> - (unsigned long)__va((map_base + map_size) << PAGE_SHIFT));
>> -
>> + xfree(dmar_table_raw);
>> +
>> /* acpi_parse_dmar() zaps APCI DMAR signature in TXT heap table */
>> /* but dom0 will read real table, so must zap it there too */
>> dmar_table = NULL;
>> diff -r 1764b0cba2e9 -r efcb14260292 xen/include/asm-x86/fixmap.h
>> --- a/xen/include/asm-x86/fixmap.h Thu Oct 01 16:13:03 2009 +0100
>> +++ b/xen/include/asm-x86/fixmap.h Thu Oct 01 16:14:10 2009 +0100
>> @@ -51,6 +51,7 @@ enum fixed_addresses {
>> FIX_TBOOT_SHARED_BASE,
>> FIX_MSIX_IO_RESERV_BASE,
>> FIX_MSIX_IO_RESERV_END = FIX_MSIX_IO_RESERV_BASE + FIX_MSIX_MAX_PAGES -1,
>> + FIX_TBOOT_MAP_ADDRESS,
>> __end_of_fixed_addresses
>> };
>>
>>
>> _______________________________________________
>> Xen-changelog mailing list
>> Xen-changelog@xxxxxxxxxxxxxxxxxxx
>> http://lists.xensource.com/xen-changelog
>>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
|