On Wed, Apr 8, 2015 at 4:33 PM, Julien Grall <julien.gr...@citrix.com> wrote:
> Hi Tamas, > > One minor question. > > On 26/03/15 22:05, Tamas K Lengyel wrote: > > + /* > > + * We had a mem_access permission limiting the access, but the page > type > > + * could also be limiting, so we need to check that as well. > > + */ > > + maddr = p2m_lookup(current->domain, ipa, &t); > > + if ( maddr == INVALID_PADDR ) > > + goto err; > > + > > + mfn = maddr >> PAGE_SHIFT; > > + if ( !mfn_valid(mfn) ) > > + goto err; > > + > > + /* > > + * Base type doesn't allow r/w > > + */ > > + if ( t != p2m_ram_rw ) > > + goto err; > > + > > + page = mfn_to_page(mfn); > > + > > + if ( unlikely(!get_page(page, current->domain)) ) > > + page = NULL; > > I just found that this code is very similar to get_page_from_gfn. Would > it make sense to use it? > Yea I could reuse that code, although it has some extra stuff that I don't need (like converting ipa to gfn back to paddr), and type checks which I ultimately don't care about here as the only type allowed is p2m_ram_rw. So the current version is probably a bit tighter. > > > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c > > index ad046e8..5d90609 100644 > > --- a/xen/arch/arm/traps.c > > +++ b/xen/arch/arm/traps.c > > @@ -1988,7 +1988,7 @@ static void do_trap_data_abort_guest(struct > cpu_user_regs *regs, > > if (dabt.s1ptw) > > goto bad_data_abort; > > > > - rc = gva_to_ipa(info.gva, &info.gpa); > > + rc = gva_to_ipa(info.gva, &info.gpa, GV2M_READ); > > The correct value would be to use dabt.write in order to give either > GV2M_READ or GV2M_WRITE. Although you keep compatibility and it's out of > the scope of this patch. > > I will send a follow-up when your series will be pushed. > Yea, my goal here was not to change existing behavior. > Regards, > > -- > Julien Grall > Thanks, Tamas
_______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
