|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC 14/16] xen/arm: p2m: Extend p2m_get_entry to return the value of bit[0] (valid bit)
On Mon, 5 Nov 2018, Julien Grall wrote:
> Hi Stefano,
>
> On 02/11/2018 23:44, Stefano Stabellini wrote:
> > On Mon, 8 Oct 2018, Julien Grall wrote:
> > > With the recent changes, a P2M entry may be populated but may as not
> > > valid. In some situation, it would be useful to know whether the entry
> > > has been marked available to guest in order to perform a specific
> > > action. So extend p2m_get_entry to return the value of bit[0] (valid bit).
> > >
> > > Signed-off-by: Julien Grall <julien.grall@xxxxxxx>
> > > ---
> > > xen/arch/arm/mem_access.c | 6 +++---
> > > xen/arch/arm/p2m.c | 20 ++++++++++++++++----
> > > xen/include/asm-arm/p2m.h | 3 ++-
> > > 3 files changed, 21 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/xen/arch/arm/mem_access.c b/xen/arch/arm/mem_access.c
> > > index 9239bdf323..f434510b2a 100644
> > > --- a/xen/arch/arm/mem_access.c
> > > +++ b/xen/arch/arm/mem_access.c
> > > @@ -70,7 +70,7 @@ static int __p2m_get_mem_access(struct domain *d, gfn_t
> > > gfn,
> > > * No setting was found in the Radix tree. Check if the
> > > * entry exists in the page-tables.
> > > */
> > > - mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL);
> > > + mfn_t mfn = p2m_get_entry(p2m, gfn, NULL, NULL, NULL, NULL);
> > > if ( mfn_eq(mfn, INVALID_MFN) )
> > > return -ESRCH;
> > > @@ -199,7 +199,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva,
> > > unsigned long flag,
> > > * We had a mem_access permission limiting the access, but the page
> > > type
> > > * could also be limiting, so we need to check that as well.
> > > */
> > > - mfn = p2m_get_entry(p2m, gfn, &t, NULL, NULL);
> > > + mfn = p2m_get_entry(p2m, gfn, &t, NULL, NULL, NULL);
> > > if ( mfn_eq(mfn, INVALID_MFN) )
> > > goto err;
> > > @@ -405,7 +405,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn,
> > > uint32_t nr,
> > > gfn = gfn_next_boundary(gfn, order) )
> > > {
> > > p2m_type_t t;
> > > - mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order);
> > > + mfn_t mfn = p2m_get_entry(p2m, gfn, &t, NULL, &order, NULL);
> > > if ( !mfn_eq(mfn, INVALID_MFN) )
> > > diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
> > > index 12b459924b..df6b48d73b 100644
> > > --- a/xen/arch/arm/p2m.c
> > > +++ b/xen/arch/arm/p2m.c
> > > @@ -306,10 +306,14 @@ static int p2m_next_level(struct p2m_domain *p2m,
> > > bool read_only,
> > > *
> > > * If the entry is not present, INVALID_MFN will be returned and the
> > > * page_order will be set according to the order of the invalid range.
> > > + *
> > > + * valid will contain the value of bit[0] (e.g valid bit) of the
> > > + * entry.
> > > */
> > > mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
> > > p2m_type_t *t, p2m_access_t *a,
> > > - unsigned int *page_order)
> > > + unsigned int *page_order,
> > > + bool *valid)
> > > {
> > > paddr_t addr = gfn_to_gaddr(gfn);
> > > unsigned int level = 0;
> > > @@ -317,6 +321,7 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
> > > int rc;
> > > mfn_t mfn = INVALID_MFN;
> > > p2m_type_t _t;
> > > + bool _valid;
> > > /* Convenience aliases */
> > > const unsigned int offsets[4] = {
> > > @@ -334,6 +339,10 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t
> > > gfn,
> > > *t = p2m_invalid;
> > > + /* Allow valid to be NULL */
> > > + valid = valid?: &_valid;
> > > + *valid = false;
> >
> > Why not a simple:
> >
> > if ( valid )
> > *valid = false;
> >
> > especially given that you do the same if ( valid ) check below.
> > In fact, it doesn' look like we need _valid?
>
> I thought I dropped the if ( valid ) below. I would actually prefer to keep
> _valid and avoid using if ( ... ) everywhere.
>
> This makes the code slightly easier to follow.
_valid is a good trick, but I don't think is worth doing it in this
change: it is easy to follow anyway. Up to you, I am fine either way.
> >
> >
> > > /* XXX: Check if the mapping is lower than the mapped gfn */
> > > /* This gfn is higher than the highest the p2m map currently holds
> > > */
> > > @@ -379,6 +388,9 @@ mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
> > > * to the GFN.
> > > */
> > > mfn = mfn_add(mfn, gfn_x(gfn) & ((1UL << level_orders[level]) -
> > > 1));
> > > +
> > > + if ( valid )
> > > + *valid = lpae_is_valid(entry);
> > > }
> > > out_unmap:
> > > @@ -397,7 +409,7 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn,
> > > p2m_type_t *t)
> > > struct p2m_domain *p2m = p2m_get_hostp2m(d);
> > > p2m_read_lock(p2m);
> > > - mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL);
> > > + mfn = p2m_get_entry(p2m, gfn, t, NULL, NULL, NULL);
> > > p2m_read_unlock(p2m);
> > > return mfn;
> > > @@ -1464,7 +1476,7 @@ int relinquish_p2m_mapping(struct domain *d)
> > > for ( ; gfn_x(start) < gfn_x(end);
> > > start = gfn_next_boundary(start, order) )
> > > {
> > > - mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order);
> > > + mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order, NULL);
> > > count++;
> > > /*
> > > @@ -1527,7 +1539,7 @@ int p2m_cache_flush_range(struct domain *d, gfn_t
> > > start, gfn_t end)
> > > for ( ; gfn_x(start) < gfn_x(end); start = next_gfn )
> > > {
> > > - mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order);
> > > + mfn_t mfn = p2m_get_entry(p2m, start, &t, NULL, &order, NULL);
> > > next_gfn = gfn_next_boundary(start, order);
> > > diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h
> > > index d7afa2bbe8..92213dd1ab 100644
> > > --- a/xen/include/asm-arm/p2m.h
> > > +++ b/xen/include/asm-arm/p2m.h
> > > @@ -211,7 +211,8 @@ mfn_t p2m_lookup(struct domain *d, gfn_t gfn,
> > > p2m_type_t *t);
> > > */
> > > mfn_t p2m_get_entry(struct p2m_domain *p2m, gfn_t gfn,
> > > p2m_type_t *t, p2m_access_t *a,
> > > - unsigned int *page_order);
> > > + unsigned int *page_order,
> > > + bool *valid);
> > > /*
> > > * Direct set a p2m entry: only for use by the P2M code.
> > > --
> > > 2.11.0
> > >
>
> Cheers,
>
> --
> Julien Grall
>
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |