[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2023092342-staunch-chafe-1598@gregkh>
Date: Sat, 23 Sep 2023 09:58:45 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Nuno Das Neves <nunodasneves@...ux.microsoft.com>
Cc: linux-hyperv@...r.kernel.org, linux-kernel@...r.kernel.org,
x86@...nel.org, linux-arm-kernel@...ts.infradead.org,
linux-arch@...r.kernel.org, patches@...ts.linux.dev,
mikelley@...rosoft.com, kys@...rosoft.com, wei.liu@...nel.org,
haiyangz@...rosoft.com, decui@...rosoft.com,
apais@...ux.microsoft.com, Tianyu.Lan@...rosoft.com,
ssengar@...ux.microsoft.com, mukeshrathor@...rosoft.com,
stanislav.kinsburskiy@...il.com, jinankjain@...ux.microsoft.com,
vkuznets@...hat.com, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com,
will@...nel.org, catalin.marinas@....com
Subject: Re: [PATCH v3 15/15] Drivers: hv: Add modules to expose /dev/mshv to
VMMs running on Hyper-V
On Fri, Sep 22, 2023 at 11:38:35AM -0700, Nuno Das Neves wrote:
> +static int mshv_vtl_get_vsm_regs(void)
> +{
> + struct hv_register_assoc registers[2];
> + union hv_input_vtl input_vtl;
> + int ret, count = 2;
> +
> + input_vtl.as_uint8 = 0;
> + registers[0].name = HV_REGISTER_VSM_CODE_PAGE_OFFSETS;
> + registers[1].name = HV_REGISTER_VSM_CAPABILITIES;
> +
> + ret = hv_call_get_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
> + count, input_vtl, registers);
> + if (ret)
> + return ret;
> +
> + mshv_vsm_page_offsets.as_uint64 = registers[0].value.reg64;
> + mshv_vsm_capabilities.as_uint64 = registers[1].value.reg64;
> +
> + pr_debug("%s: VSM code page offsets: %#016llx\n", __func__,
> + mshv_vsm_page_offsets.as_uint64);
> + pr_info("%s: VSM capabilities: %#016llx\n", __func__,
> + mshv_vsm_capabilities.as_uint64);
When drivers are working properly, they are quiet. This is very noisy
and probably is leaking memory addresses to userspace?
Also, there is NEVER a need for __func__ in a pr_debug() line, it has
that for you automatically.
Also, drivers should never call pr_*() calls, always use the proper
dev_*() calls instead.
> +
> + return ret;
> +}
> +
> +static int mshv_vtl_configure_vsm_partition(void)
> +{
> + union hv_register_vsm_partition_config config;
> + struct hv_register_assoc reg_assoc;
> + union hv_input_vtl input_vtl;
> +
> + config.as_u64 = 0;
> + config.default_vtl_protection_mask = HV_MAP_GPA_PERMISSIONS_MASK;
> + config.enable_vtl_protection = 1;
> + config.zero_memory_on_reset = 1;
> + config.intercept_vp_startup = 1;
> + config.intercept_cpuid_unimplemented = 1;
> +
> + if (mshv_vsm_capabilities.intercept_page_available) {
> + pr_debug("%s: using intercept page", __func__);
Again, __func__ is not needed, you are providing it twice here for no
real reason except to waste storage space :)
> + config.intercept_page = 1;
> + }
> +
> + reg_assoc.name = HV_REGISTER_VSM_PARTITION_CONFIG;
> + reg_assoc.value.reg64 = config.as_u64;
> + input_vtl.as_uint8 = 0;
> +
> + return hv_call_set_vp_registers(HV_VP_INDEX_SELF, HV_PARTITION_ID_SELF,
> + 1, input_vtl, ®_assoc);
None of this needs to be unwound if initialization fails later on?
thanks,
greg k-h
Powered by blists - more mailing lists