Re: [Xen-devel] [PATCH v3 3/3] xen/arm: fix mask calculation in pdx_init_mask

[Julien Grall <julien.grall@xxxxxxx>]

Hi Jan,

On 6/4/19 7:56 AM, Jan Beulich wrote:
> > > > On 04.06.19 at 00:02, <sstabellini@xxxxxxxxxx> wrote:
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -482,7 +482,14 @@ static void __init init_pdx(void)
> >   {
> >       paddr_t bank_start, bank_size, bank_end;
> >   
> > -    u64 mask = pdx_init_mask(bootinfo.mem.bank[0].start);
> > +    /*
> > +     * Arm does not have any restrictions on the bits to compress. Pass 0 to
> > +     * let the common code further restrict the mask.
> > +     *
> > +     * If the logic changes in pfn_pdx_hole_setup we might have to
> > +     * update this function too.
> > +     */
> > +    u64 mask = pdx_init_mask(0x0);
> Seeing Julien's clarification on the previous version's discussion,
> how about switching this one to uint64_t as well at this occasion?
>
> > --- a/xen/common/pdx.c
> > +++ b/xen/common/pdx.c
> > @@ -50,9 +50,12 @@ static u64 __init fill_mask(u64 mask)
> >       return mask;
> >   }
> >   
> > +/*
> > + * We don't compress the first MAX_ORDER bit of the addresses.
> > + */
> This is a single line comment.
>
> >   u64 __init pdx_init_mask(u64 base_addr)
> It wouldn't hamper patch readability much if even this one was
> switched to uint64_t at the same time, thus restoring consistency
> with ...
On Arm we don't tend to mix clean-up and fix. It would be preferable if 
the switch to uint64_t is done in a separate patch.
Cheers,

--
Julien Grall

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