[PATCH for-4.11] x86/XPTI: Fix up stale comments concerning mappings

[PATCH for-4.11] x86/XPTI: Fix up stale comments concerning mappings

[Andrew Cooper]

The comments became stale when c/s d1d6fc97d66 "x86/xpti: really hide almost
all of Xen image" altered how the stubs were mapped.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Juergen Gross <jgross@suse.com>
---
 xen/arch/x86/smpboot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index fd9050e..fcc383d 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -794,7 +794,7 @@ static int setup_cpu_root_pgt(unsigned int cpu)
     /* SH_LINEAR_PT inserted together with guest mappings. */
     /* PERDOMAIN inserted during context switch. */
 
-    /* One-time setup of common_pgt, which maps .text.entry and the stubs. */
+    /* One-time setup of common_pgt, which maps .text.entry. */
     if ( unlikely(!root_get_intpte(common_pgt)) )
     {
         const char *ptr;
@@ -811,7 +811,7 @@ static int setup_cpu_root_pgt(unsigned int cpu)
 
     rpt[root_table_offset(XEN_VIRT_START)] = common_pgt;
 
-    /* Install direct map page table entries for stack, IDT, and TSS. */
+    /* Install per-pcpu mappings for the stack, IDT, TSS and stubs. */
     for ( off = rc = 0; !rc && off < STACK_SIZE; off += PAGE_SIZE )
         if ( !memguard_is_stack_guard_page(off) )
             rc = clone_mapping(__va(__pa(stack_base[cpu])) + off, rpt);
-- 
2.1.4


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

Re: [PATCH for-4.11] x86/XPTI: Fix up stale comments concerning mappings

[Jan Beulich]

>>> On 25.05.18 at 12:52, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -794,7 +794,7 @@ static int setup_cpu_root_pgt(unsigned int cpu)
>      /* SH_LINEAR_PT inserted together with guest mappings. */
>      /* PERDOMAIN inserted during context switch. */
>  
> -    /* One-time setup of common_pgt, which maps .text.entry and the stubs. */
> +    /* One-time setup of common_pgt, which maps .text.entry. */
>      if ( unlikely(!root_get_intpte(common_pgt)) )
>      {
>          const char *ptr;

I don't understand this removal - common_pgt still is used to map the stubs.
It is just that the addition of the leaf entry for the stub space now happens
later. If you really want to change it, how about "One-time setup of
common_pgt and its .text.entry mappings"?

> @@ -811,7 +811,7 @@ static int setup_cpu_root_pgt(unsigned int cpu)
>  
>      rpt[root_table_offset(XEN_VIRT_START)] = common_pgt;
>  
> -    /* Install direct map page table entries for stack, IDT, and TSS. */
> +    /* Install per-pcpu mappings for the stack, IDT, TSS and stubs. */

Just for my own education: Back in school (i.e. quite long ago) I had been
told that in such enumerations - specifically other than in German - a
comma also belongs right ahead of the "and". Is that not the case anymore
nowadays?

Jan



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

Re: [PATCH for-4.11] x86/XPTI: Fix up stale comments concerning mappings

[Andrew Cooper]

On 25/05/18 12:17, Jan Beulich wrote:
>>>> On 25.05.18 at 12:52, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -794,7 +794,7 @@ static int setup_cpu_root_pgt(unsigned int cpu)
>>      /* SH_LINEAR_PT inserted together with guest mappings. */
>>      /* PERDOMAIN inserted during context switch. */
>>  
>> -    /* One-time setup of common_pgt, which maps .text.entry and the stubs. */
>> +    /* One-time setup of common_pgt, which maps .text.entry. */
>>      if ( unlikely(!root_get_intpte(common_pgt)) )
>>      {
>>          const char *ptr;
> I don't understand this removal - common_pgt still is used to map the stubs.

Hang on.  If this is the case, then surely you SP3 fix need to retain
its "no other CPUs using this mapping" check when destroying the mapping.

> It is just that the addition of the leaf entry for the stub space now happens
> later. If you really want to change it, how about "One-time setup of
> common_pgt and its .text.entry mappings"?
>
>> @@ -811,7 +811,7 @@ static int setup_cpu_root_pgt(unsigned int cpu)
>>  
>>      rpt[root_table_offset(XEN_VIRT_START)] = common_pgt;
>>  
>> -    /* Install direct map page table entries for stack, IDT, and TSS. */
>> +    /* Install per-pcpu mappings for the stack, IDT, TSS and stubs. */
> Just for my own education: Back in school (i.e. quite long ago) I had been
> told that in such enumerations - specifically other than in German - a
> comma also belongs right ahead of the "and". Is that not the case anymore
> nowadays?

This known as "The Oxford Comma".  It is one of the more argued-over
areas of English grammar, but is optional (insofar that, some places
demand its presence, some places refuse to allow it, and most don't care
either way).

~Andrew

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

Re: [PATCH for-4.11] x86/XPTI: Fix up stale comments concerning mappings

[Jan Beulich]

>>> On 25.05.18 at 13:31, <andrew.cooper3@citrix.com> wrote:
> On 25/05/18 12:17, Jan Beulich wrote:
>>>>> On 25.05.18 at 12:52, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/arch/x86/smpboot.c
>>> +++ b/xen/arch/x86/smpboot.c
>>> @@ -794,7 +794,7 @@ static int setup_cpu_root_pgt(unsigned int cpu)
>>>      /* SH_LINEAR_PT inserted together with guest mappings. */
>>>      /* PERDOMAIN inserted during context switch. */
>>>  
>>> -    /* One-time setup of common_pgt, which maps .text.entry and the stubs. 
> */
>>> +    /* One-time setup of common_pgt, which maps .text.entry. */
>>>      if ( unlikely(!root_get_intpte(common_pgt)) )
>>>      {
>>>          const char *ptr;
>> I don't understand this removal - common_pgt still is used to map the stubs.
> 
> Hang on.  If this is the case, then surely you SP3 fix need to retain
> its "no other CPUs using this mapping" check when destroying the mapping.

No. All the CPUs share the L4 and L3 entries, but L2 entries may differ
(when there are more than 512 CPUs), and L1 entries will be distinct for
all CPUs. (I take it you mean "S3 fix" rather than "SP3 fix".)

>> It is just that the addition of the leaf entry for the stub space now happens
>> later. If you really want to change it, how about "One-time setup of
>> common_pgt and its .text.entry mappings"?
>>
>>> @@ -811,7 +811,7 @@ static int setup_cpu_root_pgt(unsigned int cpu)
>>>  
>>>      rpt[root_table_offset(XEN_VIRT_START)] = common_pgt;
>>>  
>>> -    /* Install direct map page table entries for stack, IDT, and TSS. */
>>> +    /* Install per-pcpu mappings for the stack, IDT, TSS and stubs. */
>> Just for my own education: Back in school (i.e. quite long ago) I had been
>> told that in such enumerations - specifically other than in German - a
>> comma also belongs right ahead of the "and". Is that not the case anymore
>> nowadays?
> 
> This known as "The Oxford Comma".

Ouch - and I've asked someone sitting in Cambridge. ;-)

Jan



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