| Message ID | 1e2ebd306719ae2fbc37b050349a8299dc22b1b0.1367188423.git.julien.grall@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
On Mon, 2013-04-29 at 00:01 +0100, Julien Grall wrote: > @@ -754,11 +759,21 @@ void gic_interrupt(struct cpu_user_regs *regs, int is_fiq) > > int gicv_setup(struct domain *d) > { > + /* TODO: Retrieve distributor and CPU guest base address from the > + * guest DTS > + * For the moment we use dom0 DTS FWIW I think what we will need here eventually is domctl's so the toolstack can set this stuff explicitly to match the DT it generates, not to pass the guest DTB to the hypervisor and parse it or anything like that (maybe that's not what you were suggesting). > /* Number of ranks of interrupt registers for a domain */ > @@ -79,7 +77,16 @@ int domain_vgic_init(struct domain *d) > int i; > > d->arch.vgic.ctlr = 0; > - d->arch.vgic.nr_lines = 32; > + > + /** Javadoc? ;-) Other than those two nits: Acked-by: Ian Campbell <ian.campbell@citrix.com> > + * Currently nr_lines in vgic and gic doesn't have the same meanings > + * Here nr_lines = number of SPIs > + */ > + if ( d->domain_id == 0 ) > + d->arch.vgic.nr_lines = gic_number_lines() - 32; > + else > + d->arch.vgic.nr_lines = 0; /* We don't need SPIs for the guest */ > +
On 04/29/2013 04:41 PM, Ian Campbell wrote: > On Mon, 2013-04-29 at 00:01 +0100, Julien Grall wrote: >> @@ -754,11 +759,21 @@ void gic_interrupt(struct cpu_user_regs *regs, int is_fiq) >> >> int gicv_setup(struct domain *d) >> { >> + /* TODO: Retrieve distributor and CPU guest base address from the >> + * guest DTS >> + * For the moment we use dom0 DTS > > FWIW I think what we will need here eventually is domctl's so the > toolstack can set this stuff explicitly to match the DT it generates, > not to pass the guest DTB to the hypervisor and parse it or anything > like that (maybe that's not what you were suggesting). I don't have a particular plan for parsing/generate the guest DTB. We can: 1) Replace the GIC base address in the guest DTB 2) Use the GIC base address found in the DTB I would prefer the second solution, and use the first when the node doesn't exist. I believe the guest DTB is currently appended to the kernel. So do we really need to create a new hypercall for this purpose? We can add some logic in the toolstack to take the kernel and the device tree in arguments and concatenate it. >> /* Number of ranks of interrupt registers for a domain */ >> @@ -79,7 +77,16 @@ int domain_vgic_init(struct domain *d) >> int i; >> >> d->arch.vgic.ctlr = 0; >> - d->arch.vgic.nr_lines = 32; >> + >> + /** > > Javadoc? ;-) Right. Bad habit :). I will replace by /*.
On Mon, 2013-04-29 at 17:42 +0100, Julien Grall wrote: > On 04/29/2013 04:41 PM, Ian Campbell wrote: > > > On Mon, 2013-04-29 at 00:01 +0100, Julien Grall wrote: > >> @@ -754,11 +759,21 @@ void gic_interrupt(struct cpu_user_regs *regs, int is_fiq) > >> > >> int gicv_setup(struct domain *d) > >> { > >> + /* TODO: Retrieve distributor and CPU guest base address from the > >> + * guest DTS > >> + * For the moment we use dom0 DTS > > > > FWIW I think what we will need here eventually is domctl's so the > > toolstack can set this stuff explicitly to match the DT it generates, > > not to pass the guest DTB to the hypervisor and parse it or anything > > like that (maybe that's not what you were suggesting). > > I don't have a particular plan for parsing/generate the guest DTB. > We can: > 1) Replace the GIC base address in the guest DTB > 2) Use the GIC base address found in the DTB > > I would prefer the second solution, and use the first when the node > doesn't exist. Use the second -- yes. Use the first when the node doesn't exist -- no, the tools should ensure it always exists. > I believe the guest DTB is currently appended to the kernel. So do we > really need to create a new hypercall for this purpose? > We can add some logic in the toolstack to take the kernel and the device > tree in arguments and concatenate it. We cannot rely on CONFIG_ARM_APPEND_DTB long term, eventually the toolstack needs to generate the DTB corresponding to the guest configuration and supply it to the kernel through the appropriate register. In any case regardless of whether the guest DTB is appended or in a register we do not want the hypervisor to have to parse the guest DTB. > >> /* Number of ranks of interrupt registers for a domain */ > >> @@ -79,7 +77,16 @@ int domain_vgic_init(struct domain *d) > >> int i; > >> > >> d->arch.vgic.ctlr = 0; > >> - d->arch.vgic.nr_lines = 32; > >> + > >> + /** > > > > Javadoc? ;-) > > > Right. Bad habit :). I will replace by /*. >
diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c index ea34e9c..bf0c1fd 100644 --- a/xen/arch/arm/gic.c +++ b/xen/arch/arm/gic.c @@ -56,6 +56,11 @@ static DEFINE_PER_CPU(uint64_t, lr_mask); static unsigned nr_lrs; +unsigned int gic_number_lines(void) +{ + return gic.lines; +} + irq_desc_t *__irq_to_desc(int irq) { if (irq < NR_LOCAL_IRQS) return &this_cpu(local_irq_desc)[irq]; @@ -754,11 +759,21 @@ void gic_interrupt(struct cpu_user_regs *regs, int is_fiq) int gicv_setup(struct domain *d) { + /* TODO: Retrieve distributor and CPU guest base address from the + * guest DTS + * For the moment we use dom0 DTS + */ + d->arch.vgic.dbase = gic.dbase; + d->arch.vgic.cbase = gic.cbase; + + + d->arch.vgic.nr_lines = 0; + /* map the gic virtual cpu interface in the gic cpu interface region of * the guest */ - return map_mmio_regions(d, gic.cbase, - gic.cbase + (2 * PAGE_SIZE) - 1, - gic.vbase); + return map_mmio_regions(d, d->arch.vgic.cbase, + d->arch.vgic.cbase + (2 * PAGE_SIZE) - 1, + gic.vbase); } static void maintenance_interrupt(int irq, void *dev_id, struct cpu_user_regs *regs) diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c index 6aaafe9..cb39848 100644 --- a/xen/arch/arm/vgic.c +++ b/xen/arch/arm/vgic.c @@ -29,8 +29,6 @@ #include "io.h" #include <asm/gic.h> -#define VGIC_DISTR_BASE_ADDRESS 0x000000002c001000 - #define REG(n) (n/4) /* Number of ranks of interrupt registers for a domain */ @@ -79,7 +77,16 @@ int domain_vgic_init(struct domain *d) int i; d->arch.vgic.ctlr = 0; - d->arch.vgic.nr_lines = 32; + + /** + * Currently nr_lines in vgic and gic doesn't have the same meanings + * Here nr_lines = number of SPIs + */ + if ( d->domain_id == 0 ) + d->arch.vgic.nr_lines = gic_number_lines() - 32; + else + d->arch.vgic.nr_lines = 0; /* We don't need SPIs for the guest */ + d->arch.vgic.shared_irqs = xmalloc_array(struct vgic_irq_rank, DOMAIN_NR_RANKS(d)); d->arch.vgic.pending_irqs = @@ -162,7 +169,7 @@ static int vgic_distr_mmio_read(struct vcpu *v, mmio_info_t *info) struct cpu_user_regs *regs = guest_cpu_user_regs(); register_t *r = select_user_reg(regs, dabt.reg); struct vgic_irq_rank *rank; - int offset = (int)(info->gpa - VGIC_DISTR_BASE_ADDRESS); + int offset = (int)(info->gpa - v->domain->arch.vgic.dbase); int gicd_reg = REG(offset); switch ( gicd_reg ) @@ -375,7 +382,7 @@ static int vgic_distr_mmio_write(struct vcpu *v, mmio_info_t *info) struct cpu_user_regs *regs = guest_cpu_user_regs(); register_t *r = select_user_reg(regs, dabt.reg); struct vgic_irq_rank *rank; - int offset = (int)(info->gpa - VGIC_DISTR_BASE_ADDRESS); + int offset = (int)(info->gpa - v->domain->arch.vgic.dbase); int gicd_reg = REG(offset); uint32_t tr; @@ -597,7 +604,9 @@ write_ignore: static int vgic_distr_mmio_check(struct vcpu *v, paddr_t addr) { - return addr >= VGIC_DISTR_BASE_ADDRESS && addr < (VGIC_DISTR_BASE_ADDRESS+PAGE_SIZE); + struct domain *d = v->domain; + + return (addr >= (d->arch.vgic.dbase)) && (addr < (d->arch.vgic.dbase + PAGE_SIZE)); } const struct mmio_handler vgic_distr_mmio_handler = { diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h index 410b8e7..a083eaf 100644 --- a/xen/include/asm-arm/domain.h +++ b/xen/include/asm-arm/domain.h @@ -90,13 +90,16 @@ struct arch_domain */ spinlock_t lock; int ctlr; - int nr_lines; + int nr_lines; /* Number of SPIs */ struct vgic_irq_rank *shared_irqs; /* * SPIs are domain global, SGIs and PPIs are per-VCPU and stored in * struct arch_vcpu. */ struct pending_irq *pending_irqs; + /* Base address for guest GIC */ + paddr_t dbase; /* Distributor base address */ + paddr_t cbase; /* CPU base address */ } vgic; struct vpl011 { diff --git a/xen/include/asm-arm/gic.h b/xen/include/asm-arm/gic.h index 945e8db..127a501 100644 --- a/xen/include/asm-arm/gic.h +++ b/xen/include/asm-arm/gic.h @@ -187,6 +187,9 @@ extern void send_SGI_allbutself(enum gic_sgi sgi); /* print useful debug info */ extern void gic_dump_info(struct vcpu *v); +/* Number of interrupt lines */ +extern unsigned int gic_number_lines(void); + /* IRQ translation function for the device tree */ int gic_irq_xlate(const u32 *intspec, unsigned int intsize, unsigned int *out_hwirq, unsigned int *out_type);
- Define VGIC base address per domain. For the moment the base addresses to dom0 base addresses. - The number of interrupt lines (ie number of SPIs) is equal to: * 0 for guests * number of host SPIs for dom0 Signed-off-by: Julien Grall <julien.grall@linaro.org> --- xen/arch/arm/gic.c | 21 ++++++++++++++++++--- xen/arch/arm/vgic.c | 21 +++++++++++++++------ xen/include/asm-arm/domain.h | 5 ++++- xen/include/asm-arm/gic.h | 3 +++ 4 files changed, 40 insertions(+), 10 deletions(-)