xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
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

  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).