Hi
Am 01.02.24 um 07:00 schrieb [email protected]:
> From: Michael Kelley <[email protected]>
>
> 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 <[email protected]>
Reviewed-by: Thomas Zimmermann <[email protected]>
> ---
> 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)
From: Thomas Zimmermann <[email protected]> Sent: Thursday, February 1, 2024 12:17 AM
>
> Hi
>
> Am 01.02.24 um 07:00 schrieb [email protected]:
> > From: Michael Kelley <[email protected]>
> >
> > 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 <[email protected]>
>
> Reviewed-by: Thomas Zimmermann <[email protected]>
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;
> > }
> >
> > /*
>
On 2/9/24 16:23, Michael Kelley wrote:
> From: Thomas Zimmermann <[email protected]> Sent: Thursday, February 1, 2024 12:17 AM
>>
>> Hi
>>
>> Am 01.02.24 um 07:00 schrieb [email protected]:
>>> From: Michael Kelley <[email protected]>
>>>
>>> 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 <[email protected]>
>>
>> Reviewed-by: Thomas Zimmermann <[email protected]>
>
> 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;
>>> }
>>>
>>> /*
>>
>
>
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 <[email protected]> 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.
From: Wei Liu <[email protected]> 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 <[email protected]> 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