[PATCH] PCI: vgaarb: Remove stale "not implemented" notes from docs

[PATCH] PCI: vgaarb: Remove stale "not implemented" notes from docs

[Christian Melendez Nuñez]

The vgaarb kerneldoc and Documentation/gpu/vgaarbiter.rst both
describe "unlock all" and "target default" as "(TODO: not
implemented yet)". Both are actually implemented already:

- "unlock all" is handled in vga_arb_write() via the "all"
  strncmp branch, which releases both VGA_RSRC_LEGACY_IO and
  VGA_RSRC_LEGACY_MEM.
- "target default" is handled in the same function via the
  "default" strncmp branch, which resolves the target to
  vga_default_device().

Drop the stale disclaimers so the documentation matches actual
behavior.

This is my first submission to the kernel; happy to take any
feedback on the patch itself or how I should be doing this.

Signed-off-by: Christian Melendez Nuñez <chrismelnu@gmail.com>
---
 Documentation/gpu/vgaarbiter.rst | 9 ++++-----
 drivers/pci/vgaarb.c             | 7 +++----
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/Documentation/gpu/vgaarbiter.rst b/Documentation/gpu/vgaarbiter.rst
index d1e953712cc2..c3a728130b54 100644
--- a/Documentation/gpu/vgaarbiter.rst
+++ b/Documentation/gpu/vgaarbiter.rst
@@ -65,8 +65,7 @@ write
         unlock <io_state>
                 release locks on target
         unlock all
-                release all locks on target held by this user (not implemented
-                yet)
+                release all locks on target held by this user
         decodes <io_state>
                 set the legacy decoding attributes for the card
 
@@ -74,9 +73,9 @@ write
                 event if something changes on any card (not just the target)
 
         card_ID is of the form "PCI:domain:bus:dev.fn". It can be set to "default"
-        to go back to the system default card (TODO: not implemented yet). Currently,
-        only PCI is supported as a prefix, but the userland API may support other bus
-        types in the future, even if the current kernel implementation doesn't.
+        to go back to the system default card. Currently, only PCI is supported
+        as a prefix, but the userland API may support other bus types in the
+        future, even if the current kernel implementation doesn't.
 
 Note about locks:
 
diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index c360eee11dd9..4ae96ae60d8b 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -1008,10 +1008,9 @@ EXPORT_SYMBOL(vga_client_register);
  * poll         : event if something change on any card (not just the target)
  *
  * card_ID is of the form "PCI:domain:bus:dev.fn". It can be set to "default"
- * to go back to the system default card (TODO: not implemented yet).
- * Currently, only PCI is supported as a prefix, but the userland API may
- * support other bus types in the future, even if the current kernel
- * implementation doesn't.
+ * to go back to the system default card. Currently, only PCI is supported
+ * as a prefix, but the userland API may support other bus types in the
+ * future, even if the current kernel implementation doesn't.
  *
  * Note about locks:
  *
-- 
2.34.1

Re: [PATCH] PCI: vgaarb: Remove stale "not implemented" notes from docs

[Christian Melendez Nuñez]

> [Severity: Medium]
> Is the "unlock all" command fully implemented and working as expected?
>
> If the user holds only one type of lock, wouldn't this fail with -EINVAL
> instead of unlocking the resources? Should the TODO disclaimer remain
> until this logic is fixed?

This is correct.  "unlock all" sets both VGA_RSRC_LEGACY_IO and
VGA_RSRC_LEGACY_MEM unconditionally:

    if (strncmp(curr_pos, "all", 3) == 0)
        io_state = VGA_RSRC_LEGACY_IO | VGA_RSRC_LEGACY_MEM;

but "lock" allows acquiring them independently via vga_str_to_iostate(),
so a caller holding only one of them hits

    if (io_state & VGA_RSRC_LEGACY_MEM && uc->mem_cnt == 0) {
        ret_val = -EINVAL;
        goto done;
    }

and returns -EINVAL without releasing the lock it does hold. So "release
all locks on target held by this user" is not accurate unless both are
held, and the caveat should stay.

I'll wait for further review before sending a v2.

Thanks,
Christian