| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <BY3PR18MB469219E1401B9E1A997EA51BD4ABA@BY3PR18MB4692.namprd18.prod.outlook.com>
Date: Sun, 5 Nov 2023 15:15:45 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Nischala Yelchuri <niyelchu@...ux.microsoft.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-fbdev@...r.kernel.org" <linux-fbdev@...r.kernel.org>,
"kys@...rosoft.com" <kys@...rosoft.com>,
"haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
"wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>,
"tglx@...utronix.de" <tglx@...utronix.de>,
"mingo@...hat.com" <mingo@...hat.com>,
"bp@...en8.de" <bp@...en8.de>,
"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
"drawat.floss@...il.com" <drawat.floss@...il.com>,
"maarten.lankhorst@...ux.intel.com"
<maarten.lankhorst@...ux.intel.com>,
"mripard@...nel.org" <mripard@...nel.org>,
"tzimmermann@...e.de" <tzimmermann@...e.de>,
"airlied@...il.com" <airlied@...il.com>,
"daniel@...ll.ch" <daniel@...ll.ch>,
"dri-devel@...ts.freedesktop.org" <dri-devel@...ts.freedesktop.org>,
"deller@....de" <deller@....de>
CC: "mhkelley@...look.com" <mhkelley@...look.com>,
"singhabhinav9051571833@...il.com" <singhabhinav9051571833@...il.com>,
"niyelchu@...rosoft.com" <niyelchu@...rosoft.com>
Subject: RE: [PATCH] Replace ioremap_cache() with memremap()
From: Nischala Yelchuri <niyelchu@...ux.microsoft.com> 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
> 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 <niyelchu@...ux.microsoft.com>
> ---
> 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.c
> b/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.
> 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.c
> b/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.
> 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
Powered by blists - more mailing lists