Patchwork [WIP] Bare-metal register browsing

login
register
mail settings
Submitter Vladimir Prus
Date April 20, 2015, 6:29 a.m.
Message ID <55349CDB.8010100@codesourcery.com>
Download mbox | patch
Permalink /patch/6326/
State New
Headers show

Comments

Vladimir Prus - April 20, 2015, 6:29 a.m.
The attached patches implement accessing peripheral registers on bare-metal targets. Typically,
these registers are memory-mapped, so one can poke at them using memory operations, but it's
far from convenient. Also, on some targets the registers might require a custom way of access,
which makes things even less convenient.

This patch allows target XML to describe 'spaces' - contains of registers, which can be further grouped.
Given that descrpiption, GDB allows one to do something like:

	(gdb) print $io.GPIO_PORTF.GPIO_PORTFDIR
	$7 = -1

to access registers. One can also do

	(gdb) ptype $io

to see top-level register groups in space 'io', and so find the desired register.

Internally, new qXfer object, called 'spaces' is used to implement this functionality.
Remote target is in position to use any mechanism it sees fit to access the registers.
However, there's also a shortcut that allow the target xml to specify that space is
memory-mapped, and GDB will use ordinary memory operations. One can therefore write
target xml that will enable register browsing with unmodified remote side - in fact
I've used pristine OpenOCD to test this patch.

This was written back in 2006 by Jim Blandy, Daniel Jacobowitz and myself, and was used in
a product for that time, so somewhat time-tested. What I post is functional, but it does
not yet add MI integration and in particular support for read-sensitive registers - those
where reading has side effects, for which frontend should take special care. These bits
are somewhat tricky, so before posting a complete patch I'd appreciate feedback on the
current state. Any big design objections, or any other guidance?

Thanks,
Yao Qi - April 24, 2015, 9:47 a.m.
Vladimir Prus <vladimir@codesourcery.com> writes:

Hi Vladimir,

> The attached patches implement accessing peripheral registers on
> bare-metal targets. Typically,
> these registers are memory-mapped, so one can poke at them using
> memory operations, but it's
> far from convenient. Also, on some targets the registers might require
> a custom way of access,
> which makes things even less convenient.
>
> This patch allows target XML to describe 'spaces' - contains of
> registers, which can be further grouped.
> Given that descrpiption, GDB allows one to do something like:
>
> 	(gdb) print $io.GPIO_PORTF.GPIO_PORTFDIR
> 	$7 = -1
>
> to access registers. One can also do
>
> 	(gdb) ptype $io
>
> to see top-level register groups in space 'io', and so find the
> desired register.

What does the xml using 'spaces' look like?  A small example would be
useful.  Target description "reg" has already had a component "type",
can't we extend "type" for memory-mapped registers?  I am trying to
understand how useful it is to add 'spaces' here.
Vladimir Prus - April 27, 2015, 6:25 p.m.
On 04/24/2015 12:47 PM, Yao Qi wrote:>

Vladimir Prus <vladimir@codesourcery.com> writes:
 >
 > Hi Vladimir,
 >
 >> The attached patches implement accessing peripheral registers on
 >> bare-metal targets. Typically,
 >> these registers are memory-mapped, so one can poke at them using
 >> memory operations, but it's
 >> far from convenient. Also, on some targets the registers might require
 >> a custom way of access,
 >> which makes things even less convenient.
 >>
 >> This patch allows target XML to describe 'spaces' - contains of
 >> registers, which can be further grouped.
 >> Given that descrpiption, GDB allows one to do something like:
 >>
 >> 	(gdb) print $io.GPIO_PORTF.GPIO_PORTFDIR
 >> 	$7 = -1
 >>
 >> to access registers. One can also do
 >>
 >> 	(gdb) ptype $io
 >>
 >> to see top-level register groups in space 'io', and so find the
 >> desired register.
 >
 > What does the xml using 'spaces' look like?  A small example would be
 > useful.  Target description "reg" has already had a component "type",
 > can't we extend "type" for memory-mapped registers?  I am trying to
 > understand how useful it is to add 'spaces' here.

Hi Yao,

Here's extract from an actual file

	<space annex="memory" name="io">
	<group name="UART2">
	<reg bitsize="32" name="UART2CTL" offset="0x4000e030" read-sensitive="no" save-restore="yes" type="UART0_UART0CTL"/>
	<reg bitsize="32" name="UART2LTIM" offset="0x4000e098" read-sensitive="no" save-restore="yes" type="UART0_UART0LTIM"/>
	</group>
	<group name="UART3">
	size="32" name="UART3CTL" offset="0x4000f030" read-sensitive="no" save-restore="yes" type="UART0_UART0CTL"/>
	<reg bitsize="32" name="UART3LTIM" offset="0x4000f098" read-sensitive="no" save-restore="yes" type="UART0_UART0LTIM"/>
	</group>

The full file can be found at https://drive.google.com/open?id=0BzXbGnw_jIF6cThVZHU2T3l0SXM&authuser=0


The space and group elements are not 100% required - it might be possible to instead have something like:

	<struct id="UART2">
		<field offset="0x4000e030" bitsize="32" type="int" name="UAR2CTL"/>
	</struct>
	<struct id="UART3">
		<field offset="0x4000f030" bitsize="32" type="int" name="UART3CTL"/>
	</struct>
	<struct id="io">
		<field name="UART2" type="UART2"/>
		<field name="UART3" type="UART3"/>
	</struct>
	<reg name="io" type="io"/>

But it seems to me that this overloading of existing concepts might not be perfect:

- Normally, top level 'reg' element are accessed using ordinary packets, so we'd need to special case 'reg' above
- Using 'reg' element to refer to large number of registers, and using 'field' to refer to registers can be
   confusing.
- GDB type system does not support 'offset' field for physical address, and unlike space, there is no easy
   place to add this information.

So it seems to me that the original syntax is more straight-forward representation of hardware.

Thanks,
Volodya
Vladimir Prus - June 1, 2015, 6:36 p.m.
Yao,

did you have any further comments?

Thanks,

