Re: [Xen-devel] [PATCH 1/2] x86/tboot: invalidate FIX_TBOOT_MAP_ADDRESS mapping after use
On Mar 5, 2015 15:14, Jan Beulich wrote:
On 05.03.15 at 05:45, wrote:
>> On Feb 18, 2015 17:03, Jan Beulich wrote:
>>> In order for commit cbeeaa7d ("x86/nmi: fix shootdown of pcpus running
>>> in VMX non-root mode")'s re-use of that fixmap entry to not cause
>>> undesirable (in crash context) cross-CPU TLB flushes, invalidate the
>>> fixmap entry right after use.
>>>
>>> Signed-off-by: Jan Beulich
>>>
>>> --- a/xen/arch/x86/tboot.c
>>> +++ b/xen/arch/x86/tboot.c
>>> @@ -138,6 +138,7 @@ void __init tboot_probe(void)
>>>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);
>>> +__set_fixmap(FIX_TBOOT_MAP_ADDRESS, 0, 0);
>>> }
>>>
>>> /* definitions from xen/drivers/passthrough/vtd/iommu.h
>>> @@ -476,6 +477,8 @@ int __init tboot_parse_dmar_table(acpi_t
>>> 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;
>>> +__set_fixmap(FIX_TBOOT_MAP_ADDRESS, 0, 0);
>>> +
>>> rc = dmar_handler(dmar_table);
>>> xfree(dmar_table_raw);
>>>
>>
>> It might be better to move the fixmap invalidations into
>> tboot_copy_memory like below.
>
> I considered this, but dropped the idea due to resulting in several
> redundant invalidations.
>From performance perspective, I am ok with your final changes.
Jimmy
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 2/2] x86/tboot: simplify DMAR table copying
On Feb 18, 2015 17:03, Jan Beulich wrote:
> There's no need for more than one variable, no need for casts, and no
> point in using the type-safe xmalloc_array() here.
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/tboot.c
> +++ b/xen/arch/x86/tboot.c
> @@ -435,13 +435,12 @@ int __init tboot_protect_mem_regions(voi
>
> int __init tboot_parse_dmar_table(acpi_table_handler dmar_handler) {
> -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;
> +void *dmar_table;
>
> if ( !tboot_in_measured_env() )
> return acpi_table_parse(ACPI_SIG_DMAR, dmar_handler); @@ -
> 474,13 +473,12 @@ int __init tboot_parse_dmar_table(acpi_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;
> +dmar_table = xmalloc_bytes(dmar_table_length);
> +tboot_copy_memory(dmar_table, dmar_table_length, pa);
> __set_fixmap(FIX_TBOOT_MAP_ADDRESS, 0, 0);
>
> rc = dmar_handler(dmar_table);
> -xfree(dmar_table_raw);
> +xfree(dmar_table);
>
> /* acpi_parse_dmar() zaps APCI DMAR signature in TXT heap table */
> /* but dom0 will read real table, so must zap it there too */
>
Ack.
Jimmy
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH 1/2] x86/tboot: invalidate FIX_TBOOT_MAP_ADDRESS mapping after use
On Feb 18, 2015 17:03, Jan Beulich wrote:
> In order for commit cbeeaa7d ("x86/nmi: fix shootdown of pcpus
> running in VMX non-root mode")'s re-use of that fixmap entry to not
> cause undesirable (in crash context) cross-CPU TLB flushes, invalidate
> the fixmap entry right after use.
>
> Signed-off-by: Jan Beulich
>
> --- a/xen/arch/x86/tboot.c
> +++ b/xen/arch/x86/tboot.c
> @@ -138,6 +138,7 @@ void __init tboot_probe(void)
>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);
> +__set_fixmap(FIX_TBOOT_MAP_ADDRESS, 0, 0);
> }
>
> /* definitions from xen/drivers/passthrough/vtd/iommu.h
> @@ -476,6 +477,8 @@ int __init tboot_parse_dmar_table(acpi_t
> 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;
> +__set_fixmap(FIX_TBOOT_MAP_ADDRESS, 0, 0);
> +
> rc = dmar_handler(dmar_table);
> xfree(dmar_table_raw);
>
>
It might be better to move the fixmap invalidations into tboot_copy_memory like
below.
diff -r 37706d651593 xen/arch/x86/tboot.c
--- a/xen/arch/x86/tboot.c Wed Mar 04 10:03:48 2015 +0100
+++ b/xen/arch/x86/tboot.c Thu Mar 05 12:40:25 2015 +0800
@@ -88,6 +88,8 @@ static void __init tboot_copy_memory(uns
}
va[i] = map_addr[pa + i - (map_base << PAGE_SHIFT)];
}
+
+__set_fixmap(FIX_TBOOT_MAP_ADDRESS, 0, 0);
}
void __init tboot_probe(void)
Thanks
Jimmy
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
