Messages in this thread | | | Date | Mon, 5 Feb 2024 13:14:27 +0000 | From | Oliver Upton <> | Subject | Re: [PATCH v4 02/10] KVM: arm64: Add ptdump registration with debugfs for the stage-2 pagetables |
| |
On Thu, Feb 01, 2024 at 11:20:53AM +0000, Sebastian Ene wrote: > On Thu, Dec 21, 2023 at 06:14:20PM +0000, Oliver Upton wrote: > > Hi Oliver, > > I am planning to split the series based on your suggestion and I > wanted to make sure that I understand your feedback. > > > On Mon, Dec 18, 2023 at 01:58:52PM +0000, Sebastian Ene wrote: > > > +config PTDUMP_STAGE2_DEBUGFS > > > + bool "Present the stage-2 pagetables to debugfs" > > > + depends on PTDUMP_DEBUGFS && KVM > > > + default n > > > + help > > > + Say Y here if you want to show the stage-2 kernel pagetables > > > + layout in a debugfs file. This information is only useful for kernel developers > > > + who are working in architecture specific areas of the kernel. > > > + It is probably not a good idea to enable this feature in a production > > > + kernel. > > > > It isn't really a good idea to mount debugfs at all in a production > > system. There are already plenty worse interfaces lurking in that > > filesystem. The pKVM portions already depend on CONFIG_NVHE_EL2_DEBUG, > > so I don't see a need for this Kconfig option. > > > > I created a separate option because I wanted to re-use the parsing > functionality from the already existing ptdump code for EL1. This option > is turned off in production and only enabled for debug. > > I was thinking to make use of the `CONFIG_NVHE_EL2_DEBUG` but then I abandoned > this ideea as one can use ptdump for vHE as well.
Fair enough. I was going to say we could just have KVM follow CONFIG_PTDUMP_DEBUGFS, but it doesn't matter either way.
> > > +void kvm_ptdump_register_host(void) > > > +{ > > > + if (!is_protected_kvm_enabled()) > > > + return; > > > + > > > + kvm_ptdump_debugfs_register(&host_reg, "host_page_tables", > > > + kvm_debugfs_dir); > > > +} > > > + > > > +static int __init kvm_host_ptdump_init(void) > > > +{ > > > + host_reg.priv = (void *)host_s2_pgtable_pages(); > > > + return 0; > > > +} > > > + > > > +device_initcall(kvm_host_ptdump_init); > > > > Why can't all of this be called from finalize_pkvm()? > > > > I guess it can be called from finalize_pkvm before the is_protected_kvm_enabled > check. This should work for nvhe & vhe as well.
What does nvhe and vhe modes have to do with it? I thought this was for hooking up the host's S2, which does not exist outside protected mode.
-- Thanks, Oliver
| |