| Message ID | 0250cbb0a6b9d18e12369720a47e96772347829c.1457470980.git.alistair.francis@xilinx.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Alistair Francis <alistair.francis@xilinx.com> writes: > Add memory io handlers that glue the register API to the memory API. > Just translation functions at this stage. Although it does allow for > devices to be created without all-in-one mmio r/w handlers. > > This patch also adds the RegisterInfoArray struct, which allows all of > the individual RegisterInfo structs to be grouped into a single memory > region. > > Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> > Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> > --- > V5: > - Convert to using only one memory region > > hw/core/register.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ > include/hw/register.h | 51 +++++++++++++++++++++++++++++++++++++ > 2 files changed, 121 insertions(+) > > diff --git a/hw/core/register.c b/hw/core/register.c > index 1656f71..e1cd0c4 100644 > --- a/hw/core/register.c > +++ b/hw/core/register.c > @@ -146,3 +146,73 @@ void register_reset(RegisterInfo *reg) > > register_write_val(reg, reg->access->reset); > } > + > +static inline void register_write_memory(void *opaque, hwaddr addr, > + uint64_t value, unsigned size, bool be) > +{ > + RegisterInfoArray *reg_array = opaque; > + RegisterInfo *reg = NULL; > + uint64_t we = ~0; > + int i, shift = 0; > + > + for (i = 0; i < reg_array->num_elements; i++) { > + if (reg_array->r[i]->access->decode.addr == addr) { > + reg = reg_array->r[i]; > + break; > + } > + } > + assert(reg); > + > + /* Generate appropriate write enable mask and shift values */ > + if (reg->data_size < size) { > + we = MAKE_64BIT_MASK(0, reg->data_size * 8); > + shift = 8 * (be ? reg->data_size - size : 0); > + } else if (reg->data_size >= size) { > + we = MAKE_64BIT_MASK(0, size * 8); > + } > + > + register_write(reg, value << shift, we << shift); > +} > + > +void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value, > + unsigned size) > +{ > + register_write_memory(opaque, addr, value, size, true); > +} > + > + > +void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value, > + unsigned size) > +{ > + register_write_memory(opaque, addr, value, size, false); > +} > + > +static inline uint64_t register_read_memory(void *opaque, hwaddr addr, > + unsigned size, bool be) > +{ > + RegisterInfoArray *reg_array = opaque; > + RegisterInfo *reg = NULL; > + int i, shift; > + > + for (i = 0; i < reg_array->num_elements; i++) { > + if (reg_array->r[i]->access->decode.addr == addr) { > + reg = reg_array->r[i]; > + break; > + } > + } > + assert(reg); > + > + shift = 8 * (be ? reg->data_size - size : 0); > + > + return (register_read(reg) >> shift) & MAKE_64BIT_MASK(0, size * 8); > +} > + > +uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size) > +{ > + return register_read_memory(opaque, addr, size, true); > +} > + > +uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size) > +{ > + return register_read_memory(opaque, addr, size, false); > +} > diff --git a/include/hw/register.h b/include/hw/register.h > index baa08f5..726a914 100644 > --- a/include/hw/register.h > +++ b/include/hw/register.h > @@ -15,6 +15,7 @@ > > typedef struct RegisterInfo RegisterInfo; > typedef struct RegisterAccessInfo RegisterAccessInfo; > +typedef struct RegisterInfoArray RegisterInfoArray; > > /** > * Access description for a register that is part of guest accessible device > @@ -51,6 +52,10 @@ struct RegisterAccessInfo { > void (*post_write)(RegisterInfo *reg, uint64_t val); > > uint64_t (*post_read)(RegisterInfo *reg, uint64_t val); > + > + struct { > + hwaddr addr; > + } decode; > }; > > /** > @@ -82,6 +87,26 @@ struct RegisterInfo { > }; > > /** > + * This structure is used to group all of the individual registers which are > + * modeled using the RegisterInfo strucutre. > + * > + * @r is an aray containing of all the relevent RegisterInfo structures. > + * > + * @num_elements is the number of elements in the array r > + * > + * @mem: optional Memory region for the register > + */ > + > +struct RegisterInfoArray { > + /* <private> */ > + MemoryRegion mem; I never see this used. I thought with the other changes the memory region enclosed the group of registers. Wouldn't a private mem violate that? > + > + /* <public> */ > + int num_elements; > + RegisterInfo **r; > +}; Without the extra bits why re-invent GArray? > + > +/** > * write a value to a register, subject to its restrictions > * @reg: register to write to > * @val: value to write > @@ -105,4 +130,30 @@ uint64_t register_read(RegisterInfo *reg); > > void register_reset(RegisterInfo *reg); > > +/** > + * Memory API MMIO write handler that will write to a Register API register. > + * _be for big endian variant and _le for little endian. > + * @opaque: RegisterInfo to write to > + * @addr: Address to write > + * @value: Value to write > + * @size: Number of bytes to write > + */ > + > +void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value, > + unsigned size); > +void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value, > + unsigned size); > + > +/** > + * Memory API MMIO read handler that will read from a Register API register. > + * _be for big endian variant and _le for little endian. > + * @opaque: RegisterInfo to read from > + * @addr: Address to read > + * @size: Number of bytes to read > + * returns: Value read from register > + */ > + > +uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size); > +uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size); > + > #endif -- Alex Bennée
On Tue, Mar 22, 2016 at 9:56 AM, Alex Bennée <alex.bennee@linaro.org> wrote: > > Alistair Francis <alistair.francis@xilinx.com> writes: > >> Add memory io handlers that glue the register API to the memory API. >> Just translation functions at this stage. Although it does allow for >> devices to be created without all-in-one mmio r/w handlers. >> >> This patch also adds the RegisterInfoArray struct, which allows all of >> the individual RegisterInfo structs to be grouped into a single memory >> region. >> >> Signed-off-by: Peter Crosthwaite <peter.crosthwaite@xilinx.com> >> Signed-off-by: Alistair Francis <alistair.francis@xilinx.com> >> --- >> V5: >> - Convert to using only one memory region >> >> hw/core/register.c | 70 +++++++++++++++++++++++++++++++++++++++++++++++++++ >> include/hw/register.h | 51 +++++++++++++++++++++++++++++++++++++ >> 2 files changed, 121 insertions(+) >> >> diff --git a/hw/core/register.c b/hw/core/register.c >> index 1656f71..e1cd0c4 100644 >> --- a/hw/core/register.c >> +++ b/hw/core/register.c >> @@ -146,3 +146,73 @@ void register_reset(RegisterInfo *reg) >> >> register_write_val(reg, reg->access->reset); >> } >> + >> +static inline void register_write_memory(void *opaque, hwaddr addr, >> + uint64_t value, unsigned size, bool be) >> +{ >> + RegisterInfoArray *reg_array = opaque; >> + RegisterInfo *reg = NULL; >> + uint64_t we = ~0; >> + int i, shift = 0; >> + >> + for (i = 0; i < reg_array->num_elements; i++) { >> + if (reg_array->r[i]->access->decode.addr == addr) { >> + reg = reg_array->r[i]; >> + break; >> + } >> + } >> + assert(reg); >> + >> + /* Generate appropriate write enable mask and shift values */ >> + if (reg->data_size < size) { >> + we = MAKE_64BIT_MASK(0, reg->data_size * 8); >> + shift = 8 * (be ? reg->data_size - size : 0); >> + } else if (reg->data_size >= size) { >> + we = MAKE_64BIT_MASK(0, size * 8); >> + } >> + >> + register_write(reg, value << shift, we << shift); >> +} >> + >> +void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value, >> + unsigned size) >> +{ >> + register_write_memory(opaque, addr, value, size, true); >> +} >> + >> + >> +void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value, >> + unsigned size) >> +{ >> + register_write_memory(opaque, addr, value, size, false); >> +} >> + >> +static inline uint64_t register_read_memory(void *opaque, hwaddr addr, >> + unsigned size, bool be) >> +{ >> + RegisterInfoArray *reg_array = opaque; >> + RegisterInfo *reg = NULL; >> + int i, shift; >> + >> + for (i = 0; i < reg_array->num_elements; i++) { >> + if (reg_array->r[i]->access->decode.addr == addr) { >> + reg = reg_array->r[i]; >> + break; >> + } >> + } >> + assert(reg); >> + >> + shift = 8 * (be ? reg->data_size - size : 0); >> + >> + return (register_read(reg) >> shift) & MAKE_64BIT_MASK(0, size * 8); >> +} >> + >> +uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size) >> +{ >> + return register_read_memory(opaque, addr, size, true); >> +} >> + >> +uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size) >> +{ >> + return register_read_memory(opaque, addr, size, false); >> +} >> diff --git a/include/hw/register.h b/include/hw/register.h >> index baa08f5..726a914 100644 >> --- a/include/hw/register.h >> +++ b/include/hw/register.h >> @@ -15,6 +15,7 @@ >> >> typedef struct RegisterInfo RegisterInfo; >> typedef struct RegisterAccessInfo RegisterAccessInfo; >> +typedef struct RegisterInfoArray RegisterInfoArray; >> >> /** >> * Access description for a register that is part of guest accessible device >> @@ -51,6 +52,10 @@ struct RegisterAccessInfo { >> void (*post_write)(RegisterInfo *reg, uint64_t val); >> >> uint64_t (*post_read)(RegisterInfo *reg, uint64_t val); >> + >> + struct { >> + hwaddr addr; >> + } decode; >> }; >> >> /** >> @@ -82,6 +87,26 @@ struct RegisterInfo { >> }; >> >> /** >> + * This structure is used to group all of the individual registers which are >> + * modeled using the RegisterInfo strucutre. >> + * >> + * @r is an aray containing of all the relevent RegisterInfo structures. >> + * >> + * @num_elements is the number of elements in the array r >> + * >> + * @mem: optional Memory region for the register >> + */ >> + >> +struct RegisterInfoArray { >> + /* <private> */ >> + MemoryRegion mem; > > I never see this used. I thought with the other changes the memory It isn't used here, it is used later on in the series when the register_init_block32() function is added. I can move it back to the patch if you wish. > region enclosed the group of registers. Wouldn't a private mem violate that? Yes, you are right. I'll remove the private comment. > >> + >> + /* <public> */ >> + int num_elements; >> + RegisterInfo **r; >> +}; > > Without the extra bits why re-invent GArray? What do you mean? Thanks, Alistair > >> + >> +/** >> * write a value to a register, subject to its restrictions >> * @reg: register to write to >> * @val: value to write >> @@ -105,4 +130,30 @@ uint64_t register_read(RegisterInfo *reg); >> >> void register_reset(RegisterInfo *reg); >> >> +/** >> + * Memory API MMIO write handler that will write to a Register API register. >> + * _be for big endian variant and _le for little endian. >> + * @opaque: RegisterInfo to write to >> + * @addr: Address to write >> + * @value: Value to write >> + * @size: Number of bytes to write >> + */ >> + >> +void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value, >> + unsigned size); >> +void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value, >> + unsigned size); >> + >> +/** >> + * Memory API MMIO read handler that will read from a Register API register. >> + * _be for big endian variant and _le for little endian. >> + * @opaque: RegisterInfo to read from >> + * @addr: Address to read >> + * @size: Number of bytes to read >> + * returns: Value read from register >> + */ >> + >> +uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size); >> +uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size); >> + >> #endif > > > -- > Alex Bennée >
diff --git a/hw/core/register.c b/hw/core/register.c index 1656f71..e1cd0c4 100644 --- a/hw/core/register.c +++ b/hw/core/register.c @@ -146,3 +146,73 @@ void register_reset(RegisterInfo *reg) register_write_val(reg, reg->access->reset); } + +static inline void register_write_memory(void *opaque, hwaddr addr, + uint64_t value, unsigned size, bool be) +{ + RegisterInfoArray *reg_array = opaque; + RegisterInfo *reg = NULL; + uint64_t we = ~0; + int i, shift = 0; + + for (i = 0; i < reg_array->num_elements; i++) { + if (reg_array->r[i]->access->decode.addr == addr) { + reg = reg_array->r[i]; + break; + } + } + assert(reg); + + /* Generate appropriate write enable mask and shift values */ + if (reg->data_size < size) { + we = MAKE_64BIT_MASK(0, reg->data_size * 8); + shift = 8 * (be ? reg->data_size - size : 0); + } else if (reg->data_size >= size) { + we = MAKE_64BIT_MASK(0, size * 8); + } + + register_write(reg, value << shift, we << shift); +} + +void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value, + unsigned size) +{ + register_write_memory(opaque, addr, value, size, true); +} + + +void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value, + unsigned size) +{ + register_write_memory(opaque, addr, value, size, false); +} + +static inline uint64_t register_read_memory(void *opaque, hwaddr addr, + unsigned size, bool be) +{ + RegisterInfoArray *reg_array = opaque; + RegisterInfo *reg = NULL; + int i, shift; + + for (i = 0; i < reg_array->num_elements; i++) { + if (reg_array->r[i]->access->decode.addr == addr) { + reg = reg_array->r[i]; + break; + } + } + assert(reg); + + shift = 8 * (be ? reg->data_size - size : 0); + + return (register_read(reg) >> shift) & MAKE_64BIT_MASK(0, size * 8); +} + +uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size) +{ + return register_read_memory(opaque, addr, size, true); +} + +uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size) +{ + return register_read_memory(opaque, addr, size, false); +} diff --git a/include/hw/register.h b/include/hw/register.h index baa08f5..726a914 100644 --- a/include/hw/register.h +++ b/include/hw/register.h @@ -15,6 +15,7 @@ typedef struct RegisterInfo RegisterInfo; typedef struct RegisterAccessInfo RegisterAccessInfo; +typedef struct RegisterInfoArray RegisterInfoArray; /** * Access description for a register that is part of guest accessible device @@ -51,6 +52,10 @@ struct RegisterAccessInfo { void (*post_write)(RegisterInfo *reg, uint64_t val); uint64_t (*post_read)(RegisterInfo *reg, uint64_t val); + + struct { + hwaddr addr; + } decode; }; /** @@ -82,6 +87,26 @@ struct RegisterInfo { }; /** + * This structure is used to group all of the individual registers which are + * modeled using the RegisterInfo strucutre. + * + * @r is an aray containing of all the relevent RegisterInfo structures. + * + * @num_elements is the number of elements in the array r + * + * @mem: optional Memory region for the register + */ + +struct RegisterInfoArray { + /* <private> */ + MemoryRegion mem; + + /* <public> */ + int num_elements; + RegisterInfo **r; +}; + +/** * write a value to a register, subject to its restrictions * @reg: register to write to * @val: value to write @@ -105,4 +130,30 @@ uint64_t register_read(RegisterInfo *reg); void register_reset(RegisterInfo *reg); +/** + * Memory API MMIO write handler that will write to a Register API register. + * _be for big endian variant and _le for little endian. + * @opaque: RegisterInfo to write to + * @addr: Address to write + * @value: Value to write + * @size: Number of bytes to write + */ + +void register_write_memory_be(void *opaque, hwaddr addr, uint64_t value, + unsigned size); +void register_write_memory_le(void *opaque, hwaddr addr, uint64_t value, + unsigned size); + +/** + * Memory API MMIO read handler that will read from a Register API register. + * _be for big endian variant and _le for little endian. + * @opaque: RegisterInfo to read from + * @addr: Address to read + * @size: Number of bytes to read + * returns: Value read from register + */ + +uint64_t register_read_memory_be(void *opaque, hwaddr addr, unsigned size); +uint64_t register_read_memory_le(void *opaque, hwaddr addr, unsigned size); + #endif