Re: [PATCH 6.0 20/20] fbdev/core: Remove remove_conflicting_pci_framebuffers()

Boris V.
2022-11-01T11:42:11Z

On 01/11/2022 11:34, Thomas Zimmermann wrote:
> (cc: Alex Williamson)
>
> Hi
>
> Am 01.11.22 um 09:42 schrieb Boris V.:
>> On 24/10/2022 13:31, Greg Kroah-Hartman wrote:
>>> From: Thomas Zimmermann <tzimmermann@suse.de>
>>>
>>> commit 9d69ef1838150c7d87afc1a87aa658c637217585 upstream.
>>>
>>> Remove remove_conflicting_pci_framebuffers() and implement similar
>>> functionality in aperture_remove_conflicting_pci_device(), which was
>>> the only caller. Removes an otherwise unused interface and streamlines
>>> the aperture helper. No functional changes.
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
>>> Link:
>>> https://patchwork.freedesktop.org/patch/msgid/20220718072322.8927-5-tzimmermann@suse.de
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>>   drivers/video/aperture.c         |   30 ++++++++++++++----------
>>>   drivers/video/fbdev/core/fbmem.c |   48
>>> ---------------------------------------
>>>   include/linux/fb.h               |    2 -
>>>   3 files changed, 18 insertions(+), 62 deletions(-)
>>>
>>> --- a/drivers/video/aperture.c
>>> +++ b/drivers/video/aperture.c
>>> @@ -335,30 +335,36 @@ EXPORT_SYMBOL(aperture_remove_conflictin
>>>    */
>>>   int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
>>> const char *name)
>>>   {
>>> +    bool primary = false;
>>>       resource_size_t base, size;
>>>       int bar, ret;
>>> -    /*
>>> -     * WARNING: Apparently we must kick fbdev drivers before vgacon,
>>> -     * otherwise the vga fbdev driver falls over.
>>> -     */
>>> -#if IS_REACHABLE(CONFIG_FB)
>>> -    ret = remove_conflicting_pci_framebuffers(pdev, name);
>>> -    if (ret)
>>> -        return ret;
>>> +#ifdef CONFIG_X86
>>> +    primary = pdev->resource[PCI_ROM_RESOURCE].flags &
>>> IORESOURCE_ROM_SHADOW;
>>>   #endif
>>> -    ret = vga_remove_vgacon(pdev);
>>> -    if (ret)
>>> -        return ret;
>>>       for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
>>>           if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
>>>               continue;
>>> +
>>>           base = pci_resource_start(pdev, bar);
>>>           size = pci_resource_len(pdev, bar);
>>> -        aperture_detach_devices(base, size);
>>> +        ret = aperture_remove_conflicting_devices(base, size,
>>> primary, name);
>>> +        if (ret)
>>> +            break;
>>>       }
>>> +    if (ret)
>>> +        return ret;
>>> +
>>> +    /*
>>> +     * WARNING: Apparently we must kick fbdev drivers before vgacon,
>>> +     * otherwise the vga fbdev driver falls over.
>>> +     */
>>> +    ret = vga_remove_vgacon(pdev);
>>> +    if (ret)
>>> +        return ret;
>>> +
>>>       return 0;
>>>   }
>>> --- a/drivers/video/fbdev/core/fbmem.c
>>> +++ b/drivers/video/fbdev/core/fbmem.c
>>> @@ -1788,54 +1788,6 @@ int remove_conflicting_framebuffers(stru
>>>   EXPORT_SYMBOL(remove_conflicting_framebuffers);
>>>   /**
>>> - * remove_conflicting_pci_framebuffers - remove firmware-configured
>>> framebuffers for PCI devices
>>> - * @pdev: PCI device
>>> - * @name: requesting driver name
>>> - *
>>> - * This function removes framebuffer devices (eg. initialized by
>>> firmware)
>>> - * using memory range configured for any of @pdev's memory bars.
>>> - *
>>> - * The function assumes that PCI device with shadowed ROM drives a
>>> primary
>>> - * display and so kicks out vga16fb.
>>> - */
>>> -int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, const
>>> char *name)
>>> -{
>>> -    struct apertures_struct *ap;
>>> -    bool primary = false;
>>> -    int err, idx, bar;
>>> -
>>> -    for (idx = 0, bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
>>> -        if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
>>> -            continue;
>>> -        idx++;
>>> -    }
>>> -
>>> -    ap = alloc_apertures(idx);
>>> -    if (!ap)
>>> -        return -ENOMEM;
>>> -
>>> -    for (idx = 0, bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
>>> -        if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
>>> -            continue;
>>> -        ap->ranges[idx].base = pci_resource_start(pdev, bar);
>>> -        ap->ranges[idx].size = pci_resource_len(pdev, bar);
>>> -        pci_dbg(pdev, "%s: bar %d: 0x%lx -> 0x%lx\n", __func__, bar,
>>> -            (unsigned long)pci_resource_start(pdev, bar),
>>> -            (unsigned long)pci_resource_end(pdev, bar));
>>> -        idx++;
>>> -    }
>>> -
>>> -#ifdef CONFIG_X86
>>> -    primary = pdev->resource[PCI_ROM_RESOURCE].flags &
>>> -                    IORESOURCE_ROM_SHADOW;
>>> -#endif
>>> -    err = remove_conflicting_framebuffers(ap, name, primary);
>>> -    kfree(ap);
>>> -    return err;
>>> -}
>>> -EXPORT_SYMBOL(remove_conflicting_pci_framebuffers);
>>> -
>>> -/**
>>>    *    register_framebuffer - registers a frame buffer device
>>>    *    @fb_info: frame buffer info structure
>>>    *
>>> --- a/include/linux/fb.h
>>> +++ b/include/linux/fb.h
>>> @@ -615,8 +615,6 @@ extern ssize_t fb_sys_write(struct fb_in
>>>   /* drivers/video/fbmem.c */
>>>   extern int register_framebuffer(struct fb_info *fb_info);
>>>   extern void unregister_framebuffer(struct fb_info *fb_info);
>>> -extern int remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
>>> -                           const char *name);
>>>   extern int remove_conflicting_framebuffers(struct apertures_struct
>>> *a,
>>>                          const char *name, bool primary);
>>>   extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
>>>
>>>
>>>
>>>
>>
>> Hello,
>>
>> this patch seems to disable console/framebuffer when vfio-pci is used.
>> I hava 2 nvidia GPUs one is used for host and other is passed through
>> to VM.
>
> Vfio uses this helper to unload the driver before passing it to a VM
> AFAIU. But unless you're using nouveau, you're on your own.
>
> Best regards
> Thomas
>

But this happens at boot, when vfio-pci module is loaded. Not when VM is
started.
And console/framebuffer is unloaded for primary/boot GPU, not the one
passed to VM.
Also no GPU driver is loaded at this point. And there was no problem
before, it stopped working with 6.0.4 kernel.