Messages in this thread | | | Date | Mon, 5 Feb 2024 16:05:21 +0000 | From | Sebastian Ene <> | Subject | Re: [PATCH v4 02/10] KVM: arm64: Add ptdump registration with debugfs for the stage-2 pagetables |
| |
On Mon, Feb 05, 2024 at 01:14:27PM +0000, Oliver Upton wrote: > 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. >
True I guess there is no other need for the initialization portion in this function. I will split the series to address the non-protected support first.
Thanks, Seb
> -- > Thanks, > Oliver
| |