RE: [PATCH] Replace ioremap_cache() with memremap()
From: Michael Kelley <hidden>
Date: 2023-11-05 15:15:53
Also in:
dri-devel, linux-hyperv, lkml
From: Nischala Yelchuri <redacted> Sent: Wednesday, November 1, 2023 9:02 AM
It's customary for the patch "Subject:" line to have a prefix indicating the area of the code being modified. This patch touches on multiple Hyper-V drivers, so there's not a clear choice for prefix. I would suggest using "Drivers: hv:" as is commonly used for Hyper-V code, though other reviewers might have a different suggestion. > Current Hyper-V code for CoCo VMs uses ioremap_cache() to map normal
quoted hunk ↗ jump to hunk
memory as decrypted. ioremap_cache() is ideally used to map I/O device memory when it has the characteristics of normal memory. It should not be used to map normal memory as the returned pointer has the __iomem attribute. Fix current code by replacing ioremap_cache() with memremap(). No functional change intended. Signed-off-by: Nischala Yelchuri <redacted> --- arch/x86/hyperv/hv_init.c | 6 +++--- drivers/gpu/drm/hyperv/hyperv_drm_drv.c | 2 +- drivers/hv/hv.c | 13 +++++++------ drivers/video/fbdev/hyperv_fb.c | 6 +++--- 4 files changed, 14 insertions(+), 13 deletions(-)diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c index 21556ad87..fae43c040 100644 --- a/arch/x86/hyperv/hv_init.c +++ b/arch/x86/hyperv/hv_init.c@@ -68,9 +68,9 @@ static int hyperv_init_ghcb(void) */ rdmsrl(MSR_AMD64_SEV_ES_GHCB, ghcb_gpa); - /* Mask out vTOM bit. ioremap_cache() maps decrypted */ + /* Mask out vTOM bit. memremap() maps decrypted with MEMREMAP_DEC */ 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 | MEMREMAP_DEC); if (!ghcb_va) return -ENOMEM;@@ -238,7 +238,7 @@ static int hv_cpu_die(unsigned int cpu) if (hv_ghcb_pg) { ghcb_va = (void **)this_cpu_ptr(hv_ghcb_pg); if (*ghcb_va) - iounmap(*ghcb_va); + memunmap(*ghcb_va); *ghcb_va = NULL; }diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.cb/drivers/gpu/drm/hyperv/hyperv_drm_drv.c index d511d17c5..d6fec9bd3 100644--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c@@ -92,7 +92,7 @@ static int hyperv_setup_vram(struct hyperv_drm_device*hv, * connect to display properly for ARM64 Linux VM, as the host also maps * the VRAM cacheable. */ - hv->vram = ioremap_cache(hv->mem->start, hv->fb_size); + hv->vram = memremap(hv->mem->start, hv->fb_size, MEMREMAP_WB | MEMREMAP_DEC);
This change has some additional implications that must be accounted for. The hv->vram field is declared as void __iomem * in hyperv_drm.h. The __iomem attribute should be dropped. Then the use of IOSYS_MAP_INIT_VADDR_IOMEM() in hyperv_blit_to_vram_rect() should be changed to IOSYS_MAP_INIT_VADDR(). This has the desirable effect of allowing normal memcpy() functions to be used instead of the _toio()/_fromio() variants.
quoted hunk ↗ jump to hunk
if (!hv->vram) { drm_err(dev, "Failed to map vram\n"); ret = -ENOMEM;diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c index 51e5018ac..399bfa392 100644 --- a/drivers/hv/hv.c +++ b/drivers/hv/hv.c@@ -274,11 +274,12 @@ void hv_synic_enable_regs(unsigned int cpu) simp.simp_enabled = 1; if (ms_hyperv.paravisor_present || hv_root_partition) { - /* Mask out vTOM bit. ioremap_cache() maps decrypted */ + /* Mask out vTOM bit. memremap() maps decrypted with MEMREMAP_DEC */ u64 base = (simp.base_simp_gpa << HV_HYP_PAGE_SHIFT) & ~ms_hyperv.shared_gpa_boundary; hv_cpu->synic_message_page - = (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE); + = memremap(base, + HV_HYP_PAGE_SIZE, MEMREMAP_WB | MEMREMAP_DEC); if (!hv_cpu->synic_message_page) pr_err("Fail to map synic message page.\n"); } else {@@ -293,11 +294,11 @@ void hv_synic_enable_regs(unsigned int cpu) siefp.siefp_enabled = 1; if (ms_hyperv.paravisor_present || hv_root_partition) { - /* Mask out vTOM bit. ioremap_cache() maps decrypted */ + /* Mask out vTOM bit. memremap() maps decrypted with MEMREMAP_DEC */ u64 base = (siefp.base_siefp_gpa << HV_HYP_PAGE_SHIFT) & ~ms_hyperv.shared_gpa_boundary; hv_cpu->synic_event_page - = (void *)ioremap_cache(base, HV_HYP_PAGE_SIZE); + = memremap(base, HV_HYP_PAGE_SIZE, MEMREMAP_WB | MEMREMAP_DEC); if (!hv_cpu->synic_event_page) pr_err("Fail to map synic event page.\n"); } else {@@ -376,7 +377,7 @@ void hv_synic_disable_regs(unsigned int cpu) */ simp.simp_enabled = 0; if (ms_hyperv.paravisor_present || hv_root_partition) { - iounmap(hv_cpu->synic_message_page); + memunmap(hv_cpu->synic_message_page); hv_cpu->synic_message_page = NULL; } else { simp.base_simp_gpa = 0;@@ -388,7 +389,7 @@ void hv_synic_disable_regs(unsigned int cpu) siefp.siefp_enabled = 0; if (ms_hyperv.paravisor_present || hv_root_partition) { - iounmap(hv_cpu->synic_event_page); + memunmap(hv_cpu->synic_event_page); hv_cpu->synic_event_page = NULL; } else { siefp.base_siefp_gpa = 0;diff --git a/drivers/video/fbdev/hyperv_fb.cb/drivers/video/fbdev/hyperv_fb.c index bf59daf86..cd9ec1f6c 100644--- a/drivers/video/fbdev/hyperv_fb.c +++ b/drivers/video/fbdev/hyperv_fb.c@@ -1034,7 +1034,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) * VM Connect to display properly for ARM64 Linux VM, as the host also * maps the VRAM cacheable. */ - fb_virt = ioremap_cache(par->mem->start, screen_fb_size); + fb_virt = memremap(par->mem->start, screen_fb_size, MEMREMAP_WB | MEMREMAP_DEC);
There's a similar situation here: the local variable fb_virt is declared as void __iomem *. The __iomem attribute should be dropped.
quoted hunk ↗ jump to hunk
if (!fb_virt) goto err2;@@ -1068,7 +1068,7 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info) return 0; err3: - iounmap(fb_virt); + memunmap(fb_virt); err2: vmbus_free_mmio(par->mem->start, screen_fb_size); par->mem = NULL;@@ -1086,7 +1086,7 @@ static void hvfb_putmem(struct hv_device *hdev, struct fb_info *info) if (par->need_docopy) { vfree(par->dio_vp); - iounmap(info->screen_base); + memunmap(info->screen_base); vmbus_free_mmio(par->mem->start, screen_fb_size); } else { hvfb_release_phymem(hdev, info->fix.smem_start, --2.34.1