Re: [Xen-devel] [PATCH V14 3/7] xen/arm: Allow hypervisor access to mem_access protected pages

[Tamas K Lengyel <tklengyel@xxxxxxxxxxxxx>]

On Wed, Apr 8, 2015 at 4:33 PM, Julien Grall <julien.grall@xxxxxxxxxx> 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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel