[PATCH] x86/dom0: change align type to int

[PATCH] x86/dom0: change align type to int

[Roger Pau Monne]

There's no reason to use long to store the alignment, since the bigger
page size is 1GB, and the alignment is stored as a frame number.

Reported-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/hvm/dom0_build.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
index 5ae3a32060..b696100043 100644
--- a/xen/arch/x86/hvm/dom0_build.c
+++ b/xen/arch/x86/hvm/dom0_build.c
@@ -92,8 +92,7 @@ static int __init pvh_populate_memory_range(struct domain *d,
                                             unsigned long nr_pages)
 {
     struct {
-        unsigned long align;
-        unsigned int order;
+        unsigned int align, order;
     } static const __initconst orders[] = {
         /* NB: must be sorted by decreasing size. */
         { .align = PFN_DOWN(GB(1)), .order = PAGE_ORDER_1G },
-- 
2.17.2 (Apple Git-113)


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

Re: [PATCH] x86/dom0: change align type to int

[Jan Beulich]

>>> On 04.01.19 at 16:14, <roger.pau@citrix.com> wrote:
> There's no reason to use long to store the alignment, since the bigger

biggest?

> page size is 1GB, and the alignment is stored as a frame number.
> 
> Reported-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> ---
>  xen/arch/x86/hvm/dom0_build.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> index 5ae3a32060..b696100043 100644
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -92,8 +92,7 @@ static int __init pvh_populate_memory_range(struct domain *d,
>                                              unsigned long nr_pages)
>  {
>      struct {
> -        unsigned long align;
> -        unsigned int order;
> +        unsigned int align, order;
>      } static const __initconst orders[] = {
>          /* NB: must be sorted by decreasing size. */
>          { .align = PFN_DOWN(GB(1)), .order = PAGE_ORDER_1G },

That's not enough afaict, you also need to use 1UL in

            end = (start + nr_pages) & ~(orders[0].align - 1);

Jan


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

Re: [PATCH] x86/dom0: change align type to int

[Roger Pau Monné]

On Fri, Jan 04, 2019 at 08:35:38AM -0700, Jan Beulich wrote:
> >>> On 04.01.19 at 16:14, <roger.pau@citrix.com> wrote:
> > There's no reason to use long to store the alignment, since the bigger
> 
> biggest?
> 
> > page size is 1GB, and the alignment is stored as a frame number.
> > 
> > Reported-by: Jan Beulich <JBeulich@suse.com>
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> > ---
> > Cc: Jan Beulich <jbeulich@suse.com>
> > Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> > Cc: Wei Liu <wei.liu2@citrix.com>
> > ---
> >  xen/arch/x86/hvm/dom0_build.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> > index 5ae3a32060..b696100043 100644
> > --- a/xen/arch/x86/hvm/dom0_build.c
> > +++ b/xen/arch/x86/hvm/dom0_build.c
> > @@ -92,8 +92,7 @@ static int __init pvh_populate_memory_range(struct domain *d,
> >                                              unsigned long nr_pages)
> >  {
> >      struct {
> > -        unsigned long align;
> > -        unsigned int order;
> > +        unsigned int align, order;
> >      } static const __initconst orders[] = {
> >          /* NB: must be sorted by decreasing size. */
> >          { .align = PFN_DOWN(GB(1)), .order = PAGE_ORDER_1G },
> 
> That's not enough afaict, you also need to use 1UL in
> 
>             end = (start + nr_pages) & ~(orders[0].align - 1);

Right, I'm afraid I prefer to keep align as unsigned long in order to
avoid such mistakes.

Thanks, Roger.

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