| Message ID | 20170316112030.20419-5-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, 16 Mar 2017, Andre Przywara wrote: > Instead of directly manipulating the tables in memory, an ITS driver > sends commands via a ring buffer in normal system memory 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> Reviewed-by: Stefano Stabellini <sstabellini@kernel.org> > --- > xen/arch/arm/gic-v3-its.c | 57 ++++++++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/gic_v3_its.h | 6 +++++ > 2 files changed, 63 insertions(+) > > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c > index 9982fe9..e5601ed 100644 > --- a/xen/arch/arm/gic-v3-its.c > +++ b/xen/arch/arm/gic-v3-its.c > @@ -20,10 +20,13 @@ > > #include <xen/lib.h> > #include <xen/mm.h> > +#include <xen/sizes.h> > #include <asm/gic_v3_defs.h> > #include <asm/gic_v3_its.h> > #include <asm/io.h> > > +#define ITS_CMD_QUEUE_SZ SZ_64K > + > LIST_HEAD(host_its_list); > > bool gicv3_its_host_has_its(void) > @@ -56,6 +59,55 @@ static uint64_t encode_propbaser_phys_addr(paddr_t addr, unsigned 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; > + void *buffer; > + unsigned int order; > + > + 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; > + > + /* The ITS command buffer needs to be 64K aligned. */ > + order = max(get_order_from_bytes(ITS_CMD_QUEUE_SZ), 16U - PAGE_SHIFT); > + buffer = alloc_xenheap_pages(order, 0); > + if ( !buffer ) > + return NULL; > + > + if ( virt_to_maddr(buffer) & ~GENMASK(51, 12) ) > + { > + free_xenheap_pages(buffer, 0); > + return NULL; > + } > + memset(buffer, 0, ITS_CMD_QUEUE_SZ); > + > + reg |= GITS_VALID_BIT | virt_to_maddr(buffer); > + reg |= ((ITS_CMD_QUEUE_SZ / SZ_4K) - 1) & GITS_CBASER_SIZE_MASK; > + writeq_relaxed(reg, cbasereg); > + reg = readq_relaxed(cbasereg); > + > + /* If the ITS dropped shareability, drop cacheability as well. */ > + if ( (reg & GITS_BASER_SHAREABILITY_MASK) == 0 ) > + { > + reg &= ~GITS_BASER_INNER_CACHEABILITY_MASK; > + writeq_relaxed(reg, cbasereg); > + } > + > + /* > + * If the command queue memory is mapped as uncached, we need to flush > + * it on every access. > + */ > + if ( !(reg & GITS_BASER_INNER_CACHEABILITY_MASK) ) > + { > + its->flags |= HOST_ITS_FLUSH_CMD_QUEUE; > + dprintk(XENLOG_WARNING, "using non-cacheable ITS command queue\n"); > + } > + > + return buffer; > +} > + > /* The ITS BASE registers work with page sizes of 4K, 16K or 64K. */ > #define BASER_PAGE_BITS(sz) ((sz) * 2 + 12) > > @@ -175,6 +227,11 @@ static int gicv3_its_init_single_its(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 a6c0acc..d5facf0 100644 > --- a/xen/include/asm-arm/gic_v3_its.h > +++ b/xen/include/asm-arm/gic_v3_its.h > @@ -74,8 +74,12 @@ > #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 > + > #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; > @@ -83,6 +87,8 @@ struct host_its { > paddr_t addr; > paddr_t size; > void __iomem *its_base; > + void *cmd_buf; > + unsigned int flags; > }; > > > -- > 2.9.0 >
Hi Andre, On 16/03/17 11:20, Andre Przywara wrote: > Instead of directly manipulating the tables in memory, an ITS driver > sends commands via a ring buffer in normal system memory 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 | 57 ++++++++++++++++++++++++++++++++++++++++ > xen/include/asm-arm/gic_v3_its.h | 6 +++++ > 2 files changed, 63 insertions(+) > > diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c > index 9982fe9..e5601ed 100644 > --- a/xen/arch/arm/gic-v3-its.c > +++ b/xen/arch/arm/gic-v3-its.c > @@ -20,10 +20,13 @@ > > #include <xen/lib.h> > #include <xen/mm.h> > +#include <xen/sizes.h> > #include <asm/gic_v3_defs.h> > #include <asm/gic_v3_its.h> > #include <asm/io.h> > > +#define ITS_CMD_QUEUE_SZ SZ_64K I thought you were planning to increase the size to 1MB? > + > LIST_HEAD(host_its_list); > > bool gicv3_its_host_has_its(void) > @@ -56,6 +59,55 @@ static uint64_t encode_propbaser_phys_addr(paddr_t addr, unsigned 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; > + void *buffer; > + unsigned int order; > + > + 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; > + > + /* The ITS command buffer needs to be 64K aligned. */ Looking at the spec, the command buffer does not need to be 64K aligned. On the previous version, you made it 4K aligned. So why this restriction? > + order = max(get_order_from_bytes(ITS_CMD_QUEUE_SZ), 16U - PAGE_SHIFT); > + buffer = alloc_xenheap_pages(order, 0); I am not sure to understand why you move from _zalloc to alloc_xenheap_*. The resulting behavior will be exactly the same but the former result to a simpler code. > + if ( !buffer ) > + return NULL; > + > + if ( virt_to_maddr(buffer) & ~GENMASK(51, 12) ) > + { > + free_xenheap_pages(buffer, 0); > + return NULL; > + } > + memset(buffer, 0, ITS_CMD_QUEUE_SZ); > + > + reg |= GITS_VALID_BIT | virt_to_maddr(buffer); > + reg |= ((ITS_CMD_QUEUE_SZ / SZ_4K) - 1) & GITS_CBASER_SIZE_MASK; > + writeq_relaxed(reg, cbasereg); > + reg = readq_relaxed(cbasereg); > + > + /* If the ITS dropped shareability, drop cacheability as well. */ > + if ( (reg & GITS_BASER_SHAREABILITY_MASK) == 0 ) > + { > + reg &= ~GITS_BASER_INNER_CACHEABILITY_MASK; > + writeq_relaxed(reg, cbasereg); > + } > + > + /* > + * If the command queue memory is mapped as uncached, we need to flush > + * it on every access. > + */ > + if ( !(reg & GITS_BASER_INNER_CACHEABILITY_MASK) ) > + { > + its->flags |= HOST_ITS_FLUSH_CMD_QUEUE; > + dprintk(XENLOG_WARNING, "using non-cacheable ITS command queue\n"); Please use printk, the message is useful even for non-debug build. > + } > + > + return buffer; > +} > + Cheers,
On 22/03/17 15:23, Julien Grall wrote: > Hi Andre, > > On 16/03/17 11:20, Andre Przywara wrote: >> Instead of directly manipulating the tables in memory, an ITS driver >> sends commands via a ring buffer in normal system memory 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 | 57 >> ++++++++++++++++++++++++++++++++++++++++ >> xen/include/asm-arm/gic_v3_its.h | 6 +++++ >> 2 files changed, 63 insertions(+) >> >> diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c >> index 9982fe9..e5601ed 100644 >> --- a/xen/arch/arm/gic-v3-its.c >> +++ b/xen/arch/arm/gic-v3-its.c >> @@ -20,10 +20,13 @@ >> >> #include <xen/lib.h> >> #include <xen/mm.h> >> +#include <xen/sizes.h> >> #include <asm/gic_v3_defs.h> >> #include <asm/gic_v3_its.h> >> #include <asm/io.h> >> >> +#define ITS_CMD_QUEUE_SZ SZ_64K > > I thought you were planning to increase the size to 1MB? Good, you noticed ;-) Indeed forgot to change it ... >> + >> LIST_HEAD(host_its_list); >> >> bool gicv3_its_host_has_its(void) >> @@ -56,6 +59,55 @@ static uint64_t encode_propbaser_phys_addr(paddr_t >> addr, unsigned 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; >> + void *buffer; >> + unsigned int order; >> + >> + 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; >> + >> + /* The ITS command buffer needs to be 64K aligned. */ > > Looking at the spec, the command buffer does not need to be 64K aligned. > On the previous version, you made it 4K aligned. So why this restriction? As you have already learnt, the GIC is more subtle sometimes ;-) Read the description at "Physical_Address, bits [51:12]" in the CBASER paragraph and tell me what you take from it. I decided to read it as "has to be 64K aligned". Happy to correct this otherwise. >> + order = max(get_order_from_bytes(ITS_CMD_QUEUE_SZ), 16U - >> PAGE_SHIFT); >> + buffer = alloc_xenheap_pages(order, 0); > > I am not sure to understand why you move from _zalloc to > alloc_xenheap_*. The resulting behavior will be exactly the same but the > former result to a simpler code. ^^^^^^^^^^^^^^^^ Really? I checked and found that alloc_xenheap_pages gives me physically contiguous allocation in the given order, where *alloc just guarantees virtually contiguous pages. > >> + if ( !buffer ) >> + return NULL; >> + >> + if ( virt_to_maddr(buffer) & ~GENMASK(51, 12) ) >> + { >> + free_xenheap_pages(buffer, 0); >> + return NULL; >> + } >> + memset(buffer, 0, ITS_CMD_QUEUE_SZ); >> + >> + reg |= GITS_VALID_BIT | virt_to_maddr(buffer); >> + reg |= ((ITS_CMD_QUEUE_SZ / SZ_4K) - 1) & GITS_CBASER_SIZE_MASK; >> + writeq_relaxed(reg, cbasereg); >> + reg = readq_relaxed(cbasereg); >> + >> + /* If the ITS dropped shareability, drop cacheability as well. */ >> + if ( (reg & GITS_BASER_SHAREABILITY_MASK) == 0 ) >> + { >> + reg &= ~GITS_BASER_INNER_CACHEABILITY_MASK; >> + writeq_relaxed(reg, cbasereg); >> + } >> + >> + /* >> + * If the command queue memory is mapped as uncached, we need to >> flush >> + * it on every access. >> + */ >> + if ( !(reg & GITS_BASER_INNER_CACHEABILITY_MASK) ) >> + { >> + its->flags |= HOST_ITS_FLUSH_CMD_QUEUE; >> + dprintk(XENLOG_WARNING, "using non-cacheable ITS command >> queue\n"); > > Please use printk, the message is useful even for non-debug build. OK. Cheers, Andre. > >> + } >> + >> + return buffer; >> +} >> + > > Cheers, >
Hi Andre, On 22/03/17 16:31, André Przywara wrote: > On 22/03/17 15:23, Julien Grall wrote: >> On 16/03/17 11:20, Andre Przywara wrote: >>> + >>> LIST_HEAD(host_its_list); >>> >>> bool gicv3_its_host_has_its(void) >>> @@ -56,6 +59,55 @@ static uint64_t encode_propbaser_phys_addr(paddr_t >>> addr, unsigned 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; >>> + void *buffer; >>> + unsigned int order; >>> + >>> + 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; >>> + >>> + /* The ITS command buffer needs to be 64K aligned. */ >> >> Looking at the spec, the command buffer does not need to be 64K aligned. >> On the previous version, you made it 4K aligned. So why this restriction? > > As you have already learnt, the GIC is more subtle sometimes ;-) Read > the description at "Physical_Address, bits [51:12]" in the CBASER > paragraph and tell me what you take from it. I decided to read it as > "has to be 64K aligned". > Happy to correct this otherwise. Hmmm. I think you are right. This is a little bit weird, but fine. Lets keep like that. > >>> + order = max(get_order_from_bytes(ITS_CMD_QUEUE_SZ), 16U - >>> PAGE_SHIFT); >>> + buffer = alloc_xenheap_pages(order, 0); >> >> I am not sure to understand why you move from _zalloc to >> alloc_xenheap_*. The resulting behavior will be exactly the same but the >> former result to a simpler code. > ^^^^^^^^^^^^^^^^ > Really? > I checked and found that alloc_xenheap_pages gives me physically > contiguous allocation in the given order, where *alloc just guarantees > virtually contiguous pages. Yes. Give a look at my answer on patch #3. Regards,
diff --git a/xen/arch/arm/gic-v3-its.c b/xen/arch/arm/gic-v3-its.c index 9982fe9..e5601ed 100644 --- a/xen/arch/arm/gic-v3-its.c +++ b/xen/arch/arm/gic-v3-its.c @@ -20,10 +20,13 @@ #include <xen/lib.h> #include <xen/mm.h> +#include <xen/sizes.h> #include <asm/gic_v3_defs.h> #include <asm/gic_v3_its.h> #include <asm/io.h> +#define ITS_CMD_QUEUE_SZ SZ_64K + LIST_HEAD(host_its_list); bool gicv3_its_host_has_its(void) @@ -56,6 +59,55 @@ static uint64_t encode_propbaser_phys_addr(paddr_t addr, unsigned 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; + void *buffer; + unsigned int order; + + 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; + + /* The ITS command buffer needs to be 64K aligned. */ + order = max(get_order_from_bytes(ITS_CMD_QUEUE_SZ), 16U - PAGE_SHIFT); + buffer = alloc_xenheap_pages(order, 0); + if ( !buffer ) + return NULL; + + if ( virt_to_maddr(buffer) & ~GENMASK(51, 12) ) + { + free_xenheap_pages(buffer, 0); + return NULL; + } + memset(buffer, 0, ITS_CMD_QUEUE_SZ); + + reg |= GITS_VALID_BIT | virt_to_maddr(buffer); + reg |= ((ITS_CMD_QUEUE_SZ / SZ_4K) - 1) & GITS_CBASER_SIZE_MASK; + writeq_relaxed(reg, cbasereg); + reg = readq_relaxed(cbasereg); + + /* If the ITS dropped shareability, drop cacheability as well. */ + if ( (reg & GITS_BASER_SHAREABILITY_MASK) == 0 ) + { + reg &= ~GITS_BASER_INNER_CACHEABILITY_MASK; + writeq_relaxed(reg, cbasereg); + } + + /* + * If the command queue memory is mapped as uncached, we need to flush + * it on every access. + */ + if ( !(reg & GITS_BASER_INNER_CACHEABILITY_MASK) ) + { + its->flags |= HOST_ITS_FLUSH_CMD_QUEUE; + dprintk(XENLOG_WARNING, "using non-cacheable ITS command queue\n"); + } + + return buffer; +} + /* The ITS BASE registers work with page sizes of 4K, 16K or 64K. */ #define BASER_PAGE_BITS(sz) ((sz) * 2 + 12) @@ -175,6 +227,11 @@ static int gicv3_its_init_single_its(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 a6c0acc..d5facf0 100644 --- a/xen/include/asm-arm/gic_v3_its.h +++ b/xen/include/asm-arm/gic_v3_its.h @@ -74,8 +74,12 @@ #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 + #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; @@ -83,6 +87,8 @@ struct host_its { paddr_t addr; paddr_t size; void __iomem *its_base; + void *cmd_buf; + unsigned int flags; };
Instead of directly manipulating the tables in memory, an ITS driver sends commands via a ring buffer in normal system memory 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 | 57 ++++++++++++++++++++++++++++++++++++++++ xen/include/asm-arm/gic_v3_its.h | 6 +++++ 2 files changed, 63 insertions(+)