[PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()

[PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()

[mhkelley58]

From: Michael Kelley <mhklinux@outlook.com>

A recent commit removing the use of screen_info introduced a logic
error. The error causes hvfb_getmem() to always return -ENOMEM
for Generation 2 VMs. As a result, the Hyper-V frame buffer
device fails to initialize. The error was introduced by removing
an "else if" clause, leaving Gen2 VMs to always take the -ENOMEM
error path.

Fix the problem by removing the error path "else" clause. Gen 2
VMs now always proceed through the MMIO memory allocation code,
but with "base" and "size" defaulting to 0.

Fixes: 0aa0838c84da ("fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers")
Signed-off-by: Michael Kelley <mhklinux@outlook.com>
---
 drivers/video/fbdev/hyperv_fb.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index c26ee6fd73c9..8fdccf033b2d 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -1010,8 +1010,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
 			goto getmem_done;
 		}
 		pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Using MMIO instead.\n");
-	} else {
-		goto err1;
 	}
 
 	/*
-- 
2.25.1

Re: [PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()

[Thomas Zimmermann]

[-- Attachment #1.1: Type: text/plain, Size: 1782 bytes --]

Hi

Am 01.02.24 um 07:00 schrieb mhkelley58@gmail.com:
> From: Michael Kelley <mhklinux@outlook.com>
> 
> A recent commit removing the use of screen_info introduced a logic
> error. The error causes hvfb_getmem() to always return -ENOMEM
> for Generation 2 VMs. As a result, the Hyper-V frame buffer
> device fails to initialize. The error was introduced by removing
> an "else if" clause, leaving Gen2 VMs to always take the -ENOMEM
> error path.
> 
> Fix the problem by removing the error path "else" clause. Gen 2
> VMs now always proceed through the MMIO memory allocation code,
> but with "base" and "size" defaulting to 0.

Indeed, that's how it was supposed to work. IDK how I didn't notice this 
bug. Thanks a lot for the fix.

> 
> Fixes: 0aa0838c84da ("fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers")
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>

Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/video/fbdev/hyperv_fb.c | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index c26ee6fd73c9..8fdccf033b2d 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -1010,8 +1010,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
>   			goto getmem_done;
>   		}
>   		pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Using MMIO instead.\n");
> -	} else {
> -		goto err1;
>   	}
>   
>   	/*

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

RE: [PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()

[Michael Kelley]

From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, February 1, 2024 12:17 AM
> 
> Hi
> 
> Am 01.02.24 um 07:00 schrieb mhkelley58@gmail.com:
> > From: Michael Kelley <mhklinux@outlook.com>
> >
> > A recent commit removing the use of screen_info introduced a logic
> > error. The error causes hvfb_getmem() to always return -ENOMEM
> > for Generation 2 VMs. As a result, the Hyper-V frame buffer
> > device fails to initialize. The error was introduced by removing
> > an "else if" clause, leaving Gen2 VMs to always take the -ENOMEM
> > error path.
> >
> > Fix the problem by removing the error path "else" clause. Gen 2
> > VMs now always proceed through the MMIO memory allocation code,
> > but with "base" and "size" defaulting to 0.
> 
> Indeed, that's how it was supposed to work. IDK how I didn't notice this
> bug. Thanks a lot for the fix.
> 
> >
> > Fixes: 0aa0838c84da ("fbdev/hyperv_fb: Remove firmware framebufferswith aperture helpers")
> > Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> 
> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

Wei Liu and Helge Deller --

Should this fix go through the Hyper-V tree or the fbdev tree?   I'm not
aware of a reason that it really matters, but it needs to be one or the
other, and sooner rather than later, because the Hyper-V driver is broken
starting in 6.8-rc1.

Michael

> 
> > ---
> >   drivers/video/fbdev/hyperv_fb.c | 2 --
> >   1 file changed, 2 deletions(-)
> >
> > diff --git a/drivers/video/fbdev/hyperv_fb.c
> b/drivers/video/fbdev/hyperv_fb.c
> > index c26ee6fd73c9..8fdccf033b2d 100644
> > --- a/drivers/video/fbdev/hyperv_fb.c
> > +++ b/drivers/video/fbdev/hyperv_fb.c
> > @@ -1010,8 +1010,6 @@ static int hvfb_getmem(struct hv_device *hdev,
> struct fb_info *info)
> >   			goto getmem_done;
> >   		}
> >   		pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Using MMIO instead.\n");
> > -	} else {
> > -		goto err1;
> >   	}
> >
> >   	/*
> 

Re: [PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()

[Helge Deller]

On 2/9/24 16:23, Michael Kelley wrote:
> From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, February 1, 2024 12:17 AM
>>
>> Hi
>>
>> Am 01.02.24 um 07:00 schrieb mhkelley58@gmail.com:
>>> From: Michael Kelley <mhklinux@outlook.com>
>>>
>>> A recent commit removing the use of screen_info introduced a logic
>>> error. The error causes hvfb_getmem() to always return -ENOMEM
>>> for Generation 2 VMs. As a result, the Hyper-V frame buffer
>>> device fails to initialize. The error was introduced by removing
>>> an "else if" clause, leaving Gen2 VMs to always take the -ENOMEM
>>> error path.
>>>
>>> Fix the problem by removing the error path "else" clause. Gen 2
>>> VMs now always proceed through the MMIO memory allocation code,
>>> but with "base" and "size" defaulting to 0.
>>
>> Indeed, that's how it was supposed to work. IDK how I didn't notice this
>> bug. Thanks a lot for the fix.
>>
>>>
>>> Fixes: 0aa0838c84da ("fbdev/hyperv_fb: Remove firmware framebufferswith aperture helpers")
>>> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
>>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>
> Wei Liu and Helge Deller --
>
> Should this fix go through the Hyper-V tree or the fbdev tree?   I'm not
> aware of a reason that it really matters, but it needs to be one or the
> other, and sooner rather than later, because the Hyper-V driver is broken
> starting in 6.8-rc1.

I'm fine with either.
If there is an upcoming hyper-v pull request, I'm fine if this is included
there. If not, let me know and I can take it via fbdev.

Helge



>
> Michael
>
>>
>>> ---
>>>    drivers/video/fbdev/hyperv_fb.c | 2 --
>>>    1 file changed, 2 deletions(-)
>>>
>>> diff --git a/drivers/video/fbdev/hyperv_fb.c
>> b/drivers/video/fbdev/hyperv_fb.c
>>> index c26ee6fd73c9..8fdccf033b2d 100644
>>> --- a/drivers/video/fbdev/hyperv_fb.c
>>> +++ b/drivers/video/fbdev/hyperv_fb.c
>>> @@ -1010,8 +1010,6 @@ static int hvfb_getmem(struct hv_device *hdev,
>> struct fb_info *info)
>>>    			goto getmem_done;
>>>    		}
>>>    		pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Using MMIO instead.\n");
>>> -	} else {
>>> -		goto err1;
>>>    	}
>>>
>>>    	/*
>>
>
>

Re: [PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()

[Wei Liu]

On Fri, Feb 09, 2024 at 04:53:37PM +0100, Helge Deller wrote:
> On 2/9/24 16:23, Michael Kelley wrote:
> > From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, February 1, 2024 12:17 AM
[...]
> > 
> > Wei Liu and Helge Deller --
> > 
> > Should this fix go through the Hyper-V tree or the fbdev tree?   I'm not
> > aware of a reason that it really matters, but it needs to be one or the
> > other, and sooner rather than later, because the Hyper-V driver is broken
> > starting in 6.8-rc1.
> 
> I'm fine with either.
> If there is an upcoming hyper-v pull request, I'm fine if this is included
> there. If not, let me know and I can take it via fbdev.

I've applied this to the hyperv-fixes tree. Thanks.

RE: [PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()

[Michael Kelley]

From: Wei Liu <wei.liu@kernel.org> Sent: Friday, March 1, 2024 1:26 AM
> 
> On Fri, Feb 09, 2024 at 04:53:37PM +0100, Helge Deller wrote:
> > On 2/9/24 16:23, Michael Kelley wrote:
> > > From: Thomas Zimmermann <tzimmermann@suse.de> Sent: Thursday, February 1, 2024 12:17 AM
> [...]
> > >
> > > Wei Liu and Helge Deller --
> > >
> > > Should this fix go through the Hyper-V tree or the fbdev tree?   I'm not
> > > aware of a reason that it really matters, but it needs to be one or the
> > > other, and sooner rather than later, because the Hyper-V driver is broken
> > > starting in 6.8-rc1.
> >
> > I'm fine with either.
> > If there is an upcoming hyper-v pull request, I'm fine if this is included
> > there. If not, let me know and I can take it via fbdev.
> 
> I've applied this to the hyperv-fixes tree. Thanks.

Thanks, Wei, for picking up this patch as well as several others of mine.

Michael

Re: [PATCH 1/1] fbdev/hyperv_fb: Fix logic error for Gen2 VMs in hvfb_getmem()

[Saurabh Singh Sengar]

On Wed, Jan 31, 2024 at 10:00:22PM -0800, mhkelley58@gmail.com wrote:
> From: Michael Kelley <mhklinux@outlook.com>
> 
> A recent commit removing the use of screen_info introduced a logic
> error. The error causes hvfb_getmem() to always return -ENOMEM
> for Generation 2 VMs. As a result, the Hyper-V frame buffer
> device fails to initialize. The error was introduced by removing
> an "else if" clause, leaving Gen2 VMs to always take the -ENOMEM
> error path.
> 
> Fix the problem by removing the error path "else" clause. Gen 2
> VMs now always proceed through the MMIO memory allocation code,
> but with "base" and "size" defaulting to 0.
> 
> Fixes: 0aa0838c84da ("fbdev/hyperv_fb: Remove firmware framebuffers with aperture helpers")
> Signed-off-by: Michael Kelley <mhklinux@outlook.com>
> ---
>  drivers/video/fbdev/hyperv_fb.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
> index c26ee6fd73c9..8fdccf033b2d 100644
> --- a/drivers/video/fbdev/hyperv_fb.c
> +++ b/drivers/video/fbdev/hyperv_fb.c
> @@ -1010,8 +1010,6 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
>  			goto getmem_done;
>  		}
>  		pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Using MMIO instead.\n");
> -	} else {
> -		goto err1;
>  	}
>  
>  	/*
> -- 
> 2.25.1
>
Reviewed-by: Saurabh Sengar <ssengar@linux.microsoft.com>