Re: [Xen-devel] [PATCH v3 02/38] arm/p2m: Expose p2m_*lock helpers
Hi Julien,
On 09/02/2016 12:15 PM, Julien Grall wrote:
> On 02/09/16 11:12, Sergej Proskurin wrote:
>> Hi Julien,
>
> Hello Sergej,
>
>> On 09/01/2016 05:48 PM, Julien Grall wrote:
>>> Hello Sergej,
>>>
>>> On 16/08/16 23:16, Sergej Proskurin wrote:
This commit exposes the "p2m_*lock" helpers, as they will be used within
the file ./xen/arch/arm/altp2m.c, as will be shown in the following
commits.
Signed-off-by: Sergej Proskurin
---
Cc: Stefano Stabellini
Cc: Julien Grall
---
xen/arch/arm/p2m.c| 12 ++--
xen/include/asm-arm/p2m.h | 16
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index da6c7d4..08114d8 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -62,14 +62,14 @@ static inline bool_t p2m_is_superpage(lpae_t pte,
unsigned int level)
return (level < 3) && p2m_mapping(pte);
}
-static inline void p2m_write_lock(struct p2m_domain *p2m)
+void p2m_write_lock(struct p2m_domain *p2m)
>>>
>>> This will introduce an overhead when locking the p2m. Those helpers
>>> should be moved a inline in p2m.h and not transform to a functions.
>>>
>>
>> I did not move the definitions of the functions into the header file to
>> avoid exposing the static function p2m_flush_tlb. However, what I can do
>> is to declare the function prototypes in p2m.h and define them as inline
>> in p2m.c, which should provide the same effect.
>
> It will not have the same effect for call of p2m_*lock outside of p2m.c.
> p2m_flush_tlb is only used in p2m_write_unlock, so what is the problem to
> move the implement of all p2m_*lock but p2m_write_unlock in p2m.h?
>
There is no problem at all. I will adapt this patch based on your
suggested patch, thank you.
> commit 57b5214c6225c113b3cc50edbadce81b84e0a32c
> Author: Julien Grall
> Date: Tue Aug 2 15:48:48 2016 +0100
>
> xen/arm: p2m: Export p2m_*_lock helpers
>
> Early patch exported the p2m interface (p2m_get_entry and p2m_set_entry)
> to allow splitting xen/arch/arm/p2m.c. Those functions require the
> callers to lock the p2m, so we need to export p2m_*_lock helpers.
>
> All helpers but p2m_write_unlock but p2m_write_unlock are moved in
> xen/include/asm-arm/p2m.h to allow inlining. The helpers
> p2m_write_unlock is kept in p2m.c because it depends on a static
> function.
>
> Signed-off-by: Julien Grall
>
> ---
> Changes in v2:
> - Patch added
>
> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> index f4a808c..e41ed3c 100644
> --- a/xen/arch/arm/p2m.c
> +++ b/xen/arch/arm/p2m.c
> @@ -62,14 +62,10 @@ static inline bool_t p2m_is_superpage(lpae_t pte,
> unsigned int level)
> return (level < 3) && p2m_mapping(pte);
> }
>
> -static inline void p2m_write_lock(struct p2m_domain *p2m)
> -{
> -write_lock(&p2m->lock);
> -}
> -
> static void p2m_flush_tlb(struct p2m_domain *p2m);
>
> -static inline void p2m_write_unlock(struct p2m_domain *p2m)
> +/* Unlock the flush and do a P2M TLB flush if necessary */
> +void p2m_write_unlock(struct p2m_domain *p2m)
> {
> if ( p2m->need_flush )
> {
> @@ -85,26 +81,6 @@ static inline void p2m_write_unlock(struct p2m_domain *p2m)
> write_unlock(&p2m->lock);
> }
>
> -static inline void p2m_read_lock(struct p2m_domain *p2m)
> -{
> -read_lock(&p2m->lock);
> -}
> -
> -static inline void p2m_read_unlock(struct p2m_domain *p2m)
> -{
> -read_unlock(&p2m->lock);
> -}
> -
> -static inline int p2m_is_locked(struct p2m_domain *p2m)
> -{
> -return rw_is_locked(&p2m->lock);
> -}
> -
> -static inline int p2m_is_write_locked(struct p2m_domain *p2m)
> -{
> -return rw_is_write_locked(&p2m->lock);
> -}
> -
> void p2m_dump_info(struct domain *d)
> {
> struct p2m_domain *p2m = &d->arch.p2m;
> diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> index c2f4f98..9b0cade 100644
> --- a/xen/include/asm-arm/p2m.h
> +++ b/xen/include/asm-arm/p2m.h
> @@ -173,6 +173,33 @@ void p2m_restore_state(struct vcpu *n);
> /* Print debugging/statistial info about a domain's p2m */
> void p2m_dump_info(struct domain *d);
>
> +static inline void p2m_write_lock(struct p2m_domain *p2m)
> +{
> +write_lock(&p2m->lock);
> +}
> +
> +void p2m_write_unlock(struct p2m_domain *p2m);
> +
> +static inline void p2m_read_lock(struct p2m_domain *p2m)
> +{
> +read_lock(&p2m->lock);
> +}
> +
> +static inline void p2m_read_unlock(struct p2m_domain *p2m)
> +{
> +read_unlock(&p2m->lock);
> +}
> +
> +static inline int p2m_is_locked(struct p2m_domain *p2m)
> +{
> +return rw_is_locked(&p2m->lock);
> +}
> +
> +static inline int p2m_is_write_locked(struct p2m_domain *p2m)
> +{
> +return rw_is_write_locked(&p2m->lock);
> +}
> +
> /* Look up the MFN corresponding to a domain's GFN. */
> mfn_t p2m_lookup(struct domain
Re: [Xen-devel] [PATCH v3 02/38] arm/p2m: Expose p2m_*lock helpers
On 02/09/16 11:12, Sergej Proskurin wrote:
> Hi Julien,
Hello Sergej,
> On 09/01/2016 05:48 PM, Julien Grall wrote:
>> Hello Sergej,
>>
>> On 16/08/16 23:16, Sergej Proskurin wrote:
>>> This commit exposes the "p2m_*lock" helpers, as they will be used within
>>> the file ./xen/arch/arm/altp2m.c, as will be shown in the following
>>> commits.
>>>
>>> Signed-off-by: Sergej Proskurin
>>> ---
>>> Cc: Stefano Stabellini
>>> Cc: Julien Grall
>>> ---
>>> xen/arch/arm/p2m.c| 12 ++--
>>> xen/include/asm-arm/p2m.h | 16
>>> 2 files changed, 22 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
>>> index da6c7d4..08114d8 100644
>>> --- a/xen/arch/arm/p2m.c
>>> +++ b/xen/arch/arm/p2m.c
>>> @@ -62,14 +62,14 @@ static inline bool_t p2m_is_superpage(lpae_t pte,
>>> unsigned int level)
>>> return (level < 3) && p2m_mapping(pte);
>>> }
>>>
>>> -static inline void p2m_write_lock(struct p2m_domain *p2m)
>>> +void p2m_write_lock(struct p2m_domain *p2m)
>>
>> This will introduce an overhead when locking the p2m. Those helpers
>> should be moved a inline in p2m.h and not transform to a functions.
>>
>
> I did not move the definitions of the functions into the header file to
> avoid exposing the static function p2m_flush_tlb. However, what I can do
> is to declare the function prototypes in p2m.h and define them as inline
> in p2m.c, which should provide the same effect.
It will not have the same effect for call of p2m_*lock outside of p2m.c.
p2m_flush_tlb is only used in p2m_write_unlock, so what is the problem to
move the implement of all p2m_*lock but p2m_write_unlock in p2m.h?
commit 57b5214c6225c113b3cc50edbadce81b84e0a32c
Author: Julien Grall
Date: Tue Aug 2 15:48:48 2016 +0100
xen/arm: p2m: Export p2m_*_lock helpers
Early patch exported the p2m interface (p2m_get_entry and p2m_set_entry)
to allow splitting xen/arch/arm/p2m.c. Those functions require the
callers to lock the p2m, so we need to export p2m_*_lock helpers.
All helpers but p2m_write_unlock but p2m_write_unlock are moved in
xen/include/asm-arm/p2m.h to allow inlining. The helpers
p2m_write_unlock is kept in p2m.c because it depends on a static
function.
Signed-off-by: Julien Grall
---
Changes in v2:
- Patch added
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index f4a808c..e41ed3c 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -62,14 +62,10 @@ static inline bool_t p2m_is_superpage(lpae_t pte, unsigned
int level)
return (level < 3) && p2m_mapping(pte);
}
-static inline void p2m_write_lock(struct p2m_domain *p2m)
-{
-write_lock(&p2m->lock);
-}
-
static void p2m_flush_tlb(struct p2m_domain *p2m);
-static inline void p2m_write_unlock(struct p2m_domain *p2m)
+/* Unlock the flush and do a P2M TLB flush if necessary */
+void p2m_write_unlock(struct p2m_domain *p2m)
{
if ( p2m->need_flush )
{
@@ -85,26 +81,6 @@ static inline void p2m_write_unlock(struct p2m_domain *p2m)
write_unlock(&p2m->lock);
}
-static inline void p2m_read_lock(struct p2m_domain *p2m)
-{
-read_lock(&p2m->lock);
-}
-
-static inline void p2m_read_unlock(struct p2m_domain *p2m)
-{
-read_unlock(&p2m->lock);
-}
-
-static inline int p2m_is_locked(struct p2m_domain *p2m)
-{
-return rw_is_locked(&p2m->lock);
-}
-
-static inline int p2m_is_write_locked(struct p2m_domain *p2m)
-{
-return rw_is_write_locked(&p2m->lock);
-}
-
void p2m_dump_info(struct domain *d)
{
struct p2m_domain *p2m = &d->arch.p2m;
diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
index c2f4f98..9b0cade 100644
--- a/xen/include/asm-arm/p2m.h
+++ b/xen/include/asm-arm/p2m.h
@@ -173,6 +173,33 @@ void p2m_restore_state(struct vcpu *n);
/* Print debugging/statistial info about a domain's p2m */
void p2m_dump_info(struct domain *d);
+static inline void p2m_write_lock(struct p2m_domain *p2m)
+{
+write_lock(&p2m->lock);
+}
+
+void p2m_write_unlock(struct p2m_domain *p2m);
+
+static inline void p2m_read_lock(struct p2m_domain *p2m)
+{
+read_lock(&p2m->lock);
+}
+
+static inline void p2m_read_unlock(struct p2m_domain *p2m)
+{
+read_unlock(&p2m->lock);
+}
+
+static inline int p2m_is_locked(struct p2m_domain *p2m)
+{
+return rw_is_locked(&p2m->lock);
+}
+
+static inline int p2m_is_write_locked(struct p2m_domain *p2m)
+{
+return rw_is_write_locked(&p2m->lock);
+}
+
/* Look up the MFN corresponding to a domain's GFN. */
mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t);
--
Julien Grall
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 02/38] arm/p2m: Expose p2m_*lock helpers
Hi Julien, On 09/01/2016 05:48 PM, Julien Grall wrote: > Hello Sergej, > > On 16/08/16 23:16, Sergej Proskurin wrote: >> This commit exposes the "p2m_*lock" helpers, as they will be used within >> the file ./xen/arch/arm/altp2m.c, as will be shown in the following >> commits. >> >> Signed-off-by: Sergej Proskurin >> --- >> Cc: Stefano Stabellini >> Cc: Julien Grall >> --- >> xen/arch/arm/p2m.c| 12 ++-- >> xen/include/asm-arm/p2m.h | 16 >> 2 files changed, 22 insertions(+), 6 deletions(-) >> >> diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c >> index da6c7d4..08114d8 100644 >> --- a/xen/arch/arm/p2m.c >> +++ b/xen/arch/arm/p2m.c >> @@ -62,14 +62,14 @@ static inline bool_t p2m_is_superpage(lpae_t pte, >> unsigned int level) >> return (level < 3) && p2m_mapping(pte); >> } >> >> -static inline void p2m_write_lock(struct p2m_domain *p2m) >> +void p2m_write_lock(struct p2m_domain *p2m) > > This will introduce an overhead when locking the p2m. Those helpers > should be moved a inline in p2m.h and not transform to a functions. > I did not move the definitions of the functions into the header file to avoid exposing the static function p2m_flush_tlb. However, what I can do is to declare the function prototypes in p2m.h and define them as inline in p2m.c, which should provide the same effect. Best regards, ~Sergej ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v3 02/38] arm/p2m: Expose p2m_*lock helpers
Hello Sergej, On 16/08/16 23:16, Sergej Proskurin wrote: This commit exposes the "p2m_*lock" helpers, as they will be used within the file ./xen/arch/arm/altp2m.c, as will be shown in the following commits. Signed-off-by: Sergej Proskurin --- Cc: Stefano Stabellini Cc: Julien Grall --- xen/arch/arm/p2m.c| 12 ++-- xen/include/asm-arm/p2m.h | 16 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index da6c7d4..08114d8 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -62,14 +62,14 @@ static inline bool_t p2m_is_superpage(lpae_t pte, unsigned int level) return (level < 3) && p2m_mapping(pte); } -static inline void p2m_write_lock(struct p2m_domain *p2m) +void p2m_write_lock(struct p2m_domain *p2m) This will introduce an overhead when locking the p2m. Those helpers should be moved a inline in p2m.h and not transform to a functions. Regards, -- Julien Grall ___ Xen-devel mailing list Xen-devel@lists.xen.org https://lists.xen.org/xen-devel
