[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
Hi Julien, On Fri, Apr 4, 2014 at 6:00 PM, Vijay Kilari <vijay.kilari@xxxxxxxxx> wrote: > On Fri, Apr 4, 2014 at 5:48 PM, Julien Grall <julien.grall@xxxxxxxxxx> wrote: >> 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? >> > OK can move into domain io_handler structure > >> [..] >> >>> + >>> +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. > My first version is without IRQ disable. I remember Andrii suggested it >> >>> + 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? > OK. > >> >>> @@ -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); >> > OK. Though you suggested, I kept this check handler because it gives flexibility for driver to make additional checks if required apart from just checking for address range. >>> >>> #endif _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |