| Message ID | 20160928182457.12433-4-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Andre, On Thunderx, MAPD commands are failing with error 0x1, which mean DEVID out of range. On Wed, Sep 28, 2016 at 11:54 PM, Andre Przywara <andre.przywara@arm.com> wrote: > Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and > an EventID (the MSI payload or interrupt ID) to a pair of LPI number > and collection ID, which points to the target CPU. > This mapping is stored in the device and collection tables, which software > has to provide for the ITS to use. > Allocate the required memory and hand it the ITS. > We limit the number of devices to cover 4 PCI busses for now. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3.c | 5 ++ > xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- > 3 files changed, 167 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c > index b52dff3..40238a2 100644 > --- a/xen/arch/arm/gic-its.c > +++ b/xen/arch/arm/gic-its.c > @@ -21,6 +21,7 @@ > #include <xen/device_tree.h> > #include <xen/libfdt/libfdt.h> > #include <asm/p2m.h> > +#include <asm/io.h> > #include <asm/gic.h> > #include <asm/gic_v3_defs.h> > #include <asm/gic-its.h> > @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); > min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) > #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) > > +#define BASER_ATTR_MASK \ > + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ > + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ > + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) > +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) > + > +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) > +{ > + uint64_t ret; > + > + if ( page_bits < 16) > + return (uint64_t)addr & GENMASK(47, page_bits); > + > + ret = addr & GENMASK(47, 16); > + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); > +} > + > +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) > +{ > + uint64_t attr; > + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; > + int pagesz; > + int order; > + void *buffer = NULL; > + > + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; > + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; > + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; > + > + /* > + * Loop over the page sizes (4K, 16K, 64K) to find out what the host > + * supports. > + */ > + for (pagesz = 0; pagesz < 3; pagesz++) > + { > + uint64_t reg; > + int nr_bytes; > + > + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); > + order = get_order_from_bytes(nr_bytes); > + > + if ( !buffer ) > + buffer = alloc_xenheap_pages(order, 0); > + if ( !buffer ) > + return -ENOMEM; > + > + reg = attr; > + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); > + reg |= nr_bytes >> (pagesz * 2 + 12); > + reg |= regc & BASER_RO_MASK; > + reg |= GITS_BASER_VALID; > + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); > + > + writeq_relaxed(reg, basereg); > + regc = readl_relaxed(basereg); > + > + /* The host didn't like our attributes, just use what it returned. */ > + if ( (regc & BASER_ATTR_MASK) != attr ) > + attr = regc & BASER_ATTR_MASK; > + > + /* If the host accepted our page size, we are done. */ > + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) > + return 0; > + > + /* Check whether our buffer is aligned to the next page size already. */ > + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) > + { > + free_xenheap_pages(buffer, order); > + buffer = NULL; > + } > + } > + > + if ( buffer ) > + free_xenheap_pages(buffer, order); > + > + return -EINVAL; > +} > + > +int gicv3_its_init(struct host_its *hw_its) > +{ > + uint64_t reg; > + int i; > + > + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); > + if ( !hw_its->its_base ) > + return -ENOMEM; > + > + for (i = 0; i < 8; i++) > + { > + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; > + int type; > + > + reg = readq_relaxed(basereg); > + type = (reg >> 56) & 0x7; > + switch ( type ) > + { > + case GITS_BASER_TYPE_NONE: > + continue; > + case GITS_BASER_TYPE_DEVICE: > + /* TODO: find some better way of limiting the number of devices */ > + gicv3_map_baser(basereg, reg, 1024); Thunderx has larger device id values. Changing to hw supported device ids makes MAPD commands to pass and Thunderx to boot. You can refer to Thunderx bdf numbers here https://github.com/vijaykilari/its_v6/commit/e1a8ec82ad2bb00b299727d0847b89671e9ba66d > + break; > + case GITS_BASER_TYPE_COLLECTION: > + gicv3_map_baser(basereg, reg, NR_CPUS); > + break; > + default: > + continue; > + } > + } > + > + return 0; > +} > + > uint64_t gicv3_lpi_allocate_pendtable(void) > { > uint64_t reg, attr; > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 2534aa5..5cf4618 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -29,6 +29,7 @@ > #include <xen/irq.h> > #include <xen/iocap.h> > #include <xen/sched.h> > +#include <xen/err.h> > #include <xen/errno.h> > #include <xen/delay.h> > #include <xen/device_tree.h> > @@ -1548,6 +1549,7 @@ static int __init gicv3_init(void) > { > int res, i; > uint32_t reg; > + struct host_its *hw_its; > > if ( !cpu_has_gicv3 ) > { > @@ -1603,6 +1605,9 @@ static int __init gicv3_init(void) > res = gicv3_cpu_init(); > gicv3_hyp_init(); > > + list_for_each_entry(hw_its, &host_its_list, entry) > + gicv3_its_init(hw_its); > + > spin_unlock(&gicv3.lock); > > return res; > diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h > index 48c6c78..589b889 100644 > --- a/xen/include/asm-arm/gic-its.h > +++ b/xen/include/asm-arm/gic-its.h > @@ -18,6 +18,47 @@ > #ifndef __ASM_ARM_ITS_H__ > #define __ASM_ARM_ITS_H__ > > +#define LPI_OFFSET 8192 > + > +#define GITS_CTLR (0x000) > +#define GITS_IIDR (0x004) > +#define GITS_TYPER (0x008) > +#define GITS_CBASER (0x080) > +#define GITS_CWRITER (0x088) > +#define GITS_CREADR (0x090) > +#define GITS_BASER0 (0x100) > +#define GITS_BASER1 (0x108) > +#define GITS_BASER2 (0x110) > +#define GITS_BASER3 (0x118) > +#define GITS_BASER4 (0x120) > +#define GITS_BASER5 (0x128) > +#define GITS_BASER6 (0x130) > +#define GITS_BASER7 (0x138) > + > +/* Register bits */ > +#define GITS_CTLR_ENABLE 0x1 > +#define GITS_IIDR_VALUE 0x34c > + > +#define GITS_BASER_VALID BIT(63) > +#define GITS_BASER_INDIRECT BIT(62) > +#define GITS_BASER_INNER_CACHEABILITY_SHIFT 59 > +#define GITS_BASER_TYPE_SHIFT 56 > +#define GITS_BASER_OUTER_CACHEABILITY_SHIFT 53 > +#define GITS_BASER_TYPE_NONE 0UL > +#define GITS_BASER_TYPE_DEVICE 1UL > +#define GITS_BASER_TYPE_VCPU 2UL > +#define GITS_BASER_TYPE_CPU 3UL > +#define GITS_BASER_TYPE_COLLECTION 4UL > +#define GITS_BASER_TYPE_RESERVED5 5UL > +#define GITS_BASER_TYPE_RESERVED6 6UL > +#define GITS_BASER_TYPE_RESERVED7 7UL > +#define GITS_BASER_ENTRY_SIZE_SHIFT 48 > +#define GITS_BASER_SHAREABILITY_SHIFT 10 > +#define GITS_BASER_PAGE_SIZE_SHIFT 8 > +#define GITS_BASER_RO_MASK ((7UL << GITS_BASER_TYPE_SHIFT) | \ > + (31UL << GITS_BASER_ENTRY_SIZE_SHIFT) |\ > + GITS_BASER_INDIRECT) > + > #ifndef __ASSEMBLY__ > #include <xen/device_tree.h> > > @@ -27,6 +68,7 @@ struct host_its { > const struct dt_device_node *dt_node; > paddr_t addr; > paddr_t size; > + void __iomem *its_base; > }; > > extern struct list_head host_its_list; > @@ -42,8 +84,9 @@ void gicv3_its_dt_init(const struct dt_device_node *node); > uint64_t gicv3_lpi_get_proptable(void); > uint64_t gicv3_lpi_allocate_pendtable(void); > > -/* Initialize the host structures for LPIs. */ > +/* Initialize the host structures for LPIs and the host ITSes. */ > int gicv3_lpi_init_host_lpis(int nr_lpis); > +int gicv3_its_init(struct host_its *hw_its); > > #else > > @@ -62,6 +105,10 @@ static inline int gicv3_lpi_init_host_lpis(int nr_lpis) > { > return 0; > } > +static inline int gicv3_its_init(struct host_its *hw_its) > +{ > + return 0; > +} > #endif /* CONFIG_HAS_ITS */ > > #endif /* __ASSEMBLY__ */ > -- > 2.9.0 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
Hi, On 09/10/16 14:55, Vijay Kilari wrote: > Hi Andre, > > On Thunderx, MAPD commands are failing with error 0x1, > which mean DEVID out of range. MAPD commands from Dom0, you mean? And thanks for giving it a try! > On Wed, Sep 28, 2016 at 11:54 PM, Andre Przywara <andre.przywara@arm.com> wrote: >> Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and >> an EventID (the MSI payload or interrupt ID) to a pair of LPI number >> and collection ID, which points to the target CPU. >> This mapping is stored in the device and collection tables, which software >> has to provide for the ITS to use. >> Allocate the required memory and hand it the ITS. >> We limit the number of devices to cover 4 PCI busses for now. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/gic-v3.c | 5 ++ >> xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- >> 3 files changed, 167 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c >> index b52dff3..40238a2 100644 >> --- a/xen/arch/arm/gic-its.c >> +++ b/xen/arch/arm/gic-its.c >> @@ -21,6 +21,7 @@ >> #include <xen/device_tree.h> >> #include <xen/libfdt/libfdt.h> >> #include <asm/p2m.h> >> +#include <asm/io.h> >> #include <asm/gic.h> >> #include <asm/gic_v3_defs.h> >> #include <asm/gic-its.h> >> @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); >> min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) >> #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) >> >> +#define BASER_ATTR_MASK \ >> + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ >> + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ >> + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) >> +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) >> + >> +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) >> +{ >> + uint64_t ret; >> + >> + if ( page_bits < 16) >> + return (uint64_t)addr & GENMASK(47, page_bits); >> + >> + ret = addr & GENMASK(47, 16); >> + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); >> +} >> + >> +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) >> +{ >> + uint64_t attr; >> + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; >> + int pagesz; >> + int order; >> + void *buffer = NULL; >> + >> + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; >> + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; >> + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; >> + >> + /* >> + * Loop over the page sizes (4K, 16K, 64K) to find out what the host >> + * supports. >> + */ >> + for (pagesz = 0; pagesz < 3; pagesz++) >> + { >> + uint64_t reg; >> + int nr_bytes; >> + >> + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); >> + order = get_order_from_bytes(nr_bytes); >> + >> + if ( !buffer ) >> + buffer = alloc_xenheap_pages(order, 0); >> + if ( !buffer ) >> + return -ENOMEM; >> + >> + reg = attr; >> + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); >> + reg |= nr_bytes >> (pagesz * 2 + 12); >> + reg |= regc & BASER_RO_MASK; >> + reg |= GITS_BASER_VALID; >> + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); >> + >> + writeq_relaxed(reg, basereg); >> + regc = readl_relaxed(basereg); >> + >> + /* The host didn't like our attributes, just use what it returned. */ >> + if ( (regc & BASER_ATTR_MASK) != attr ) >> + attr = regc & BASER_ATTR_MASK; >> + >> + /* If the host accepted our page size, we are done. */ >> + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) >> + return 0; >> + >> + /* Check whether our buffer is aligned to the next page size already. */ >> + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) >> + { >> + free_xenheap_pages(buffer, order); >> + buffer = NULL; >> + } >> + } >> + >> + if ( buffer ) >> + free_xenheap_pages(buffer, order); >> + >> + return -EINVAL; >> +} >> + >> +int gicv3_its_init(struct host_its *hw_its) >> +{ >> + uint64_t reg; >> + int i; >> + >> + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); >> + if ( !hw_its->its_base ) >> + return -ENOMEM; >> + >> + for (i = 0; i < 8; i++) >> + { >> + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; >> + int type; >> + >> + reg = readq_relaxed(basereg); >> + type = (reg >> 56) & 0x7; >> + switch ( type ) >> + { >> + case GITS_BASER_TYPE_NONE: >> + continue; >> + case GITS_BASER_TYPE_DEVICE: >> + /* TODO: find some better way of limiting the number of devices */ >> + gicv3_map_baser(basereg, reg, 1024); > > Thunderx has larger device id values. > Changing to hw supported device ids makes MAPD commands to pass > and Thunderx to boot. Ah, thanks for the heads up. Obviously this "1024" is a hack. Julien wanted to use platform specific code for the Dom0 device mapping, which could take care of those cases. I am not so happy with this, since it requires code for each and every platform. Instead I was thinking of using the PV PCI calls that Dom0 issues anyway to get an idea of how many devices we need, but this maybe too late in the boot process. We would need to take a closer look into this. > You can refer to Thunderx bdf numbers here > https://github.com/vijaykilari/its_v6/commit/e1a8ec82ad2bb00b299727d0847b89671e9ba66d Thanks for the link. To be honest, I deliberately didn't look into the previous patches to get a clean start. I guess I will take a look now. So you need something like 200,000 devices to cover those "three and some" segments? I guess this means no excuse anymore for postponing indirect mapping ;-) Cheers, Andre.
On Wed, Sep 28, 2016 at 11:54 PM, Andre Przywara <andre.przywara@arm.com> wrote: > Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and > an EventID (the MSI payload or interrupt ID) to a pair of LPI number > and collection ID, which points to the target CPU. > This mapping is stored in the device and collection tables, which software > has to provide for the ITS to use. > Allocate the required memory and hand it the ITS. > We limit the number of devices to cover 4 PCI busses for now. Thunderx has more than 4 PCI busses > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3.c | 5 ++ > xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- > 3 files changed, 167 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c > index b52dff3..40238a2 100644 > --- a/xen/arch/arm/gic-its.c > +++ b/xen/arch/arm/gic-its.c > @@ -21,6 +21,7 @@ > #include <xen/device_tree.h> > #include <xen/libfdt/libfdt.h> > #include <asm/p2m.h> > +#include <asm/io.h> > #include <asm/gic.h> > #include <asm/gic_v3_defs.h> > #include <asm/gic-its.h> > @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); > min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) > #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) > > +#define BASER_ATTR_MASK \ > + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ > + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ > + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) > +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) > + > +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) > +{ > + uint64_t ret; > + > + if ( page_bits < 16) > + return (uint64_t)addr & GENMASK(47, page_bits); > + > + ret = addr & GENMASK(47, 16); > + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); why this mask and shift for?. > +} > + > +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) > +{ > + uint64_t attr; > + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; > + int pagesz; > + int order; > + void *buffer = NULL; > + > + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; > + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; > + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; > + > + /* > + * Loop over the page sizes (4K, 16K, 64K) to find out what the host > + * supports. > + */ > + for (pagesz = 0; pagesz < 3; pagesz++) > + { > + uint64_t reg; > + int nr_bytes; > + > + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); > + order = get_order_from_bytes(nr_bytes); > + > + if ( !buffer ) > + buffer = alloc_xenheap_pages(order, 0); Don't we need to reset to zero all the pages before handing memory to ITS hw? > + if ( !buffer ) > + return -ENOMEM; > + > + reg = attr; > + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); > + reg |= nr_bytes >> (pagesz * 2 + 12); > + reg |= regc & BASER_RO_MASK; > + reg |= GITS_BASER_VALID; > + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); > + > + writeq_relaxed(reg, basereg); > + regc = readl_relaxed(basereg); > + > + /* The host didn't like our attributes, just use what it returned. */ > + if ( (regc & BASER_ATTR_MASK) != attr ) > + attr = regc & BASER_ATTR_MASK; > + > + /* If the host accepted our page size, we are done. */ > + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) > + return 0; > + > + /* Check whether our buffer is aligned to the next page size already. */ > + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) > + { > + free_xenheap_pages(buffer, order); > + buffer = NULL; > + } > + } > + > + if ( buffer ) > + free_xenheap_pages(buffer, order); > + > + return -EINVAL; > +} > + > +int gicv3_its_init(struct host_its *hw_its) > +{ > + uint64_t reg; > + int i; > + > + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); > + if ( !hw_its->its_base ) > + return -ENOMEM; > + > + for (i = 0; i < 8; i++) > + { > + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; > + int type; > + > + reg = readq_relaxed(basereg); > + type = (reg >> 56) & 0x7; define a macro for these constants > + switch ( type ) > + { > + case GITS_BASER_TYPE_NONE: > + continue; > + case GITS_BASER_TYPE_DEVICE: > + /* TODO: find some better way of limiting the number of devices */ > + gicv3_map_baser(basereg, reg, 1024); > + break; > + case GITS_BASER_TYPE_COLLECTION: > + gicv3_map_baser(basereg, reg, NR_CPUS); > + break; > + default: > + continue; > + } > + } > + > + return 0; > +} > + > uint64_t gicv3_lpi_allocate_pendtable(void) > { > uint64_t reg, attr; > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 2534aa5..5cf4618 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -29,6 +29,7 @@ > #include <xen/irq.h> > #include <xen/iocap.h> > #include <xen/sched.h> > +#include <xen/err.h> > #include <xen/errno.h> > #include <xen/delay.h> > #include <xen/device_tree.h> > @@ -1548,6 +1549,7 @@ static int __init gicv3_init(void) > { > int res, i; > uint32_t reg; > + struct host_its *hw_its; > > if ( !cpu_has_gicv3 ) > { > @@ -1603,6 +1605,9 @@ static int __init gicv3_init(void) > res = gicv3_cpu_init(); > gicv3_hyp_init(); > > + list_for_each_entry(hw_its, &host_its_list, entry) > + gicv3_its_init(hw_its); > + > spin_unlock(&gicv3.lock); > > return res; > diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h > index 48c6c78..589b889 100644 > --- a/xen/include/asm-arm/gic-its.h > +++ b/xen/include/asm-arm/gic-its.h > @@ -18,6 +18,47 @@ > #ifndef __ASM_ARM_ITS_H__ > #define __ASM_ARM_ITS_H__ > > +#define LPI_OFFSET 8192 > + > +#define GITS_CTLR (0x000) > +#define GITS_IIDR (0x004) > +#define GITS_TYPER (0x008) > +#define GITS_CBASER (0x080) > +#define GITS_CWRITER (0x088) > +#define GITS_CREADR (0x090) > +#define GITS_BASER0 (0x100) > +#define GITS_BASER1 (0x108) > +#define GITS_BASER2 (0x110) > +#define GITS_BASER3 (0x118) > +#define GITS_BASER4 (0x120) > +#define GITS_BASER5 (0x128) > +#define GITS_BASER6 (0x130) > +#define GITS_BASER7 (0x138) > + > +/* Register bits */ > +#define GITS_CTLR_ENABLE 0x1 > +#define GITS_IIDR_VALUE 0x34c > + > +#define GITS_BASER_VALID BIT(63) > +#define GITS_BASER_INDIRECT BIT(62) > +#define GITS_BASER_INNER_CACHEABILITY_SHIFT 59 > +#define GITS_BASER_TYPE_SHIFT 56 > +#define GITS_BASER_OUTER_CACHEABILITY_SHIFT 53 > +#define GITS_BASER_TYPE_NONE 0UL > +#define GITS_BASER_TYPE_DEVICE 1UL > +#define GITS_BASER_TYPE_VCPU 2UL > +#define GITS_BASER_TYPE_CPU 3UL > +#define GITS_BASER_TYPE_COLLECTION 4UL > +#define GITS_BASER_TYPE_RESERVED5 5UL > +#define GITS_BASER_TYPE_RESERVED6 6UL > +#define GITS_BASER_TYPE_RESERVED7 7UL > +#define GITS_BASER_ENTRY_SIZE_SHIFT 48 > +#define GITS_BASER_SHAREABILITY_SHIFT 10 > +#define GITS_BASER_PAGE_SIZE_SHIFT 8 > +#define GITS_BASER_RO_MASK ((7UL << GITS_BASER_TYPE_SHIFT) | \ > + (31UL << GITS_BASER_ENTRY_SIZE_SHIFT) |\ > + GITS_BASER_INDIRECT) > + > #ifndef __ASSEMBLY__ > #include <xen/device_tree.h> > > @@ -27,6 +68,7 @@ struct host_its { > const struct dt_device_node *dt_node; > paddr_t addr; > paddr_t size; > + void __iomem *its_base; > }; > > extern struct list_head host_its_list; > @@ -42,8 +84,9 @@ void gicv3_its_dt_init(const struct dt_device_node *node); > uint64_t gicv3_lpi_get_proptable(void); > uint64_t gicv3_lpi_allocate_pendtable(void); > > -/* Initialize the host structures for LPIs. */ > +/* Initialize the host structures for LPIs and the host ITSes. */ > int gicv3_lpi_init_host_lpis(int nr_lpis); > +int gicv3_its_init(struct host_its *hw_its); > > #else > > @@ -62,6 +105,10 @@ static inline int gicv3_lpi_init_host_lpis(int nr_lpis) > { > return 0; > } > +static inline int gicv3_its_init(struct host_its *hw_its) > +{ > + return 0; > +} > #endif /* CONFIG_HAS_ITS */ > > #endif /* __ASSEMBLY__ */ > -- > 2.9.0 > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > https://lists.xen.org/xen-devel
On Wed, 28 Sep 2016, Andre Przywara wrote: > Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and > an EventID (the MSI payload or interrupt ID) to a pair of LPI number > and collection ID, which points to the target CPU. > This mapping is stored in the device and collection tables, which software > has to provide for the ITS to use. > Allocate the required memory and hand it the ITS. > We limit the number of devices to cover 4 PCI busses for now. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3.c | 5 ++ > xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- > 3 files changed, 167 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c > index b52dff3..40238a2 100644 > --- a/xen/arch/arm/gic-its.c > +++ b/xen/arch/arm/gic-its.c > @@ -21,6 +21,7 @@ > #include <xen/device_tree.h> > #include <xen/libfdt/libfdt.h> > #include <asm/p2m.h> > +#include <asm/io.h> > #include <asm/gic.h> > #include <asm/gic_v3_defs.h> > #include <asm/gic-its.h> > @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); > min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) > #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) > > +#define BASER_ATTR_MASK \ > + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ > + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ > + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) > +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) > + > +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) > +{ > + uint64_t ret; > + > + if ( page_bits < 16) > + return (uint64_t)addr & GENMASK(47, page_bits); > + > + ret = addr & GENMASK(47, 16); > + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); > +} > + > +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) Shouldn't this be called its_map_baser? > +{ > + uint64_t attr; > + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; The spec says "This field is read-only and specifies the number of bytes per entry, minus one." Do we need to increment it by 1? > + int pagesz; > + int order; > + void *buffer = NULL; > + > + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; > + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; > + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; > + > + /* > + * Loop over the page sizes (4K, 16K, 64K) to find out what the host > + * supports. > + */ Is this really the best way to do it? Can't we assume ITS supports 4K, given that Xen requires 4K pages at the moment? Is it actually possible to find hardware that supports 4K but with an ITS that only support 64K or 16K pages? It seems insane to me. Otherwise can't we probe the page size somehow? > + for (pagesz = 0; pagesz < 3; pagesz++) > + { > + uint64_t reg; > + int nr_bytes; > + > + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); > + order = get_order_from_bytes(nr_bytes); > + > + if ( !buffer ) > + buffer = alloc_xenheap_pages(order, 0); > + if ( !buffer ) > + return -ENOMEM; > + > + reg = attr; > + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); > + reg |= nr_bytes >> (pagesz * 2 + 12); > + reg |= regc & BASER_RO_MASK; > + reg |= GITS_BASER_VALID; > + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); > + > + writeq_relaxed(reg, basereg); > + regc = readl_relaxed(basereg); > + > + /* The host didn't like our attributes, just use what it returned. */ > + if ( (regc & BASER_ATTR_MASK) != attr ) > + attr = regc & BASER_ATTR_MASK; > + > + /* If the host accepted our page size, we are done. */ > + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) > + return 0; > + > + /* Check whether our buffer is aligned to the next page size already. */ > + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) > + { > + free_xenheap_pages(buffer, order); > + buffer = NULL; > + } > + } > + > + if ( buffer ) > + free_xenheap_pages(buffer, order); > + > + return -EINVAL; > +} > + > +int gicv3_its_init(struct host_its *hw_its) > +{ > + uint64_t reg; > + int i; > + > + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); > + if ( !hw_its->its_base ) > + return -ENOMEM; > + > + for (i = 0; i < 8; i++) Code style. Unfortunately we don't have a script to check, but please refer to CODING_STYLE. I'd prefer if every number was #define'ed, including `8' (something like GITS_BASER_MAX). > + { > + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; > + int type; > + > + reg = readq_relaxed(basereg); > + type = (reg >> 56) & 0x7; Please #define 56 and 0x7 > + switch ( type ) > + { > + case GITS_BASER_TYPE_NONE: > + continue; > + case GITS_BASER_TYPE_DEVICE: > + /* TODO: find some better way of limiting the number of devices */ > + gicv3_map_baser(basereg, reg, 1024); An hardcoded max value might be OK, but please #define it. > + break; > + case GITS_BASER_TYPE_COLLECTION: > + gicv3_map_baser(basereg, reg, NR_CPUS); > + break; > + default: > + continue; > + } > + } > + > + return 0; > +} > + > uint64_t gicv3_lpi_allocate_pendtable(void) > { > uint64_t reg, attr; > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 2534aa5..5cf4618 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -29,6 +29,7 @@ > #include <xen/irq.h> > #include <xen/iocap.h> > #include <xen/sched.h> > +#include <xen/err.h> > #include <xen/errno.h> > #include <xen/delay.h> > #include <xen/device_tree.h> > @@ -1548,6 +1549,7 @@ static int __init gicv3_init(void) > { > int res, i; > uint32_t reg; > + struct host_its *hw_its; > > if ( !cpu_has_gicv3 ) > { > @@ -1603,6 +1605,9 @@ static int __init gicv3_init(void) > res = gicv3_cpu_init(); > gicv3_hyp_init(); > > + list_for_each_entry(hw_its, &host_its_list, entry) > + gicv3_its_init(hw_its); > + > spin_unlock(&gicv3.lock); > > return res; > diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h > index 48c6c78..589b889 100644 > --- a/xen/include/asm-arm/gic-its.h > +++ b/xen/include/asm-arm/gic-its.h > @@ -18,6 +18,47 @@ > #ifndef __ASM_ARM_ITS_H__ > #define __ASM_ARM_ITS_H__ > > +#define LPI_OFFSET 8192 > + > +#define GITS_CTLR (0x000) > +#define GITS_IIDR (0x004) > +#define GITS_TYPER (0x008) > +#define GITS_CBASER (0x080) > +#define GITS_CWRITER (0x088) > +#define GITS_CREADR (0x090) > +#define GITS_BASER0 (0x100) > +#define GITS_BASER1 (0x108) > +#define GITS_BASER2 (0x110) > +#define GITS_BASER3 (0x118) > +#define GITS_BASER4 (0x120) > +#define GITS_BASER5 (0x128) > +#define GITS_BASER6 (0x130) > +#define GITS_BASER7 (0x138) > + > +/* Register bits */ > +#define GITS_CTLR_ENABLE 0x1 > +#define GITS_IIDR_VALUE 0x34c > + > +#define GITS_BASER_VALID BIT(63) > +#define GITS_BASER_INDIRECT BIT(62) > +#define GITS_BASER_INNER_CACHEABILITY_SHIFT 59 > +#define GITS_BASER_TYPE_SHIFT 56 > +#define GITS_BASER_OUTER_CACHEABILITY_SHIFT 53 > +#define GITS_BASER_TYPE_NONE 0UL > +#define GITS_BASER_TYPE_DEVICE 1UL > +#define GITS_BASER_TYPE_VCPU 2UL > +#define GITS_BASER_TYPE_CPU 3UL > +#define GITS_BASER_TYPE_COLLECTION 4UL > +#define GITS_BASER_TYPE_RESERVED5 5UL > +#define GITS_BASER_TYPE_RESERVED6 6UL > +#define GITS_BASER_TYPE_RESERVED7 7UL > +#define GITS_BASER_ENTRY_SIZE_SHIFT 48 > +#define GITS_BASER_SHAREABILITY_SHIFT 10 > +#define GITS_BASER_PAGE_SIZE_SHIFT 8 > +#define GITS_BASER_RO_MASK ((7UL << GITS_BASER_TYPE_SHIFT) | \ > + (31UL << GITS_BASER_ENTRY_SIZE_SHIFT) |\ > + GITS_BASER_INDIRECT) > + > #ifndef __ASSEMBLY__ > #include <xen/device_tree.h> > > @@ -27,6 +68,7 @@ struct host_its { > const struct dt_device_node *dt_node; > paddr_t addr; > paddr_t size; > + void __iomem *its_base; > }; > > extern struct list_head host_its_list; > @@ -42,8 +84,9 @@ void gicv3_its_dt_init(const struct dt_device_node *node); > uint64_t gicv3_lpi_get_proptable(void); > uint64_t gicv3_lpi_allocate_pendtable(void); > > -/* Initialize the host structures for LPIs. */ > +/* Initialize the host structures for LPIs and the host ITSes. */ > int gicv3_lpi_init_host_lpis(int nr_lpis); > +int gicv3_its_init(struct host_its *hw_its); > > #else > > @@ -62,6 +105,10 @@ static inline int gicv3_lpi_init_host_lpis(int nr_lpis) > { > return 0; > } > +static inline int gicv3_its_init(struct host_its *hw_its) > +{ > + return 0; > +} > #endif /* CONFIG_HAS_ITS */ > > #endif /* __ASSEMBLY__ */ > -- > 2.9.0 >
Hi Stefano, On 26/10/2016 23:57, Stefano Stabellini wrote: >> + int pagesz; >> + int order; >> + void *buffer = NULL; >> + >> + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; >> + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; >> + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; >> + >> + /* >> + * Loop over the page sizes (4K, 16K, 64K) to find out what the host >> + * supports. >> + */ > > Is this really the best way to do it? Can't we assume ITS supports 4K, > given that Xen requires 4K pages at the moment? Is it actually possible > to find hardware that supports 4K but with an ITS that only support 64K > or 16K pages? It seems insane to me. Otherwise can't we probe the page > size somehow? By reading the spec (8.19.1 in IHI 0069C): "If the GIC implementation supports only a single, fixed page size, this field might be RO. When this register has an architecturally-defined reset value, if this field is implemented as an RW field, it resets to a value that is architecturally UNKNOWN." As the reset value is architecturally unknown the only way to find out the correct page size is to try them one by one. The GIC is a separate component of the platform and will be programed using physical address (and not virtual one). It would be fine to have a BASE registers supporting only 64K to save few lines in the GIC. Regards,
Hi Andre, On 28/09/2016 19:24, Andre Przywara wrote: > Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and > an EventID (the MSI payload or interrupt ID) to a pair of LPI number > and collection ID, which points to the target CPU. > This mapping is stored in the device and collection tables, which software > has to provide for the ITS to use. > Allocate the required memory and hand it the ITS. > We limit the number of devices to cover 4 PCI busses for now. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > --- > xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/gic-v3.c | 5 ++ > xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- > 3 files changed, 167 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c > index b52dff3..40238a2 100644 > --- a/xen/arch/arm/gic-its.c > +++ b/xen/arch/arm/gic-its.c > @@ -21,6 +21,7 @@ > #include <xen/device_tree.h> > #include <xen/libfdt/libfdt.h> > #include <asm/p2m.h> > +#include <asm/io.h> > #include <asm/gic.h> > #include <asm/gic_v3_defs.h> > #include <asm/gic-its.h> > @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); > min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) > #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) > > +#define BASER_ATTR_MASK \ > + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ > + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ > + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) > +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) I did not notice in the previous patch. But I would rather introduce GENMASK_ULL to avoid any issue the day we decide to port on 32-bits (for similar reason as BIT). > + > +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) Please make page_bits unsigned int. > +{ > + uint64_t ret; > + > + if ( page_bits < 16) Coding style. > + return (uint64_t)addr & GENMASK(47, page_bits); > + > + ret = addr & GENMASK(47, 16); > + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); > +} > + > +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) unsigned int nr_items Also can you find a better name for regc and/or reg. The naming is very similar and could be error-prone. > +{ > + uint64_t attr; > + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; unsigned int here. > + int pagesz; Same. > + int order; Same. > + void *buffer = NULL; > + > + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; > + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; > + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; > + > + /* > + * Loop over the page sizes (4K, 16K, 64K) to find out what the host > + * supports. > + */ > + for (pagesz = 0; pagesz < 3; pagesz++) Coding style: for ( ... ) > + { > + uint64_t reg; > + int nr_bytes; unsigned int nr_bytes. > + > + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); > + order = get_order_from_bytes(nr_bytes); > + > + if ( !buffer ) > + buffer = alloc_xenheap_pages(order, 0); > + if ( !buffer ) > + return -ENOMEM; > + > + reg = attr; > + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); > + reg |= nr_bytes >> (pagesz * 2 + 12); > + reg |= regc & BASER_RO_MASK; > + reg |= GITS_BASER_VALID; > + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); > + > + writeq_relaxed(reg, basereg); > + regc = readl_relaxed(basereg); Should not it be readq_relaxed? > + > + /* The host didn't like our attributes, just use what it returned. */ > + if ( (regc & BASER_ATTR_MASK) != attr ) > + attr = regc & BASER_ATTR_MASK; Should we flush the cache if the GIC does not support shareability? > + > + /* If the host accepted our page size, we are done. */ > + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) You probably want a define for 3UL << GITS_BASER_PAGE_SIZE_SHIFT. > + return 0; > + > + /* Check whether our buffer is aligned to the next page size already. */ > + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) Well the buffer could be aligned to the next page size but the size not page aligned. So you would provide the GIC memory that it cannot touch. So you have to always reallocate the buffer from scratch to avoid this issue. > + { > + free_xenheap_pages(buffer, order); > + buffer = NULL; > + } > + } > + > + if ( buffer ) > + free_xenheap_pages(buffer, order); > + > + return -EINVAL; > +} > + > +int gicv3_its_init(struct host_its *hw_its) > +{ > + uint64_t reg; > + int i; > + > + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); > + if ( !hw_its->its_base ) > + return -ENOMEM; > + > + for (i = 0; i < 8; i++) Coding style: for ( ... ) > + { > + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; > + int type; > + > + reg = readq_relaxed(basereg); > + type = (reg >> 56) & 0x7; > + switch ( type ) > + { > + case GITS_BASER_TYPE_NONE: > + continue; > + case GITS_BASER_TYPE_DEVICE: > + /* TODO: find some better way of limiting the number of devices */ > + gicv3_map_baser(basereg, reg, 1024); Flat table may use a huge amount of memory on some platform (depending how many device are present and how the ID space is sparsed). It would probably worth to add a TODO regarding the support of one/two-level table. Also, the limitation could be dropped if we use the one/two-level table as the amount of memory pre-allocated will be smaller. > + break; > + case GITS_BASER_TYPE_COLLECTION: > + gicv3_map_baser(basereg, reg, NR_CPUS); > + break; > + default: > + continue; > + } > + } > + > + return 0; > +} > + > uint64_t gicv3_lpi_allocate_pendtable(void) > { > uint64_t reg, attr; > diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c > index 2534aa5..5cf4618 100644 > --- a/xen/arch/arm/gic-v3.c > +++ b/xen/arch/arm/gic-v3.c > @@ -29,6 +29,7 @@ > #include <xen/irq.h> > #include <xen/iocap.h> > #include <xen/sched.h> > +#include <xen/err.h> > #include <xen/errno.h> > #include <xen/delay.h> > #include <xen/device_tree.h> > @@ -1548,6 +1549,7 @@ static int __init gicv3_init(void) > { > int res, i; > uint32_t reg; > + struct host_its *hw_its; > > if ( !cpu_has_gicv3 ) > { > @@ -1603,6 +1605,9 @@ static int __init gicv3_init(void) > res = gicv3_cpu_init(); > gicv3_hyp_init(); > > + list_for_each_entry(hw_its, &host_its_list, entry) > + gicv3_its_init(hw_its); > + > spin_unlock(&gicv3.lock); > > return res; > diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h > index 48c6c78..589b889 100644 > --- a/xen/include/asm-arm/gic-its.h > +++ b/xen/include/asm-arm/gic-its.h > @@ -18,6 +18,47 @@ > #ifndef __ASM_ARM_ITS_H__ > #define __ASM_ARM_ITS_H__ > > +#define LPI_OFFSET 8192 > + > +#define GITS_CTLR (0x000) > +#define GITS_IIDR (0x004) > +#define GITS_TYPER (0x008) > +#define GITS_CBASER (0x080) > +#define GITS_CWRITER (0x088) > +#define GITS_CREADR (0x090) > +#define GITS_BASER0 (0x100) > +#define GITS_BASER1 (0x108) > +#define GITS_BASER2 (0x110) > +#define GITS_BASER3 (0x118) > +#define GITS_BASER4 (0x120) > +#define GITS_BASER5 (0x128) > +#define GITS_BASER6 (0x130) > +#define GITS_BASER7 (0x138) > + > +/* Register bits */ > +#define GITS_CTLR_ENABLE 0x1 > +#define GITS_IIDR_VALUE 0x34c > + > +#define GITS_BASER_VALID BIT(63) > +#define GITS_BASER_INDIRECT BIT(62) > +#define GITS_BASER_INNER_CACHEABILITY_SHIFT 59 > +#define GITS_BASER_TYPE_SHIFT 56 > +#define GITS_BASER_OUTER_CACHEABILITY_SHIFT 53 > +#define GITS_BASER_TYPE_NONE 0UL > +#define GITS_BASER_TYPE_DEVICE 1UL > +#define GITS_BASER_TYPE_VCPU 2UL > +#define GITS_BASER_TYPE_CPU 3UL > +#define GITS_BASER_TYPE_COLLECTION 4UL > +#define GITS_BASER_TYPE_RESERVED5 5UL > +#define GITS_BASER_TYPE_RESERVED6 6UL > +#define GITS_BASER_TYPE_RESERVED7 7UL > +#define GITS_BASER_ENTRY_SIZE_SHIFT 48 > +#define GITS_BASER_SHAREABILITY_SHIFT 10 > +#define GITS_BASER_PAGE_SIZE_SHIFT 8 > +#define GITS_BASER_RO_MASK ((7UL << GITS_BASER_TYPE_SHIFT) | \ > + (31UL << GITS_BASER_ENTRY_SIZE_SHIFT) |\ > + GITS_BASER_INDIRECT) > + > #ifndef __ASSEMBLY__ > #include <xen/device_tree.h> > > @@ -27,6 +68,7 @@ struct host_its { > const struct dt_device_node *dt_node; > paddr_t addr; > paddr_t size; > + void __iomem *its_base; > }; > > extern struct list_head host_its_list; > @@ -42,8 +84,9 @@ void gicv3_its_dt_init(const struct dt_device_node *node); > uint64_t gicv3_lpi_get_proptable(void); > uint64_t gicv3_lpi_allocate_pendtable(void); > > -/* Initialize the host structures for LPIs. */ > +/* Initialize the host structures for LPIs and the host ITSes. */ > int gicv3_lpi_init_host_lpis(int nr_lpis); > +int gicv3_its_init(struct host_its *hw_its); > > #else > > @@ -62,6 +105,10 @@ static inline int gicv3_lpi_init_host_lpis(int nr_lpis) > { > return 0; > } Please add a newline here and ... > +static inline int gicv3_its_init(struct host_its *hw_its) > +{ > + return 0; > +} here. > #endif /* CONFIG_HAS_ITS */ > > #endif /* __ASSEMBLY__ */ > Regards,
Hi, On 24/10/16 15:30, Vijay Kilari wrote: > On Wed, Sep 28, 2016 at 11:54 PM, Andre Przywara <andre.przywara@arm.com> wrote: >> Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and >> an EventID (the MSI payload or interrupt ID) to a pair of LPI number >> and collection ID, which points to the target CPU. >> This mapping is stored in the device and collection tables, which software >> has to provide for the ITS to use. >> Allocate the required memory and hand it the ITS. >> We limit the number of devices to cover 4 PCI busses for now. > > Thunderx has more than 4 PCI busses Yeah, I am thinking about a proper solution for that hack. We may use a default of 4 buses and allow platform to override this. Or make this a configuration value. Or copy the Linux behaviour by limiting the number of pages to some sensible value (16MB, if I got this correctly). Anyway I think we need indirect table support to save Thunder from allocating too much memory for this. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/gic-v3.c | 5 ++ >> xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- >> 3 files changed, 167 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c >> index b52dff3..40238a2 100644 >> --- a/xen/arch/arm/gic-its.c >> +++ b/xen/arch/arm/gic-its.c >> @@ -21,6 +21,7 @@ >> #include <xen/device_tree.h> >> #include <xen/libfdt/libfdt.h> >> #include <asm/p2m.h> >> +#include <asm/io.h> >> #include <asm/gic.h> >> #include <asm/gic_v3_defs.h> >> #include <asm/gic-its.h> >> @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); >> min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) >> #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) >> >> +#define BASER_ATTR_MASK \ >> + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ >> + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ >> + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) >> +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) >> + >> +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) >> +{ >> + uint64_t ret; >> + >> + if ( page_bits < 16) >> + return (uint64_t)addr & GENMASK(47, page_bits); >> + >> + ret = addr & GENMASK(47, 16); >> + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); > > why this mask and shift for?. According to the (current issue of the) spec bits 48-51 of the address are stored in bits 12-15 of the register. >> +} >> + >> +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) >> +{ >> + uint64_t attr; >> + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; >> + int pagesz; >> + int order; >> + void *buffer = NULL; >> + >> + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; >> + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; >> + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; >> + >> + /* >> + * Loop over the page sizes (4K, 16K, 64K) to find out what the host >> + * supports. >> + */ >> + for (pagesz = 0; pagesz < 3; pagesz++) >> + { >> + uint64_t reg; >> + int nr_bytes; >> + >> + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); >> + order = get_order_from_bytes(nr_bytes); >> + >> + if ( !buffer ) >> + buffer = alloc_xenheap_pages(order, 0); > Don't we need to reset to zero all the pages before handing > memory to ITS hw? True. Will fix it. >> + if ( !buffer ) >> + return -ENOMEM; >> + >> + reg = attr; >> + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); >> + reg |= nr_bytes >> (pagesz * 2 + 12); >> + reg |= regc & BASER_RO_MASK; >> + reg |= GITS_BASER_VALID; >> + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); >> + >> + writeq_relaxed(reg, basereg); >> + regc = readl_relaxed(basereg); >> + >> + /* The host didn't like our attributes, just use what it returned. */ >> + if ( (regc & BASER_ATTR_MASK) != attr ) >> + attr = regc & BASER_ATTR_MASK; >> + >> + /* If the host accepted our page size, we are done. */ >> + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) >> + return 0; >> + >> + /* Check whether our buffer is aligned to the next page size already. */ >> + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) >> + { >> + free_xenheap_pages(buffer, order); >> + buffer = NULL; >> + } >> + } >> + >> + if ( buffer ) >> + free_xenheap_pages(buffer, order); >> + >> + return -EINVAL; >> +} >> + >> +int gicv3_its_init(struct host_its *hw_its) >> +{ >> + uint64_t reg; >> + int i; >> + >> + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); >> + if ( !hw_its->its_base ) >> + return -ENOMEM; >> + >> + for (i = 0; i < 8; i++) >> + { >> + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; >> + int type; >> + >> + reg = readq_relaxed(basereg); >> + type = (reg >> 56) & 0x7; > > define a macro for these constants Sure. Cheers, Andre.
Hi, On 26/10/16 23:57, Stefano Stabellini wrote: > On Wed, 28 Sep 2016, Andre Przywara wrote: >> Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and >> an EventID (the MSI payload or interrupt ID) to a pair of LPI number >> and collection ID, which points to the target CPU. >> This mapping is stored in the device and collection tables, which software >> has to provide for the ITS to use. >> Allocate the required memory and hand it the ITS. >> We limit the number of devices to cover 4 PCI busses for now. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> --- >> xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ >> xen/arch/arm/gic-v3.c | 5 ++ >> xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- >> 3 files changed, 167 insertions(+), 1 deletion(-) >> >> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c >> index b52dff3..40238a2 100644 >> --- a/xen/arch/arm/gic-its.c >> +++ b/xen/arch/arm/gic-its.c >> @@ -21,6 +21,7 @@ >> #include <xen/device_tree.h> >> #include <xen/libfdt/libfdt.h> >> #include <asm/p2m.h> >> +#include <asm/io.h> >> #include <asm/gic.h> >> #include <asm/gic_v3_defs.h> >> #include <asm/gic-its.h> >> @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); >> min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) >> #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) >> >> +#define BASER_ATTR_MASK \ >> + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ >> + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ >> + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) >> +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) >> + >> +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) >> +{ >> + uint64_t ret; >> + >> + if ( page_bits < 16) >> + return (uint64_t)addr & GENMASK(47, page_bits); >> + >> + ret = addr & GENMASK(47, 16); >> + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); >> +} >> + >> +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) > > Shouldn't this be called its_map_baser? Yes, the BASER registers are an ITS property. >> +{ >> + uint64_t attr; >> + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; > > The spec says "This field is read-only and specifies the number of > bytes per entry, minus one." Do we need to increment it by 1? Mmh, looks so. I guess it worked because the number gets dwarfed by the page size round up below. >> + int pagesz; >> + int order; >> + void *buffer = NULL; >> + >> + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; >> + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; >> + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; >> + >> + /* >> + * Loop over the page sizes (4K, 16K, 64K) to find out what the host >> + * supports. >> + */ > > Is this really the best way to do it? Can't we assume ITS supports 4K, > given that Xen requires 4K pages at the moment? The ITS pages are totally independent from the core's MMU page size. So the spec says: "If the GIC implementation supports only a single, fixed page size, this field might be RO." I take it that this means that the only implemented page size could be 64K, for instance. And in fact the KVM ITS emulation advertises exactly this to a guest. > Is it actually possible > to find hardware that supports 4K but with an ITS that only support 64K > or 16K pages? It seems insane to me. Otherwise can't we probe the page > size somehow? We can probe by writing and seeing if it sticks - that's what the code does. Is it really so horrible? I agree it's nasty, but isn't it basically a loop around the code needed anyway? Yes to the rest of the comments. Cheers, Andre. >> + for (pagesz = 0; pagesz < 3; pagesz++) >> + { >> + uint64_t reg; >> + int nr_bytes; >> + >> + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); >> + order = get_order_from_bytes(nr_bytes); >> + >> + if ( !buffer ) >> + buffer = alloc_xenheap_pages(order, 0); >> + if ( !buffer ) >> + return -ENOMEM; >> + >> + reg = attr; >> + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); >> + reg |= nr_bytes >> (pagesz * 2 + 12); >> + reg |= regc & BASER_RO_MASK; >> + reg |= GITS_BASER_VALID; >> + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); >> + >> + writeq_relaxed(reg, basereg); >> + regc = readl_relaxed(basereg); >> + >> + /* The host didn't like our attributes, just use what it returned. */ >> + if ( (regc & BASER_ATTR_MASK) != attr ) >> + attr = regc & BASER_ATTR_MASK; >> + >> + /* If the host accepted our page size, we are done. */ >> + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) >> + return 0; >> + >> + /* Check whether our buffer is aligned to the next page size already. */ >> + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) >> + { >> + free_xenheap_pages(buffer, order); >> + buffer = NULL; >> + } >> + } >> + >> + if ( buffer ) >> + free_xenheap_pages(buffer, order); >> + >> + return -EINVAL; >> +} >> + >> +int gicv3_its_init(struct host_its *hw_its) >> +{ >> + uint64_t reg; >> + int i; >> + >> + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); >> + if ( !hw_its->its_base ) >> + return -ENOMEM; >> + >> + for (i = 0; i < 8; i++) > > Code style. Unfortunately we don't have a script to check, but please > refer to CODING_STYLE. I'd prefer if every number was #define'ed, > including `8' (something like GITS_BASER_MAX). > > >> + { >> + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; >> + int type; >> + >> + reg = readq_relaxed(basereg); >> + type = (reg >> 56) & 0x7; > > Please #define 56 and 0x7 > > >> + switch ( type ) >> + { >> + case GITS_BASER_TYPE_NONE: >> + continue; >> + case GITS_BASER_TYPE_DEVICE: >> + /* TODO: find some better way of limiting the number of devices */ >> + gicv3_map_baser(basereg, reg, 1024); > > An hardcoded max value might be OK, but please #define it. > >
On Thu, 10 Nov 2016, Andre Przywara wrote: > Hi, > > On 26/10/16 23:57, Stefano Stabellini wrote: > > On Wed, 28 Sep 2016, Andre Przywara wrote: > >> Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and > >> an EventID (the MSI payload or interrupt ID) to a pair of LPI number > >> and collection ID, which points to the target CPU. > >> This mapping is stored in the device and collection tables, which software > >> has to provide for the ITS to use. > >> Allocate the required memory and hand it the ITS. > >> We limit the number of devices to cover 4 PCI busses for now. > >> > >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> > >> --- > >> xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ > >> xen/arch/arm/gic-v3.c | 5 ++ > >> xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- > >> 3 files changed, 167 insertions(+), 1 deletion(-) > >> > >> diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c > >> index b52dff3..40238a2 100644 > >> --- a/xen/arch/arm/gic-its.c > >> +++ b/xen/arch/arm/gic-its.c > >> @@ -21,6 +21,7 @@ > >> #include <xen/device_tree.h> > >> #include <xen/libfdt/libfdt.h> > >> #include <asm/p2m.h> > >> +#include <asm/io.h> > >> #include <asm/gic.h> > >> #include <asm/gic_v3_defs.h> > >> #include <asm/gic-its.h> > >> @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); > >> min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) > >> #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) > >> > >> +#define BASER_ATTR_MASK \ > >> + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ > >> + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ > >> + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) > >> +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) > >> + > >> +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) > >> +{ > >> + uint64_t ret; > >> + > >> + if ( page_bits < 16) > >> + return (uint64_t)addr & GENMASK(47, page_bits); > >> + > >> + ret = addr & GENMASK(47, 16); > >> + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); > >> +} > >> + > >> +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) > > > > Shouldn't this be called its_map_baser? > > Yes, the BASER registers are an ITS property. > > >> +{ > >> + uint64_t attr; > >> + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; > > > > The spec says "This field is read-only and specifies the number of > > bytes per entry, minus one." Do we need to increment it by 1? > > Mmh, looks so. I guess it worked because the number gets dwarfed by the > page size round up below. > > >> + int pagesz; > >> + int order; > >> + void *buffer = NULL; > >> + > >> + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; > >> + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; > >> + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; > >> + > >> + /* > >> + * Loop over the page sizes (4K, 16K, 64K) to find out what the host > >> + * supports. > >> + */ > > > > Is this really the best way to do it? Can't we assume ITS supports 4K, > > given that Xen requires 4K pages at the moment? > > The ITS pages are totally independent from the core's MMU page size. > So the spec says: "If the GIC implementation supports only a single, > fixed page size, this field might be RO." > I take it that this means that the only implemented page size could be > 64K, for instance. And in fact the KVM ITS emulation advertises exactly > this to a guest. > > > Is it actually possible > > to find hardware that supports 4K but with an ITS that only support 64K > > or 16K pages? It seems insane to me. Otherwise can't we probe the page > > size somehow? > > We can probe by writing and seeing if it sticks - that's what the code > does. Is it really so horrible? I agree it's nasty, but isn't it > basically a loop around the code needed anyway? It looks very strange that there isn't a better way to find that info. It looks a bit like an hack. It is also bad from a software point of view being forced to cope with all three possible page granularities. But oh well, sometimes we just have to deal with whatever the hardware offers us. > Yes to the rest of the comments. > > > >> + for (pagesz = 0; pagesz < 3; pagesz++) > >> + { > >> + uint64_t reg; > >> + int nr_bytes; > >> + > >> + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); > >> + order = get_order_from_bytes(nr_bytes); > >> + > >> + if ( !buffer ) > >> + buffer = alloc_xenheap_pages(order, 0); > >> + if ( !buffer ) > >> + return -ENOMEM; > >> + > >> + reg = attr; > >> + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); > >> + reg |= nr_bytes >> (pagesz * 2 + 12); > >> + reg |= regc & BASER_RO_MASK; > >> + reg |= GITS_BASER_VALID; > >> + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); > >> + > >> + writeq_relaxed(reg, basereg); > >> + regc = readl_relaxed(basereg); > >> + > >> + /* The host didn't like our attributes, just use what it returned. */ > >> + if ( (regc & BASER_ATTR_MASK) != attr ) > >> + attr = regc & BASER_ATTR_MASK; > >> + > >> + /* If the host accepted our page size, we are done. */ > >> + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) > >> + return 0; > >> + > >> + /* Check whether our buffer is aligned to the next page size already. */ > >> + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) > >> + { > >> + free_xenheap_pages(buffer, order); > >> + buffer = NULL; > >> + } > >> + } > >> + > >> + if ( buffer ) > >> + free_xenheap_pages(buffer, order); > >> + > >> + return -EINVAL; > >> +} > >> + > >> +int gicv3_its_init(struct host_its *hw_its) > >> +{ > >> + uint64_t reg; > >> + int i; > >> + > >> + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); > >> + if ( !hw_its->its_base ) > >> + return -ENOMEM; > >> + > >> + for (i = 0; i < 8; i++) > > > > Code style. Unfortunately we don't have a script to check, but please > > refer to CODING_STYLE. I'd prefer if every number was #define'ed, > > including `8' (something like GITS_BASER_MAX). > > > > > >> + { > >> + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; > >> + int type; > >> + > >> + reg = readq_relaxed(basereg); > >> + type = (reg >> 56) & 0x7; > > > > Please #define 56 and 0x7 > > > > > >> + switch ( type ) > >> + { > >> + case GITS_BASER_TYPE_NONE: > >> + continue; > >> + case GITS_BASER_TYPE_DEVICE: > >> + /* TODO: find some better way of limiting the number of devices */ > >> + gicv3_map_baser(basereg, reg, 1024); > > > > An hardcoded max value might be OK, but please #define it. > > > > >
diff --git a/xen/arch/arm/gic-its.c b/xen/arch/arm/gic-its.c index b52dff3..40238a2 100644 --- a/xen/arch/arm/gic-its.c +++ b/xen/arch/arm/gic-its.c @@ -21,6 +21,7 @@ #include <xen/device_tree.h> #include <xen/libfdt/libfdt.h> #include <asm/p2m.h> +#include <asm/io.h> #include <asm/gic.h> #include <asm/gic_v3_defs.h> #include <asm/gic-its.h> @@ -38,6 +39,119 @@ static DEFINE_PER_CPU(void *, pending_table); min_t(unsigned int, lpi_data.host_lpi_bits, CONFIG_HOST_LPI_BITS) #define MAX_HOST_LPIS (BIT(MAX_HOST_LPI_BITS) - 8192) +#define BASER_ATTR_MASK \ + ((0x3UL << GITS_BASER_SHAREABILITY_SHIFT) | \ + (0x7UL << GITS_BASER_OUTER_CACHEABILITY_SHIFT) | \ + (0x7UL << GITS_BASER_INNER_CACHEABILITY_SHIFT)) +#define BASER_RO_MASK (GENMASK(52, 48) | GENMASK(58, 56)) + +static uint64_t encode_phys_addr(paddr_t addr, int page_bits) +{ + uint64_t ret; + + if ( page_bits < 16) + return (uint64_t)addr & GENMASK(47, page_bits); + + ret = addr & GENMASK(47, 16); + return ret | (addr & GENMASK(51, 48)) >> (48 - 12); +} + +static int gicv3_map_baser(void __iomem *basereg, uint64_t regc, int nr_items) +{ + uint64_t attr; + int entry_size = (regc >> GITS_BASER_ENTRY_SIZE_SHIFT) & 0x1f; + int pagesz; + int order; + void *buffer = NULL; + + attr = GIC_BASER_InnerShareable << GITS_BASER_SHAREABILITY_SHIFT; + attr |= GIC_BASER_CACHE_SameAsInner << GITS_BASER_OUTER_CACHEABILITY_SHIFT; + attr |= GIC_BASER_CACHE_RaWaWb << GITS_BASER_INNER_CACHEABILITY_SHIFT; + + /* + * Loop over the page sizes (4K, 16K, 64K) to find out what the host + * supports. + */ + for (pagesz = 0; pagesz < 3; pagesz++) + { + uint64_t reg; + int nr_bytes; + + nr_bytes = ROUNDUP(nr_items * entry_size, BIT(pagesz * 2 + 12)); + order = get_order_from_bytes(nr_bytes); + + if ( !buffer ) + buffer = alloc_xenheap_pages(order, 0); + if ( !buffer ) + return -ENOMEM; + + reg = attr; + reg |= (pagesz << GITS_BASER_PAGE_SIZE_SHIFT); + reg |= nr_bytes >> (pagesz * 2 + 12); + reg |= regc & BASER_RO_MASK; + reg |= GITS_BASER_VALID; + reg |= encode_phys_addr(virt_to_maddr(buffer), pagesz * 2 + 12); + + writeq_relaxed(reg, basereg); + regc = readl_relaxed(basereg); + + /* The host didn't like our attributes, just use what it returned. */ + if ( (regc & BASER_ATTR_MASK) != attr ) + attr = regc & BASER_ATTR_MASK; + + /* If the host accepted our page size, we are done. */ + if ( (reg & (3UL << GITS_BASER_PAGE_SIZE_SHIFT)) == pagesz ) + return 0; + + /* Check whether our buffer is aligned to the next page size already. */ + if ( !(virt_to_maddr(buffer) & (BIT(pagesz * 2 + 12 + 2) - 1)) ) + { + free_xenheap_pages(buffer, order); + buffer = NULL; + } + } + + if ( buffer ) + free_xenheap_pages(buffer, order); + + return -EINVAL; +} + +int gicv3_its_init(struct host_its *hw_its) +{ + uint64_t reg; + int i; + + hw_its->its_base = ioremap_nocache(hw_its->addr, hw_its->size); + if ( !hw_its->its_base ) + return -ENOMEM; + + for (i = 0; i < 8; i++) + { + void __iomem *basereg = hw_its->its_base + GITS_BASER0 + i * 8; + int type; + + reg = readq_relaxed(basereg); + type = (reg >> 56) & 0x7; + switch ( type ) + { + case GITS_BASER_TYPE_NONE: + continue; + case GITS_BASER_TYPE_DEVICE: + /* TODO: find some better way of limiting the number of devices */ + gicv3_map_baser(basereg, reg, 1024); + break; + case GITS_BASER_TYPE_COLLECTION: + gicv3_map_baser(basereg, reg, NR_CPUS); + break; + default: + continue; + } + } + + return 0; +} + uint64_t gicv3_lpi_allocate_pendtable(void) { uint64_t reg, attr; diff --git a/xen/arch/arm/gic-v3.c b/xen/arch/arm/gic-v3.c index 2534aa5..5cf4618 100644 --- a/xen/arch/arm/gic-v3.c +++ b/xen/arch/arm/gic-v3.c @@ -29,6 +29,7 @@ #include <xen/irq.h> #include <xen/iocap.h> #include <xen/sched.h> +#include <xen/err.h> #include <xen/errno.h> #include <xen/delay.h> #include <xen/device_tree.h> @@ -1548,6 +1549,7 @@ static int __init gicv3_init(void) { int res, i; uint32_t reg; + struct host_its *hw_its; if ( !cpu_has_gicv3 ) { @@ -1603,6 +1605,9 @@ static int __init gicv3_init(void) res = gicv3_cpu_init(); gicv3_hyp_init(); + list_for_each_entry(hw_its, &host_its_list, entry) + gicv3_its_init(hw_its); + spin_unlock(&gicv3.lock); return res; diff --git a/xen/include/asm-arm/gic-its.h b/xen/include/asm-arm/gic-its.h index 48c6c78..589b889 100644 --- a/xen/include/asm-arm/gic-its.h +++ b/xen/include/asm-arm/gic-its.h @@ -18,6 +18,47 @@ #ifndef __ASM_ARM_ITS_H__ #define __ASM_ARM_ITS_H__ +#define LPI_OFFSET 8192 + +#define GITS_CTLR (0x000) +#define GITS_IIDR (0x004) +#define GITS_TYPER (0x008) +#define GITS_CBASER (0x080) +#define GITS_CWRITER (0x088) +#define GITS_CREADR (0x090) +#define GITS_BASER0 (0x100) +#define GITS_BASER1 (0x108) +#define GITS_BASER2 (0x110) +#define GITS_BASER3 (0x118) +#define GITS_BASER4 (0x120) +#define GITS_BASER5 (0x128) +#define GITS_BASER6 (0x130) +#define GITS_BASER7 (0x138) + +/* Register bits */ +#define GITS_CTLR_ENABLE 0x1 +#define GITS_IIDR_VALUE 0x34c + +#define GITS_BASER_VALID BIT(63) +#define GITS_BASER_INDIRECT BIT(62) +#define GITS_BASER_INNER_CACHEABILITY_SHIFT 59 +#define GITS_BASER_TYPE_SHIFT 56 +#define GITS_BASER_OUTER_CACHEABILITY_SHIFT 53 +#define GITS_BASER_TYPE_NONE 0UL +#define GITS_BASER_TYPE_DEVICE 1UL +#define GITS_BASER_TYPE_VCPU 2UL +#define GITS_BASER_TYPE_CPU 3UL +#define GITS_BASER_TYPE_COLLECTION 4UL +#define GITS_BASER_TYPE_RESERVED5 5UL +#define GITS_BASER_TYPE_RESERVED6 6UL +#define GITS_BASER_TYPE_RESERVED7 7UL +#define GITS_BASER_ENTRY_SIZE_SHIFT 48 +#define GITS_BASER_SHAREABILITY_SHIFT 10 +#define GITS_BASER_PAGE_SIZE_SHIFT 8 +#define GITS_BASER_RO_MASK ((7UL << GITS_BASER_TYPE_SHIFT) | \ + (31UL << GITS_BASER_ENTRY_SIZE_SHIFT) |\ + GITS_BASER_INDIRECT) + #ifndef __ASSEMBLY__ #include <xen/device_tree.h> @@ -27,6 +68,7 @@ struct host_its { const struct dt_device_node *dt_node; paddr_t addr; paddr_t size; + void __iomem *its_base; }; extern struct list_head host_its_list; @@ -42,8 +84,9 @@ void gicv3_its_dt_init(const struct dt_device_node *node); uint64_t gicv3_lpi_get_proptable(void); uint64_t gicv3_lpi_allocate_pendtable(void); -/* Initialize the host structures for LPIs. */ +/* Initialize the host structures for LPIs and the host ITSes. */ int gicv3_lpi_init_host_lpis(int nr_lpis); +int gicv3_its_init(struct host_its *hw_its); #else @@ -62,6 +105,10 @@ static inline int gicv3_lpi_init_host_lpis(int nr_lpis) { return 0; } +static inline int gicv3_its_init(struct host_its *hw_its) +{ + return 0; +} #endif /* CONFIG_HAS_ITS */ #endif /* __ASSEMBLY__ */
Each ITS maps a pair of a DeviceID (usually the PCI b/d/f triplet) and an EventID (the MSI payload or interrupt ID) to a pair of LPI number and collection ID, which points to the target CPU. This mapping is stored in the device and collection tables, which software has to provide for the ITS to use. Allocate the required memory and hand it the ITS. We limit the number of devices to cover 4 PCI busses for now. Signed-off-by: Andre Przywara <andre.przywara@arm.com> --- xen/arch/arm/gic-its.c | 114 ++++++++++++++++++++++++++++++++++++++++++ xen/arch/arm/gic-v3.c | 5 ++ xen/include/asm-arm/gic-its.h | 49 +++++++++++++++++- 3 files changed, 167 insertions(+), 1 deletion(-)