| Message ID | 1379256713-13197-1-git-send-email-peter.maydell@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
On Sun, Sep 15, 2013 at 03:51:53PM +0100, Peter Maydell wrote: > The documentation of how overlapping memory regions behave and how > the priority system works was rather brief, and confusion about > priorities seems to be quite common for developers trying to understand > how the memory region system works, so expand and clarify it. > This includes a worked example with overlaps, documentation of the > behaviour when an overlapped container has "holes", and mention > that it's valid for a region to have both MMIO callbacks and > subregions (and how this interacts with priorities when it does). > > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Great, thanks a lot! Minor comments below: > --- > docs/memory.txt | 48 +++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 47 insertions(+), 1 deletion(-) > > diff --git a/docs/memory.txt b/docs/memory.txt > index feb9fe9..bd0ef6e 100644 > --- a/docs/memory.txt > +++ b/docs/memory.txt > @@ -45,6 +45,10 @@ MemoryRegion): > can overlay a subregion of RAM with MMIO or ROM, or a PCI controller > that does not prevent card from claiming overlapping BARs. > > + It is valid for regions which are not "pure containers" I would add "that is, MMIO, RAM or ROM" > to have subregions; > + this means that any addresses within the container's region which are > + not claimed by a subregion maybe stress "by any subregion" > are handled by the container's MMIO callbacks. RAM doesn't have MMIO callbacks (at least at the API level), maybe say something like "cause an access to the container itself (e.g. invoke container's MMIO callbacks or modify container's RAM)" is better? > + > - alias: a subsection of another region. Aliases allow a region to be > split apart into discontiguous regions. Examples of uses are memory banks > used when the guest address space is smaller than the amount of RAM > @@ -81,6 +85,45 @@ allows the region to overlap any other region in the same container, and > specifies a priority that allows the core to decide which of two regions at > the same address are visible (highest wins). > > +If the higher priority region in an overlap is a container or alias, then > +the lower priority region will appear in any "holes" that the higher priority > +region has left by not mapping subregions Maybe add "(or recursively - holes that some of the subregions left - if some of the subregions are containers or aliases)" > to that area of its address range. > +For example, suppose we have a container A of size 0x8000 with two subregions > +B and C. B is a container mapped at 0x2000, size 0x4000, priority 1; C is > +an MMIO region mapped at 0x0, size 0x6000, priority 2. B currently has two > +of its own subregions: D of size 0x1000 at offset 0 and E of size 0x1000 at > +offset 0x2000. As a diagram: > + > + 0 1000 2000 3000 4000 5000 6000 7000 8000 > + |------|------|------|------|------|------|------|-------| > + A: [ ] > + C: [CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC] > + B: [ ] > + D: [DDDDD] > + E: [EEEEE] > + > +The regions that will be seen within this address range then are: > + [CCCCCCCCCCCC][DDDDD][CCCCC][EEEEE][CCCCC] > + > +Since B has higher priority than C, its subregions appear in the flat map > +even where they overlap with C. In ranges where B has not mapped anything > +C's region appears. > + > +If B had provided its own MMIO operations (ie it was not a pure container) > +then these would be used for any addresses in its range not handled by > +D or E, and the result would be: > + [CCCCCCCCCCCC][DDDDD][BBBBB][EEEEE][BBBBB] > + > +Priority values are local to a container, because the priorities of two > +regions are only compared when they are both children of the same container. > +This means that the device in charge of the container (typically modelling > +a bus or a memory controller) can use them to manage the interaction of > +its child regions without any side effects on other parts of the system. > +In the example above, the priorities of D and E are unimportant because > +they do not overlap each other. It is the relative priority of B and C > +that causes D and E to appear on top of C: D and E's priorities are never > +compared against the priority of C. > + > Visibility > ---------- > The memory core uses the following rules to select a memory region when the > @@ -93,8 +136,11 @@ guest accesses an address: > - if the subregion is a leaf (RAM or MMIO), the search terminates Maybe add "And the leaf is selected" > - if the subregion is a container, the same algorithm is used within the > subregion (after the address is adjusted by the subregion offset) > - - if the subregion is an alias, the search is continues at the alias target > + - if the subregion is an alias, the search is continued at the alias target > (after the address is adjusted by the subregion offset and alias offset) > + - if a recursive search within a container or alias subregion does not > + find a match (because of a "hole" in the container's coverage of its > + address range), we continue with the next subregion in priority order > This makes it look like the one way for a search to terminate is with RAM or MMIO. There are two other cases: - non pure container -> container can be selected - no match is found -> nothing is selected > Example memory map > ------------------ > -- > 1.7.11.4
On 15 September 2013 16:24, Michael S. Tsirkin <mst@redhat.com> wrote: > On Sun, Sep 15, 2013 at 03:51:53PM +0100, Peter Maydell wrote: >> The documentation of how overlapping memory regions behave and how >> the priority system works was rather brief, and confusion about >> priorities seems to be quite common for developers trying to understand >> how the memory region system works, so expand and clarify it. >> This includes a worked example with overlaps, documentation of the >> behaviour when an overlapped container has "holes", and mention >> that it's valid for a region to have both MMIO callbacks and >> subregions (and how this interacts with priorities when it does). >> >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > Great, thanks a lot! > Minor comments below: > >> --- >> docs/memory.txt | 48 +++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 47 insertions(+), 1 deletion(-) >> >> diff --git a/docs/memory.txt b/docs/memory.txt >> index feb9fe9..bd0ef6e 100644 >> --- a/docs/memory.txt >> +++ b/docs/memory.txt >> @@ -45,6 +45,10 @@ MemoryRegion): >> can overlay a subregion of RAM with MMIO or ROM, or a PCI controller >> that does not prevent card from claiming overlapping BARs. >> >> + It is valid for regions which are not "pure containers" > > I would add "that is, MMIO, RAM or ROM" Actually maybe it would be better to instead have this new paragraph go at the bottom of the 'types of region' section rather than inside the 'container' section. Containers with MMIO etc are a bit odd because I think conceptually it's easier to think of them as a kind of container but API-wise you do it by just creating one of the other types of region and then using the subregion APIs on it. So I put the explanation in the part describing containers, but it looks a bit odd. If we moved it we would have: It is valid to add subregions to a region which is not a pure container (that is, to an MMIO, RAM or ROM region). This means that the region will act like a container, except that any addresses within the container's region which are not claimed by any subregion are handled by the container itself (ie by its MMIO callbacks or RAM backing). However it is generally possible to achieve the same effect with a pure container one of whose subregions is a low priority "background" region covering the whole address range; this is often clearer and is preferred. Subregions cannot be added to an alias region. >> to have subregions; >> + this means that any addresses within the container's region which are >> + not claimed by a subregion > > maybe stress "by any subregion" Agreed. >> are handled by the container's MMIO callbacks. > > RAM doesn't have MMIO callbacks (at least at the API level), > maybe say something like "cause an access to the container > itself (e.g. invoke container's MMIO callbacks or > modify container's RAM)" is better? I'm kind of unsure about container RAM/ROM, which is why I didn't specifically mention it: it's not forbidden by assertions, and it will have a reasonably straightforward effect by analogy with containers with MMIO callbacks, but it's hard to see why you'd want it. (We only use the container-with-IO for a particular effect with the system IO space's region.) But I've tweaked the wording in my suggested new para above along these lines. >> + >> - alias: a subsection of another region. Aliases allow a region to be >> split apart into discontiguous regions. Examples of uses are memory banks >> used when the guest address space is smaller than the amount of RAM >> @@ -81,6 +85,45 @@ allows the region to overlap any other region in the same container, and >> specifies a priority that allows the core to decide which of two regions at >> the same address are visible (highest wins). >> >> +If the higher priority region in an overlap is a container or alias, then >> +the lower priority region will appear in any "holes" that the higher priority >> +region has left by not mapping subregions > Maybe add > "(or recursively - holes that some of the subregions > left - if some of the subregions are containers or aliases)" >> to that area of its address range. Yes -- though I've put it as an extra sentence rather than inserting it into the middle of an already fairly long sentence: (This applies recursively -- if the subregions are themselves containers or aliases that leave holes then the lower priority region will appear in these holes too.) >> Visibility >> ---------- >> The memory core uses the following rules to select a memory region when the >> @@ -93,8 +136,11 @@ guest accesses an address: >> - if the subregion is a leaf (RAM or MMIO), the search terminates > > Maybe add > "And the leaf is selected" Not sure what you mean by "selected" here. >> - if the subregion is a container, the same algorithm is used within the >> subregion (after the address is adjusted by the subregion offset) >> - - if the subregion is an alias, the search is continues at the alias target >> + - if the subregion is an alias, the search is continued at the alias target >> (after the address is adjusted by the subregion offset and alias offset) >> + - if a recursive search within a container or alias subregion does not >> + find a match (because of a "hole" in the container's coverage of its >> + address range), we continue with the next subregion in priority order >> > > This makes it look like the one way for a search to terminate > is with RAM or MMIO. > There are two other cases: > - non pure container -> container can be selected > - no match is found -> nothing is selected How about: - if a recursive search within a container or alias subregion does not find a match (because of a "hole" in the container's coverage of its address range), then if this is a container with its own MMIO or RAM backing the search terminates with the container itself. Otherwise we continue with the next subregion in priority order and then one level of bullet point nesting out (ie lining up with "all direct subregions...") - if none of the subregions match then the search terminates with no match found ? -- PMM
On Sun, Sep 15, 2013 at 05:55:54PM +0100, Peter Maydell wrote: > On 15 September 2013 16:24, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Sun, Sep 15, 2013 at 03:51:53PM +0100, Peter Maydell wrote: > >> The documentation of how overlapping memory regions behave and how > >> the priority system works was rather brief, and confusion about > >> priorities seems to be quite common for developers trying to understand > >> how the memory region system works, so expand and clarify it. > >> This includes a worked example with overlaps, documentation of the > >> behaviour when an overlapped container has "holes", and mention > >> that it's valid for a region to have both MMIO callbacks and > >> subregions (and how this interacts with priorities when it does). > >> > >> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > > > > Great, thanks a lot! > > Minor comments below: > > > >> --- > >> docs/memory.txt | 48 +++++++++++++++++++++++++++++++++++++++++++++++- > >> 1 file changed, 47 insertions(+), 1 deletion(-) > >> > >> diff --git a/docs/memory.txt b/docs/memory.txt > >> index feb9fe9..bd0ef6e 100644 > >> --- a/docs/memory.txt > >> +++ b/docs/memory.txt > >> @@ -45,6 +45,10 @@ MemoryRegion): > >> can overlay a subregion of RAM with MMIO or ROM, or a PCI controller > >> that does not prevent card from claiming overlapping BARs. > >> > >> + It is valid for regions which are not "pure containers" > > > > I would add "that is, MMIO, RAM or ROM" > > Actually maybe it would be better to instead have this new paragraph > go at the bottom of the 'types of region' section rather than inside > the 'container' section. Containers with MMIO etc are a bit odd because > I think conceptually it's easier to think of them as a kind of container but > API-wise you do it by just creating one of the other types of region and > then using the subregion APIs on it. So I put the explanation in the > part describing containers, but it looks a bit odd. If we moved it we would > have: > > It is valid to add subregions to a region which is not a pure container > (that is, to an MMIO, RAM or ROM region). This means that the region > will act like a container, except that any addresses within the container's > region which are not claimed by any subregion are handled by the > container itself (ie by its MMIO callbacks or RAM backing). However > it is generally possible to achieve the same effect with a pure container > one of whose subregions is a low priority "background" region covering > the whole address range; this is often clearer and is preferred. > Subregions cannot be added to an alias region. > > >> to have subregions; > >> + this means that any addresses within the container's region which are > >> + not claimed by a subregion > > > > maybe stress "by any subregion" > > Agreed. > > >> are handled by the container's MMIO callbacks. > > > > RAM doesn't have MMIO callbacks (at least at the API level), > > maybe say something like "cause an access to the container > > itself (e.g. invoke container's MMIO callbacks or > > modify container's RAM)" is better? > > I'm kind of unsure about container RAM/ROM, which is why I didn't > specifically mention it: it's not forbidden by assertions, and it will > have a reasonably straightforward effect by analogy with containers > with MMIO callbacks, but it's hard to see why you'd want it. (We only > use the container-with-IO for a particular effect with the system IO > space's region.) But I've tweaked the wording in my suggested new > para above along these lines. > > >> + > >> - alias: a subsection of another region. Aliases allow a region to be > >> split apart into discontiguous regions. Examples of uses are memory banks > >> used when the guest address space is smaller than the amount of RAM > >> @@ -81,6 +85,45 @@ allows the region to overlap any other region in the same container, and > >> specifies a priority that allows the core to decide which of two regions at > >> the same address are visible (highest wins). > >> > >> +If the higher priority region in an overlap is a container or alias, then > >> +the lower priority region will appear in any "holes" that the higher priority > >> +region has left by not mapping subregions > > Maybe add > > "(or recursively - holes that some of the subregions > > left - if some of the subregions are containers or aliases)" > >> to that area of its address range. > > Yes -- though I've put it as an extra sentence rather than inserting it into > the middle of an already fairly long sentence: > > (This applies recursively -- if the subregions are themselves containers or > aliases that leave holes then the lower priority region will appear in these > holes too.) > > >> Visibility > >> ---------- > >> The memory core uses the following rules to select a memory region when the > >> @@ -93,8 +136,11 @@ guest accesses an address: > >> - if the subregion is a leaf (RAM or MMIO), the search terminates > > > > Maybe add > > "And the leaf is selected" > > Not sure what you mean by "selected" here. Well search terminates but what is the result? It says "to select a memory region" so for the result I said "is selected". > >> - if the subregion is a container, the same algorithm is used within the > >> subregion (after the address is adjusted by the subregion offset) > >> - - if the subregion is an alias, the search is continues at the alias target > >> + - if the subregion is an alias, the search is continued at the alias target > >> (after the address is adjusted by the subregion offset and alias offset) > >> + - if a recursive search within a container or alias subregion does not > >> + find a match (because of a "hole" in the container's coverage of its > >> + address range), we continue with the next subregion in priority order > >> > > > > This makes it look like the one way for a search to terminate > > is with RAM or MMIO. > > There are two other cases: > > - non pure container -> container can be selected > > - no match is found -> nothing is selected > > How about: > - if a recursive search within a container or alias subregion does not > find a match (because of a "hole" in the container's coverage of its > address range), then if this is a container with its own MMIO or RAM > backing the search terminates with the container itself. Otherwise > we continue with the next subregion in priority order > > and then one level of bullet point nesting out (ie lining up with > "all direct subregions...") > - if none of the subregions match then the search terminates with > no match found > > ? > > -- PMM Looks good.
On 15 September 2013 18:07, Michael S. Tsirkin <mst@redhat.com> wrote: > On Sun, Sep 15, 2013 at 05:55:54PM +0100, Peter Maydell wrote: >> On 15 September 2013 16:24, Michael S. Tsirkin <mst@redhat.com> wrote: >> > On Sun, Sep 15, 2013 at 03:51:53PM +0100, Peter Maydell wrote: >> >> The memory core uses the following rules to select a memory region when the >> >> @@ -93,8 +136,11 @@ guest accesses an address: >> >> - if the subregion is a leaf (RAM or MMIO), the search terminates >> > >> > Maybe add >> > "And the leaf is selected" >> >> Not sure what you mean by "selected" here. > > Well search terminates but what is the result? > It says "to select a memory region" > so for the result I said "is selected". Oh, I see. I thought you were suggesting "if the subregion is a leaf and the leaf is selected, the search terminates"... My suggested text below uses the phrasing "the search terminates with X", so perhaps "the search terminates with this leaf region" ? Better, make both of them be "the search terminates, returning X". -- PMM
On Sun, Sep 15, 2013 at 06:29:51PM +0100, Peter Maydell wrote: > On 15 September 2013 18:07, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Sun, Sep 15, 2013 at 05:55:54PM +0100, Peter Maydell wrote: > >> On 15 September 2013 16:24, Michael S. Tsirkin <mst@redhat.com> wrote: > >> > On Sun, Sep 15, 2013 at 03:51:53PM +0100, Peter Maydell wrote: > >> >> The memory core uses the following rules to select a memory region when the > >> >> @@ -93,8 +136,11 @@ guest accesses an address: > >> >> - if the subregion is a leaf (RAM or MMIO), the search terminates > >> > > >> > Maybe add > >> > "And the leaf is selected" > >> > >> Not sure what you mean by "selected" here. > > > > Well search terminates but what is the result? > > It says "to select a memory region" > > so for the result I said "is selected". > > Oh, I see. I thought you were suggesting "if the subregion is a leaf > and the leaf is selected, the search terminates"... > > My suggested text below uses the phrasing "the search terminates > with X", so perhaps "the search terminates with this leaf region" ? > Better, make both of them be "the search terminates, returning X". > > -- PMM Sounds good.
diff --git a/docs/memory.txt b/docs/memory.txt index feb9fe9..bd0ef6e 100644 --- a/docs/memory.txt +++ b/docs/memory.txt @@ -45,6 +45,10 @@ MemoryRegion): can overlay a subregion of RAM with MMIO or ROM, or a PCI controller that does not prevent card from claiming overlapping BARs. + It is valid for regions which are not "pure containers" to have subregions; + this means that any addresses within the container's region which are + not claimed by a subregion are handled by the container's MMIO callbacks. + - alias: a subsection of another region. Aliases allow a region to be split apart into discontiguous regions. Examples of uses are memory banks used when the guest address space is smaller than the amount of RAM @@ -81,6 +85,45 @@ allows the region to overlap any other region in the same container, and specifies a priority that allows the core to decide which of two regions at the same address are visible (highest wins). +If the higher priority region in an overlap is a container or alias, then +the lower priority region will appear in any "holes" that the higher priority +region has left by not mapping subregions to that area of its address range. +For example, suppose we have a container A of size 0x8000 with two subregions +B and C. B is a container mapped at 0x2000, size 0x4000, priority 1; C is +an MMIO region mapped at 0x0, size 0x6000, priority 2. B currently has two +of its own subregions: D of size 0x1000 at offset 0 and E of size 0x1000 at +offset 0x2000. As a diagram: + + 0 1000 2000 3000 4000 5000 6000 7000 8000 + |------|------|------|------|------|------|------|-------| + A: [ ] + C: [CCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCCC] + B: [ ] + D: [DDDDD] + E: [EEEEE] + +The regions that will be seen within this address range then are: + [CCCCCCCCCCCC][DDDDD][CCCCC][EEEEE][CCCCC] + +Since B has higher priority than C, its subregions appear in the flat map +even where they overlap with C. In ranges where B has not mapped anything +C's region appears. + +If B had provided its own MMIO operations (ie it was not a pure container) +then these would be used for any addresses in its range not handled by +D or E, and the result would be: + [CCCCCCCCCCCC][DDDDD][BBBBB][EEEEE][BBBBB] + +Priority values are local to a container, because the priorities of two +regions are only compared when they are both children of the same container. +This means that the device in charge of the container (typically modelling +a bus or a memory controller) can use them to manage the interaction of +its child regions without any side effects on other parts of the system. +In the example above, the priorities of D and E are unimportant because +they do not overlap each other. It is the relative priority of B and C +that causes D and E to appear on top of C: D and E's priorities are never +compared against the priority of C. + Visibility ---------- The memory core uses the following rules to select a memory region when the @@ -93,8 +136,11 @@ guest accesses an address: - if the subregion is a leaf (RAM or MMIO), the search terminates - if the subregion is a container, the same algorithm is used within the subregion (after the address is adjusted by the subregion offset) - - if the subregion is an alias, the search is continues at the alias target + - if the subregion is an alias, the search is continued at the alias target (after the address is adjusted by the subregion offset and alias offset) + - if a recursive search within a container or alias subregion does not + find a match (because of a "hole" in the container's coverage of its + address range), we continue with the next subregion in priority order Example memory map ------------------
The documentation of how overlapping memory regions behave and how the priority system works was rather brief, and confusion about priorities seems to be quite common for developers trying to understand how the memory region system works, so expand and clarify it. This includes a worked example with overlaps, documentation of the behaviour when an overlapped container has "holes", and mention that it's valid for a region to have both MMIO callbacks and subregions (and how this interacts with priorities when it does). Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- docs/memory.txt | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-)