From: Jan Beulich <jbeulich@suse.com>
To: Hongyan Xia <hx242@xen.org>
Cc: xen-devel@lists.xenproject.org,
"Roger Pau Monné" <roger.pau@citrix.com>,
julien@xen.org, "Wei Liu" <wl@xen.org>,
"Andrew Cooper" <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2 1/5] x86/shim: map and unmap page tables in replace_va_mapping
Date: Thu, 9 Apr 2020 11:42:02 +0200 [thread overview]
Message-ID: <ddbad9f5-307e-7b1d-0cc7-cd7ed684f680@suse.com> (raw)
In-Reply-To: <7638095024ec3379a8d9ddadfe47e36da168e4dd.1586352238.git.hongyxia@amazon.com>
On 08.04.2020 15:36, Hongyan Xia wrote:
> --- a/xen/arch/x86/pv/shim.c
> +++ b/xen/arch/x86/pv/shim.c
> @@ -168,16 +168,17 @@ const struct platform_bad_page *__init pv_shim_reserved_pages(unsigned int *size
> static void __init replace_va_mapping(struct domain *d, l4_pgentry_t *l4start,
> unsigned long va, mfn_t mfn)
> {
> - l4_pgentry_t *pl4e = l4start + l4_table_offset(va);
> - l3_pgentry_t *pl3e = l4e_to_l3e(*pl4e) + l3_table_offset(va);
> - l2_pgentry_t *pl2e = l3e_to_l2e(*pl3e) + l2_table_offset(va);
> - l1_pgentry_t *pl1e = l2e_to_l1e(*pl2e) + l1_table_offset(va);
> + l4_pgentry_t l4e = l4start[l4_table_offset(va)];
> + l3_pgentry_t l3e = l3e_from_l4e(l4e, l3_table_offset(va));
> + l2_pgentry_t l2e = l2e_from_l3e(l3e, l2_table_offset(va));
> + l1_pgentry_t *pl1e = map_l1t_from_l2e(l2e) + l1_table_offset(va);
> struct page_info *page = mfn_to_page(l1e_get_mfn(*pl1e));
>
> put_page_and_type(page);
>
> *pl1e = l1e_from_mfn(mfn, (!is_pv_32bit_domain(d) ? L1_PROT
> : COMPAT_L1_PROT));
> + UNMAP_DOMAIN_PAGE(pl1e);
> }
As said before, here and below I think it should be unmap_domain_page().
> --- a/xen/include/asm-x86/page.h
> +++ b/xen/include/asm-x86/page.h
> @@ -196,6 +196,19 @@ static inline l4_pgentry_t l4e_from_paddr(paddr_t pa, unsigned int flags)
> #define map_l2t_from_l3e(x) (l2_pgentry_t *)map_domain_page(l3e_get_mfn(x))
> #define map_l3t_from_l4e(x) (l3_pgentry_t *)map_domain_page(l4e_get_mfn(x))
>
> +/* Unlike lYe_to_lXe(), lXe_from_lYe() do not rely on the direct map. */
> +#define l2e_from_l3e(l3e, offset) ({ \
> + const l2_pgentry_t *l2t = map_l2t_from_l3e(l3e); \
> + l2_pgentry_t l2e = l2t[offset]; \
> + UNMAP_DOMAIN_PAGE(l2t); \
> + l2e; })
> +
> +#define l3e_from_l4e(l4e, offset) ({ \
> + const l3_pgentry_t *l3t = map_l3t_from_l4e(l4e); \
> + l3_pgentry_t l3e = l3t[offset]; \
> + UNMAP_DOMAIN_PAGE(l3t); \
> + l3e; })
I think l1e_from_l2e() should be introduced at the same time, even
if for now it's unused. I also think, like we do elsewhere, that
macro-local variables would better have _ suffixes, to avoid
possible variable aliasing issues.
Jan
next prev parent reply other threads:[~2020-04-09 9:42 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-08 13:36 [PATCH v2 0/5] use new API for Xen page tables Hongyan Xia
2020-04-08 13:36 ` [PATCH v2 1/5] x86/shim: map and unmap page tables in replace_va_mapping Hongyan Xia
2020-04-09 9:42 ` Jan Beulich [this message]
2020-04-14 16:53 ` Hongyan Xia
2020-04-15 6:13 ` Jan Beulich
2020-04-08 13:36 ` [PATCH v2 2/5] x86_64/mm: map and unmap page tables in m2p_mapped Hongyan Xia
2020-04-15 7:54 ` Jan Beulich
2020-04-08 13:36 ` [PATCH v2 3/5] x86_64/mm: map and unmap page tables in share_hotadd_m2p_table Hongyan Xia
2020-04-15 7:55 ` Jan Beulich
2020-04-08 13:36 ` [PATCH v2 4/5] x86_64/mm: map and unmap page tables in destroy_compat_m2p_mapping Hongyan Xia
2020-04-15 8:21 ` [PATCH 0/2] x86: high compat r/o M2P table handling adjustments Jan Beulich
2020-04-15 8:23 ` [PATCH 1/2] x86: drop unnecessary page table walking in compat r/o M2P handling Jan Beulich
2020-04-15 9:59 ` Hongyan Xia
2020-04-15 10:34 ` Jan Beulich
2020-04-15 10:50 ` Hongyan Xia
2020-04-15 11:16 ` Wei Liu
2020-04-15 8:23 ` [PATCH 2/2] x86: drop high compat r/o M2P table address range Jan Beulich
2020-04-27 19:52 ` Wei Liu
2020-04-28 6:14 ` [PATCH 0/2] x86: high compat r/o M2P table handling adjustments Jan Beulich
2020-04-15 8:25 ` [PATCH v2 4/5] x86_64/mm: map and unmap page tables in destroy_compat_m2p_mapping Jan Beulich
2020-04-08 13:36 ` [PATCH v2 5/5] x86_64/mm: map and unmap page tables in destroy_m2p_mapping Hongyan Xia
2020-04-15 8:32 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ddbad9f5-307e-7b1d-0cc7-cd7ed684f680@suse.com \
--to=jbeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=hx242@xen.org \
--cc=julien@xen.org \
--cc=roger.pau@citrix.com \
--cc=wl@xen.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.