| Message ID | 1466429845-28240-9-git-send-email-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Andrew, On 20/06/16 15:53, Andrew Cooper wrote: > On 20/06/16 14:37, Julien Grall wrote: >> p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename >> the variable to *gfn* and use typesafe to avoid possible misusage. >> >> Signed-off-by: Julien Grall <julien.grall@arm.com> > > On arm32, xen_pfn_t was uint64_t, but gfn_t is unsigned long. > > Is the truncation ok? The PFN will be encoded on 28 bits maximum (40 bits address). Unless we want to check that the guest effectively zeroed the unused bits, I think the truncation is fine. FWIW, this is not the only place where the truncation xen_pfn_t (aka uin64_t) -> unsigned long happens. Regards,
On 20/06/16 16:36, Andrew Cooper wrote: > On 20/06/16 16:03, Julien Grall wrote: >> Hi Andrew, >> >> On 20/06/16 15:53, Andrew Cooper wrote: >>> On 20/06/16 14:37, Julien Grall wrote: >>>> p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename >>>> the variable to *gfn* and use typesafe to avoid possible misusage. >>>> >>>> Signed-off-by: Julien Grall <julien.grall@arm.com> >>> >>> On arm32, xen_pfn_t was uint64_t, but gfn_t is unsigned long. >>> >>> Is the truncation ok? >> >> The PFN will be encoded on 28 bits maximum (40 bits address). Unless >> we want to check that the guest effectively zeroed the unused bits, I >> think the truncation is fine. > > Ok - I was just checking that it wasn't happening accidentally. I will mention it in the commit message. Cheers,
diff --git a/xen/arch/arm/domctl.c b/xen/arch/arm/domctl.c index 30453d8..b94e97c 100644 --- a/xen/arch/arm/domctl.c +++ b/xen/arch/arm/domctl.c @@ -30,7 +30,7 @@ long arch_do_domctl(struct xen_domctl *domctl, struct domain *d, if ( e < s ) return -EINVAL; - return p2m_cache_flush(d, s, e); + return p2m_cache_flush(d, _gfn(s), _gfn(e)); } case XEN_DOMCTL_bind_pt_irq: { diff --git a/xen/arch/arm/p2m.c b/xen/arch/arm/p2m.c index f3330dd..9149981 100644 --- a/xen/arch/arm/p2m.c +++ b/xen/arch/arm/p2m.c @@ -1469,16 +1469,16 @@ int relinquish_p2m_mapping(struct domain *d) d->arch.p2m.default_access); } -int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn) +int p2m_cache_flush(struct domain *d, gfn_t start, gfn_t end) { struct p2m_domain *p2m = &d->arch.p2m; - start_mfn = MAX(start_mfn, p2m->lowest_mapped_gfn); - end_mfn = MIN(end_mfn, p2m->max_mapped_gfn); + start = gfn_max(start, _gfn(p2m->lowest_mapped_gfn)); + end = gfn_min(end, _gfn(p2m->max_mapped_gfn)); return apply_p2m_changes(d, CACHEFLUSH, - pfn_to_paddr(start_mfn), - pfn_to_paddr(end_mfn), + pfn_to_paddr(gfn_x(start)), + pfn_to_paddr(gfn_x(end)), pfn_to_paddr(INVALID_MFN), MATTR_MEM, 0, p2m_invalid, d->arch.p2m.default_access); diff --git a/xen/include/asm-arm/p2m.h b/xen/include/asm-arm/p2m.h index f204482..976e51e 100644 --- a/xen/include/asm-arm/p2m.h +++ b/xen/include/asm-arm/p2m.h @@ -139,7 +139,7 @@ void p2m_dump_info(struct domain *d); mfn_t p2m_lookup(struct domain *d, gfn_t gfn, p2m_type_t *t); /* Clean & invalidate caches corresponding to a region of guest address space */ -int p2m_cache_flush(struct domain *d, xen_pfn_t start_mfn, xen_pfn_t end_mfn); +int p2m_cache_flush(struct domain *d, gfn_t start, gfn_t end); /* Setup p2m RAM mapping for domain d from start-end. */ int p2m_populate_ram(struct domain *d, paddr_t start, paddr_t end);
p2m_cache_flush is expecting GFNs in parameter and not MFNs. Rename the variable to *gfn* and use typesafe to avoid possible misusage. Signed-off-by: Julien Grall <julien.grall@arm.com> --- Changes in v2: - Drop _gfn suffix --- xen/arch/arm/domctl.c | 2 +- xen/arch/arm/p2m.c | 10 +++++----- xen/include/asm-arm/p2m.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-)