On 4/27/2015 9:25 PM, Vladimir Prus wrote:
>
> On 04/24/2015 12:47 PM, Yao Qi wrote:>
>
> Vladimir Prus <vladimir@codesourcery.com> writes:
>  >
>  > Hi Vladimir,
>  >
>  >> The attached patches implement accessing peripheral registers on
>  >> bare-metal targets. Typically,
>  >> these registers are memory-mapped, so one can poke at them using
>  >> memory operations, but it's
>  >> far from convenient. Also, on some targets the registers might require
>  >> a custom way of access,
>  >> which makes things even less convenient.
>  >>
>  >> This patch allows target XML to describe 'spaces' - contains of
>  >> registers, which can be further grouped.
>  >> Given that descrpiption, GDB allows one to do something like:
>  >>
>  >>     (gdb) print $io.GPIO_PORTF.GPIO_PORTFDIR
>  >>     $7 = -1
>  >>
>  >> to access registers. One can also do
>  >>
>  >>     (gdb) ptype $io
>  >>
>  >> to see top-level register groups in space 'io', and so find the
>  >> desired register.
>  >
>  > What does the xml using 'spaces' look like?  A small example would be
>  > useful.  Target description "reg" has already had a component "type",
>  > can't we extend "type" for memory-mapped registers?  I am trying to
>  > understand how useful it is to add 'spaces' here.
>
> Hi Yao,
>
> Here's extract from an actual file
>
>      <space annex="memory" name="io">
>      <group name="UART2">
>      <reg bitsize="32" name="UART2CTL" offset="0x4000e030"
> read-sensitive="no" save-restore="yes" type="UART0_UART0CTL"/>
>      <reg bitsize="32" name="UART2LTIM" offset="0x4000e098"
> read-sensitive="no" save-restore="yes" type="UART0_UART0LTIM"/>
>      </group>
>      <group name="UART3">
>      size="32" name="UART3CTL" offset="0x4000f030" read-sensitive="no"
> save-restore="yes" type="UART0_UART0CTL"/>
>      <reg bitsize="32" name="UART3LTIM" offset="0x4000f098"
> read-sensitive="no" save-restore="yes" type="UART0_UART0LTIM"/>
>      </group>
>
> The full file can be found at
> https://drive.google.com/open?id=0BzXbGnw_jIF6cThVZHU2T3l0SXM&authuser=0
>
>
> The space and group elements are not 100% required - it might be
> possible to instead have something like:
>
>      <struct id="UART2">
>          <field offset="0x4000e030" bitsize="32" type="int"
> name="UAR2CTL"/>
>      </struct>
>      <struct id="UART3">
>          <field offset="0x4000f030" bitsize="32" type="int"
> name="UART3CTL"/>
>      </struct>
>      <struct id="io">
>          <field name="UART2" type="UART2"/>
>          <field name="UART3" type="UART3"/>
>      </struct>
>      <reg name="io" type="io"/>
>
> But it seems to me that this overloading of existing concepts might not
> be perfect:
>
> - Normally, top level 'reg' element are accessed using ordinary packets,
> so we'd need to special case 'reg' above
> - Using 'reg' element to refer to large number of registers, and using
> 'field' to refer to registers can be
>    confusing.
> - GDB type system does not support 'offset' field for physical address,
> and unlike space, there is no easy
>    place to add this information.
>
> So it seems to me that the original syntax is more straight-forward
> representation of hardware.
>
> Thanks,
> Volodya
>
Yao Qi - June 2, 2015, 1 p.m.
Vladimir Prus <vladimir.prus@gmail.com> writes:

> 	<space annex="memory" name="io">
> 	<group name="UART2">
> 	<reg bitsize="32" name="UART2CTL" offset="0x4000e030" read-sensitive="no" save-restore="yes" type="UART0_UART0CTL"/>
> 	<reg bitsize="32" name="UART2LTIM" offset="0x4000e098" read-sensitive="no" save-restore="yes" type="UART0_UART0LTIM"/>
> 	</group>
> 	<group name="UART3">
> 	size="32" name="UART3CTL" offset="0x4000f030" read-sensitive="no" save-restore="yes" type="UART0_UART0CTL"/>
> 	<reg bitsize="32" name="UART3LTIM" offset="0x4000f098" read-sensitive="no" save-restore="yes" type="UART0_UART0LTIM"/>
> 	</group>

It is good to have a "group" element, so that we can easily define a
group of registers.  However, what is the usefulness of element "space"?
to define a group of "group" elements?
Vladimir Prus - June 3, 2015, 7:48 p.m.
On 6/2/2015 4:00 PM, Yao Qi wrote:
> Vladimir Prus <vladimir.prus@gmail.com> writes:
>
>> 	<space annex="memory" name="io">
>> 	<group name="UART2">
>> 	<reg bitsize="32" name="UART2CTL" offset="0x4000e030" read-sensitive="no" save-restore="yes" type="UART0_UART0CTL"/>
>> 	<reg bitsize="32" name="UART2LTIM" offset="0x4000e098" read-sensitive="no" save-restore="yes" type="UART0_UART0LTIM"/>
>> 	</group>
>> 	<group name="UART3">
>> 	size="32" name="UART3CTL" offset="0x4000f030" read-sensitive="no" save-restore="yes" type="UART0_UART0CTL"/>
>> 	<reg bitsize="32" name="UART3LTIM" offset="0x4000f098" read-sensitive="no" save-restore="yes" type="UART0_UART0LTIM"/>
>> 	</group>
>
> It is good to have a "group" element, so that we can easily define a
> group of registers.  However, what is the usefulness of element "space"?
> to define a group of "group" elements?

The original motivation was to indicate that particular set of registers are not usual registers, and must
be accessed in a different way. It's not strictly necessary, we can rewrite the above as:

    <group name="io" target-object="spaces" annex="memory">
	<group name="UART2">
		<reg offset="0x4000e030" .../>
         </group>
    </group>

Maybe my patch can be modified to not introduce a new target object, after all. We can use:

    <group name="io" target-object="memory">
	<group name="UART2">
		<reg offset="0x4000e030" .../>
         </group>
    </group>

to describe memory-mapped io registers.

It sounds like this should be possible to implement. What do you think?

Two questions:

- Is 'group' ok, or 'register-group' would be more clear? We found that the size of these XML
files can be sometimes a problem

- I'd propose that 'target-object' and 'annex' attribute are only allowed for top-level 'group' element,
   and not for top-level 'reg' or nested 'group' element, to make implementation simpler. Is that fine?

Thanks,
Volodya
Yao Qi - June 4, 2015, 2:38 p.m.
Vladimir Prus <vladimir.prus@gmail.com> writes:

> Maybe my patch can be modified to not introduce a new target object, after all. We can use:
>
>    <group name="io" target-object="memory">
> 	<group name="UART2">
> 		<reg offset="0x4000e030" .../>
>         </group>
>    </group>
>
> to describe memory-mapped io registers.
>
> It sounds like this should be possible to implement. What do you
> think?

That is fine to me, but I am not sure the meaning of
target-object="memory" here?  Does this mean this group of registers are
mapped to memory?  Does "offset=0x4000e030" mean this register is mapped
at address 0x4000e030?  If the answers of both questions are yes, is
target-object="memory" still necessary?  Without it, we can still define
a group of memory-mapped registers like:

 <group name="io">
     <reg offset="0x4000e030" name="UART1_1">
     <reg offset="0x4000e034" name="UART1_2">
     <reg offset="0x4000e038" name="UART1_3">
 </group>

