From: Abhinav Singh <singhabhinav9051571833@xxxxxxxxx> Sent: Tuesday, October 24, 2023 4:29 AM > Subject lines usually have a prefix to indicate the area of the kernel the patch is for. We're not always super consistent with the prefixes, but you can look at the commit log for a file to see what is typically used. In this case, the prefix is usually "x86/hyperv:" > > This patch fixes sparse complaining about the removal of __iomem address > space when casting the return value of this function ioremap_cache(...) > from `void __ioremap*` to `void*`. Should avoid wording like "this patch" in commit messages. See the commit message guidelines in the "Describe your changes" section of Documentation/process/submitting-patches.rst. A better approach is to just state the problem: "Sparse complains about the removal .....". Then describe the fix. Also avoid pronouns like "I" or "you". > > I think there are two way of fixing it, first one is changing the > datatype of variable `ghcb_va` from `void*` to `void __iomem*` . > Second way of fixing it is using the memremap(...) which is > done in this patch. > > Signed-off-by: Abhinav Singh <singhabhinav9051571833@xxxxxxxxx> > --- > arch/x86/hyperv/hv_init.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c > index 21556ad87f4b..c14161add274 100644 > --- a/arch/x86/hyperv/hv_init.c > +++ b/arch/x86/hyperv/hv_init.c > @@ -70,7 +70,7 @@ static int hyperv_init_ghcb(void) > > /* Mask out vTOM bit. ioremap_cache() maps decrypted */ This comment mentions ioremap_cache(). Since you are changing to use memremap() instead, the comment should be updated to match. > ghcb_gpa &= ~ms_hyperv.shared_gpa_boundary; > - ghcb_va = (void *)ioremap_cache(ghcb_gpa, HV_HYP_PAGE_SIZE); > + ghcb_va = memremap(ghcb_gpa, HV_HYP_PAGE_SIZE, MEMREMAP_WB); As noted in the comment, ioremap_cache() provides a mapping that accesses the memory as decrypted. To be equivalent, the call to memremap() should include the MEMREMAP_DEC flag so that it also is assured of producing a decrypted mapping. Also, corresponding to the current ioremap_cache() call here, there's an iounmap() call in hv_cpu_die(). To maintain proper pairing, that iounmap() call should be changed to memunmap(). It turns out there are other occurrences of this same pattern in Hyper-V specific code in the Linux kernel. See hv_synic_enable_regs(), for example. Did "sparse" flag the same problem in those occurrences? It turns out that Nischala Yelchuri at Microsoft is concurrently working on fixing this occurrence as well as the others we know about in Hyper-V specific code. Michael > > -- > 2.39.2
