Re: [Xen-devel] [PATCH] x86/HVM: honor p2m_ram_ro in hvm_map_guest_frame_rw()

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Fri, Jul 24, 2015 at 04:37:25AM -0600, Jan Beulich wrote:
> >>> On 24.07.15 at 12:26, <wei.liu2@xxxxxxxxxx> wrote:
> > On Fri, Jul 24, 2015 at 03:41:26AM -0600, Jan Beulich wrote:
> >> @@ -3618,7 +3618,12 @@ static void *__hvm_map_guest_frame(unsig
> >>      }
> >>  
> >>      if ( writable )
> > 
> > I don't claim I know this piece of code, but checking the pointer but
> > not the content looks suspicious.
> > 
> >> -        paging_mark_dirty(d, page_to_mfn(page));
> >> +    {
> >> +        if ( !p2m_is_discard_write(p2mt) )
> >> +            paging_mark_dirty(d, page_to_mfn(page));
> >> +        else
> >> +            *writable = 0;
> > 
> > You then set *writable here, which makes it even more suspicious.
> 
> Why? A caller _wanting_ a writable mapping passes non-NULL as
> the pointer argument (pre-initialized to point to a variable holding
> TRUE aka 1). Upon return the variable will have got set to FALSE
> aka 0 if the page shouldn't be written to.
> 

If this is the convention for using this function then the code you
write is of course fine.

Wei.

> Jan

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