Re: [Xen-devel] [RFC PATCH v1 08/10] xen/arm: Add support for GIC v3

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Sun, 2014-03-23 at 14:49 +0000, Julien Grall wrote:
> Hello Vijay,
> 
> For next time, can you try to quote only what you need? It's hard to 
> find your answer in a long mail.
> 
> On 22/03/14 10:21, Vijay Kilari wrote:
> > On Thu, Mar 20, 2014 at 10:10 PM, Julien Grall <julien.grall@xxxxxxxxxx> 
> > wrote:
> >> Please check all the file against CODING_STYLE. I won't shout anymore on
> >> every coding style error in this mail.
> > OK. Is there any script to check coding style of Xen?
> 
> Unfortunately no. It might be interesting to have one for Xen as the 
> coding style differs from Linux. People are often confusing between them :)
> 
> >>> +static void save_state(struct vcpu *v)
> >>> +{
> >>> +    int i;
> >>> +    struct gic_state_data *d;
> >>> +    d = (struct gic_state_data *)v->arch.gic_state;
> >>> +
> >>> +    /* No need for spinlocks here because interrupts are disabled around
> >>> +     * this call and it only accesses struct vcpu fields that cannot be
> >>> +     * accessed simultaneously by another pCPU.
> >>> +     */
> >>> +    for ( i=0; i<nr_lrs; i++)
> >>> +        d->gic_lr[i] = gich_read_lr(i);
> >>
> >> You are introducing a helper to read/write lr. How the compiler handle
> >> it? Will it optimize?
> >>
> >> For me it seems very slow...
> >    because LR registers are system registers, we have to
> > use READ/WRITE_SYSREG
> 
> Is it possible to read all of them (without looking at nr_lrs)?

What's the concern here? That the compiler doesn't inline and then do
dead code elimination of gich_read_lr to effectively unroll the loop
into a series of sysreg reads+memory stores?

> I see that KVM is using this solution, e.g not looping.

You mean they've just manually unrolled the loop? That's likely to be as
much to do with having to write it in asm any way as anything else.

I'm not saying it isn't worth measuring+optimising this code, but the
measuring bit should happen first not just guessing what might optimise
badly (a simple form of "measure" might be "look at the disassembly").

Ian.


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