lkml.org 
[lkml]   [2023]   [Oct]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
SubjectRe: [PATCH] Fixing warning cast removes address space '__iomem' of expression
From
On 10/29/23 01:40, Michael Kelley (LINUX) wrote:
> From: Abhinav Singh <singhabhinav9051571833@gmail.com> 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@gmail.com>
>> ---
>> 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?

The particular warning msg for this case is like this "warning: cast
removes address space '__iomem' of expression". I only saw these warning
one time inside the arch/x86/ directory.

>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.

So should I continue or not with this patch?

>
> Michael
>
>>
>> --
>> 2.39.2
>

Thanks for taking out the time for reviewing this and giving the
suggestions.

Thank You,
Abhinav Singh



\
 
 \ /
  Last update: 2023-10-28 23:27    [W:0.108 / U:0.052 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site