[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 01/15] xen/arm: register mmio handler at runtime
Hello vijay, Thank your for the patch. On 04/04/2014 12:56 PM, vijay.kilari@xxxxxxxxx wrote: [..] > -#define MMIO_HANDLER_NR ARRAY_SIZE(mmio_handlers) > +static DEFINE_SPINLOCK(handler_lock); Why a global lock rather than a domain lock? [..] > + > +void register_mmio_handler(struct domain *d, struct mmio_handler * handle) > +{ > + unsigned long flags; > + struct io_handler *handler = d->arch.io_handlers; > + BUG_ON(handler->num_entries >= MAX_IO_HANDLER); > + > + spin_lock_irqsave(&handler_lock, flags); You don't need to disable the IRQ here. > + handler->mmio_handlers[handler->num_entries++] = handle; > + spin_unlock_irqrestore(&handler_lock, flags); > +} > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/io.h b/xen/arch/arm/io.h > index 8d252c0..5fc1660 100644 > --- a/xen/arch/arm/io.h > +++ b/xen/arch/arm/io.h As you are modifying this header. Can you move it in include/asm-arm? > @@ -40,10 +40,14 @@ struct mmio_handler { > mmio_write_t write_handler; > }; > > -extern const struct mmio_handler vgic_distr_mmio_handler; > -extern const struct mmio_handler vuart_mmio_handler; > +#define MAX_IO_HANDLER 16 > +struct io_handler { > + int num_entries; > + struct mmio_handler *mmio_handlers[MAX_IO_HANDLER]; > +}; > > extern int handle_mmio(mmio_info_t *info); > +void register_mmio_handler(struct domain *d, struct mmio_handler * handle); As I said on the previous version, it would be nice to remove check callback in the mmio_handler and replace it by addr/size. It's better if we might want to change the place in the memory following the guest. So the result function would be: register_mmio_handler(struct domain *d, read_t read, write_t write, addr, size); > > #endif > > diff --git a/xen/arch/arm/vgic.c b/xen/arch/arm/vgic.c > index 8616534..77b561e 100644 > --- a/xen/arch/arm/vgic.c > +++ b/xen/arch/arm/vgic.c > @@ -35,6 +35,7 @@ > /* Number of ranks of interrupt registers for a domain */ > #define DOMAIN_NR_RANKS(d) (((d)->arch.vgic.nr_lines+31)/32) > > +static struct mmio_handler vgic_distr_mmio_handler; > /* > * Rank containing GICD_<FOO><n> for GICD_<FOO> with > * <b>-bits-per-interrupt > @@ -107,6 +108,7 @@ int domain_vgic_init(struct domain *d) > } > for (i=0; i<DOMAIN_NR_RANKS(d); i++) > spin_lock_init(&d->arch.vgic.shared_irqs[i].lock); > + register_mmio_handler(d, &vgic_distr_mmio_handler); > return 0; > } > > @@ -673,7 +675,7 @@ static int vgic_distr_mmio_check(struct vcpu *v, paddr_t > addr) > return (addr >= (d->arch.vgic.dbase)) && (addr < (d->arch.vgic.dbase + > PAGE_SIZE)); > } > > -const struct mmio_handler vgic_distr_mmio_handler = { > +static struct mmio_handler vgic_distr_mmio_handler = { Why did you remove the const? [..] > diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h > index 50b9b54..23dac85 100644 > --- a/xen/include/asm-arm/domain.h > +++ b/xen/include/asm-arm/domain.h > @@ -116,6 +116,7 @@ struct arch_domain > struct hvm_domain hvm_domain; > xen_pfn_t *grant_table_gpfn; > > + struct io_handler *io_handlers; Why do you need a pointer here? I think can can directly use struct io_handler iohandlers. Regards, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |