Hi Oliver,
On 11/10/22 8:42 AM, Oliver Upton wrote:
On Thu, Nov 10, 2022 at 08:23:36AM +0800, Gavin Shan wrote:
On 11/8/22 5:56 AM, Oliver Upton wrote:
Passing new arguments by value to the visitor callbacks is extremely
inflexible for stuffing new parameters used by only some of the
visitors. Use a context structure instead and pass the pointer through
to the visitor callback.
While at it, redefine the 'flags' parameter to the visitor to contain
the bit indicating the phase of the walk. Pass the entire set of flags
through the context structure such that the walker can communicate
additional state to the visitor callback.
No functional change intended.
Signed-off-by: Oliver Upton <oliver.upton@xxxxxxxxx>
---
arch/arm64/include/asm/kvm_pgtable.h | 15 +-
arch/arm64/kvm/hyp/nvhe/mem_protect.c | 10 +-
arch/arm64/kvm/hyp/nvhe/setup.c | 16 +-
arch/arm64/kvm/hyp/pgtable.c | 269 +++++++++++++-------------
4 files changed, 154 insertions(+), 156 deletions(-)
Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx>
One nit below.
diff --git a/arch/arm64/include/asm/kvm_pgtable.h b/arch/arm64/include/asm/kvm_pgtable.h
index 3252eb50ecfe..607f9bb8aab4 100644
--- a/arch/arm64/include/asm/kvm_pgtable.h
+++ b/arch/arm64/include/asm/kvm_pgtable.h
@@ -199,10 +199,17 @@ enum kvm_pgtable_walk_flags {
KVM_PGTABLE_WALK_TABLE_POST = BIT(2),
};
-typedef int (*kvm_pgtable_visitor_fn_t)(u64 addr, u64 end, u32 level,
- kvm_pte_t *ptep,
- enum kvm_pgtable_walk_flags flag,
- void * const arg);
+struct kvm_pgtable_visit_ctx {
+ kvm_pte_t *ptep;
+ void *arg;
+ u64 addr;
+ u64 end;
+ u32 level;
+ enum kvm_pgtable_walk_flags flags;
+};
+
+typedef int (*kvm_pgtable_visitor_fn_t)(const struct kvm_pgtable_visit_ctx *ctx,
+ enum kvm_pgtable_walk_flags visit);
Does it make sense to reorder these fields in the context struct based on
their properties.
The ordering was a deliberate optimization for space. Your suggestion
has 8 bytes of implicit padding:
Right, so how about to rearrange these fields like below? It makes
more sense to have @arg after addr/end/ptep.
struct kvm_pgtable_visit_ctx {
enum kvm_pgtable_walk_flags flags;
u32 level;
u64 addr;
u64 end;
kvm_pte_t *ptep;
void *arg;
};
[...]
Thanks,
Gavin
