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

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

> >>+
> >>+        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. 

..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.
> 
> Regards,
> 
> -- 
> Julien Grall

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