[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH 12/18] arm/altp2m: Cosmetic fixes - function prototypes.
Hi Sergej,
On 04/07/16 12:45, Sergej Proskurin wrote:
This commit changes the prototype of the following functions:
- apply_p2m_changes
- apply_one_level
- p2m_shatter_page
- p2m_create_table
- __p2m_lookup
- __p2m_get_mem_access
These changes are required as our implementation reuses most of the
existing ARM p2m implementation to set page table attributes of the
individual altp2m views. Therefore, exiting function prototypes have
been extended to hold another argument (of type struct p2m_domain *).
This allows to specify the p2m/altp2m domain that should be processed by
the individual function -- instead of accessing the host's default p2m
domain.
I am actually reworking the whole p2m code to be complain with the ARM
architecture (such as break-before-make) and make easier to implement
new features such as altp2m.
Would it be possible to send this patch separately with nothing altp2m
related in it?
Signed-off-by: Sergej Proskurin <proskurin@xxxxxxxxxxxxx>
---
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
Cc: Julien Grall <julien.grall@xxxxxxx>
---
xen/arch/arm/p2m.c | 80 +++++++++++++++++++++++++++++-------------------------
1 file changed, 43 insertions(+), 37 deletions(-)
diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c
index 019f10e..9c8fefd 100644
--- a/xen/arch/arm/p2m.c
+++ b/xen/arch/arm/p2m.c
@@ -200,9 +200,8 @@ void flush_tlb_domain(struct domain *d)
* There are no processor functions to do a stage 2 only lookup therefore we
* do a a software walk.
*/
-static paddr_t __p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
+static paddr_t __p2m_lookup(struct p2m_domain *p2m, paddr_t paddr, p2m_type_t
*t)
{
- struct p2m_domain *p2m = &d->arch.p2m;
const unsigned int offsets[4] = {
zeroeth_table_offset(paddr),
first_table_offset(paddr),
@@ -282,10 +281,11 @@ err:
paddr_t p2m_lookup(struct domain *d, paddr_t paddr, p2m_type_t *t)
{
paddr_t ret;
- struct p2m_domain *p2m = &d->arch.p2m;
+ struct vcpu *v = current;
+ struct p2m_domain *p2m = altp2m_active(d) ? p2m_get_altp2m(v) :
p2m_get_hostp2m(d);
This change is wrong, this function is called in hypercalls to translate
an IPA for another domain to an MFN. So current->domain != d.
spin_lock(&p2m->lock);
- ret = __p2m_lookup(d, paddr, t);
+ ret = __p2m_lookup(p2m, paddr, t);
spin_unlock(&p2m->lock);
return ret;
@@ -441,10 +441,12 @@ static inline void p2m_remove_pte(lpae_t *p, bool_t
flush_cache)
*
* level_shift is the number of bits at the level we want to create.
*/
-static int p2m_create_table(struct domain *d, lpae_t *entry,
- int level_shift, bool_t flush_cache)
+static int p2m_create_table(struct domain *d,
Please drop "struct domain *d", it was only here to get the p2m.
+ struct p2m_domain *p2m,
+ lpae_t *entry,
+ int level_shift,
+ bool_t flush_cache)
{
- struct p2m_domain *p2m = &d->arch.p2m;
struct page_info *page;
lpae_t *p;
lpae_t pte;
@@ -502,10 +504,9 @@ static int p2m_create_table(struct domain *d, lpae_t
*entry,
return 0;
}
-static int __p2m_get_mem_access(struct domain *d, gfn_t gfn,
+static int __p2m_get_mem_access(struct p2m_domain *p2m, gfn_t gfn,
xenmem_access_t *access)
{
- struct p2m_domain *p2m = p2m_get_hostp2m(d);
void *i;
unsigned int index;
@@ -548,7 +549,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.
*/
- paddr_t maddr = __p2m_lookup(d, gfn_x(gfn) << PAGE_SHIFT, NULL);
+ paddr_t maddr = __p2m_lookup(p2m, gfn_x(gfn) << PAGE_SHIFT, NULL);
if ( INVALID_PADDR == maddr )
return -ESRCH;
@@ -677,17 +678,17 @@ static const paddr_t level_shifts[] =
{ ZEROETH_SHIFT, FIRST_SHIFT, SECOND_SHIFT, THIRD_SHIFT };
static int p2m_shatter_page(struct domain *d,
Ditto.
+ struct p2m_domain *p2m,
lpae_t *entry,
unsigned int level,
bool_t flush_cache)
{
const paddr_t level_shift = level_shifts[level];
- int rc = p2m_create_table(d, entry,
+ int rc = p2m_create_table(d, p2m, entry,
level_shift - PAGE_SHIFT, flush_cache);
if ( !rc )
{
- struct p2m_domain *p2m = &d->arch.p2m;
p2m->stats.shattered[level]++;
p2m->stats.mappings[level]--;
p2m->stats.mappings[level+1] += LPAE_ENTRIES;
@@ -704,6 +705,7 @@ static int p2m_shatter_page(struct domain *d,
* -ve == (-Exxx) error.
*/
static int apply_one_level(struct domain *d,
Ditto.
+ struct p2m_domain *p2m,
lpae_t *entry,
unsigned int level,
bool_t flush_cache,
@@ -721,7 +723,6 @@ static int apply_one_level(struct domain *d,
const paddr_t level_mask = level_masks[level];
const paddr_t level_shift = level_shifts[level];
- struct p2m_domain *p2m = &d->arch.p2m;
lpae_t pte;
const lpae_t orig_pte = *entry;
int rc;
[...]
@@ -1011,6 +1012,7 @@ static void update_reference_mapping(struct page_info
*page,
}
static int apply_p2m_changes(struct domain *d,
I would like to see "struct domain *d" dropped completely. If we really
need it, we could introduce a back pointer to domain.
+ struct p2m_domain *p2m,
enum p2m_operation op,
paddr_t start_gpaddr,
paddr_t end_gpaddr,
[...]
@@ -1743,6 +1745,9 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
long flag)
xenmem_access_t xma;
p2m_type_t t;
struct page_info *page = NULL;
+ struct vcpu *v = current;
+ struct domain *d = v->domain;
+ struct p2m_domain *p2m = altp2m_active(d) ? p2m_get_altp2m(v) :
p2m_get_hostp2m(d);
This patch is supposed to be "comestic fixes", so this change does not
belong to this patch.
However, the changes within this function look wrong to me because
p2m_mem_access_check_and_get_page is used by Xen to get the page when
copying data to/from the guest. If the entry is not yet replicated in
the altp2m, you will fail to copy the data.
You may also want to consider how get_page_from_gva would work if an
altp2m is used.
rc = gva_to_ipa(gva, &ipa, flag);
This is not related to this patch, but I think gva_to_ipa can fail to
translate a VA to an IPA if the stage-1 page table reside in memory
which was restricted by memaccess.
if ( rc < 0 )
@@ -1752,7 +1757,7 @@ p2m_mem_access_check_and_get_page(vaddr_t gva, unsigned
long flag)
* We do this first as this is faster in the default case when no
* permission is set on the page.
*/
- rc = __p2m_get_mem_access(current->domain, _gfn(paddr_to_pfn(ipa)), &xma);
+ rc = __p2m_get_mem_access(p2m, _gfn(paddr_to_pfn(ipa)), &xma);
if ( rc < 0 )
goto err;
@@ -1801,7 +1806,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.
*/
- maddr = __p2m_lookup(current->domain, ipa, &t);
+ maddr = __p2m_lookup(p2m, ipa, &t);
if ( maddr == INVALID_PADDR )
goto err;
@@ -2125,7 +2130,7 @@ long p2m_set_mem_access(struct domain *d, gfn_t gfn,
uint32_t nr,
return 0;
}
- rc = apply_p2m_changes(d, MEMACCESS,
+ rc = apply_p2m_changes(d, p2m, MEMACCESS,
pfn_to_paddr(gfn_x(gfn) + start),
pfn_to_paddr(gfn_x(gfn) + nr),
0, MATTR_MEM, mask, 0, a);
@@ -2141,10 +2146,11 @@ int p2m_get_mem_access(struct domain *d, gfn_t gfn,
xenmem_access_t *access)
{
int ret;
- struct p2m_domain *p2m = p2m_get_hostp2m(d);
+ struct vcpu *v = current;
+ struct p2m_domain *p2m = altp2m_active(d) ? p2m_get_altp2m(v) :
p2m_get_hostp2m(d);
Same remark as above, this patch is only "comestic" and functional
changes does not belong to this patch.
spin_lock(&p2m->lock);
- ret = __p2m_get_mem_access(d, gfn, access);
+ ret = __p2m_get_mem_access(p2m, gfn, access);
spin_unlock(&p2m->lock);
return ret;
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel
|
