|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v3 7/8] xen/arm: Rework the interface of p2m_lookup and use typesafe gfn and mfn
On Tue, 21 Jun 2016, Julien Grall wrote:
> The prototype and the declaration of p2m_lookup disagree on how the
> function should be used. One expect a frame number whilst the other
> an address.
>
> Thankfully, everyone is using with an address today. However, most of
> the callers have to convert a guest frame to an address. Modify
> the interface to take a guest physical frame in parameter and return
> a machine frame.
>
> Whilst modifying the interface, use typesafe gfn and mfn for clarity
> and catching possible misusage.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> ---
> xen/arch/arm/p2m.c | 37 ++++++++++++++++++++-----------------
> xen/arch/arm/traps.c | 21 +++++++++++----------
> xen/include/asm-arm/p2m.h | 7 +++----
> 3 files changed, 34 insertions(+), 31 deletions(-)
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index 47cb383..f3330dd 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -140,14 +140,15 @@ void flush_tlb_domain(struct domain *d)
> }
>
> /*
> - * Lookup the MFN corresponding to a domain's PFN.
> + * Lookup the MFN corresponding to a domain's GFN.
> *
> * There are no processor functions to do a stage 2 only lookup therefore we
> * do a a software walk.
> */
> -static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
> +static mfn_t __p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
> {
> struct p2m_domain *p2m = &d->arch.p2m;
> + const paddr_t paddr = pfn_to_paddr(gfn_x(gfn));
> const unsigned int offsets[4] = {
> zeroeth_table_offset(paddr),
> first_table_offset(paddr),
> @@ -158,7 +159,7 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t
> paddr, p2m_type_t *t)
> ZEROETH_MASK, FIRST_MASK, SECOND_MASK, THIRD_MASK
> };
> lpae_t pte, *map;
> - paddr_t maddr = INVALID_PADDR;
> + mfn_t mfn = _mfn(INVALID_MFN);
It might be worth defining INVALID_MFN_T and just assign that to mfn.
> paddr_t mask = 0;
> p2m_type_t _t;
> unsigned int level, root_table;
> @@ -216,21 +217,22 @@ static paddr_t __p2m_lookup(struct domain *d, paddr_t
> paddr, p2m_type_t *t)
> {
> ASSERT(mask);
> ASSERT(pte.p2m.type != p2m_invalid);
> - maddr = (pte.bits & PADDR_MASK & mask) | (paddr & ~mask);
> + mfn = _mfn(paddr_to_pfn((pte.bits & PADDR_MASK & mask) |
> + (paddr & ~mask)));
> *t = pte.p2m.type;
> }
>
> err:
> - return maddr;
> + return mfn;
> }
>
> -paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
> +mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t)
> {
> - paddr_t ret;
> + mfn_t ret;
> struct p2m_domain *p2m = &d->arch.p2m;
>
> spin_lock(&p2m->lock);
> - ret = __p2m_lookup(d, paddr, t);
> + ret = __p2m_lookup(d, gfn, t);
> spin_unlock(&p2m->lock);
>
> return ret;
> @@ -493,8 +495,9 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t
> gfn,
> * No setting was found in the Radix tree. Check if the
> * entry exists in the page-tables.
> */
> - paddr_t maddr = __p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
> - if ( INVALID_PADDR == maddr )
> + mfn_t mfn = __p2m_lookup(d, gfn, NULL);
> +
> + if ( mfn_x(mfn) == INVALID_MFN )
> return -ESRCH;
>
> /* If entry exists then its rwx. */
> @@ -1483,8 +1486,7 @@ int p2m_cache_flush(struct domain *d, xen_pfn_t
> start_mfn, xen_pfn_t end_mfn)
>
> mfn_t gfn_to_mfn(struct domain *d, gfn_t gfn)
> {
> - paddr_t p = p2m_lookup(d, pfn_to_paddr(gfn_x(gfn)), NULL);
> - return _mfn(p >> PAGE_SHIFT);
> + return p2m_lookup(d, gfn, NULL);
> }
>
> /*
> @@ -1498,7 +1500,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
> long flag)
> {
> long rc;
> paddr_t ipa;
> - unsigned long maddr;
> + gfn_t gfn;
> unsigned long mfn;
> xenmem_access_t xma;
> p2m_type_t t;
> @@ -1508,11 +1510,13 @@ p2m_mem_access_check_and_get_page(vaddr_t gva,
> unsigned long flag)
> if ( rc < 0 )
> goto err;
>
> + gfn = _gfn(paddr_to_pfn(ipa));
> +
> /*
> * We do this first as this is faster in the default case when no
> * permission is set on the page.
> */
> - rc = __p2m_get_mem_access(current->domain, _gfn(paddr_to_pfn(ipa)),
> &xma);
> + rc = __p2m_get_mem_access(current->domain, gfn, &xma);
> if ( rc < 0 )
> goto err;
>
> @@ -1561,11 +1565,10 @@ p2m_mem_access_check_and_get_page(vaddr_t gva,
> unsigned long flag)
> * 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 )
> + mfn = mfn_x(__p2m_lookup(current->domain, gfn, &t));
> + if ( mfn == INVALID_MFN )
The conversion would go away if we had an INVALID_MFN which is mfn_t
> goto err;
>
> - mfn = maddr >> PAGE_SHIFT;
> if ( !mfn_valid(mfn) )
> goto err;
>
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 44926ca..02fe117 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2319,14 +2319,16 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t
> addr)
> {
> register_t ttbcr = READ_SYSREG(TCR_EL1);
> uint64_t ttbr0 = READ_SYSREG64(TTBR0_EL1);
> - paddr_t paddr;
> uint32_t offset;
> uint32_t *first = NULL, *second = NULL;
> + mfn_t mfn;
> +
> + mfn = p2m_lookup(d, _gfn(paddr_to_pfn(ttbr0)), NULL);
>
> printk("dom%d VA 0x%08"PRIvaddr"\n", d->domain_id, addr);
> printk(" TTBCR: 0x%08"PRIregister"\n", ttbcr);
> printk(" TTBR0: 0x%016"PRIx64" = 0x%"PRIpaddr"\n",
> - ttbr0, p2m_lookup(d, ttbr0 & PAGE_MASK, NULL));
> + ttbr0, pfn_to_paddr(mfn_x(mfn)));
>
> if ( ttbcr & TTBCR_EAE )
> {
> @@ -2339,32 +2341,31 @@ void dump_guest_s1_walk(struct domain *d, vaddr_t
> addr)
> return;
> }
>
> - paddr = p2m_lookup(d, ttbr0 & PAGE_MASK, NULL);
> - if ( paddr == INVALID_PADDR )
> + if ( mfn_x(mfn) == INVALID_MFN )
> {
> printk("Failed TTBR0 maddr lookup\n");
> goto done;
> }
> - first = map_domain_page(_mfn(paddr_to_pfn(paddr)));
> + first = map_domain_page(mfn);
>
> offset = addr >> (12+10);
> printk("1ST[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
> - offset, paddr, first[offset]);
> + offset, pfn_to_paddr(mfn_x(mfn)), first[offset]);
> if ( !(first[offset] & 0x1) ||
> !(first[offset] & 0x2) )
> goto done;
>
> - paddr = p2m_lookup(d, first[offset] & PAGE_MASK, NULL);
> + mfn = p2m_lookup(d, _gfn(paddr_to_pfn(first[offset])), NULL);
>
> - if ( paddr == INVALID_PADDR )
> + if ( mfn_x(mfn) == INVALID_MFN )
> {
> printk("Failed L1 entry maddr lookup\n");
> goto done;
> }
> - second = map_domain_page(_mfn(paddr_to_pfn(paddr)));
> + second = map_domain_page(mfn);
> offset = (addr >> 12) & 0x3FF;
> printk("2ND[0x%"PRIx32"] (0x%"PRIpaddr") = 0x%08"PRIx32"\n",
> - offset, paddr, second[offset]);
> + offset, pfn_to_paddr(mfn_x(mfn)), second[offset]);
>
> done:
> if (second) unmap_domain_page(second);
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index 0d1e61e..f204482 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -135,8 +135,8 @@ void p2m_restore_state(struct vcpu *n);
> /* Print debugging/statistial info about a domain's p2m */
> void p2m_dump_info(struct domain *d);
>
> -/* Look up the MFN corresponding to a domain's PFN. */
> -paddr_t p2m_lookup(struct domain *d, paddr_t gpfn, p2m_type_t *t);
> +/* Look up the MFN corresponding to a domain's GFN. */
> +mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t);
>
> /* Clean & invalidate caches corresponding to a region of guest address
> space */
> int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t
> end_mfn);
> @@ -201,8 +201,7 @@ static inline struct page_info *get_page_from_gfn(
> {
> struct page_info *page;
> p2m_type_t p2mt;
> - paddr_t maddr = p2m_lookup(d, pfn_to_paddr(gfn), &p2mt);
> - unsigned long mfn = maddr >> PAGE_SHIFT;
> + unsigned long mfn = mfn_x(p2m_lookup(d, _gfn(gfn), &p2mt));
>
> if (t)
> *t = p2mt;
> --
> 1.9.1
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |