[PATCH V2] libxc: fix xc_translate_foreign_address()

[PATCH V2] libxc: fix xc_translate_foreign_address()

[Razvan Cojocaru]

Currently xc_translate_foreign_address() only checks for the PSE bit on
level 2 entries (that's 2 MB pages on x64 and 32-bit with PAE, and 4 MB
pages on 32-bit). But the Linux kernel sometimes uses 1 GB pages. This
patch fixes that, by checking the PSE bit on level 3 entries if the guest
has 4 translation levels (that means 64-bit guests only).

Signed-off-by: Cristian-Bogdan Sirb <csirb@bitdefender.com>
---
Changes since V1:
 - Added header comment.
---
 tools/libxc/include/xenctrl.h | 3 +++
 tools/libxc/xc_pagetab.c      | 2 +-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
index 2d97d36..ca4bb6c 100644
--- a/tools/libxc/include/xenctrl.h
+++ b/tools/libxc/include/xenctrl.h
@@ -1460,6 +1460,9 @@ int xc_lockprof_query(xc_interface *xch,
 void *xc_memalign(xc_interface *xch, size_t alignment, size_t size);
 
 /**
+ * Avoid using this function, as it does not work for all cases (such
+ * as 4M superpages, or guests using PSE36). Only used for debugging.
+ *
  * Translates a virtual address in the context of a given domain and
  * vcpu returning the GFN containing the address (that is, an MFN for 
  * PV guests, a PFN for HVM guests).  Returns 0 for failure.
diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c
index 92eebd6..db25c20 100644
--- a/tools/libxc/xc_pagetab.c
+++ b/tools/libxc/xc_pagetab.c
@@ -93,7 +93,7 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom,
             return 0;
         }
         paddr = pte & 0x000ffffffffff000ull;
-        if (level == 2 && (pte & PTE_PSE)) {
+        if ((level == 2 || (level == 3 && pt_levels == 4)) && (pte & PTE_PSE)) {
             mask = ((mask ^ ~-mask) >> 1); /* All bits below first set bit */
             return ((paddr & ~mask) | (virt & mask)) >> PAGE_SHIFT;
         }
-- 
1.9.1


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

Re: [PATCH V2] libxc: fix xc_translate_foreign_address()

[Julien Grall]

Hi Razvan,

On 05/04/17 15:53, Razvan Cojocaru wrote:
> Currently xc_translate_foreign_address() only checks for the PSE bit on
> level 2 entries (that's 2 MB pages on x64 and 32-bit with PAE, and 4 MB
> pages on 32-bit). But the Linux kernel sometimes uses 1 GB pages. This
> patch fixes that, by checking the PSE bit on level 3 entries if the guest
> has 4 translation levels (that means 64-bit guests only).
>
> Signed-off-by: Cristian-Bogdan Sirb <csirb@bitdefender.com>

He is the author of the patch, correct? If so, he should be the author 
as well (e.g From: ...).

Also, you may want to add your signed-off-by here.

Cheers,

> ---
> Changes since V1:
>  - Added header comment.
> ---
>  tools/libxc/include/xenctrl.h | 3 +++
>  tools/libxc/xc_pagetab.c      | 2 +-
>  2 files changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> index 2d97d36..ca4bb6c 100644
> --- a/tools/libxc/include/xenctrl.h
> +++ b/tools/libxc/include/xenctrl.h
> @@ -1460,6 +1460,9 @@ int xc_lockprof_query(xc_interface *xch,
>  void *xc_memalign(xc_interface *xch, size_t alignment, size_t size);
>
>  /**
> + * Avoid using this function, as it does not work for all cases (such
> + * as 4M superpages, or guests using PSE36). Only used for debugging.
> + *
>   * Translates a virtual address in the context of a given domain and
>   * vcpu returning the GFN containing the address (that is, an MFN for
>   * PV guests, a PFN for HVM guests).  Returns 0 for failure.
> diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c
> index 92eebd6..db25c20 100644
> --- a/tools/libxc/xc_pagetab.c
> +++ b/tools/libxc/xc_pagetab.c
> @@ -93,7 +93,7 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom,
>              return 0;
>          }
>          paddr = pte & 0x000ffffffffff000ull;
> -        if (level == 2 && (pte & PTE_PSE)) {
> +        if ((level == 2 || (level == 3 && pt_levels == 4)) && (pte & PTE_PSE)) {
>              mask = ((mask ^ ~-mask) >> 1); /* All bits below first set bit */
>              return ((paddr & ~mask) | (virt & mask)) >> PAGE_SHIFT;
>          }
>