and we may even can define a group of normal registers and memory-mapped
registers, (even it is not likely in practise)

 <group name="io">
     <reg offset="0x4000e030" name="UART1_1">
     <reg offset="0x4000e034" name="UART1_2">
     <reg name="UART1_3">
 </group>

In this case, UART1_1 and UART1_2 are memory-mapped, while UART1_3 is
not.  IMO, memory-map-ness is an attribute of each register instead of a
group, so better to define such attribute on each register level.

>
> Two questions:
>
> - Is 'group' ok, or 'register-group' would be more clear? We found that the size of these XML
> files can be sometimes a problem

I don't have preference here.

>
> - I'd propose that 'target-object' and 'annex' attribute are only allowed for top-level 'group' element,
>   and not for top-level 'reg' or nested 'group' element, to make implementation simpler. Is that fine?

You have to explain the meaning of target-object and annex first and
what are the possible values of them.

My last concern is about the testing of these new things in target
description.  We need some test cases that people can run in their own
dev env, without involving setting up OpenOCD/JTAG probe/embedded boards.
Vladimir Prus - June 9, 2015, 8:49 p.m.
Yao,

On 6/4/2015 5:38 PM, Yao Qi wrote:
> Vladimir Prus <vladimir.prus@gmail.com> writes:
>
>> Maybe my patch can be modified to not introduce a new target object, after all. We can use:
>>
>>     <group name="io" target-object="memory">
>> 	<group name="UART2">
>> 		<reg offset="0x4000e030" .../>
>>          </group>
>>     </group>
>>
>> to describe memory-mapped io registers.
>>
>> It sounds like this should be possible to implement. What do you
>> think?
>
> That is fine to me, but I am not sure the meaning of
> target-object="memory" here?  Does this mean this group of registers are
> mapped to memory?

It's a bit more generic - it means that to obtain values of any register
in this group, GDB should perform qXfer of the specified target object
and annex, using register's offset. Memory is the most typical target object,
but in our case, we had other sorts of registers, so I'd prefer the design to
not lock us into memory-mapped registers.

> Does "offset=0x4000e030" mean this register is mapped
> at address 0x4000e030?  If the answers of both questions are yes, is
> target-object="memory" still necessary?  Without it, we can still define
> a group of memory-mapped registers like:
>
>   <group name="io">
>       <reg offset="0x4000e030" name="UART1_1">
>       <reg offset="0x4000e034" name="UART1_2">
>       <reg offset="0x4000e038" name="UART1_3">
>   </group>
>
> and we may even can define a group of normal registers and memory-mapped
> registers, (even it is not likely in practise)
>
>   <group name="io">
>       <reg offset="0x4000e030" name="UART1_1">
>       <reg offset="0x4000e034" name="UART1_2">
>       <reg name="UART1_3">
>   </group>
>
> In this case, UART1_1 and UART1_2 are memory-mapped, while UART1_3 is
> not.  IMO, memory-map-ness is an attribute of each register instead of a
> group, so better to define such attribute on each register level.

It is possible in theory, but I think it has two drawbacks.

First, I think specifying target object is more explicit (and therefore better
than implicit, especially for machine-oriented format) and more generic, as it allows
us to use other target objects.

Second, implementing such mixed registers group is extra complexity, and we did
not find any need for that in practice.

May I suggest we start from a simple use case, where an alternative access mechanism
can only be specified for a top-level group, and it's explicitly specified by
target-object attribute? Should the need arise for mixing differently-accessed
registers inside one group, that can be implemented later with extra code.

>> Two questions:
>>
>> - Is 'group' ok, or 'register-group' would be more clear? We found that the size of these XML
>> files can be sometimes a problem
>
> I don't have preference here.
>
>>
>> - I'd propose that 'target-object' and 'annex' attribute are only allowed for top-level 'group' element,
>>    and not for top-level 'reg' or nested 'group' element, to make implementation simpler. Is that fine?
>
> You have to explain the meaning of target-object and annex first and
> what are the possible values of them.
>
> My last concern is about the testing of these new things in target
> description.  We need some test cases that people can run in their own
> dev env, without involving setting up OpenOCD/JTAG probe/embedded boards.

That's a valid concern. I was using a particular board with USB interface, so did not need a probe,
but even that is not trivial to setup or automate.

If we were to test on a regular computer, I suppose the only way is to have some variables in a C program,
determine their addresses after debug session start, and generate target XML dynamically from that?

- Volodya
Yao Qi - June 11, 2015, 8:56 a.m.
On 09/06/15 21:49, Vladimir Prus wrote:
> It's a bit more generic - it means that to obtain values of any register
> in this group, GDB should perform qXfer of the specified target object
> and annex, using register's offset. Memory is the most typical target
> object,
> but in our case, we had other sorts of registers, so I'd prefer the
> design to
> not lock us into memory-mapped registers.
>

I don't object to it.

>> Does "offset=0x4000e030" mean this register is mapped
>> at address 0x4000e030?  If the answers of both questions are yes, is
>> target-object="memory" still necessary?  Without it, we can still define
>> a group of memory-mapped registers like:
>>
>>   <group name="io">
>>       <reg offset="0x4000e030" name="UART1_1">
>>       <reg offset="0x4000e034" name="UART1_2">
>>       <reg offset="0x4000e038" name="UART1_3">
>>   </group>
>>
>> and we may even can define a group of normal registers and memory-mapped
>> registers, (even it is not likely in practise)
>>
>>   <group name="io">
>>       <reg offset="0x4000e030" name="UART1_1">
>>       <reg offset="0x4000e034" name="UART1_2">
>>       <reg name="UART1_3">
>>   </group>
>>
>> In this case, UART1_1 and UART1_2 are memory-mapped, while UART1_3 is
>> not.  IMO, memory-map-ness is an attribute of each register instead of a
>> group, so better to define such attribute on each register level.
>
> It is possible in theory, but I think it has two drawbacks.
>
> First, I think specifying target object is more explicit (and therefore
> better
> than implicit, especially for machine-oriented format) and more generic,
> as it allows
> us to use other target objects.
>
> Second, implementing such mixed registers group is extra complexity, and
> we did
> not find any need for that in practice.

If such mixed registers group brings extra complexity in the
implementation, then I am inclined to start from a simple one.

