| Message ID | 9ade638a-6a63-8f32-9d09-cf9207964f45@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 <proskurin@sec.in.tum.de> >>>> --- >>>> Cc: Stefano Stabellini <sstabellini@kernel.org> >>>> Cc: Julien Grall <julien.grall@arm.com> >>>> --- >>>> 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 <julien.grall@arm.com> > 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 <julien.grall@arm.com> > > --- > 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); > > Cheers, ~Sergej
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);