From: Julien Grall <julien.grall@arm.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>,
"andrii_anisov@epam.com" <andrii_anisov@epam.com>,
WeiLiu <wl@xen.org>,
Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>,
George Dunlap <George.Dunlap@eu.citrix.com>,
Andrew Cooper <andrew.cooper3@citrix.com>,
Ian Jackson <Ian.Jackson@eu.citrix.com>, Tim Deegan <tim@xen.org>,
oleksandr_tyshchenko@epam.com,
xen-devel <xen-devel@lists.xenproject.org>,
Roger Pau Monne <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v3 07/14] xen: Introduce HAS_M2P config and use to protect mfn_to_gmfn call
Date: Tue, 4 Jun 2019 17:22:29 +0100 [thread overview]
Message-ID: <b85178eb-05ad-da33-3f97-9fdcdf65727b@arm.com> (raw)
In-Reply-To: <5CF698EB0200007800235319@prv1-mh.provo.novell.com>
Hi Jan,
On 6/4/19 5:14 PM, Jan Beulich wrote:
>>>> On 03.06.19 at 18:03, <julien.grall@arm.com> wrote:
>> While Arm never had a M2P, the implementation of mfn_to_gmfn is pretty
>> bogus as we directly return the MFN passed in parameter.
>>
>> Thankfully, the use of mfn_to_gmfn is pretty limited on Arm today. There
>> are only 3 callers:
>> - iommu_hwdom_init: mfn_to_gmfn is used for creating IOMMU
>> page-tables when the P2M is not shared with the IOMMU. No issues so
>> far as Arm does not yet support non-shared P2M case.
>> - memory_exchange: Arm cannot not use it because steal_page is not
>> implemented.
>> - getdomaininfo: Toolstack may map the shared page. It looks like
>> this is mostly used for mapping the P2M of PV guest. Therefore the
>> issue might be minor.
>>
>> Implementing the M2P on Arm is not planned. The M2P would require
>> significant
>> amount of VA address (very tough on 32-bit) that can hardly be justified with
>> the current use of mfn_to_gmfn.
>> - iommu_hwdom_init: mfn_to_gmfn is used because the creating of the
>> IOMMU page-tables is delayed until the first device is assigned.
>> In the embedded case, we will known in most of the times what
>> devices are assigned during the domain creation. So it is possible
>> to take to enable the IOMMU from start. See [1] for the patch.
>> - memory_exchange: This does not work and I haven't seen any
>> request for it so far.
>> - getdomaininfo: The structure on Arm does not seem to contain a lot
>> of useful information on Arm. It is unclear whether we want to
>> allow the toolstack mapping it on Arm.
>>
>> This patch introduces a config option HAS_M2P to tell whether an
>> architecture implements the M2P.
>> - iommu_hwdom_init: For now, we require the M2P support when the IOMMU
>> is not sharing the P2M.
>> - memory_exchange: The hypercall is marked as not supported when the
>> M2P does not exist.
>
> But where's the connection between there being M2P and the
> availability of this operation? I think I've suggested so before:
> Why don't we simply disable this operation for translated
> guests (in an independent patch)?
And I answered that mfn_to_gmfn() is used in the function. I really
don't want to implement the macro on Arm (even if it is dummy).
You haven't answered back to that comment and I assumed this was fine
with you...
>
>> - getdomaininfo: A new helper is introduced to wrap the call to
>> mfn_to_gfn/mfn_to_gmfn. For Arm, a fixed value will be provided that will
>> fail on mapping if used.
>
> This reads slightly stale now.
ok.
>
>> --- a/xen/drivers/passthrough/iommu.c
>> +++ b/xen/drivers/passthrough/iommu.c
>> @@ -188,9 +188,10 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>> hd->need_sync = iommu_hwdom_strict && !iommu_use_hap_pt(d);
>> if ( need_iommu_pt_sync(d) )
>> {
>> + int rc = 0;
>> +#ifdef CONFIG_HAS_M2P
>> struct page_info *page;
>> unsigned int i = 0, flush_flags = 0;
>> - int rc = 0;
>>
>> page_list_for_each ( page, &d->page_list )
>> {
>> @@ -221,6 +222,11 @@ void __hwdom_init iommu_hwdom_init(struct domain *d)
>> if ( rc )
>> printk(XENLOG_WARNING "d%d: IOMMU mapping failed: %d\n",
>> d->domain_id, rc);
>> +#else
>> + ASSERT_UNREACHABLE();
>> + rc = -EOPNOTSUPP;
>> +#endif
>> +
>> }
>
> Please don't add a blank line ahead of a closing brace.
ok.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
next prev parent reply other threads:[~2019-06-04 16:22 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-06-03 16:03 [PATCH v3 00/14] xen/arm: Properly disable M2P on Arm Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 01/14] xen/x86: Make mfn_to_gfn typesafe Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-04 16:06 ` Jan Beulich
2019-07-29 12:13 ` Julien Grall
2019-08-19 13:50 ` George Dunlap
2019-06-03 16:03 ` [PATCH v3 02/14] xen/x86: Use mfn_to_gfn rather than mfn_to_gmfn Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 03/14] xen/grant-table: Make arch specific macros typesafe Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 04/14] xen: Convert hotplug page function to use typesafe MFN Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 05/14] xen: Convert is_xen_fixed_mfn " Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 06/14] xen: Convert is_xen_heap_mfn " Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 07/14] xen: Introduce HAS_M2P config and use to protect mfn_to_gmfn call Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-04 16:14 ` Jan Beulich
2019-06-04 16:22 ` Julien Grall [this message]
2019-06-05 7:14 ` Jan Beulich
2019-06-05 9:35 ` Julien Grall
2019-06-05 10:10 ` Jan Beulich
2019-07-29 12:08 ` Julien Grall
2019-06-03 16:03 ` [PATCH v3 08/14] xen: Remove mfn_to_gmfn macro Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 09/14] xen/x86: mm: Re-implement set_gpfn_from_mfn() as a static inline function Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-04 16:21 ` Jan Beulich
2019-06-04 16:23 ` Julien Grall
2019-06-05 7:18 ` Jan Beulich
2019-06-03 16:03 ` [PATCH v3 10/14] xen/x86: pv: Convert update_intpte() to use typesafe MFN Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 11/14] xen/x86: p2m: Remove duplicate error message in p2m_pt_audit_p2m() Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-05 10:43 ` Jan Beulich
2019-06-05 10:44 ` Julien Grall
2019-06-03 16:03 ` [PATCH v3 12/14] xen/x86: p2m: Rework printk format in audit_p2m() Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-03 16:03 ` [PATCH v3 13/14] xen/mm: Convert {s, g}et_gpfn_from_mfn() to use typesafe MFN Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-05 11:01 ` Jan Beulich
2019-06-05 11:03 ` Julien Grall
2019-06-03 16:03 ` [PATCH v3 14/14] xen/mm: Provide dummy M2P-related helpers when !CONFIG_HAVE_M2P Julien Grall
2019-06-03 16:03 ` [Xen-devel] " Julien Grall
2019-06-05 11:07 ` Jan Beulich
2019-06-05 11:17 ` Julien Grall
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=b85178eb-05ad-da33-3f97-9fdcdf65727b@arm.com \
--to=julien.grall@arm.com \
--cc=George.Dunlap@eu.citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=JBeulich@suse.com \
--cc=andrew.cooper3@citrix.com \
--cc=andrii_anisov@epam.com \
--cc=konrad.wilk@oracle.com \
--cc=oleksandr_tyshchenko@epam.com \
--cc=roger.pau@citrix.com \
--cc=sstabellini@kernel.org \
--cc=tim@xen.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).