>
> May I suggest we start from a simple use case, where an alternative
> access mechanism
> can only be specified for a top-level group, and it's explicitly
> specified by
> target-object attribute? Should the need arise for mixing
> differently-accessed
> registers inside one group, that can be implemented later with extra code.
>
>>> Two questions:
>>>
>>> - Is 'group' ok, or 'register-group' would be more clear? We found
>>> that the size of these XML
>>> files can be sometimes a problem
>>
>> I don't have preference here.
>>
>>>
>>> - I'd propose that 'target-object' and 'annex' attribute are only
>>> allowed for top-level 'group' element,
>>>    and not for top-level 'reg' or nested 'group' element, to make
>>> implementation simpler. Is that fine?
>>
>> You have to explain the meaning of target-object and annex first and
>> what are the possible values of them.
>>
>> My last concern is about the testing of these new things in target
>> description.  We need some test cases that people can run in their own
>> dev env, without involving setting up OpenOCD/JTAG probe/embedded boards.
>
> That's a valid concern. I was using a particular board with USB
> interface, so did not need a probe,
> but even that is not trivial to setup or automate.
>
> If we were to test on a regular computer, I suppose the only way is to
> have some variables in a C program,
> determine their addresses after debug session start, and generate target
> XML dynamically from that?

Yes, that is what I want to suggest too.
Vladimir Prus - June 15, 2015, 1:51 p.m.
On 6/11/2015 11:56 AM, Yao Qi wrote:
> On 09/06/15 21:49, Vladimir Prus wrote:
>> It's a bit more generic - it means that to obtain values of any register
>> in this group, GDB should perform qXfer of the specified target object
>> and annex, using register's offset. Memory is the most typical target
>> object,
>> but in our case, we had other sorts of registers, so I'd prefer the
>> design to
>> not lock us into memory-mapped registers.
>>
>
> I don't object to it.
>
>>> Does "offset=0x4000e030" mean this register is mapped
>>> at address 0x4000e030?  If the answers of both questions are yes, is
>>> target-object="memory" still necessary?  Without it, we can still define
>>> a group of memory-mapped registers like:
>>>
>>>   <group name="io">
>>>       <reg offset="0x4000e030" name="UART1_1">
>>>       <reg offset="0x4000e034" name="UART1_2">
>>>       <reg offset="0x4000e038" name="UART1_3">
>>>   </group>
>>>
>>> and we may even can define a group of normal registers and memory-mapped
>>> registers, (even it is not likely in practise)
>>>
>>>   <group name="io">
>>>       <reg offset="0x4000e030" name="UART1_1">
>>>       <reg offset="0x4000e034" name="UART1_2">
>>>       <reg name="UART1_3">
>>>   </group>
>>>
>>> In this case, UART1_1 and UART1_2 are memory-mapped, while UART1_3 is
>>> not.  IMO, memory-map-ness is an attribute of each register instead of a
>>> group, so better to define such attribute on each register level.
>>
>> It is possible in theory, but I think it has two drawbacks.
>>
>> First, I think specifying target object is more explicit (and therefore
>> better
>> than implicit, especially for machine-oriented format) and more generic,
>> as it allows
>> us to use other target objects.
>>
>> Second, implementing such mixed registers group is extra complexity, and
>> we did
>> not find any need for that in practice.
>
> If such mixed registers group brings extra complexity in the
> implementation, then I am inclined to start from a simple one.

Yes, I think it brings extra complexity.

Thanks for your comments; I plan to update the patch to:

- Use top-level group for register browsing, as opposed to <space> element
- Do some form of testing that does not require hardware.

Thanks,
Volodya

Patch

From 94f8524b20b514ed91e6f20bc9cc2ce83a186ad9 Mon Sep 17 00:00:00 2001
From: Vladimir Prus <vladimir@codesourcery.com>
Date: Tue, 14 Apr 2015 21:56:36 +0300
Subject: [PATCH 2/2] Initial register browsing patches.

---
 gdb/remote.c              |  21 +++
 gdb/target-descriptions.c | 328 +++++++++++++++++++++++++++++++++++++++++++++-
 gdb/target-descriptions.h |  19 ++-
 gdb/target.h              |   5 +
 gdb/xml-tdesc.c           | 188 +++++++++++++++++++++++++-
 5 files changed, 552 insertions(+), 9 deletions(-)

diff --git a/gdb/remote.c b/gdb/remote.c
index dcd24c4..b18d428 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1255,6 +1255,7 @@  enum {
   PACKET_qXfer_libraries,
   PACKET_qXfer_libraries_svr4,
   PACKET_qXfer_memory_map,
+  PACKET_qXfer_spaces,
   PACKET_qXfer_spu_read,
   PACKET_qXfer_spu_write,
   PACKET_qXfer_osdata,
@@ -8945,6 +8946,20 @@  remote_xfer_partial (struct target_ops *ops, enum target_object object,
 	return TARGET_XFER_E_IO;
     }
 
+  if (object == TARGET_OBJECT_SPACES)
+    {
+      if (readbuf)
+	return remote_read_qxfer (ops, "spaces", annex, readbuf,
+				  offset, len, xfered_len,
+				  &remote_protocol_packets
+				  [PACKET_qXfer_spaces]);
+      else
+	return remote_write_qxfer (ops, "spaces", annex, writebuf,
+				   offset, len, xfered_len,
+				   &remote_protocol_packets
+				   [PACKET_qXfer_spaces]);
+    }
+
   /* Only handle flash writes.  */
   if (writebuf != NULL)
     {
@@ -12231,6 +12246,12 @@  Show the maximum size of the address (in bits) in a memory packet."), NULL,
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_memory_map],
 			 "qXfer:memory-map:read", "memory-map", 0);
 
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_spaces],
+			 "qXfer:spaces:read", "read-spaces", 0);
+
+  add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_spaces],
+			 "qXfer:spaces:write", "write-spaces", 0);
+
   add_packet_config_cmd (&remote_protocol_packets[PACKET_qXfer_spu_read],
                          "qXfer:spu:read", "read-spu-object", 0);
 
diff --git a/gdb/target-descriptions.c b/gdb/target-descriptions.c
index 0eec6be..b65ef42 100644
--- a/gdb/target-descriptions.c
+++ b/gdb/target-descriptions.c
@@ -23,6 +23,7 @@ 
 #include "arch-utils.h"
 #include "gdbcmd.h"
 #include "gdbtypes.h"
+#include "observer.h"
 #include "reggroups.h"
 #include "target.h"
 #include "target-descriptions.h"
@@ -34,6 +35,7 @@ 
 #include "gdb_obstack.h"
 #include "hashtab.h"
 #include "inferior.h"
+#include "splay-tree.h"
 
 /* Types.  */
 
@@ -84,12 +86,31 @@  typedef struct tdesc_reg
 } *tdesc_reg_p;
 DEF_VEC_P(tdesc_reg_p);
 