-- 
Julien Grall

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

Re: [PATCH V2] libxc: fix xc_translate_foreign_address()

[Razvan Cojocaru]

Hello Julien,

> On 05/04/17 15:53, Razvan Cojocaru wrote:
>> Currently xc_translate_foreign_address() only checks for the PSE bit on
>> level 2 entries (that's 2 MB pages on x64 and 32-bit with PAE, and 4 MB
>> pages on 32-bit). But the Linux kernel sometimes uses 1 GB pages. This
>> patch fixes that, by checking the PSE bit on level 3 entries if the guest
>> has 4 translation levels (that means 64-bit guests only).
>>
>> Signed-off-by: Cristian-Bogdan Sirb <csirb@bitdefender.com>
> 
> He is the author of the patch, correct? If so, he should be the author
> as well (e.g From: ...).
> 
> Also, you may want to add your signed-off-by here.

Alright, I can add my Signed-off for the comment (though it's not much
work, hence my initial reluctance :) ) in which case I suppose it's
alright to resend V3 from my address.


Thanks,
Razvan

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

Re: [PATCH V2] libxc: fix xc_translate_foreign_address()

[Wei Liu]

On Wed, Apr 05, 2017 at 03:58:05PM +0100, Julien Grall wrote:
> Hi Razvan,
> 
> On 05/04/17 15:53, Razvan Cojocaru wrote:
> > Currently xc_translate_foreign_address() only checks for the PSE bit on
> > level 2 entries (that's 2 MB pages on x64 and 32-bit with PAE, and 4 MB
> > pages on 32-bit). But the Linux kernel sometimes uses 1 GB pages. This
> > patch fixes that, by checking the PSE bit on level 3 entries if the guest
> > has 4 translation levels (that means 64-bit guests only).
> > 
> > Signed-off-by: Cristian-Bogdan Sirb <csirb@bitdefender.com>
> 
> He is the author of the patch, correct? If so, he should be the author as
> well (e.g From: ...).
> 
> Also, you may want to add your signed-off-by here.
> 

Julien, if you agree to let this patch go in 4.9. I can fix those up.

Razvan, please tell me what you want me to do.

Code-wise:

Acked-by: Wei Liu <wei.liu2@citrix.com>


> Cheers,
> 
> > ---
> > Changes since V1:
> >  - Added header comment.
> > ---
> >  tools/libxc/include/xenctrl.h | 3 +++
> >  tools/libxc/xc_pagetab.c      | 2 +-
> >  2 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
> > index 2d97d36..ca4bb6c 100644
> > --- a/tools/libxc/include/xenctrl.h
> > +++ b/tools/libxc/include/xenctrl.h
> > @@ -1460,6 +1460,9 @@ int xc_lockprof_query(xc_interface *xch,
> >  void *xc_memalign(xc_interface *xch, size_t alignment, size_t size);
> > 
> >  /**
> > + * Avoid using this function, as it does not work for all cases (such
> > + * as 4M superpages, or guests using PSE36). Only used for debugging.
> > + *
> >   * Translates a virtual address in the context of a given domain and
> >   * vcpu returning the GFN containing the address (that is, an MFN for
> >   * PV guests, a PFN for HVM guests).  Returns 0 for failure.
> > diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c
> > index 92eebd6..db25c20 100644
> > --- a/tools/libxc/xc_pagetab.c
> > +++ b/tools/libxc/xc_pagetab.c
> > @@ -93,7 +93,7 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom,
> >              return 0;
> >          }
> >          paddr = pte & 0x000ffffffffff000ull;
> > -        if (level == 2 && (pte & PTE_PSE)) {
> > +        if ((level == 2 || (level == 3 && pt_levels == 4)) && (pte & PTE_PSE)) {
> >              mask = ((mask ^ ~-mask) >> 1); /* All bits below first set bit */
> >              return ((paddr & ~mask) | (virt & mask)) >> PAGE_SHIFT;
> >          }
> > 
> 
> -- 
> Julien Grall

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

Re: [PATCH V2] libxc: fix xc_translate_foreign_address()

[Razvan Cojocaru]

On 04/05/2017 06:03 PM, Wei Liu wrote:
> On Wed, Apr 05, 2017 at 03:58:05PM +0100, Julien Grall wrote:
>> Hi Razvan,
>>
>> On 05/04/17 15:53, Razvan Cojocaru wrote:
>>> Currently xc_translate_foreign_address() only checks for the PSE bit on
>>> level 2 entries (that's 2 MB pages on x64 and 32-bit with PAE, and 4 MB
>>> pages on 32-bit). But the Linux kernel sometimes uses 1 GB pages. This
>>> patch fixes that, by checking the PSE bit on level 3 entries if the guest
>>> has 4 translation levels (that means 64-bit guests only).
>>>
>>> Signed-off-by: Cristian-Bogdan Sirb <csirb@bitdefender.com>
>>
>> He is the author of the patch, correct? If so, he should be the author as
>> well (e.g From: ...).
>>
>> Also, you may want to add your signed-off-by here.
>>
> 
> Julien, if you agree to let this patch go in 4.9. I can fix those up.
> 
> Razvan, please tell me what you want me to do.
> 
> Code-wise:
> 
> Acked-by: Wei Liu <wei.liu2@citrix.com>

Whatever you'd like is fine with me. I don't care about my Signed-off-by
either way, but if the header comment header I've added justifies it
then please feel free to add:

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

in which case I suppose you won't need to also modify the From:.


Thanks,
Razvan

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

Re: [PATCH V2] libxc: fix xc_translate_foreign_address()

[Wei Liu]

On Wed, Apr 05, 2017 at 06:53:33PM +0300, Razvan Cojocaru wrote:
> On 04/05/2017 06:03 PM, Wei Liu wrote:
> > On Wed, Apr 05, 2017 at 03:58:05PM +0100, Julien Grall wrote:
> >> Hi Razvan,
> >>
> >> On 05/04/17 15:53, Razvan Cojocaru wrote:
> >>> Currently xc_translate_foreign_address() only checks for the PSE bit on
> >>> level 2 entries (that's 2 MB pages on x64 and 32-bit with PAE, and 4 MB
> >>> pages on 32-bit). But the Linux kernel sometimes uses 1 GB pages. This
> >>> patch fixes that, by checking the PSE bit on level 3 entries if the guest
> >>> has 4 translation levels (that means 64-bit guests only).
> >>>
> >>> Signed-off-by: Cristian-Bogdan Sirb <csirb@bitdefender.com>
> >>
> >> He is the author of the patch, correct? If so, he should be the author as
> >> well (e.g From: ...).
> >>
> >> Also, you may want to add your signed-off-by here.
> >>
> > 
> > Julien, if you agree to let this patch go in 4.9. I can fix those up.
> > 
> > Razvan, please tell me what you want me to do.
> > 
> > Code-wise:
> > 
> > Acked-by: Wei Liu <wei.liu2@citrix.com>
> 
> Whatever you'd like is fine with me. I don't care about my Signed-off-by
> either way, but if the header comment header I've added justifies it
> then please feel free to add:
> 
> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
> 

Yes.

> in which case I suppose you won't need to also modify the From:.
> 

Who should be the author of this patch? I can't make a decision on your
behalf...

Wei.

> 
> Thanks,
> Razvan

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

Re: [PATCH V2] libxc: fix xc_translate_foreign_address()

[Razvan Cojocaru]

On 04/05/2017 06:56 PM, Wei Liu wrote:
> On Wed, Apr 05, 2017 at 06:53:33PM +0300, Razvan Cojocaru wrote:
>> On 04/05/2017 06:03 PM, Wei Liu wrote:
>>> On Wed, Apr 05, 2017 at 03:58:05PM +0100, Julien Grall wrote:
>>>> Hi Razvan,
>>>>
>>>> On 05/04/17 15:53, Razvan Cojocaru wrote:
>>>>> Currently xc_translate_foreign_address() only checks for the PSE bit on
>>>>> level 2 entries (that's 2 MB pages on x64 and 32-bit with PAE, and 4 MB
>>>>> pages on 32-bit). But the Linux kernel sometimes uses 1 GB pages. This
>>>>> patch fixes that, by checking the PSE bit on level 3 entries if the guest
>>>>> has 4 translation levels (that means 64-bit guests only).
>>>>>
>>>>> Signed-off-by: Cristian-Bogdan Sirb <csirb@bitdefender.com>
>>>>
>>>> He is the author of the patch, correct? If so, he should be the author as
>>>> well (e.g From: ...).
>>>>
>>>> Also, you may want to add your signed-off-by here.
>>>>
>>>
>>> Julien, if you agree to let this patch go in 4.9. I can fix those up.
>>>
>>> Razvan, please tell me what you want me to do.
>>>
>>> Code-wise:
>>>
>>> Acked-by: Wei Liu <wei.liu2@citrix.com>
>>
>> Whatever you'd like is fine with me. I don't care about my Signed-off-by
>> either way, but if the header comment header I've added justifies it
>> then please feel free to add:
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
>>
> 
> Yes.
> 
>> in which case I suppose you won't need to also modify the From:.
>>
> 
> Who should be the author of this patch? I can't make a decision on your
> behalf...

