Re: [Xen-devel] [PATCH v2] mem_access: sanitize code around sending vm_event request

[Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>]

On Aug 2, 2016 06:45, "Jan Beulich" <JBeulich@xxxxxxxx> wrote:
>
> >>> On 01.08.16 at 18:52, <tamas.lengyel@xxxxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/hvm/hvm.c
> > +++ b/xen/arch/x86/hvm/hvm.c
> > @@ -1707,7 +1707,7 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
> >      int rc, fall_through = 0, paged = 0;
> >      int sharing_enomem = 0;
> >      vm_event_request_t *req_ptr = NULL;
> > -    bool_t ap2m_active;
> > +    bool_t ap2m_active, sync = 0;
> >
> >      /* On Nested Virtualization, walk the guest page table.
> >       * If this succeeds, all is fine.
> > @@ -1846,11 +1846,12 @@ int hvm_hap_nested_page_fault(paddr_t gpa, unsigned long gla,
> >                  }
> >              }
> >
> > -            if ( p2m_mem_access_check(gpa, gla, npfec, &req_ptr) )
> > -            {
> > +            sync = p2m_mem_access_check(gpa, gla, npfec, &req_ptr);
> > +
> > +            if ( !sync ) {
>
> Coding style. If you dropped the brace entirely, you could at once
> adjust ...
>
> >                  fall_through = 1;
> >              } else {
>
> ... coding style here.
>
> > -                /* Rights not promoted, vcpu paused, work here is done */
> > +                /* Rights not promoted (aka. sync event), work here is done */
>
> Comment style. More of these elsewhere.
>
> > +int hvm_monitor_mem_access(struct vcpu* v, bool_t sync,
>
> Coding style.
>
> > +                           vm_event_request_t *req)
> > +{
> > +    return monitor_traps(v, sync, req);
> > +}
>
> Overall - is this a useful wrapper? Why can't the caller(s) call
> monitor_traps() directly? And if you really want to keep it, it would
> probably better be an inline one.
>

The reason for this wrapper is to avoid having to include the common monitor header here. I can move it into the hvm monitor header as inline, that's no problem.

Thanks,
Tamas

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