Re: [Xen-devel] [PATCH v7 10/24] xsplice: Implement support for applying/reverting/replacing patches.

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

On Mon, Apr 11, 2016 at 11:14:46AM +0100, Andrew Cooper wrote:
> On 10/04/16 22:14, Konrad Rzeszutek Wilk wrote:
> > diff --git a/xen/common/xsplice.c b/xen/common/xsplice.c
> > index cbee264..9a53cf4 100644
> > --- a/xen/common/xsplice.c
> > +++ b/xen/common/xsplice.c
> > @@ -3,6 +3,7 @@
> >   *
> >   */
> >  
> > +#include <xen/cpu.h>
> >  #include <xen/err.h>
> >  #include <xen/guest_access.h>
> >  #include <xen/keyhandler.h>
> > @@ -11,17 +12,28 @@
> >  #include <xen/mm.h>
> >  #include <xen/sched.h>
> >  #include <xen/smp.h>
> > +#include <xen/softirq.h>
> >  #include <xen/spinlock.h>
> >  #include <xen/vmap.h>
> > +#include <xen/wait.h>
> >  #include <xen/xsplice_elf.h>
> >  #include <xen/xsplice.h>
> >  
> >  #include <asm/event.h>
> >  #include <public/sysctl.h>
> >  
> > +/*
> > + * Protects against payload_list operations and also allows only one
> > + * caller in schedule_work.
> > + */
> 
> This comment really should be added in a previous patch.

The earlier patch did not have schedule work. It is introduced here.

I made the #2 patch (xen/xsplice: Hypervisor implementation of XEN_XSPLICE_op)
have:

+/* Protects against payload_list operations. */
 static DEFINE_SPINLOCK(payload_lock);


And this one:


-/* Protects against payload_list operations. */
+/*
+ * Protects against payload_list operations and also allows only one
+ * caller in schedule_work.
+ */


Would that work for you?
> 
> > @@ -254,6 +292,95 @@ static int secure_payload(struct payload *payload, 
> > struct xsplice_elf *elf)
> >      return rc;
> >  }
> >  
> > +static int check_special_sections(const struct xsplice_elf *elf)
> > +{
> > +    unsigned int i;
> > +    static const char *const names[] = { ".xsplice.funcs" };
> > +    unsigned int count[ARRAY_SIZE(names)] = { 0 };
> > +
> > +    for ( i = 0; i < ARRAY_SIZE(names); i++ )
> > +    {
> > +        const struct xsplice_elf_sec *sec;
> > +
> > +        sec = xsplice_elf_sec_by_name(elf, names[i]);
> > +        if ( !sec )
> > +        {
> > +            printk(XENLOG_ERR XSPLICE "%s: %s is missing!\n",
> > +                   elf->name, names[i]);
> > +            return -EINVAL;
> > +        }
> > +
> > +        if ( !sec->sec->sh_size )
> 
> printk(... "%s is empty\n").
> 
> Otherwise, Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

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