Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <91effdc2-c357-5bd9-a355-7f1ad0a848e4@gmx.de>
Date:   Wed, 27 Apr 2022 21:24:02 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.8.0
Subject: Re: [PATCH] video: hyperv_fb: Allow resolutions with size > 64 MB for
 Gen1
Content-Language: en-US
To:     Saurabh Sengar <ssengar@linux.microsoft.com>,
        ssengar@microsoft.com, kys@microsoft.com, haiyangz@microsoft.com,
        sthemmin@microsoft.com, wei.liu@kernel.org, decui@microsoft.com,
        linux-hyperv@vger.kernel.org, linux-fbdev@vger.kernel.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <1651067273-6635-1-git-send-email-ssengar@linux.microsoft.com>
From:   Helge Deller <deller@gmx.de>
In-Reply-To: <1651067273-6635-1-git-send-email-ssengar@linux.microsoft.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On 4/27/22 15:47, Saurabh Sengar wrote:
> This patch fixes a bug where GEN1 VMs doesn't allow resolutions greater
> than 64 MB size (eg 7680x4320). Unnecessary PCI check limits Gen1 VRAM
> to legacy PCI BAR size only (ie 64MB). Thus any, resolution requesting
> greater then 64MB (eg 7680x4320) would fail. MMIO region assigning this
> memory shouldn't be limited by PCI bar size.

Is that right?
Allocating more memory than what the PCI bar states?
That sounds to me that theoretically the "now bigger" memory framebuffer c=
ould
overwrite other memory areas or mapped PCI bars.
I'd like to see some other person from Microsoft to please comment/ack/nac=
k.

Helge

> Signed-off-by: Saurabh Sengar <ssengar@linux.microsoft.com>
> ---
>  drivers/video/fbdev/hyperv_fb.c | 19 +------------------
>  1 file changed, 1 insertion(+), 18 deletions(-)
>
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyper=
v_fb.c
> index c8e0ea2..58c304a 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -1009,7 +1009,6 @@ static int hvfb_getmem(struct hv_device *hdev, str=
uct fb_info *info)
>  	struct pci_dev *pdev  =3D NULL;
>  	void __iomem *fb_virt;
>  	int gen2vm =3D efi_enabled(EFI_BOOT);
> -	resource_size_t pot_start, pot_end;
>  	phys_addr_t paddr;
>  	int ret;
>
> @@ -1060,23 +1059,7 @@ static int hvfb_getmem(struct hv_device *hdev, st=
ruct fb_info *info)
>  	dio_fb_size =3D
>  		screen_width * screen_height * screen_depth / 8;
>
> -	if (gen2vm) {
> -		pot_start =3D 0;
> -		pot_end =3D -1;
> -	} else {
> -		if (!(pci_resource_flags(pdev, 0) & IORESOURCE_MEM) ||
> -		    pci_resource_len(pdev, 0) < screen_fb_size) {
> -			pr_err("Resource not available or (0x%lx < 0x%lx)\n",
> -			       (unsigned long) pci_resource_len(pdev, 0),
> -			       (unsigned long) screen_fb_size);
> -			goto err1;
> -		}
> -
> -		pot_end =3D pci_resource_end(pdev, 0);
> -		pot_start =3D pot_end - screen_fb_size + 1;
> -	}
> -
> -	ret =3D vmbus_allocate_mmio(&par->mem, hdev, pot_start, pot_end,
> +	ret =3D vmbus_allocate_mmio(&par->mem, hdev, 0, -1,
>  				  screen_fb_size, 0x100000, true);
>  	if (ret !=3D 0) {
>  		pr_err("Unable to allocate framebuffer memory\n");