+typedef struct tdesc_space
+{
+  /* The name of the GDB variable ('$foo') representing this space.  */
+  char *name;
+
+  /* The annex we do qXfer requests on to read and write its members.  */
+  char *annex;
+
+  /* The struct type we built to describe this space's layout.  */
+  struct tdesc_type *tdesc_type;
+
+  /* The mapping from local structure offsets to remote register
+     addresses.  */
+  void *offset_map;
+} *tdesc_space_p;
+DEF_VEC_P(tdesc_space_p);
+
+
 /* A named type from a target description.  */
 
 typedef struct tdesc_type_field
 {
   char *name;
   struct tdesc_type *type;
+  int read_sensitive;
   int start, end;
 } tdesc_type_field;
 DEF_VEC_O(tdesc_type_field);
@@ -177,6 +198,9 @@  typedef struct tdesc_feature
 
   /* The types associated with this feature.  */
   VEC(tdesc_type_p) *types;
+
+  /* The spaces associated with this feature.  */
+  VEC(tdesc_space_p) *spaces;
 } *tdesc_feature_p;
 DEF_VEC_P(tdesc_feature_p);
 
@@ -257,6 +281,10 @@  struct target_desc_info
   char *filename;
 };
 
+/* Prototypes.  */
+
+static void tdesc_free_space (struct tdesc_space *space);
+
 /* Get the inferior INF's target description info, allocating one on
    the stop if necessary.  */
 
@@ -1316,6 +1344,13 @@  tdesc_create_vector (struct tdesc_feature *feature, const char *name,
   return type;
 }
 
