Re: wrap around in a memory region property?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Fri, Oct 05, 2018 at 07:05:31AM -0500, Rob Herring wrote:
> On Fri, Oct 5, 2018 at 1:05 AM AKASHI Takahiro
> <takahiro.akashi@xxxxxxxxxx> wrote:
> >
> > # This issue was raised from a discussion on my patch set of kexec_file
> >   support on arm64. Please see the thread[1] for the background.
> >
> > Let's think about the case that we want to modify a device tree blob and
> > dynamically add a sort of "memory region" property to some node:
> >         mem_range = <addr size>;
> >
> > When #address_cells == 1 and #size_cells == 1, how we should deal with
> > the memory if "addr + size" overflows a 32-bit range and wraps around?
> >
> > One instance is (from Frank's email),
> > >          / memory@0xfffff000 {
> > >               reg = <0xfffff000 0x2000>;
> > >            }
> > >
> > >       or must it be specified as (option 1):
> > >
> > >          / memory@0xfffff000 {
> > >               reg = <0xfffff000 0x1000 0x0 0x1000>
> 
> Normally, we'd put these in address order.
> 
> > >            }
> > >
> > >       or (option 2):
> > >
> > >          / memory@0xfffff000 {
> > >               reg = <0xfffff000 0x1000>;
> > >            }
> > >            memory@0 {
> > >               reg = <0x0 0x1000>;
> > >            }
> 
> Either option 1 or 2 is valid. Probably 1 is more common.
> 
> Note that even these wrap. They wrap to 0 which I think we have to
> support,

Well, that depends on how you define it.  If you naively try to
compute end = (start + size), then yes, that arithmetic will wrap.
The range itself does *not* wrap, since it's not inclusive at the end.

But however you look at it, yes, we absolutely need to handle this
case.  It occurs frequently in practice, and there's not really any
other way we could encode that case.

> but going past that shouldn't be allowed IMO.

I mostly agree.  Regardless of how we define it in the spec, I think
trying to use a wrapping range like that should befg
avoided, because it would be very confusing.

For completeness of the spec, I think the right way to define it is
that anything which extends beyond the range of the address space it
appears in is inaccessible, rather than wrapping to somewhere else in
that address space.

Rationale: That way the behaviour of a reg which extends beyond
#address-cells in its containing bus matches that of a reg which
extends beyond one of the 'ranges' windows in its parent bridge.  It
might exist in some theoretical sense, but is inaccessible to anything
further up the tree.

Additionally if you do have a weird device whose registers are wrapped
around the 32-bit boundary, this forces you to be explicit about that
wrapping in the DT.  That means less changes if the next version of
the device is designed for, say, a 40-bit bus, and needs to wrap
around that (buses which are notionally "64-bit" but only actually
implement 40-50 address lines are pretty common).

I can sort of imagine some (pretty contrived) cases where this might
make some sense.  e.g. a bus with #address-cells defined as 1 not
because the bus itself can't address beyond 32-bits, but because we
know the cpu or the bridge above can't address beyond that.  In that
case devices could notionally have registers above 32-bits, they just
wouldn't be usable (of course that would better be described with an
#address-cells = 2 bus and a bridge providing a 32-bit window into
it).

> We had numerous discussions in the past about this in context of dma-ranges.
> 
> > Or does it imply, in fact,
> >     0x0fffff000-0x0ffffffff, and
> >     0x100000000-0x100000fff ?
> > (because neither #address_cells nor #size_cells, in theory, prevents
> > the host from having >32-bit memory space.)
> 
> No. If you have more than 32-bits, increase the cells.
> 
> Rob
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Device Tree]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux Audio Users]     [Photos]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]

  Powered by Linux