Hi Jan,
On 21/11/2019 10:41, Jan Beulich wrote:
On 21.11.2019 11:07, Julien Grall wrote:
On 21/11/2019 09:04, Jan Beulich wrote:
On 20.11.2019 21:22, Julien Grall wrote:
On 14/11/2019 16:43, Jan Beulich wrote:
TBD: Does Arm actually have anything to check against in its
arch_notify_gfn()?
I understand that we want to keep the code mostly generic, but I am a
bit concerned of the extra cost to use notify_gfn() (and indirectly
iommu_notify_gfn()) for doing nothing.
I can't see any direct use of this for the foreseable future on Arm. So
could we gate this under a config option?
This is an option, sure. Alternatively I could see about making this
an inline function, but iirc there were header dependency issues.
Then again - is a call to a function doing almost nothing really so
much extra overhead on Arm.
AFAICT, this is a workaround for AMD driver. So any impact (no matter
the size) feels not right for Arm.
In this particular case, the only thing I request is to protect the
notify_gfn & callback with !CONFIG_IOMMU_FORCE_SHARE.
Oh, there already is a suitable config option. Will do (and
cover share_p2m_table() at the same time).
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -946,6 +946,16 @@ map_grant_ref(
return;
}
+ if ( paging_mode_translate(ld) /* && (op->flags & GNTMAP_host_map) */ &&
I think this wants an explanation in the code why the check is commented.
Hmm, in such a case I'd rather omit the commented condition. It
being commented has the purpose of documenting itself.
I fail to understand why GNTMAP_host_map would always be true in the case.
AFAIU the code, this is only correct for paging_mode_external(ld) == 1.
Does it mean that paging_mode_translate(ld) and paging_mode_external(ld)
are always equal? If so, what's the point of having two macro (and two
flags)?
Historical reasons. Nowadays translate == external == refcounts on
x86. But since this is common code, perhaps I better un-comment that
part of the conditional.
For this patch, this would be the ideal solution.
We might want to consider to reduce to one macro (maybe
paging_mode_translate()) if we don't expect new architecture to return a
different value for those 3 macros.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
