From: Julien Grall <julien.grall@linaro.org>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: xen-devel@lists.xenproject.org, tim@xen.org,
Jan Beulich <jbeulich@suse.com>,
stefano.stabellini@citrix.com
Subject: Re: [PATCH v8 4/4] xen/arm: grant: Add another entry to map MFN 1:1 in dom0 p2m
Date: Wed, 21 May 2014 15:01:51 +0100 [thread overview]
Message-ID: <537CB1CF.1030206@linaro.org> (raw)
In-Reply-To: <1400680247.4856.100.camel@kazak.uk.xensource.com>
On 05/21/2014 02:50 PM, Ian Campbell wrote:
>>>> diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
>>>> index 21b4572..9f85800 100644
>>>> --- a/xen/drivers/passthrough/arm/smmu.c
>>>> +++ b/xen/drivers/passthrough/arm/smmu.c
>>>> @@ -1536,6 +1536,48 @@ static void arm_smmu_iommu_domain_teardown(struct domain *d)
>>>> xfree(smmu_domain);
>>>> }
>>>>
>>>> +static int arm_smmu_map_page(struct domain *d, unsigned long gfn,
>>>> + unsigned long mfn, unsigned int flags)
>>>> +{
>>>> + p2m_type_t t;
>>>> +
>>>> + /* This function should only be used by gnttab code when the domain
>>>> + * is direct mapped and gfn == mfn.
>>>
>>> Is gfn !+ mfn an ASSERT-worthy condition?
>>
>> The ASSERT would only be for debug build. I'd like to have a safe guard
>> for non-debug build just in case.
>
> That's a BUG_ON then I think, assuming it would be a coding error in the
> hypervisor (rather than e.g. a guest trying to exploit the issue
> somehow).
The guest should not be able to exploit this issue. I will add a BUG_ON.
>>> Is gnttab the only possible user?
>>
>> For ARM yes.
>
> OK
>
> (out of curiosity what are the other users on x86?)
It's used for create the IOMMU PT.
>>>> + * This is only valid when the domain is directed mapped
>>>> + */
>>>> + return guest_physmap_add_entry(d, gfn, mfn, 0, t);
>>>> +}
>>>> +
>>>> +static int arm_smmu_unmap_page(struct domain *d, unsigned long gfn)
>>>> +{
>>>> + /* This function should only be used by gnttab code when the domain
>>>> + * is direct mapped
>>>> + */
>>>> + if ( !is_domain_direct_mapped(d) )
>>>> + return -EINVAL;
>>>> +
>>>> + guest_physmap_remove_page(d, gfn, gfn, 0);
>>>
>>> I think 0 here is really PAGE_ORDER_4K, is it? (other callers of this
>>> function seem to be inconsistent about this)
>>
>> Yes, assuming the guest page will always be 4K.
>
> Even if not then PAGE_ORDER_4K will make good fodder for grep...
I will use it in the next version.
>> What about introducing "dummy type" such as p2m_notype_{ro,rw} which
>> could be use in such case?
>
> notype is effectively "ram" I think, but that doesn't seem quite right
> either.
>
> I'm just worried that p2m type bits are in limited supply so I want to
> be sure using new ones is justified.
We don't really need to store those type in the P2M. We only need them
to choose the page attributes.
We could introduce a virtual type (i.e value > p2m_max_real_type) and
store p2m_invalid in the P2M.
Regards,
--
Julien Grall
next prev parent reply other threads:[~2014-05-21 14:01 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-05-19 16:23 [PATCH v8 0/4] IOMMU support for ARM Julien Grall
2014-05-19 16:23 ` [PATCH v8 1/4] xen/arm: p2m: Clean cache PT when the IOMMU doesn't support coherent walk Julien Grall
2014-05-19 16:23 ` [PATCH v8 2/4] xen: iommu: Define PAGE_{SHIFT, SIZE, ALIGN, MASK)_64K Julien Grall
2014-05-19 16:23 ` [PATCH v8 3/4] drivers/passthrough: arm: Add support for SMMU drivers Julien Grall
2014-05-19 16:24 ` [PATCH v8 4/4] xen/arm: grant: Add another entry to map MFN 1:1 in dom0 p2m Julien Grall
2014-05-20 7:46 ` Jan Beulich
2014-05-20 9:21 ` Ian Campbell
2014-05-20 10:48 ` Julien Grall
2014-05-21 13:27 ` Ian Campbell
2014-05-21 13:42 ` Julien Grall
2014-05-21 13:50 ` Ian Campbell
2014-05-21 14:01 ` Julien Grall [this message]
2014-05-21 14:40 ` Ian Campbell
2014-05-21 13:51 ` Ian Campbell
2014-05-21 13:54 ` Julien Grall
2014-05-21 14:04 ` Ian Campbell
2014-05-21 13:27 ` [PATCH v8 0/4] IOMMU support for ARM Ian Campbell
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=537CB1CF.1030206@linaro.org \
--to=julien.grall@linaro.org \
--cc=Ian.Campbell@citrix.com \
--cc=jbeulich@suse.com \
--cc=stefano.stabellini@citrix.com \
--cc=tim@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).