>>> On 07.03.18 at 16:51, <andrew.coop...@citrix.com> wrote:
> @@ -175,18 +175,47 @@ void init_or_livepatch apply_alternatives(const struct
> alt_instr *start,
> * So be careful if you want to change the scan order to any other
> * order.
> */
> - for ( a = start; a < end; a++ )
> + for ( a = base = start; a < end; a++ )
> {
> uint8_t *orig = ALT_ORIG_PTR(a);
> uint8_t *repl = ALT_REPL_PTR(a);
> uint8_t buf[MAX_PATCH_LEN];
> + unsigned int total_len = a->orig_len + a->pad_len;
>
> - BUG_ON(a->repl_len > a->orig_len);
> - BUG_ON(a->orig_len > sizeof(buf));
> + BUG_ON(a->repl_len > total_len);
> + BUG_ON(total_len > sizeof(buf));
> BUG_ON(a->cpuid >= NCAPINTS * 32);
>
> + /*
> + * Detect sequences of alt_instr's patching the same origin site, and
> + * keep base pointing at the first alt_instr entry. This is so we
> can
> + * refer to a single ->priv field for patching decisions.
> + *
> + * ->priv being nonzero means that the origin site has already been
> + * modified, and we shouldn't try to optimise the nops again.
> + */
> + if ( ALT_ORIG_PTR(base) != orig )
> + base = a;
I don't understand why you need the new "priv" field - have a
boolean local variable which you reset instead of base here, and
which you check/set instead of base->priv below.
Everything else looks fine to me.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