+char *
+tdesc_struct_name (struct tdesc_type *type)
+{
+  gdb_assert (type->kind == TDESC_TYPE_STRUCT);
+  return type->name;
+}
+
 struct tdesc_type *
 tdesc_create_struct (struct tdesc_feature *feature, const char *name)
 {
@@ -1369,7 +1404,7 @@  tdesc_create_flags (struct tdesc_feature *feature, const char *name,
    is only valid until the next call to tdesc_add_field (the vector
    might be reallocated).  */
 
-void
+struct tdesc_type_field *
 tdesc_add_field (struct tdesc_type *type, const char *field_name,
 		 struct tdesc_type *field_type)
 {
@@ -1382,6 +1417,15 @@  tdesc_add_field (struct tdesc_type *type, const char *field_name,
   f.type = field_type;
 
   VEC_safe_push (tdesc_type_field, type->u.u.fields, &f);
+  return VEC_last (tdesc_type_field, type->u.u.fields);
+}
+
+/* Mark FIELD as read-sensitive.  */
+
+void
+tdesc_make_field_sensitive (struct tdesc_type_field *field)
+{
+  field->read_sensitive = 1;
 }
 
 /* Add a new bitfield.  */
@@ -1419,6 +1463,7 @@  static void
 tdesc_free_feature (struct tdesc_feature *feature)
 {
   struct tdesc_reg *reg;
+  struct tdesc_space *space;
   struct tdesc_type *type;
   int ix;
 
@@ -1426,6 +1471,10 @@  tdesc_free_feature (struct tdesc_feature *feature)
     tdesc_free_reg (reg);
   VEC_free (tdesc_reg_p, feature->registers);
 
+  for (ix = 0; VEC_iterate (tdesc_space_p, feature->spaces, ix, space); ix++)
+    tdesc_free_space (space);
+  VEC_free (tdesc_space_p, feature->spaces);
+
   for (ix = 0; VEC_iterate (tdesc_type_p, feature->types, ix, type); ix++)
     tdesc_free_type (type);
   VEC_free (tdesc_type_p, feature->types);
@@ -1543,6 +1592,281 @@  set_tdesc_osabi (struct target_desc *target_desc, enum gdb_osabi osabi)
 }
 
 
+/* Register space support.  */
+
+static void
+tdesc_free_space (struct tdesc_space *space)
+{
+  xfree (space->name);
+  xfree (space->annex);
+  if (space->offset_map != NULL)
+    splay_tree_delete (space->offset_map);
+  /* As for target-defined types, we can not free the space's
+     type.  */
+
+  xfree (space);
+}
+
+struct tdesc_space *
+tdesc_create_space (struct tdesc_feature *feature, const char *name,
+		    const char *annex, struct tdesc_type *tdesc_type)
+{
+  struct tdesc_space *space = XCNEW (struct tdesc_space);
+
+  space->name = xstrdup (name);
+  space->annex = xstrdup (annex);
+  space->tdesc_type = tdesc_type;
+
+  VEC_safe_push (tdesc_space_p, feature->spaces, space);
+
+  return space;
+}
+
+/* Record that the bits at offset FROM in SPACE, in the corresponding
+   GDB type, should be fetched from offset TO on the target.  */
+
+void
+space_record_offset_mapping (struct tdesc_space *space,
+			     ULONGEST from, ULONGEST to)
+{
+  splay_tree_node node;
+
+  if (space->offset_map == NULL)
+    space->offset_map = splay_tree_new (splay_tree_compare_ints, NULL, NULL);
+
+  node = splay_tree_lookup (space->offset_map, from);
+  if (node != NULL)
+    internal_error (__FILE__, __LINE__,
+		    "Duplicated entries for 0x%lx in space offset mapping: "
+		    "0x%lx and 0x%lx",
+		    (long) from, (long) node->value, (long) to);
+
+  node = splay_tree_successor (space->offset_map, from);
+  if (node != NULL)
+    internal_error (__FILE__, __LINE__,
+		    "Tried to add 0x%lx to space offset mapping after 0x%lx",
+		    (long) from, (long) node->key);
+
+  /* If two consecutive entries have the same relative offset, then we
+     don't need to record the second entry.  This optimization lets us
+     coalesce reads from the target.  */
+  node = splay_tree_predecessor (space->offset_map, from);
+  if (node != NULL && from - node->key == to - node->value)
+    return;
+
+  splay_tree_insert (space->offset_map, from, to);
+}
+
+/* Return the target offset where we can find bits from offset FROM in
+   the GDB type constructed for SPACE.  Set *NEXT_OFFSET to the
+   starting GDB offset of the next portion of SPACE, or zero if this
+   is the last portion.  The caller may read up to *NEXT_OFFSET - FROM
+   bytes from the returned target offset.  */
+
+static ULONGEST
+space_map_offset (struct tdesc_space *space, ULONGEST from,
+		  ULONGEST *next_offset)
+{
+  ULONGEST result;
+  splay_tree_node node;
+
+  if (space->offset_map == NULL)
+    {
+      *next_offset = 0;
+      return from;
+    }
+
+  node = splay_tree_lookup (space->offset_map, from);
+  if (node == NULL)
+    node = splay_tree_predecessor (space->offset_map, from);
+
+  if (node == NULL)
+    internal_error (__FILE__, __LINE__,
+		    "No predecessor for %ld in space mapping",
+		    (long) from);
+
+  result = node->value + from - node->key;
+
+  node = splay_tree_successor (space->offset_map, from);
+  if (node)
+    *next_offset = node->key;
+  else
+    *next_offset = 0;
+
+  return result;
+}
+
+/* Space registers.
+
+   If an architecture has a feature that contains spaces, we create an
+   internal variable for each space, where reads and writes to that
+   register turn into target reads and writes of a given annex.
+
+   For this, the internal variable's value is an lval_computed value,
+   with the space as a closure.  */
+
+static void
+space_value_read (struct value *v)
+{
+  struct tdesc_space *space = value_computed_closure (v);
+  unsigned length = TYPE_LENGTH (value_type (v));
+  LONGEST transferred;
+  ULONGEST offset, target_offset;
+  gdb_byte *dest = value_contents_all_raw (v);
+
+  /* value_fetch_lazy handles reading bitfields.  */
+  gdb_assert (!value_bitsize (v));
+
+  offset = value_offset (v);
+  while (length > 0)
+    {
+      ULONGEST wanted, next_offset;
+
+      /* Map the current offset to a target offset.  */
+      target_offset = space_map_offset (space, offset, &next_offset);
+      if (next_offset > 0 && next_offset - offset < length)
+	wanted = next_offset - offset;
+      else
+	wanted = length;
+
+      transferred =
+	target_read (&current_target, TARGET_OBJECT_SPACES,
+		     space->annex, dest, target_offset, wanted);
+
+      if (transferred != wanted)
+	error ("Unable to read from register space '%s'", space->name);
+
+      length -= wanted;
+      offset += wanted;
+      dest += wanted;
+    }
+}
+
+static void
+space_value_write (struct value *v, struct value *fromval)
+{
+  struct tdesc_space *space = value_computed_closure (v);
+  unsigned real_length;
+  CORE_ADDR val_offset, offset, real_start;
+  LONGEST transferred;
+  ULONGEST next_offset;
+  gdb_byte buf[16];
+
+  val_offset = value_offset (v);
+  if (value_bitsize (v))
+    val_offset += value_offset (value_parent (v));
+  offset = space_map_offset (space, val_offset, &next_offset);
+
+  real_length = TYPE_LENGTH (value_type (v)) * 8;
+  real_start = offset;
+
+  /* For bitfields, adjust to always write a register sized and
+     aligned quantity by using the container type of the bitfield.  */
+  if (value_bitsize (v))
+    {
+      unsigned bit_length, adjust;
+      LONGEST fieldval;
+
+      bit_length = value_bitsize (v);
+
+      adjust = real_start % (real_length / 8);
+      if (adjust != 0)
+	{
+	  real_start = real_start - adjust;
+	  if (real_length < (adjust * 8) + bit_length + value_bitpos (v))
+	    error (_("Unable to read multi-word value from register space '%s'"),
+		   space->annex);
+	}
+      if (real_length > 8 * sizeof (buf))
+	error (_("Unable to read multi-word value from register space '%s'"),
+	       space->annex);
+
+      transferred =
+	target_read (&current_target, TARGET_OBJECT_SPACES,
+		      space->annex, buf, real_start, real_length / 8);
+
+      if (transferred != real_length / 8)
+	error (_("Unable to read from register space '%s'"),
+	       space->annex);
+
+      fieldval = value_as_long (fromval);
+      modify_field (value_type (v), buf, fieldval,
+		    value_bitpos (v) + 8 * adjust,
+		    bit_length);
+    }
+  else
+    memcpy (buf, value_contents_all_raw (fromval), real_length / 8);
+
+  transferred =
+    target_write (&current_target, TARGET_OBJECT_SPACES,
+                  space->annex, buf, real_start, real_length / 8);
+
+  if (transferred != real_length / 8)
+    error (_("Unable to write to register space '%s' (annex '%s')"),
+           space->name, space->annex);
+}
+
+static struct lval_funcs space_value_funcs =
+  {
+    .read = space_value_read,
+    .write = space_value_write,
+  };
+
+static struct value *
+make_space_value (struct gdbarch *gdbarch, struct tdesc_feature *feature,
+		  struct tdesc_space *space)
+{
+  struct type *type = tdesc_gdb_type (gdbarch, space->tdesc_type);
+
+  return allocate_computed_value (type, &space_value_funcs, space);
+}
+
+/* When the architecture changes, remove any space variables for the
+   old architecture, and add variables for the new architecture.  */
+static void
+tdesc_arch_changed_observer (struct gdbarch *new)
+{
+  const struct target_desc *new_tdesc = gdbarch_target_desc (new);
+  struct tdesc_feature *feature;
+  struct tdesc_space *space;
+  int ix, ixs;
+
+  /* FIXME lgustavo@mentor 2012-06-06: Re-use the existing observer for
+     "architecture changed" events.  Don't check if new differs compared
+     to the old one, just do whatever is needed everytime.  */
+
+  /* FIXME: at the moment, GDB don't have a function for deleting
+     internal variables.  We need to add one, and then clean up the
+     old architecture's variables here.  */
+  if (new_tdesc)
+    for (ix = 0;
+	 VEC_iterate (tdesc_feature_p, new_tdesc->features, ix, feature);
+	 ix++)
+      for (ixs = 0;
+	   VEC_iterate (tdesc_space_p, feature->spaces, ixs, space);
+	   ixs++)
+        set_internalvar_lazy (lookup_internalvar (space->name),
+                              make_space_value (new, feature, space));
+}
+
+void
+tdesc_push_space_names (const struct target_desc *tdesc,
+			VEC(char_p) **names)
+{
+  struct tdesc_feature *feature;
+  struct tdesc_space *space;
+  int ix, ixs;
+
+  for (ix = 0;
+       VEC_iterate (tdesc_feature_p, tdesc->features, ix, feature);
+       ix++)
+    for (ixs = 0;
+	 VEC_iterate (tdesc_space_p, feature->spaces, ixs, space);
+	 ixs++)
+      VEC_safe_push (char_p, *names, space->name);
+}
+
+
 static struct cmd_list_element *tdesc_set_cmdlist, *tdesc_show_cmdlist;
 static struct cmd_list_element *tdesc_unset_cmdlist;
 
@@ -1840,6 +2164,8 @@  _initialize_target_descriptions (void)
 {
   tdesc_data = gdbarch_data_register_pre_init (tdesc_data_init);
 
+  observer_attach_architecture_changed (tdesc_arch_changed_observer);
+
   add_prefix_cmd ("tdesc", class_maintenance, set_tdesc_cmd, _("\
 Set target description specific variables."),
 		  &tdesc_set_cmdlist, "set tdesc ",
diff --git a/gdb/target-descriptions.h b/gdb/target-descriptions.h
index 285debe..e0f91f4 100644
--- a/gdb/target-descriptions.h
+++ b/gdb/target-descriptions.h
@@ -25,7 +25,9 @@ 
 struct tdesc_feature;
 struct tdesc_arch_data;
 struct tdesc_type;
+struct tdesc_type_field;
 struct tdesc_reg;
+struct tdesc_space;
 struct target_desc;
 struct target_ops;
 struct target_desc;
@@ -34,6 +36,9 @@  struct target_desc;
 struct target_desc_info;
 struct inferior;
 
+typedef char *char_p;
+DEF_VEC_P (char_p);
+
 /* Fetch the current inferior's description, and switch its current
    architecture to one which incorporates that description.  */
 
@@ -227,6 +232,7 @@  struct tdesc_type *tdesc_create_vector (struct tdesc_feature *feature,
 					const char *name,
 					struct tdesc_type *field_type,
 					int count);
+char *tdesc_struct_name (struct tdesc_type *type);
 struct tdesc_type *tdesc_create_struct (struct tdesc_feature *feature,
 					const char *name);
 void tdesc_set_struct_size (struct tdesc_type *type, LONGEST size);
@@ -235,8 +241,10 @@  struct tdesc_type *tdesc_create_union (struct tdesc_feature *feature,
 struct tdesc_type *tdesc_create_flags (struct tdesc_feature *feature,
 				       const char *name,
 				       LONGEST size);
-void tdesc_add_field (struct tdesc_type *type, const char *field_name,
-		      struct tdesc_type *field_type);
+struct tdesc_type_field *tdesc_add_field (struct tdesc_type *type, 
+                                          const char *field_name,
+                                          struct tdesc_type *field_type);
+void tdesc_make_field_sensitive (struct tdesc_type_field *field);
 void tdesc_add_bitfield (struct tdesc_type *type, const char *field_name,
 			 int start, int end);
 void tdesc_add_flag (struct tdesc_type *type, int start,
@@ -244,5 +252,12 @@  void tdesc_add_flag (struct tdesc_type *type, int start,
 void tdesc_create_reg (struct tdesc_feature *feature, const char *name,
 		       int regnum, int save_restore, const char *group,
 		       int bitsize, const char *type);
+struct tdesc_space *tdesc_create_space (struct tdesc_feature *feature,
+					const char *name, const char *annex,
+					struct tdesc_type *type);
+void space_record_offset_mapping (struct tdesc_space *,
+                                  ULONGEST from, ULONGEST to);
+void tdesc_push_space_names (const struct target_desc *tdesc,
+			     VEC(char_p) **names);
 
 #endif /* TARGET_DESCRIPTIONS_H */
diff --git a/gdb/target.h b/gdb/target.h
index f57e431..bd783f7 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -167,6 +167,11 @@  enum target_object
   /* Available target-specific features, e.g. registers and coprocessors.
      See "target-descriptions.c".  ANNEX should never be empty.  */
   TARGET_OBJECT_AVAILABLE_FEATURES,
+  /* Additional memory "spaces" defined by the target.
+     A target might have several completely independent 'spaces',
+     for example of memory mapped registers.  Each space
+     is identified by annex within this object.  */
+  TARGET_OBJECT_SPACES,
   /* Currently loaded libraries, in XML format.  */
   TARGET_OBJECT_LIBRARIES,
   /* Currently loaded libraries specific for SVR4 systems, in XML format.  */
diff --git a/gdb/xml-tdesc.c b/gdb/xml-tdesc.c
index 65744ff..11b1d94 100644
--- a/gdb/xml-tdesc.c
+++ b/gdb/xml-tdesc.c
@@ -69,6 +69,9 @@  DEF_VEC_O(tdesc_xml_cache_s);
 
 static VEC(tdesc_xml_cache_s) *xml_cache;
 
+typedef struct tdesc_type *tdesc_type_p;
+DEF_VEC_P(tdesc_type_p);
+
 /* Callback data for target description parsing.  */
 
 struct tdesc_parsing_data
@@ -92,6 +95,20 @@  struct tdesc_parsing_data
 
   /* Whether the current type is a flags type.  */
   int current_type_is_flags;
+
+  /* The space we are currently parsing, or NULL if we are not
+     parsing a space.  */
+  struct tdesc_space *current_space;
+
+  /* While parsing a space, the structure offset and default target
+     offset for the next register.  */
+  int next_gdb_offset;
+  int next_tgt_offset;
+
+  /* While parsing a space, the types for the current space and
+     any open groups.  New registers are added to the last element
+     of this.  */
+  VEC(tdesc_type_p) *space_types;
 };
 
 /* Handle the end of an <architecture> element and its value.  */
@@ -183,7 +200,8 @@  tdesc_start_reg (struct gdb_xml_parser *parser,
   struct gdb_xml_value *attrs = VEC_address (gdb_xml_value_s, attributes);
   int ix = 0, length;
   char *name, *group, *type;
-  int bitsize, regnum, save_restore;
+  int bitsize, regnum, save_restore, offset, read_sensitive;
+  struct tdesc_type *reg_type;
 
   length = VEC_length (gdb_xml_value_s, attributes);
 
@@ -210,18 +228,152 @@  tdesc_start_reg (struct gdb_xml_parser *parser,
   else
     save_restore = 1;
 
-  if (strcmp (type, "int") != 0
-      && strcmp (type, "float") != 0
-      && tdesc_named_type (data->current_feature, type) == NULL)
+  if (ix < length && strcmp (attrs[ix].name, "offset") == 0)
+    offset = * (ULONGEST *) attrs[ix++].value;
+  else
+    offset = -1;
+
+  if (ix < length && strcmp (attrs[ix].name, "read-sensitive") == 0)
+    read_sensitive = * (ULONGEST *) attrs[ix++].value;
+  else
+    read_sensitive = 0;
+
+  reg_type = tdesc_named_type (data->current_feature, type);
+  if (reg_type == NULL
+      && strcmp (type, "float") != 0)
     gdb_xml_error (parser, _("Register \"%s\" has unknown type \"%s\""),
 		   name, type);
 
-  tdesc_create_reg (data->current_feature, name, regnum, save_restore, group,
+  if (data->current_space == NULL)
+    {
+      if (read_sensitive)
+	gdb_xml_error (parser,
+		       _("Read-sensitive register \"%s\" not supported"),
+		       name);
+
+      tdesc_create_reg (data->current_feature, name, regnum, save_restore, group,
 		    bitsize, type);
 
-  data->next_regnum = regnum + 1;
+      data->next_regnum = regnum + 1;
+    }
+  else
+    {
+      struct tdesc_type *container = VEC_last (tdesc_type_p, data->space_types);
+      struct tdesc_type_field *field;
+
+      if (reg_type == NULL)
+	{
+	  if (strcmp (type, "int") == 0)
+	    {
+	      if (bitsize == 8)
+		reg_type = tdesc_named_type (data->current_feature, "int8");
+	      else if (bitsize == 16)
+		reg_type = tdesc_named_type (data->current_feature, "int16");
+	      else if (bitsize == 32)
+		reg_type = tdesc_named_type (data->current_feature, "int32");
+	      else if (bitsize == 64)
+		reg_type = tdesc_named_type (data->current_feature, "int64");
+	      else
+		gdb_xml_error (parser, _("Unrecognized size for register "
+					 "'%s': %d bits"),
+			       name, bitsize);
+	    }
+	  else
+	    gdb_xml_error (parser, _("Unsupported type for register "
+					 "'%s': %s"),
+			   name, type);
+	}
+
+      /* Create the field.  */
+      field = tdesc_add_field (container, name, reg_type);
+
+      /* If this register is read-sensitive, set the appropriate bit
+	 on its field.  */
+      if (read_sensitive)
+	tdesc_make_field_sensitive (field);
+
+      /* Set its target offset to the one given in the reg element, or
+	 place it after the preceding member.  */
+      if (offset != -1)
+	data->next_tgt_offset = offset;
+
+      space_record_offset_mapping (data->current_space, data->next_gdb_offset,
+				   data->next_tgt_offset);
+
+      data->next_gdb_offset += bitsize / TARGET_CHAR_BIT;
+      data->next_tgt_offset += bitsize / TARGET_CHAR_BIT;
+    }
+}
+
+/* Handle the start of a <space> element.  Initialize the type and
+   record it with the current feature.  */
+
+static void
+tdesc_start_space (struct gdb_xml_parser *parser,
+		   const struct gdb_xml_element *element,
+		   void *user_data, VEC(gdb_xml_value_s) *attributes)
+{
+  struct tdesc_parsing_data *data = user_data;
+  char *name = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+  char *annex = VEC_index (gdb_xml_value_s, attributes, 1)->value;
+  struct tdesc_space *space;
+  struct tdesc_type *type;
+
+  type = tdesc_create_struct (data->current_feature, name);
+  space = tdesc_create_space (data->current_feature, name, annex, type);
+
+  data->current_space = space;
+  data->next_gdb_offset = 0;
+  data->next_tgt_offset = 0;
+  gdb_assert (data->space_types == NULL);
+  VEC_safe_push (tdesc_type_p, data->space_types, type);
+}
+
+/* Handle the end of a <space> element.  */
+
+static void
+tdesc_end_space (struct gdb_xml_parser *parser,
+		 const struct gdb_xml_element *element,
+		 void *user_data, const char *body_text)
+{
+  struct tdesc_parsing_data *data = user_data;
+
+  data->current_space = NULL;
+  gdb_assert (VEC_length (tdesc_type_p, data->space_types) == 1);
+  VEC_free (tdesc_type_p, data->space_types);
+}
+
+/* Handle the start of a <group> element.  Initialize the new type and
+   record it.  */
+
+static void
+tdesc_start_group (struct gdb_xml_parser *parser,
+		   const struct gdb_xml_element *element,
+		   void *user_data, VEC(gdb_xml_value_s) *attributes)
+{
+  struct tdesc_parsing_data *data = user_data;
+  char *name = VEC_index (gdb_xml_value_s, attributes, 0)->value;
+  struct tdesc_type *type;
+
+  type = tdesc_create_struct (data->current_feature, name);
+  VEC_safe_push (tdesc_type_p, data->space_types, type);
+}
+
+/* Handle the end of a <group> element.  */
+
+static void
+tdesc_end_group (struct gdb_xml_parser *parser,
+		 const struct gdb_xml_element *element,
+		 void *user_data, const char *body_text)
+{
+  struct tdesc_parsing_data *data = user_data;
+  struct tdesc_type *type = VEC_pop (tdesc_type_p, data->space_types);
+  struct tdesc_type *prev_type = VEC_last (tdesc_type_p, data->space_types);
+
+  tdesc_add_field (prev_type, tdesc_struct_name (type), type);
 }
 
+
 /* Handle the start of a <union> element.  Initialize the type and
    record it with the current feature.  */
 
@@ -430,6 +582,27 @@  static const struct gdb_xml_attribute reg_attributes[] = {
   { NULL, GDB_XML_AF_NONE, NULL, NULL }
 };
 
+static const struct gdb_xml_attribute space_attributes[] = {
+  { "name", GDB_XML_AF_NONE, NULL, NULL },
+  { "annex", GDB_XML_AF_NONE, NULL, NULL },
+  { NULL, GDB_XML_AF_NONE, NULL, NULL }
+};
+
+static const struct gdb_xml_attribute group_attributes[] = {
+  { "name", GDB_XML_AF_NONE, NULL, NULL },
+  { NULL, GDB_XML_AF_NONE, NULL, NULL }
+};
+
+static const struct gdb_xml_element space_group_children[] = {
+  { "reg", reg_attributes, NULL,
+    GDB_XML_EF_OPTIONAL | GDB_XML_EF_REPEATABLE,
+    tdesc_start_reg, NULL },
+  { "group", group_attributes, space_group_children,
+    GDB_XML_EF_OPTIONAL | GDB_XML_EF_REPEATABLE,
+    tdesc_start_group, tdesc_end_group },
+  { NULL, NULL, NULL, GDB_XML_EF_NONE, NULL, NULL }
+};
+
 static const struct gdb_xml_attribute struct_union_attributes[] = {
   { "id", GDB_XML_AF_NONE, NULL, NULL },
   { "size", GDB_XML_AF_OPTIONAL, gdb_xml_parse_attr_ulongest, NULL},
@@ -458,6 +631,9 @@  static const struct gdb_xml_element feature_children[] = {
   { "reg", reg_attributes, NULL,
     GDB_XML_EF_OPTIONAL | GDB_XML_EF_REPEATABLE,
     tdesc_start_reg, NULL },
+  { "space", space_attributes, space_group_children,
+    GDB_XML_EF_OPTIONAL | GDB_XML_EF_REPEATABLE,
+    tdesc_start_space, tdesc_end_space },
   { "struct", struct_union_attributes, struct_union_children,
     GDB_XML_EF_OPTIONAL | GDB_XML_EF_REPEATABLE,
     tdesc_start_struct, NULL },
-- 
1.9.1