Re: [PATCH v5 01/14] KVM: arm64: Combine visitor arguments into a context structure

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux