linux-kernel - Re: [PATCH] PCI: mediatek: Fix sparse warning caused to virt_to

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20231121213755.GA258354@bhelgaas>
Date:   Tue, 21 Nov 2023 15:37:55 -0600
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
Cc:     ryder.lee@...iatek.com, jianjun.wang@...iatek.com,
        lpieralisi@...nel.org, kw@...ux.com, robh@...nel.org,
        bhelgaas@...gle.com, matthias.bgg@...il.com,
        angelogioacchino.delregno@...labora.com, linux-pci@...r.kernel.org,
        linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org, skinsburskii@...il.com
Subject: Re: [PATCH] PCI: mediatek: Fix sparse warning caused to
 virt_to_phys() prototype change

On Mon, Nov 20, 2023 at 03:40:33PM -0800, Stanislav Kinsburskii wrote:
> Explicitly cast __iomem pointer to const void* with __force to fix the
> following warning:
> 
>   warning: incorrect type in argument 1 (different address spaces)
>      expected void const volatile *address
>      got void [noderef] __iomem *

I have two questions about this:

  1) There's no other use of __force in drivers/pci, so I don't know
  what's special about pcie-mediatek.c.  There should be a way to fix
  the types so it's not needed.

  2) virt_to_phys() is not quite right to begin with because what we
  want is a *bus* address, not the CPU physical address we get from
  virt_to_phys().  Obviously the current platforms that use this must
  not apply any offset between bus and CPU physical addresses, but
  it's not something we should rely on.

  There are only three drivers (pci-aardvark.c, pcie-xilinx.c, and
  this one) that use virt_to_phys(), and they're all slightly wrong
  here.

The *_compose_msi_msg() methods could use a little more consistency
across the board.

> Signed-off-by: Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>
> ---
>  drivers/pci/controller/pcie-mediatek.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-mediatek.c b/drivers/pci/controller/pcie-mediatek.c
> index 66a8f73296fc..27f0f79810a1 100644
> --- a/drivers/pci/controller/pcie-mediatek.c
> +++ b/drivers/pci/controller/pcie-mediatek.c
> @@ -397,7 +397,7 @@ static void mtk_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
>  	phys_addr_t addr;
>  
>  	/* MT2712/MT7622 only support 32-bit MSI addresses */
> -	addr = virt_to_phys(port->base + PCIE_MSI_VECTOR);
> +	addr = virt_to_phys((__force const void *)port->base + PCIE_MSI_VECTOR);
>  	msg->address_hi = 0;
>  	msg->address_lo = lower_32_bits(addr);
>  
> @@ -520,7 +520,7 @@ static void mtk_pcie_enable_msi(struct mtk_pcie_port *port)
>  	u32 val;
>  	phys_addr_t msg_addr;
>  
> -	msg_addr = virt_to_phys(port->base + PCIE_MSI_VECTOR);
> +	msg_addr = virt_to_phys((__force const void *)port->base + PCIE_MSI_VECTOR);
>  	val = lower_32_bits(msg_addr);
>  	writel(val, port->base + PCIE_IMSI_ADDR);
>  
> 
> 
>