| Message ID | 1491404033-14458-1-git-send-email-rcojocaru@bitdefender.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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; > } >
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
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
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
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
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
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
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; }
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(-)