| Message ID | 9254e9b6dc51564636ba767181366835d3859488.1367979526.git.julien.grall@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Wed, 2013-05-08 at 03:33 +0100, Julien Grall wrote: > - Remove early parsing for GIC addresses > - Remove hard coded maintenance IRQ number > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > Changes in v2: > - use the new function request_dt_irq > --- > xen/arch/arm/gic.c | 63 ++++++++++++++++++++++++++++------------- > xen/common/device_tree.c | 42 --------------------------- > xen/include/xen/device_tree.h | 8 ------ > 3 files changed, 43 insertions(+), 70 deletions(-) > > diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c > index 1efa9a3..34304b3 100644 > --- a/xen/arch/arm/gic.c > +++ b/xen/arch/arm/gic.c > @@ -45,6 +45,7 @@ static struct { > paddr_t hbase; /* Address of virtual interface registers */ > paddr_t vbase; /* Address of virtual cpu interface registers */ > unsigned int lines; /* Number of interrupts (SPIs + PPIs + SGIs) */ > + struct dt_irq maintenance; /* IRQ maintenance */ > unsigned int cpus; > spinlock_t lock; > } gic; > @@ -352,28 +353,49 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, > /* Set up the GIC */ > void __init gic_init(void) > { > + struct dt_device_node *node; > + int res; > + > + node = dt_find_interrupt_controller("arm,cortex-a15-gic"); > + if ( !node ) > + panic("Unable to find compatible GIC in the device tree\n"); > + > + dt_device_set_used_by(node, DT_USED_BY_XEN); > + > + res = dt_device_get_address(node, 0, &gic.dbase, NULL); > + if ( res || !gic.dbase || (gic.dbase & ~PAGE_MASK) ) > + panic("GIC: Cannot find a valid address for the distributor\n"); > + > + res = dt_device_get_address(node, 1, &gic.cbase, NULL); > + if ( res || !gic.cbase || (gic.cbase & ~PAGE_MASK) ) > + panic("GIC: Cannot find a valid address for the CPU\n"); > + > + res = dt_device_get_address(node, 2, &gic.hbase, NULL); > + if ( res || !gic.hbase || (gic.hbase & ~PAGE_MASK) ) > + panic("GIC: Cannot find a valid address for the hypervisor\n"); > + > + res = dt_device_get_address(node, 3, &gic.vbase, NULL); > + if ( res || !gic.vbase || (gic.vbase & ~PAGE_MASK) ) > + panic("GIC: Cannot find a valid address for the virtual CPU\n"); > + > + res = dt_device_get_irq(node, 0, &gic.maintenance); > + if ( res ) > + panic("GIC: Cannot find the maintenance IRQ\n"); > + > + /* Set the GIC as the primary interrupt controller */ > + dt_interrupt_controller = node; > + > + /* TODO: Add check on distributor, cpu size */ > + > printk("GIC initialization:\n" > " gic_dist_addr=%"PRIpaddr"\n" > " gic_cpu_addr=%"PRIpaddr"\n" > " gic_hyp_addr=%"PRIpaddr"\n" > - " gic_vcpu_addr=%"PRIpaddr"\n", > - early_info.gic.gic_dist_addr, early_info.gic.gic_cpu_addr, > - early_info.gic.gic_hyp_addr, early_info.gic.gic_vcpu_addr); > - if ( !early_info.gic.gic_dist_addr || > - !early_info.gic.gic_cpu_addr || > - !early_info.gic.gic_hyp_addr || > - !early_info.gic.gic_vcpu_addr ) > - panic("the physical address of one of the GIC interfaces is missing\n"); > - if ( (early_info.gic.gic_dist_addr & ~PAGE_MASK) || > - (early_info.gic.gic_cpu_addr & ~PAGE_MASK) || > - (early_info.gic.gic_hyp_addr & ~PAGE_MASK) || > - (early_info.gic.gic_vcpu_addr & ~PAGE_MASK) ) > - panic("GIC interfaces not page aligned.\n"); Please can we keep this last check separate and after the print, this way we get to see why it failed (not present vs invalid). I'm ambivalent about keeping the !gic.Xbase separate or including it in the res check. Ian.
On 05/08/2013 02:46 PM, Ian Campbell wrote: > On Wed, 2013-05-08 at 03:33 +0100, Julien Grall wrote: >> - Remove early parsing for GIC addresses >> - Remove hard coded maintenance IRQ number >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> >> Changes in v2: >> - use the new function request_dt_irq >> --- >> xen/arch/arm/gic.c | 63 ++++++++++++++++++++++++++++------------- >> xen/common/device_tree.c | 42 --------------------------- >> xen/include/xen/device_tree.h | 8 ------ >> 3 files changed, 43 insertions(+), 70 deletions(-) >> >> diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c >> index 1efa9a3..34304b3 100644 >> --- a/xen/arch/arm/gic.c >> +++ b/xen/arch/arm/gic.c >> @@ -45,6 +45,7 @@ static struct { >> paddr_t hbase; /* Address of virtual interface registers */ >> paddr_t vbase; /* Address of virtual cpu interface registers */ >> unsigned int lines; /* Number of interrupts (SPIs + PPIs + SGIs) */ >> + struct dt_irq maintenance; /* IRQ maintenance */ >> unsigned int cpus; >> spinlock_t lock; >> } gic; >> @@ -352,28 +353,49 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, >> /* Set up the GIC */ >> void __init gic_init(void) >> { >> + struct dt_device_node *node; >> + int res; >> + >> + node = dt_find_interrupt_controller("arm,cortex-a15-gic"); >> + if ( !node ) >> + panic("Unable to find compatible GIC in the device tree\n"); >> + >> + dt_device_set_used_by(node, DT_USED_BY_XEN); >> + >> + res = dt_device_get_address(node, 0, &gic.dbase, NULL); >> + if ( res || !gic.dbase || (gic.dbase & ~PAGE_MASK) ) >> + panic("GIC: Cannot find a valid address for the distributor\n"); >> + >> + res = dt_device_get_address(node, 1, &gic.cbase, NULL); >> + if ( res || !gic.cbase || (gic.cbase & ~PAGE_MASK) ) >> + panic("GIC: Cannot find a valid address for the CPU\n"); >> + >> + res = dt_device_get_address(node, 2, &gic.hbase, NULL); >> + if ( res || !gic.hbase || (gic.hbase & ~PAGE_MASK) ) >> + panic("GIC: Cannot find a valid address for the hypervisor\n"); >> + >> + res = dt_device_get_address(node, 3, &gic.vbase, NULL); >> + if ( res || !gic.vbase || (gic.vbase & ~PAGE_MASK) ) >> + panic("GIC: Cannot find a valid address for the virtual CPU\n"); >> + >> + res = dt_device_get_irq(node, 0, &gic.maintenance); >> + if ( res ) >> + panic("GIC: Cannot find the maintenance IRQ\n"); >> + >> + /* Set the GIC as the primary interrupt controller */ >> + dt_interrupt_controller = node; >> + >> + /* TODO: Add check on distributor, cpu size */ >> + >> printk("GIC initialization:\n" >> " gic_dist_addr=%"PRIpaddr"\n" >> " gic_cpu_addr=%"PRIpaddr"\n" >> " gic_hyp_addr=%"PRIpaddr"\n" >> - " gic_vcpu_addr=%"PRIpaddr"\n", >> - early_info.gic.gic_dist_addr, early_info.gic.gic_cpu_addr, >> - early_info.gic.gic_hyp_addr, early_info.gic.gic_vcpu_addr); >> - if ( !early_info.gic.gic_dist_addr || >> - !early_info.gic.gic_cpu_addr || >> - !early_info.gic.gic_hyp_addr || >> - !early_info.gic.gic_vcpu_addr ) >> - panic("the physical address of one of the GIC interfaces is missing\n"); >> - if ( (early_info.gic.gic_dist_addr & ~PAGE_MASK) || >> - (early_info.gic.gic_cpu_addr & ~PAGE_MASK) || >> - (early_info.gic.gic_hyp_addr & ~PAGE_MASK) || >> - (early_info.gic.gic_vcpu_addr & ~PAGE_MASK) ) >> - panic("GIC interfaces not page aligned.\n"); > > Please can we keep this last check separate and after the print, this > way we get to see why it failed (not present vs invalid). I will fix it on the next patch series. > > I'm ambivalent about keeping the !gic.Xbase separate or including it in > the res check. I don't see a good reason why the gic base address could not be 0. I prefer to remove this check.
On Wed, 2013-05-08 at 16:49 +0100, Julien Grall wrote: > On 05/08/2013 02:46 PM, Ian Campbell wrote: > > Please can we keep this last check separate and after the print, this > > way we get to see why it failed (not present vs invalid). > > > I will fix it on the next patch series. Thanks. > > > > I'm ambivalent about keeping the !gic.Xbase separate or including it in > > the res check. > > > I don't see a good reason why the gic base address could not be 0. I > prefer to remove this check. True, agreed. Ian.
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index 1efa9a3..34304b3 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -45,6 +45,7 @@ static struct { paddr_t hbase; /* Address of virtual interface registers */ paddr_t vbase; /* Address of virtual cpu interface registers */ unsigned int lines; /* Number of interrupts (SPIs + PPIs + SGIs) */ + struct dt_irq maintenance; /* IRQ maintenance */ unsigned int cpus; spinlock_t lock; } gic; @@ -352,28 +353,49 @@ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, /* Set up the GIC */ void __init gic_init(void) { + struct dt_device_node *node; + int res; + + node = dt_find_interrupt_controller("arm,cortex-a15-gic"); + if ( !node ) + panic("Unable to find compatible GIC in the device tree\n"); + + dt_device_set_used_by(node, DT_USED_BY_XEN); + + res = dt_device_get_address(node, 0, &gic.dbase, NULL); + if ( res || !gic.dbase || (gic.dbase & ~PAGE_MASK) ) + panic("GIC: Cannot find a valid address for the distributor\n"); + + res = dt_device_get_address(node, 1, &gic.cbase, NULL); + if ( res || !gic.cbase || (gic.cbase & ~PAGE_MASK) ) + panic("GIC: Cannot find a valid address for the CPU\n"); + + res = dt_device_get_address(node, 2, &gic.hbase, NULL); + if ( res || !gic.hbase || (gic.hbase & ~PAGE_MASK) ) + panic("GIC: Cannot find a valid address for the hypervisor\n"); + + res = dt_device_get_address(node, 3, &gic.vbase, NULL); + if ( res || !gic.vbase || (gic.vbase & ~PAGE_MASK) ) + panic("GIC: Cannot find a valid address for the virtual CPU\n"); + + res = dt_device_get_irq(node, 0, &gic.maintenance); + if ( res ) + panic("GIC: Cannot find the maintenance IRQ\n"); + + /* Set the GIC as the primary interrupt controller */ + dt_interrupt_controller = node; + + /* TODO: Add check on distributor, cpu size */ + printk("GIC initialization:\n" " gic_dist_addr=%"PRIpaddr"\n" " gic_cpu_addr=%"PRIpaddr"\n" " gic_hyp_addr=%"PRIpaddr"\n" - " gic_vcpu_addr=%"PRIpaddr"\n", - early_info.gic.gic_dist_addr, early_info.gic.gic_cpu_addr, - early_info.gic.gic_hyp_addr, early_info.gic.gic_vcpu_addr); - if ( !early_info.gic.gic_dist_addr || - !early_info.gic.gic_cpu_addr || - !early_info.gic.gic_hyp_addr || - !early_info.gic.gic_vcpu_addr ) - panic("the physical address of one of the GIC interfaces is missing\n"); - if ( (early_info.gic.gic_dist_addr & ~PAGE_MASK) || - (early_info.gic.gic_cpu_addr & ~PAGE_MASK) || - (early_info.gic.gic_hyp_addr & ~PAGE_MASK) || - (early_info.gic.gic_vcpu_addr & ~PAGE_MASK) ) - panic("GIC interfaces not page aligned.\n"); - - gic.dbase = early_info.gic.gic_dist_addr; - gic.cbase = early_info.gic.gic_cpu_addr; - gic.hbase = early_info.gic.gic_hyp_addr; - gic.vbase = early_info.gic.gic_vcpu_addr; + " gic_vcpu_addr=%"PRIpaddr"\n" + " gic_maintenance_irq=%u\n", + gic.dbase, gic.cbase, gic.hbase, gic.vbase, + gic.maintenance.irq); + set_fixmap(FIXMAP_GICD, gic.dbase >> PAGE_SHIFT, DEV_SHARED); BUILD_BUG_ON(FIXMAP_ADDR(FIXMAP_GICC1) != FIXMAP_ADDR(FIXMAP_GICC2)-PAGE_SIZE); @@ -464,7 +486,7 @@ void gic_route_ppis(void) { /* XXX should get these from DT */ /* GIC maintenance */ - gic_route_irq(25, 1, 1u << smp_processor_id(), 0xa0); + gic_route_dt_irq(&gic.maintenance, 1u << smp_processor_id(), 0xa0); /* Hypervisor Timer */ gic_route_irq(26, 1, 1u << smp_processor_id(), 0xa0); /* Virtual Timer */ @@ -830,7 +852,8 @@ void gic_dump_info(struct vcpu *v) void __cpuinit init_maintenance_interrupt(void) { - request_irq(25, maintenance_interrupt, 0, "irq-maintenance", NULL); + request_dt_irq(&gic.maintenance, maintenance_interrupt, + 0, "irq-maintenance", NULL); } /* diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c index e5ff779..548d21b 100644 --- a/xen/common/device_tree.c +++ b/xen/common/device_tree.c @@ -425,46 +425,6 @@ static void __init process_cpu_node(const void *fdt, int node, cpumask_set_cpu(start, &cpu_possible_map); } -static void __init process_gic_node(const void *fdt, int node, - const char *name, - u32 address_cells, u32 size_cells) -{ - const struct fdt_property *prop; - const u32 *cell; - paddr_t start, size; - int interfaces; - - if ( address_cells < 1 || size_cells < 1 ) - { - early_printk("fdt: node `%s': invalid #address-cells or #size-cells", - name); - return; - } - - prop = fdt_get_property(fdt, node, "reg", NULL); - if ( !prop ) - { - early_printk("fdt: node `%s': missing `reg' property\n", name); - return; - } - - cell = (const u32 *)prop->data; - interfaces = device_tree_nr_reg_ranges(prop, address_cells, size_cells); - if ( interfaces < 4 ) - { - early_printk("fdt: node `%s': not enough ranges\n", name); - return; - } - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); - early_info.gic.gic_dist_addr = start; - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); - early_info.gic.gic_cpu_addr = start; - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); - early_info.gic.gic_hyp_addr = start; - device_tree_get_reg(&cell, address_cells, size_cells, &start, &size); - early_info.gic.gic_vcpu_addr = start; -} - static void __init process_multiboot_node(const void *fdt, int node, const char *name, u32 address_cells, u32 size_cells) @@ -516,8 +476,6 @@ static int __init early_scan_node(const void *fdt, process_memory_node(fdt, node, name, address_cells, size_cells); else if ( device_tree_type_matches(fdt, node, "cpu") ) process_cpu_node(fdt, node, name, address_cells, size_cells); - else if ( device_tree_node_compatible(fdt, node, "arm,cortex-a15-gic") ) - process_gic_node(fdt, node, name, address_cells, size_cells); else if ( device_tree_node_compatible(fdt, node, "xen,multiboot-module" ) ) process_multiboot_node(fdt, node, name, address_cells, size_cells); diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h index bf873d9..deef5e7 100644 --- a/xen/include/xen/device_tree.h +++ b/xen/include/xen/device_tree.h @@ -31,13 +31,6 @@ struct dt_mem_info { struct membank bank[NR_MEM_BANKS]; }; -struct dt_gic_info { - paddr_t gic_dist_addr; - paddr_t gic_cpu_addr; - paddr_t gic_hyp_addr; - paddr_t gic_vcpu_addr; -}; - struct dt_mb_module { paddr_t start; paddr_t size; @@ -52,7 +45,6 @@ struct dt_module_info { struct dt_early_info { struct dt_mem_info mem; - struct dt_gic_info gic; struct dt_module_info modules; };
- Remove early parsing for GIC addresses - Remove hard coded maintenance IRQ number Signed-off-by: Julien Grall <julien.grall@linaro.org> Changes in v2: - use the new function request_dt_irq --- xen/arch/arm/gic.c | 63 ++++++++++++++++++++++++++++------------- xen/common/device_tree.c | 42 --------------------------- xen/include/xen/device_tree.h | 8 ------ 3 files changed, 43 insertions(+), 70 deletions(-)