| Message ID | 20170130183153.28566-6-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andre, On 30/01/17 18:31, Andre Przywara wrote: > Instead of directly manipulating the tables in memory, an ITS driver > sends commands via a ring buffer to the ITS h/w to create or alter the > LPI mappings. > Allocate memory for that buffer and tell the ITS about it to be able > to send ITS commands. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic-v3-its.c | 46 ++++++++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/gic_v3_its.h | 6 ++++++ > 2 files changed, 52 insertions(+) > > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c > index c31fef6..ad7cd2a 100644 > --- a/xen/arch/arm/gic-v3-its.c > +++ b/xen/arch/arm/gic-v3-its.c > @@ -27,6 +27,8 @@ > #include <asm/gic_v3_its.h> > #include <asm/io.h> > > +#define ITS_CMD_QUEUE_SZ SZ_64K > + > #define BASER_ATTR_MASK \ > ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ > (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ > @@ -44,6 +46,45 @@ static uint64_t encode_phys_addr(paddr_t addr, int page_bits) > return ret | (addr & GENMASK(51, 48)) >> (48 - 12); > } > > +static void *its_map_cbaser(struct host_its *its) > +{ > + void __iomem *cbasereg = its->its_base + GITS_CBASER; > + uint64_t reg, regc; > + void *buffer; > + paddr_t paddr; > + > + reg = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; > + reg |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; > + reg |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; > + > + buffer = _xzalloc(ITS_CMD_QUEUE_SZ, PAGE_SIZE); s/PAGE_SIZE/SZ_4K/ > + if ( !buffer ) > + return NULL; > + paddr = virt_to_maddr(buffer); > + ASSERT(!(paddr & ~GENMASK(51, 12))); Please remove the ASSERT (see why on patch #3). > + > + reg |= GITS_VALID_BIT | paddr; > + reg |= ((ITS_CMD_QUEUE_SZ / PAGE_SIZE) - 1) & GITS_CBASER_SIZE_MASK; > + writeq_relaxed(reg, cbasereg); > + regc = readq_relaxed(cbasereg); You could re-use reg rather than introduce a new variable regc. This would avoid some confusion on which one to use. > + > + /* If the ITS dropped shareability, drop cacheability as well. */ > + if ( (regc & GITS_BASER_SHAREABILITY_MASK) == 0 ) > + { > + regc &= ~GITS_BASER_INNER_CACHEABILITY_MASK; > + writeq_relaxed(regc, cbasereg); > + } > + > + /* > + * If the command queue memory is mapped as uncached, we need to flush > + * it on every access. > + */ > + if ( !(regc & GITS_BASER_INNER_CACHEABILITY_MASK) ) > + its->flags |= HOST_ITS_FLUSH_CMD_QUEUE; Could you add a print to inform the user about the cache flush? It is something quite useful to know in the log. > + > + return buffer; > +} > + > #define PAGE_BITS(sz) ((sz) * 2 + PAGE_SHIFT) > > static int its_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) > @@ -150,6 +191,11 @@ int gicv3_its_init(struct host_its *hw_its) > } > } > > + hw_its->cmd_buf = its_map_cbaser(hw_its); > + if ( !hw_its->cmd_buf ) > + return -ENOMEM; > + writeq_relaxed(0, hw_its->its_base + GITS_CWRITER); > + > return 0; > } > > diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h > index ed44bdb..ff5572f 100644 > --- a/xen/include/asm-arm/gic_v3_its.h > +++ b/xen/include/asm-arm/gic_v3_its.h > @@ -65,9 +65,13 @@ > #define GITS_BASER_OUTER_CACHEABILITY_MASK (0x7ULL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) > #define GITS_BASER_INNER_CACHEABILITY_MASK (0x7ULL << GITS_BASER_INNER_CACHEABILITY_SHIFT) > > +#define GITS_CBASER_SIZE_MASK 0xff > + > #ifndef __ASSEMBLY__ > #include <xen/device_tree.h> > > +#define HOST_ITS_FLUSH_CMD_QUEUE (1U << 0) > + > /* data structure for each hardware ITS */ > struct host_its { > struct list_head entry; > @@ -75,6 +79,8 @@ struct host_its { > paddr_t addr; > paddr_t size; > void __iomem *its_base; > + void *cmd_buf; > + unsigned int flags; > }; > > extern struct list_head host_its_list; > Regards,
On Mon, 30 Jan 2017, Andre Przywara wrote: > Instead of directly manipulating the tables in memory, an ITS driver > sends commands via a ring buffer to the ITS h/w to create or alter the > LPI mappings. > Allocate memory for that buffer and tell the ITS about it to be able > to send ITS commands. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic-v3-its.c | 46 ++++++++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/gic_v3_its.h | 6 ++++++ > 2 files changed, 52 insertions(+) > > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c > index c31fef6..ad7cd2a 100644 > --- a/xen/arch/arm/gic-v3-its.c > +++ b/xen/arch/arm/gic-v3-its.c > @@ -27,6 +27,8 @@ > #include <asm/gic_v3_its.h> > #include <asm/io.h> > > +#define ITS_CMD_QUEUE_SZ SZ_64K > + > #define BASER_ATTR_MASK \ > ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ > (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ > @@ -44,6 +46,45 @@ static uint64_t encode_phys_addr(paddr_t addr, int page_bits) > return ret | (addr & GENMASK(51, 48)) >> (48 - 12); > } > > +static void *its_map_cbaser(struct host_its *its) > +{ > + void __iomem *cbasereg = its->its_base + GITS_CBASER; > + uint64_t reg, regc; > + void *buffer; > + paddr_t paddr; > + > + reg = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; > + reg |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; > + reg |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; > + > + buffer = _xzalloc(ITS_CMD_QUEUE_SZ, PAGE_SIZE); > + if ( !buffer ) > + return NULL; > + paddr = virt_to_maddr(buffer); > + ASSERT(!(paddr & ~GENMASK(51, 12))); > + > + reg |= GITS_VALID_BIT | paddr; > + reg |= ((ITS_CMD_QUEUE_SZ / PAGE_SIZE) - 1) & GITS_CBASER_SIZE_MASK; > + writeq_relaxed(reg, cbasereg); > + regc = readq_relaxed(cbasereg); > + > + /* If the ITS dropped shareability, drop cacheability as well. */ > + if ( (regc & GITS_BASER_SHAREABILITY_MASK) == 0 ) > + { > + regc &= ~GITS_BASER_INNER_CACHEABILITY_MASK; > + writeq_relaxed(regc, cbasereg); > + } > + > + /* > + * If the command queue memory is mapped as uncached, we need to flush > + * it on every access. > + */ > + if ( !(regc & GITS_BASER_INNER_CACHEABILITY_MASK) ) > + its->flags |= HOST_ITS_FLUSH_CMD_QUEUE; > + > + return buffer; > +} > + > #define PAGE_BITS(sz) ((sz) * 2 + PAGE_SHIFT) > > static int its_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) > @@ -150,6 +191,11 @@ int gicv3_its_init(struct host_its *hw_its) > } > } > > + hw_its->cmd_buf = its_map_cbaser(hw_its); > + if ( !hw_its->cmd_buf ) > + return -ENOMEM; > + writeq_relaxed(0, hw_its->its_base + GITS_CWRITER); Why this new write?
Hi Stefano, On 02/14/2017 12:59 AM, Stefano Stabellini wrote: > On Mon, 30 Jan 2017, Andre Przywara wrote: >> static int its_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) >> @@ -150,6 +191,11 @@ int gicv3_its_init(struct host_its *hw_its) >> } >> } >> >> + hw_its->cmd_buf = its_map_cbaser(hw_its); >> + if ( !hw_its->cmd_buf ) >> + return -ENOMEM; >> + writeq_relaxed(0, hw_its->its_base + GITS_CWRITER); > > Why this new write? This was requested by me. From the spec (8.19.5 in ARM IHI 0069C), the reset value of GITS_CWRITER is unknown. So we have to reset the register to 0 otherwise the ITS may try to read invalid command as soon as it has been enabled. FWIW, GITS_CREADR was reset to 0 by the ITS when GITS_CBASER has successfully been written (see 8.19.2). Cheers, -- Julien Grall IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On Tue, 14 Feb 2017, Julien Grall wrote: > Hi Stefano, > > On 02/14/2017 12:59 AM, Stefano Stabellini wrote: > > On Mon, 30 Jan 2017, Andre Przywara wrote: > > > static int its_map_baser(void __iomem *basereg, uint64_t regc, int > > > nr_items) > > > @@ -150,6 +191,11 @@ int gicv3_its_init(struct host_its *hw_its) > > > } > > > } > > > > > > + hw_its->cmd_buf = its_map_cbaser(hw_its); > > > + if ( !hw_its->cmd_buf ) > > > + return -ENOMEM; > > > + writeq_relaxed(0, hw_its->its_base + GITS_CWRITER); > > > > Why this new write? > > This was requested by me. From the spec (8.19.5 in ARM IHI 0069C), the > reset value of GITS_CWRITER is unknown. So we have to reset the register > to 0 otherwise the ITS may try to read invalid command as soon as it has > been enabled. > > FWIW, GITS_CREADR was reset to 0 by the ITS when GITS_CBASER has > successfully been written (see 8.19.2). All right, thanks
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index c31fef6..ad7cd2a 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -27,6 +27,8 @@ #include <asm/gic_v3_its.h> #include <asm/io.h> +#define ITS_CMD_QUEUE_SZ SZ_64K + #define BASER_ATTR_MASK \ ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ @@ -44,6 +46,45 @@ static uint64_t encode_phys_addr(paddr_t addr, int page_bits) return ret | (addr & GENMASK(51, 48)) >> (48 - 12); } +static void *its_map_cbaser(struct host_its *its) +{ + void __iomem *cbasereg = its->its_base + GITS_CBASER; + uint64_t reg, regc; + void *buffer; + paddr_t paddr; + + reg = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; + reg |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; + reg |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; + + buffer = _xzalloc(ITS_CMD_QUEUE_SZ, PAGE_SIZE); + if ( !buffer ) + return NULL; + paddr = virt_to_maddr(buffer); + ASSERT(!(paddr & ~GENMASK(51, 12))); + + reg |= GITS_VALID_BIT | paddr; + reg |= ((ITS_CMD_QUEUE_SZ / PAGE_SIZE) - 1) & GITS_CBASER_SIZE_MASK; + writeq_relaxed(reg, cbasereg); + regc = readq_relaxed(cbasereg); + + /* If the ITS dropped shareability, drop cacheability as well. */ + if ( (regc & GITS_BASER_SHAREABILITY_MASK) == 0 ) + { + regc &= ~GITS_BASER_INNER_CACHEABILITY_MASK; + writeq_relaxed(regc, cbasereg); + } + + /* + * If the command queue memory is mapped as uncached, we need to flush + * it on every access. + */ + if ( !(regc & GITS_BASER_INNER_CACHEABILITY_MASK) ) + its->flags |= HOST_ITS_FLUSH_CMD_QUEUE; + + return buffer; +} + #define PAGE_BITS(sz) ((sz) * 2 + PAGE_SHIFT) static int its_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) @@ -150,6 +191,11 @@ int gicv3_its_init(struct host_its *hw_its) } } + hw_its->cmd_buf = its_map_cbaser(hw_its); + if ( !hw_its->cmd_buf ) + return -ENOMEM; + writeq_relaxed(0, hw_its->its_base + GITS_CWRITER); + return 0; } diff --git a/xen/include/asm-arm/gic_v3_its.h b/xen/include/asm-arm/gic_v3_its.h index ed44bdb..ff5572f 100644 --- a/xen/include/asm-arm/gic_v3_its.h +++ b/xen/include/asm-arm/gic_v3_its.h @@ -65,9 +65,13 @@ #define GITS_BASER_OUTER_CACHEABILITY_MASK (0x7ULL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) #define GITS_BASER_INNER_CACHEABILITY_MASK (0x7ULL << GITS_BASER_INNER_CACHEABILITY_SHIFT) +#define GITS_CBASER_SIZE_MASK 0xff + #ifndef __ASSEMBLY__ #include <xen/device_tree.h> +#define HOST_ITS_FLUSH_CMD_QUEUE (1U << 0) + /* data structure for each hardware ITS */ struct host_its { struct list_head entry; @@ -75,6 +79,8 @@ struct host_its { paddr_t addr; paddr_t size; void __iomem *its_base; + void *cmd_buf; + unsigned int flags; }; extern struct list_head host_its_list;
Instead of directly manipulating the tables in memory, an ITS driver sends commands via a ring buffer to the ITS h/w to create or alter the LPI mappings. Allocate memory for that buffer and tell the ITS about it to be able to send ITS commands. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/gic-v3-its.c | 46 ++++++++++++++++++++++++++++++++++++++++ xen/include/asm-arm/gic_v3_its.h | 6 ++++++ 2 files changed, 52 insertions(+)