[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver
On 07/03/2014 03:02 PM, Ian Campbell wrote: > On Thu, 2014-07-03 at 14:25 +0100, Julien Grall wrote: >> On 07/03/2014 02:02 PM, Ian Campbell wrote: >>> On Thu, 2014-07-03 at 13:57 +0100, Julien Grall wrote: >>> >>>>> +struct vgic_ops { >>>>> + /* Initialize vGIC */ >>>>> + int (*vcpu_init)(struct vcpu *v); >>>>> + /* Domain specific initialization of vGIC */ >>>>> + int (*domain_init)(struct domain *d); >>>>> + /* SGI handler of vGIC */ >>>>> + int (*send_sgi)(struct vcpu *v, register_t sgir); >>>> >>>> By reviewing the VGIC-v3 support, I still don't think this is the right >>>> callback to add. You bypass the VGIC common emulation with your >>>> vgic_emulate... >>>> >>>> I would introduce a callback to emulate_sysreg rather than this send_sgi. >>> >>> Why? The vgic will either be v2 or v3, so either MMIO or sysreg, once >>> the thing has been decoded then you want to send an SGI I think, hence >>> the callback. Passing a register_t does seem odd though, I'd have >>> thought it would take an SGI number and any other flags which would then >>> be interpreted for either v2 or v3 as appropriate. >> >> The decoding depends on the vgic emulation. For now this function is >> badly implement in vgic-v3.c. >> >> What I was trying to say is send_sgi can be handled internally. If you >> are looking to the calls of this function, it's only happen within the >> file vgic-v2.c (or vgic-v3.c) >> >> But, the sysreg emulation is called outside the vgic code. So we should >> add a callaback for this. > > So the common code would have > case HSR_SYSREG_ICC_SGI0R: > gic->handle_sysreg(esr, val) > instead of > case HSR_SYSREG_ICC_SGI0R: > gic->handle_sysreg(val); > ? The is not the actual case, Actually, the common code is: case HSR_SYSREG_ICC_SGI0R: vgic_emulate(regs, hsr) where vgic_emulate is implemented in vgic-v3.c rather than in vgic.c. The function will decode the register and then call vgic_send_sgi. But, as the function send_sgi is only used internaly there is no reason to create a callback. What I ask is to have a new callback emulate_sysreg. The common code (i.e traps.c) will have: case HSR_SYSREG_ICC_SGI0R: vgic_emulate(regs, hsr). The function vgic_emulate will be implemented in vgic.c: vgic_emulate(...) { vgic->emulate_sysreg(regs, hsr); } The vgic-v3.c will implement the callback correctly. > That might be nicer, but TBH given that there is only one trappable gic > sysreg right now I don't think it is worth getting too worried about. We > can always rework this interface when gic v4 or v5 needs something more. I don't see why we should break the vgic common implementation as it does on the next series: http://www.gossamer-threads.com/lists/xen/devel/337708. 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 |