[PATCH 2/2] x86/dmar: zap DMAR signature for dom0 once in TBOOT case

[PATCH 2/2] x86/dmar: zap DMAR signature for dom0 once in TBOOT case

[Zhenzhong Duan]

When TBOOT enabled, acpi_parse_dmar() zap a copy of DMAR table rather
than the real table, so make it controled by config option based on the
fact that we already have done the real zapping in tboot_parse_dmar_table().

As said above, acpi_parse_dmar() doesn't zaps APCI DMAR signature in
real TXT heap table, fix the stale comments.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
---
 xen/arch/x86/tboot.c               |    3 +--
 xen/drivers/passthrough/vtd/dmar.c |    2 ++
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/tboot.c b/xen/arch/x86/tboot.c
index d5a5292..f22fa24 100644
--- a/xen/arch/x86/tboot.c
+++ b/xen/arch/x86/tboot.c
@@ -490,8 +490,7 @@ int __init tboot_parse_dmar_table(acpi_table_handler dmar_handler)
     rc = dmar_handler(dmar_table);
     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 */
+    /* Dom0 will read real DMAR table, so must zap it there */
     acpi_dmar_zap();
 
     return rc;
diff --git a/xen/drivers/passthrough/vtd/dmar.c b/xen/drivers/passthrough/vtd/dmar.c
index 8c5fa80..ed4c04e 100644
--- a/xen/drivers/passthrough/vtd/dmar.c
+++ b/xen/drivers/passthrough/vtd/dmar.c
@@ -866,8 +866,10 @@ static int __init acpi_parse_dmar(struct acpi_table_header *table)
     }
 
 out:
+#ifndef CONFIG_TBOOT
     /* Zap ACPI DMAR signature to prevent dom0 using vt-d HW. */
     acpi_dmar_zap();
+#endif
     return ret;
 }
 
-- 
1.7.3

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] x86/dmar: zap DMAR signature for dom0 once in TBOOT case

[Jan Beulich]

>>> On 20.08.18 at 05:32, <zhenzhong.duan@oracle.com> wrote:
> When TBOOT enabled, acpi_parse_dmar() zap a copy of DMAR table rather
> than the real table, so make it controled by config option based on the
> fact that we already have done the real zapping in tboot_parse_dmar_table().

Is this just a cosmetic change, or is there any harm done by the extra
zapping?

> As said above, acpi_parse_dmar() doesn't zaps APCI DMAR signature in
> real TXT heap table, fix the stale comments.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
> ---
>  xen/arch/x86/tboot.c               |    3 +--
>  xen/drivers/passthrough/vtd/dmar.c |    2 ++
>  2 files changed, 3 insertions(+), 2 deletions(-)

You've again failed to Cc maintainers (included now).

Jan

> --- a/xen/arch/x86/tboot.c
> +++ b/xen/arch/x86/tboot.c
> @@ -490,8 +490,7 @@ int __init tboot_parse_dmar_table(acpi_table_handler dmar_handler)
>      rc = dmar_handler(dmar_table);
>      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 */
> +    /* Dom0 will read real DMAR table, so must zap it there */
>      acpi_dmar_zap();
>  
>      return rc;
> --- a/xen/drivers/passthrough/vtd/dmar.c
> +++ b/xen/drivers/passthrough/vtd/dmar.c
> @@ -866,8 +866,10 @@ static int __init acpi_parse_dmar(struct acpi_table_header *table)
>      }
>  
>  out:
> +#ifndef CONFIG_TBOOT
>      /* Zap ACPI DMAR signature to prevent dom0 using vt-d HW. */
>      acpi_dmar_zap();
> +#endif
>      return ret;
>  }
>  



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] x86/dmar: zap DMAR signature for dom0 once in TBOOT case

[Zhenzhong Duan]

On 2018/8/20 16:30, Jan Beulich wrote:
>>>> On 20.08.18 at 05:32, <zhenzhong.duan@oracle.com> wrote:
>> When TBOOT enabled, acpi_parse_dmar() zap a copy of DMAR table rather
>> than the real table, so make it controled by config option based on the
>> fact that we already have done the real zapping in tboot_parse_dmar_table().
> 
> Is this just a cosmetic change, or is there any harm done by the extra
> zapping?
Cosmetic change, I feel it isn't necessory to zap a copy of DMAR table 
which is freed later.

Thanks
Zhenzhong
> 
>> As said above, acpi_parse_dmar() doesn't zaps APCI DMAR signature in
>> real TXT heap table, fix the stale comments.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
>> ---
>>   xen/arch/x86/tboot.c               |    3 +--
>>   xen/drivers/passthrough/vtd/dmar.c |    2 ++
>>   2 files changed, 3 insertions(+), 2 deletions(-)
> 
> You've again failed to Cc maintainers (included now).
> 
> Jan
> 
>> --- a/xen/arch/x86/tboot.c
>> +++ b/xen/arch/x86/tboot.c
>> @@ -490,8 +490,7 @@ int __init tboot_parse_dmar_table(acpi_table_handler dmar_handler)
>>       rc = dmar_handler(dmar_table);
>>       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 */
>> +    /* Dom0 will read real DMAR table, so must zap it there */
>>       acpi_dmar_zap();
>>   
>>       return rc;
>> --- a/xen/drivers/passthrough/vtd/dmar.c
>> +++ b/xen/drivers/passthrough/vtd/dmar.c
>> @@ -866,8 +866,10 @@ static int __init acpi_parse_dmar(struct acpi_table_header *table)
>>       }
>>   
>>   out:
>> +#ifndef CONFIG_TBOOT
>>       /* Zap ACPI DMAR signature to prevent dom0 using vt-d HW. */
>>       acpi_dmar_zap();
>> +#endif
>>       return ret;
>>   }
>>   
> 
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] x86/dmar: zap DMAR signature for dom0 once in TBOOT case

[Jan Beulich]

>>> On 20.08.18 at 10:40, <zhenzhong.duan@oracle.com> wrote:
> On 2018/8/20 16:30, Jan Beulich wrote:
>>>>> On 20.08.18 at 05:32, <zhenzhong.duan@oracle.com> wrote:
>>> When TBOOT enabled, acpi_parse_dmar() zap a copy of DMAR table rather
>>> than the real table, so make it controled by config option based on the
>>> fact that we already have done the real zapping in tboot_parse_dmar_table().
>> 
>> Is this just a cosmetic change, or is there any harm done by the extra
>> zapping?
> Cosmetic change, I feel it isn't necessory to zap a copy of DMAR table 
> which is freed later.

In which case please clarify this in the description.

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] x86/dmar: zap DMAR signature for dom0 once in TBOOT case

[Andrew Cooper]

On 20/08/2018 09:30, Jan Beulich wrote:
>>>> On 20.08.18 at 05:32, <zhenzhong.duan@oracle.com> wrote:
>> When TBOOT enabled, acpi_parse_dmar() zap a copy of DMAR table rather
>> than the real table, so make it controled by config option based on the
>> fact that we already have done the real zapping in tboot_parse_dmar_table().
> Is this just a cosmetic change, or is there any harm done by the extra
> zapping?

Before 123c7793797502b222300eb710cd3873dcca41ee, calling it multiple
times would require an equivalent number of reinstate calls for it to work.

That change went into Xen 4.6, whereas I guess this is a patch being
upstreamed from Oracles 4.4 patchqueue?

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH 2/2] x86/dmar: zap DMAR signature for dom0 once in TBOOT case

[Zhenzhong Duan]

On 2018/8/20 16:44, Andrew Cooper wrote:
> On 20/08/2018 09:30, Jan Beulich wrote:
>>>>> On 20.08.18 at 05:32, <zhenzhong.duan@oracle.com> wrote:
>>> When TBOOT enabled, acpi_parse_dmar() zap a copy of DMAR table rather
>>> than the real table, so make it controled by config option based on the
>>> fact that we already have done the real zapping in tboot_parse_dmar_table().
>> Is this just a cosmetic change, or is there any harm done by the extra
>> zapping?
> 
> Before 123c7793797502b222300eb710cd3873dcca41ee, calling it multiple
> times would require an equivalent number of reinstate calls for it to work.
> 
> That change went into Xen 4.6, whereas I guess this is a patch being
> upstreamed from Oracles 4.4 patchqueue?
Not for that reason. Patch is for upstream.
tboot_parse_dmar_table() called acpi_dmar_zap() the first time, 
acpi_parse_dmar called it the second time, I thought it's a dup.

Thanks
Zhenzhong

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel