[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4 1/2] x86/mem_sharing: make fork_reset more configurable
On Wed, Apr 13, 2022 at 09:41:51AM -0400, Tamas K Lengyel wrote: > Allow specify distinct parts of the fork VM to be reset. This is useful when a > fuzzing operation involves mapping in only a handful of pages that are known > ahead of time. Throwing these pages away just to be re-copied immediately is > expensive, thus allowing to specify partial resets can speed things up. > > Also allow resetting to be initiated from vm_event responses as an > optiomization. > > Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> > --- > v4: No change > v3: Rebase on simpler approach after dropping empty_p2m feature > v2: address review comments and add more sanity checking > --- > tools/include/xenctrl.h | 3 ++- > tools/libs/ctrl/xc_memshr.c | 7 ++++++- > xen/arch/x86/include/asm/mem_sharing.h | 9 +++++++++ > xen/arch/x86/mm/mem_sharing.c | 24 +++++++++++++++++++----- > xen/common/vm_event.c | 15 +++++++++++++++ > xen/include/public/memory.h | 4 +++- > xen/include/public/vm_event.h | 8 ++++++++ > 7 files changed, 62 insertions(+), 8 deletions(-) > > diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h > index 95bd5eca67..1b089a2c02 100644 > --- a/tools/include/xenctrl.h > +++ b/tools/include/xenctrl.h > @@ -2290,7 +2290,8 @@ int xc_memshr_fork(xc_interface *xch, > * > * With VMs that have a lot of memory this call may block for a long time. > */ > -int xc_memshr_fork_reset(xc_interface *xch, uint32_t forked_domain); > +int xc_memshr_fork_reset(xc_interface *xch, uint32_t forked_domain, > + bool reset_state, bool reset_memory); > > /* Debug calls: return the number of pages referencing the shared frame > backing > * the input argument. Should be one or greater. > diff --git a/tools/libs/ctrl/xc_memshr.c b/tools/libs/ctrl/xc_memshr.c > index a6cfd7dccf..a0d0b894e2 100644 > --- a/tools/libs/ctrl/xc_memshr.c > +++ b/tools/libs/ctrl/xc_memshr.c > @@ -257,12 +257,17 @@ int xc_memshr_fork(xc_interface *xch, uint32_t pdomid, > uint32_t domid, > return xc_memshr_memop(xch, domid, &mso); > } > > -int xc_memshr_fork_reset(xc_interface *xch, uint32_t domid) > +int xc_memshr_fork_reset(xc_interface *xch, uint32_t domid, bool reset_state, > + bool reset_memory) > { > xen_mem_sharing_op_t mso; > > memset(&mso, 0, sizeof(mso)); > mso.op = XENMEM_sharing_op_fork_reset; > + if ( reset_state ) > + mso.u.fork.flags |= XENMEM_FORK_RESET_STATE; > + if ( reset_memory ) > + mso.u.fork.flags |= XENMEM_FORK_RESET_MEMORY; IMO would be clearer to init mso fields at definition. > diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c > index 84cf52636b..d26a6699fc 100644 > --- a/xen/common/vm_event.c > +++ b/xen/common/vm_event.c > @@ -28,6 +28,11 @@ > #include <asm/p2m.h> > #include <asm/monitor.h> > #include <asm/vm_event.h> > + > +#ifdef CONFIG_MEM_SHARING > +#include <asm/mem_sharing.h> > +#endif > + > #include <xsm/xsm.h> > #include <public/hvm/params.h> > > @@ -394,6 +399,16 @@ static int vm_event_resume(struct domain *d, struct > vm_event_domain *ved) > if ( rsp.reason == VM_EVENT_REASON_MEM_PAGING ) > p2m_mem_paging_resume(d, &rsp); > #endif > +#ifdef CONFIG_MEM_SHARING > + if ( mem_sharing_is_fork(d) ) > + { > + bool reset_state = rsp.flags & > VM_EVENT_FLAG_RESET_FORK_STATE; > + bool reset_mem = rsp.flags & VM_EVENT_FLAG_RESET_FORK_MEMORY; > + > + if ( reset_state || reset_mem ) > + ASSERT(!mem_sharing_fork_reset(d, reset_state, > reset_mem)); Might be appropriate to destroy the domain in case fork reset fails? ASSERT will only help in debug builds. > + } > +#endif > > /* > * Check emulation flags in the arch-specific handler only, as it > diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h > index a1a0f0233a..f8d26fb77d 100644 > --- a/xen/include/public/memory.h > +++ b/xen/include/public/memory.h > @@ -541,12 +541,14 @@ struct xen_mem_sharing_op { > uint32_t gref; /* IN: gref to debug */ > } u; > } debug; > - struct mem_sharing_op_fork { /* OP_FORK */ > + struct mem_sharing_op_fork { /* OP_FORK{,_RESET} */ > domid_t parent_domain; /* IN: parent's domain id */ > /* Only makes sense for short-lived forks */ > #define XENMEM_FORK_WITH_IOMMU_ALLOWED (1u << 0) > /* Only makes sense for short-lived forks */ > #define XENMEM_FORK_BLOCK_INTERRUPTS (1u << 1) Should you add: /* Only for OP_FORK_RESET. */ > +#define XENMEM_FORK_RESET_STATE (1u << 2) > +#define XENMEM_FORK_RESET_MEMORY (1u << 3) Thanks, Roger.
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |