Re: [Xen-devel] [PATCH v5 3/7] xen/arm: keep track of reserved-memory regions

[Julien Grall <julien.grall@xxxxxxx>]

Hi,

On 8/13/19 4:14 PM, Volodymyr Babchuk wrote:
> Julien Grall writes:
> > On 8/13/19 3:23 PM, Volodymyr Babchuk wrote:
> > > Stefano Stabellini writes:
> > >
> > > >        {
> > > >            device_tree_get_reg(&cell, address_cells, size_cells, &start, &size);
> > > >            if ( !size )
> > > >                continue;
> > > > -        bootinfo.mem.bank[bootinfo.mem.nr_banks].start = start;
> > > > -        bootinfo.mem.bank[bootinfo.mem.nr_banks].size = size;
> > > > -        bootinfo.mem.nr_banks++;
> > > > +        mem->bank[mem->nr_banks].start = start;
> > > > +        mem->bank[mem->nr_banks].size = size;
> > > > +        mem->nr_banks++;
> > > >        }
> > > >
> > > > -    if ( bootinfo.mem.nr_banks == NR_MEM_BANKS )
> > > > +    if ( mem->nr_banks == NR_MEM_BANKS )
> > > Looks like you have the same off-by-one error, as in previous patch.
> > > I can see that it was there earlier. But it is good time to fix it.
> > I don't think there was an off-by-one error before this series. So
> > what do you mean?
> I explained this in patch #2. Imagine that NR_MEM_BANKS = 1 and you have
> one memory node in the dtb. You'll fill the first element of the array
> and mem->nr_banks will become 1. This is absolutely normal. But check
> above will fail, which is not right.
Ok. So the off-by-one error has been introduced by this series. So this 
should be fixed in patch #2 not here.
Cheers,

--
Julien Grall

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