Re: [Xen-devel] [PATCH v5 1/7] xen/arm: Introduce alternative runtime patching

[Julien Grall <julien.grall@xxxxxxx>]

On 22/07/16 16:38, Konrad Rzeszutek Wilk wrote:
> > > > +
> > > > +        origptr = ALT_ORIG_PTR(alt);
> > > > +        writeptr = origptr - (u32 *)_start + writemap;
> > > How about just using writeptr += ?
> > I am not sure about your suggestion here. Regardless the Linux code, the
> > origptr will not follow a pattern at each iteration. So we have to recompute
> > it everytime.
> Just ignore that. I somehow equated writeptr to writemp.
>
> ..snip..
> > > Ah wait. You are trying to preserve the Linux code!. Nevermind then.
> > >
> > > > +        replptr = ALT_REPL_PTR(alt);
> > > > +
> > > > +        nr_inst = alt->alt_len / sizeof(insn);
> > > > +
> > > > +        for ( i = 0; i < nr_inst; i++ )
> > > > +        {
> > > > +            insn = get_alt_insn(alt, origptr + i, replptr + i);
> > > > +            *(writeptr + i) = cpu_to_le32(insn);
> > > > +        }
> > > > +
> > > > +        /* Ensure the new instructions reached the memory and nuke */
> > > > +        clean_and_invalidate_dcache_va_range(writeptr,
> > > > +                                             (sizeof (*writeptr) * nr_inst));
> > > > +    }
> > > > +
> > > > +    /* Nuke the instruction cache */
> > > > +    invalidate_icache();
> > > > +
> > > > +    vunmap(writemap);
> > > > +
> > > > +    return 0;
> > > > +}
> > > > +
> > > > +/*
> > > > + * We might be patching the stop_machine state machine, so implement a
> > > > + * really simple polling protocol here.
> > > > + */
> > > > +static int __apply_alternatives_multi_stop(void *unused)
> > > > +{
> > > > +    static int patched = 0;
> > > Shouldn't this be 'atomic_t' ?
> > Does it matter? From my understanding the code will behave the same.
> Not at all under the hood.
>  But I see 'atomic_write' and they all operate on the 'atomic_t' .. hence
> the query.
The code is using *_atomic (and not atomic_*) which operate on any type. 
I know the naming is really confusing.
I would like to rename write_atomic and read_atomic to WRITE_ONCE and 
READ_ONCE.
> ..snip.
> > > This being a new file perhaps add:
> > > *
> > > * Local variables:
> > > * mode: C
> > > * c-file-style: "BSD"
> > > * c-basic-offset: 4
> > > * indent-tabs-mode: nil
> > > * End:
> > > */
> > > ?
> > It is a Linux file with Linux coding style. I would need to look what should
> > be the emacs magic block here.
> Right. I just meant that you needed the magic block.
I don't think we ever put magic block on Linux file. I will try to find 
out and update the patch.
Cheers,

--
Julien Grall

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