Re: [Xen-devel] [PATCH v3 14/17] livepatch/x86/arm: arch/x86/mm: generalize do_page_walk() and implement arch_livepatch_lookup_mfn

[Julien Grall <julien.grall@xxxxxxx>]

Hi Konrad,

On 12/09/17 01:37, Konrad Rzeszutek Wilk wrote:
> With this change we can use _do_page_walk() to implement
> arch_livepatch_lookup_mfn() which can be used to find out
> vmap virtual addresses (as under x86 virt_to_mfn won't work
> for vmap, but it does for arm!).
The reason is on ARM, virt_to_mfn is taking advantage of the hardware to 
perform the translation.
I am wondering if on x86 you could use vmap_to_mfn.

Cheers,

> Signed-off-by: Blaise Boscaccy <blaise.boscaccy@xxxxxxxxxx>
> Signed-off-by: Vegard Nossum <vegard.nossum@xxxxxxxxxx>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
>   xen/arch/arm/livepatch.c    |  7 ++++---
>   xen/arch/x86/livepatch.c    | 10 ++++++++++
>   xen/arch/x86/x86_64/mm.c    | 33 ++++++++++++++++++++++++---------
>   xen/include/asm-x86/mm.h    |  1 +
>   xen/include/xen/livepatch.h |  3 +++
>   5 files changed, 42 insertions(+), 12 deletions(-)
>
> diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
> index e1d5d58f97..1771b3c558 100644
> --- a/xen/arch/arm/livepatch.c
> +++ b/xen/arch/arm/livepatch.c
> @@ -13,9 +13,10 @@
>   #include <asm/livepatch.h>
>   #include <asm/mm.h>
>   
> -/* Override macros from asm/page.h to make them work with mfn_t */
> -#undef virt_to_mfn
> -#define virt_to_mfn(va) _mfn(__virt_to_mfn(va))
> +mfn_t arch_livepatch_lookup_mfn(unsigned long addr)
> +{
> +    return _mfn(__virt_to_mfn(addr));
> +}
>   
>   void arch_livepatch_revive(void)
>   {
> diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
> index 12287d445f..667573c6de 100644
> --- a/xen/arch/x86/livepatch.c
> +++ b/xen/arch/x86/livepatch.c
> @@ -14,6 +14,16 @@
>   #include <asm/nmi.h>
>   #include <asm/livepatch.h>
>   
> +mfn_t arch_livepatch_lookup_mfn(unsigned long addr)
> +{
> +    unsigned long cr3 = read_cr3() >> PAGE_SHIFT;
> +
> +    if ( !mfn_valid(_mfn(cr3)) )
> +        return INVALID_MFN;
> +
> +    return _do_page_walk(cr3, addr);
> +}
> +
>   void arch_livepatch_revive(void)
>   {
>       /* Nothing to do. */
> diff --git a/xen/arch/x86/x86_64/mm.c b/xen/arch/x86/x86_64/mm.c
> index 11746730b4..f8a963bbba 100644
> --- a/xen/arch/x86/x86_64/mm.c
> +++ b/xen/arch/x86/x86_64/mm.c
> @@ -44,29 +44,28 @@ unsigned int __read_mostly m2p_compat_vstart = 
> __HYPERVISOR_COMPAT_VIRT_START;
>   
>   l2_pgentry_t *compat_idle_pg_table_l2;
>   
> -void *do_page_walk(struct vcpu *v, unsigned long addr)
> +mfn_t _do_page_walk(unsigned long mfn, unsigned long addr)
>   {
> -    unsigned long mfn = pagetable_get_pfn(v->arch.guest_table);
>       l4_pgentry_t l4e, *l4t;
>       l3_pgentry_t l3e, *l3t;
>       l2_pgentry_t l2e, *l2t;
>       l1_pgentry_t l1e, *l1t;
>   
> -    if ( !is_pv_vcpu(v) || !is_canonical_address(addr) )
> -        return NULL;
> +    if ( !is_canonical_address(addr) )
> +        return INVALID_MFN;
>   
>       l4t = map_domain_page(_mfn(mfn));
>       l4e = l4t[l4_table_offset(addr)];
>       unmap_domain_page(l4t);
>       if ( !(l4e_get_flags(l4e) & _PAGE_PRESENT) )
> -        return NULL;
> +        return INVALID_MFN;
>   
>       l3t = map_l3t_from_l4e(l4e);
>       l3e = l3t[l3_table_offset(addr)];
>       unmap_domain_page(l3t);
>       mfn = l3e_get_pfn(l3e);
>       if ( !(l3e_get_flags(l3e) & _PAGE_PRESENT) || !mfn_valid(_mfn(mfn)) )
> -        return NULL;
> +        return INVALID_MFN;
>       if ( (l3e_get_flags(l3e) & _PAGE_PSE) )
>       {
>           mfn += PFN_DOWN(addr & ((1UL << L3_PAGETABLE_SHIFT) - 1));
> @@ -78,7 +77,7 @@ void *do_page_walk(struct vcpu *v, unsigned long addr)
>       unmap_domain_page(l2t);
>       mfn = l2e_get_pfn(l2e);
>       if ( !(l2e_get_flags(l2e) & _PAGE_PRESENT) || !mfn_valid(_mfn(mfn)) )
> -        return NULL;
> +        return INVALID_MFN;
>       if ( (l2e_get_flags(l2e) & _PAGE_PSE) )
>       {
>           mfn += PFN_DOWN(addr & ((1UL << L2_PAGETABLE_SHIFT) - 1));
> @@ -90,10 +89,26 @@ void *do_page_walk(struct vcpu *v, unsigned long addr)
>       unmap_domain_page(l1t);
>       mfn = l1e_get_pfn(l1e);
>       if ( !(l1e_get_flags(l1e) & _PAGE_PRESENT) || !mfn_valid(_mfn(mfn)) )
> -        return NULL;
> +        return INVALID_MFN;
>   
>    ret:
> -    return map_domain_page(_mfn(mfn)) + (addr & ~PAGE_MASK);
> +    return _mfn(mfn);
> +}
> +
> +void *do_page_walk(struct vcpu *v, unsigned long addr)
> +{
> +    mfn_t mfn;
> +    unsigned long cr3;
> +
> +    if ( !is_pv_vcpu(v) )
> +        return NULL;
> +
> +    cr3 = pagetable_get_pfn(v->arch.guest_table);
> +    mfn = _do_page_walk(cr3, addr);
> +    if ( mfn_eq(mfn, INVALID_MFN) )
> +        return NULL;
> +
> +    return map_domain_page(mfn) + (addr & ~PAGE_MASK);
>   }
>   
>   /*
> diff --git a/xen/include/asm-x86/mm.h b/xen/include/asm-x86/mm.h
> index bef45e8e9f..224a94494a 100644
> --- a/xen/include/asm-x86/mm.h
> +++ b/xen/include/asm-x86/mm.h
> @@ -540,6 +540,7 @@ int new_guest_cr3(mfn_t mfn);
>   void make_cr3(struct vcpu *v, mfn_t mfn);
>   void update_cr3(struct vcpu *v);
>   int vcpu_destroy_pagetables(struct vcpu *);
> +mfn_t _do_page_walk(unsigned long mfn, unsigned long addr);
>   void *do_page_walk(struct vcpu *v, unsigned long addr);
>   
>   int __sync_local_execstate(void);
> diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
> index 065c1a323a..e529f0e7c3 100644
> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -72,6 +72,9 @@ int arch_livepatch_secure(const void *va, unsigned int pages, 
> enum va_type types
>   
>   void arch_livepatch_init(void);
>   
> +#include <xen/mm.h> /* For mfn_t decleration. */
> +mfn_t arch_livepatch_lookup_mfn(unsigned long addr);
> +
>   #include <public/sysctl.h> /* For struct livepatch_func. */
>   #include <asm/livepatch.h>
>   int arch_livepatch_verify_func(const struct livepatch_func *func);
--
Julien Grall

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