Messages in this thread Patch in this message | | | Date | Wed, 22 Nov 2023 23:35:57 +0000 | From | Oliver Upton <> | Subject | Re: [PATCH v3 10/10] arm64: ptdump: Add support for guest stage-2 pagetables dumping |
| |
On Wed, Nov 15, 2023 at 05:16:40PM +0000, Sebastian Ene wrote: > +struct ptdump_registered_guest { > + struct list_head reg_list; > + struct ptdump_info info; > + struct kvm_pgtable_snapshot snapshot; > + rwlock_t *lock; > +};
Why can't we just store a pointer directly to struct kvm in ::private? Also, shouldn't you take a reference on struct kvm when the file is opened to protect against VM teardown?
> +static LIST_HEAD(ptdump_guest_list); > +static DEFINE_MUTEX(ptdump_list_lock);
What is the list for?
> static phys_addr_t ptdump_host_pa(void *addr) > { > return __pa(addr); > @@ -757,6 +768,63 @@ static void stage2_ptdump_walk(struct seq_file *s, struct ptdump_info *info) > kvm_pgtable_walk(pgtable, start_ipa, end_ipa, &walker); > } > > +static void guest_stage2_ptdump_walk(struct seq_file *s, > + struct ptdump_info *info) > +{ > + struct ptdump_info_file_priv *f_priv = > + container_of(info, struct ptdump_info_file_priv, info); > + struct ptdump_registered_guest *guest = info->priv; > + > + f_priv->file_priv = &guest->snapshot; > + > + read_lock(guest->lock); > + stage2_ptdump_walk(s, info); > + read_unlock(guest->lock);
Taking the mmu lock for read allows other table walkers to add new mappings and adjust the granularity of existing ones. Should this instead take the mmu lock for write?
> +} > + > +int ptdump_register_guest_stage2(struct kvm *kvm) > +{ > + struct ptdump_registered_guest *guest; > + struct kvm_s2_mmu *mmu = &kvm->arch.mmu; > + struct kvm_pgtable *pgt = mmu->pgt; > + > + guest = kzalloc(sizeof(struct ptdump_registered_guest), GFP_KERNEL);
You want GFP_KERNEL_ACCOUNT here.
-- Thanks, Oliver
| |