Re: [Xen-devel] [PATCH v8a 5/6] xen/arm: split vgic driver into generic and vgic-v2 driver

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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);
?

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.

Ian.


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel