Re: [Xen-devel] [PATCH v5 11/28] xsplice: Implement support for applying/reverting/replacing patches.

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

> That's for an individual patch I suppose? What if REPLACE has to
> revert dozens or hundreds of patches?

I don't know. That is sometihng I need to figure out.
I also need to test this out on the 8 socket machine..

> > +void arch_xsplice_apply_jmp(struct xsplice_patch_func *func)
> > +{
> > +    uint32_t val;
> 
> The way it's being used below, it clearly should be int32_t.
> 
> > +    uint8_t *old_ptr;
> > +
> > +    BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->undo));
> > +    BUILD_BUG_ON(PATCH_INSN_SIZE != (1 + sizeof val));
> > +
> > +    old_ptr = (uint8_t *)func->old_addr;
> 
> (Considering this cast, the "old_addr" member should be
> unsigned long (or void *), not uint64_t: The latest on ARM32
> such would otherwise cause problems.)

I has to be uint8_t to make the single byte modifications. Keep
also in mind that this file is only for x86.

> 
> Also - where is the verification that
> func->old_size >= PATCH_INSN_SIZE?

I made a 'arch_xsplice_verify_func' to have per-architecture check for
that.
..
> > +        sec = xsplice_elf_sec_by_name(elf, names[i]);
> > +        if ( !sec )
> > +        {
> > +            printk(XENLOG_ERR "%s%s: %s is missing!\n",
> > +                   XSPLICE, elf->name, names[i]);
> > +            return -EINVAL;
> > +        }
> > +
> > +        if ( !sec->sec->sh_size )
> > +            return -EINVAL;
> > +    }
> > +
> > +    return 0;
> > +}
> 
> So you check for there being one such section. Is having multiple
> of them okay / meaningful?

I've been going back on this. I think at this point I will say no.
I am going to look at what is involved in this in later versions.

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