Sorry, the author should definitely be Cristian-Bogdan Sirb
<csirb@bitdefender.com>. He discovered and fixed the issue, I merely
added the header comment.


Thanks,
Razvan

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

Re: [PATCH V2] libxc: fix xc_translate_foreign_address()

[Julien Grall]

On 05/04/17 16:03, Wei Liu wrote:
> On Wed, Apr 05, 2017 at 03:58:05PM +0100, Julien Grall wrote:
>> Hi Razvan,
>>
>> On 05/04/17 15:53, Razvan Cojocaru wrote:
>>> Currently xc_translate_foreign_address() only checks for the PSE bit on
>>> level 2 entries (that's 2 MB pages on x64 and 32-bit with PAE, and 4 MB
>>> pages on 32-bit). But the Linux kernel sometimes uses 1 GB pages. This
>>> patch fixes that, by checking the PSE bit on level 3 entries if the guest
>>> has 4 translation levels (that means 64-bit guests only).
>>>
>>> Signed-off-by: Cristian-Bogdan Sirb <csirb@bitdefender.com>
>>
>> He is the author of the patch, correct? If so, he should be the author as
>> well (e.g From: ...).
>>
>> Also, you may want to add your signed-off-by here.
>>
>
> Julien, if you agree to let this patch go in 4.9. I can fix those up.

Release-acked-by: Julien Grall <julien.grall@arm.com>

Cheers,

>
> Razvan, please tell me what you want me to do.
>
> Code-wise:
>
> Acked-by: Wei Liu <wei.liu2@citrix.com>
>
>
>> Cheers,
>>
>>> ---
>>> Changes since V1:
>>>  - Added header comment.
>>> ---
>>>  tools/libxc/include/xenctrl.h | 3 +++
>>>  tools/libxc/xc_pagetab.c      | 2 +-
>>>  2 files changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/libxc/include/xenctrl.h b/tools/libxc/include/xenctrl.h
>>> index 2d97d36..ca4bb6c 100644
>>> --- a/tools/libxc/include/xenctrl.h
>>> +++ b/tools/libxc/include/xenctrl.h
>>> @@ -1460,6 +1460,9 @@ int xc_lockprof_query(xc_interface *xch,
>>>  void *xc_memalign(xc_interface *xch, size_t alignment, size_t size);
>>>
>>>  /**
>>> + * Avoid using this function, as it does not work for all cases (such
>>> + * as 4M superpages, or guests using PSE36). Only used for debugging.
>>> + *
>>>   * Translates a virtual address in the context of a given domain and
>>>   * vcpu returning the GFN containing the address (that is, an MFN for
>>>   * PV guests, a PFN for HVM guests).  Returns 0 for failure.
>>> diff --git a/tools/libxc/xc_pagetab.c b/tools/libxc/xc_pagetab.c
>>> index 92eebd6..db25c20 100644
>>> --- a/tools/libxc/xc_pagetab.c
>>> +++ b/tools/libxc/xc_pagetab.c
>>> @@ -93,7 +93,7 @@ unsigned long xc_translate_foreign_address(xc_interface *xch, uint32_t dom,
>>>              return 0;
>>>          }
>>>          paddr = pte & 0x000ffffffffff000ull;
>>> -        if (level == 2 && (pte & PTE_PSE)) {
>>> +        if ((level == 2 || (level == 3 && pt_levels == 4)) && (pte & PTE_PSE)) {
>>>              mask = ((mask ^ ~-mask) >> 1); /* All bits below first set bit */
>>>              return ((paddr & ~mask) | (virt & mask)) >> PAGE_SHIFT;
>>>          }
>>>
>>
>> --
>> Julien Grall

-- 
Julien Grall

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