Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Arm virtio keyboard driver #6444

Merged
merged 5 commits into from
Dec 29, 2024

Conversation

elkoniu
Copy link
Contributor

@elkoniu elkoniu commented Nov 15, 2024

Description

This set of commits introduces virtio based keyboard driver. The main idea is to
have a keyboard driver which is more "light" than standard USB based driver.
This may be better for some small ARM based platforms with limited resources.

  • Breaking change?
    • Breaking change - Does this PR cause a break in build or boot behavior?
    • Examples: Does it add a new library class or move a module to a different repo.
  • Impacts security?
    • Security - Does this PR have a direct security impact?
    • Examples: Crypto algorithm change or buffer overflow fix.
  • Includes tests?
    • Tests - Does this PR include any explicit test code?
    • Examples: Unit tests or integration tests.

How This Was Tested

I have tested this manually with RPi5 ARM platform and qemu. For test I have navigated
over efi shell, config and grub bootloader window running from Fedora ISO.

#!/bin/bash

CODE=/home/koniu/RedHat/git/virt/edk2/Build/Ovmf3264/DEBUG_GCC5/FV/OVMF_CODE.fd
VARS=/home/koniu/RedHat/git/virt/edk2/Build/Ovmf3264/DEBUG_GCC5/FV/OVMF_VARS.fd

qemu-system-x86_64 \
    -blockdev node-name=code,driver=file,filename=${CODE},read-only=on \
    -drive if=none,id=vars,format=raw,file=${VARS},snapshot=on \
    -machine q35,kernel-irqchip=on,smm=on,pflash0=code,pflash1=vars \
    -accel kvm -m 4G -cpu host -smp cores=2,threads=1 \
    -chardev stdio,id=fwlog \
    -device isa-debugcon,iobase=0x402,chardev=fwlog \
    -net none \
    -device virtio-keyboard-pci
#!/bin/bash -eu

BUILD_CODE=edk2/Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/FV/QEMU_EFI.fd
BUILD_VARS=edk2/Build/ArmVirtQemu-AARCH64/DEBUG_GCC5/FV/QEMU_VARS.fd

FW_CODE=QEMU_EFI-pflash.raw
FW_VARS=QEMU_VARS-pflash.raw

# Make 64M padding of images (qemu requirement)
dd if=/dev/zero of=${FW_CODE} bs=1M count=64
dd if=${BUILD_CODE} of=${FW_CODE} conv=notrunc

dd if=/dev/zero of=${FW_VARS} bs=1M count=64
dd if=${BUILD_VARS} of=${FW_VARS} conv=notrunc

qemu-system-aarch64 \
    -machine virt \
    -machine pflash0=code \
    -machine pflash1=vars \
    -blockdev node-name=code,driver=file,filename="${FW_CODE}",read-only=on \
    -blockdev node-name=vars,driver=file,filename="${FW_VARS}" \
    -machine accel=tcg \
    -cpu max \
    -m 1G \
    -boot menu=on,splash-time=5000 \
    -serial stdio \
    -device virtio-keyboard-pci \
    -net none \
    -hda Fedora-Cloud-Base-Generic-41-1.4.aarch64.qcow2 \
    "${@}"

Integration Instructions

N/A

OvmfPkg/VirtioKeyboardDxe/VirtioKeyboard.c Outdated Show resolved Hide resolved
OvmfPkg/VirtioKeyboardDxe/VirtioKeyboard.c Outdated Show resolved Hide resolved
OvmfPkg/VirtioKeyboardDxe/VirtioKeyboard.c Outdated Show resolved Hide resolved
OvmfPkg/VirtioKeyboardDxe/VirtioKeyboard.c Outdated Show resolved Hide resolved
@lersek
Copy link
Member

lersek commented Dec 13, 2024

@elkoniu (sorry, I'm only here to make noise, not to review:) I'm so glad this is being upstreamed! 👍

@elkoniu elkoniu force-pushed the arm-virtio-keyboard-driver branch from 3d4a2f0 to 3502fb9 Compare December 19, 2024 00:34
@elkoniu elkoniu force-pushed the arm-virtio-keyboard-driver branch 3 times, most recently from c9d525e to 148533d Compare December 19, 2024 09:31
@elkoniu
Copy link
Contributor Author

elkoniu commented Dec 19, 2024

@elkoniu (sorry, I'm only here to make noise, not to review:) I'm so glad this is being upstreamed! 👍

Hi @lersek - thanks \o/ Btw - good to see you around :)

@elkoniu
Copy link
Contributor Author

elkoniu commented Dec 19, 2024

@kraxel @osteffenrh I have updated the code and addressed all the review notes from Gerd.

Copy link
Member

@ardbiesheuvel ardbiesheuvel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please fix the subjects of these commits?

virio -> virtio

and use package prefixes (look at git log for inspiration)

@elkoniu elkoniu force-pushed the arm-virtio-keyboard-driver branch 2 times, most recently from 66aa121 to 4ac96f9 Compare December 26, 2024 01:17
@elkoniu
Copy link
Contributor Author

elkoniu commented Dec 26, 2024

Can you please fix the subjects of these commits?

virio -> virtio

and use package prefixes (look at git log for inspiration)

Hi @ardbiesheuvel - I have addressed the issues you pointed out:

  • commit messages has been adjusted
  • typo has been fixed

I am waiting for the CI to complete and check what are the Windows build issues to address them.

@elkoniu elkoniu force-pushed the arm-virtio-keyboard-driver branch 6 times, most recently from a55f9a8 to c2fac2d Compare December 29, 2024 16:02
@elkoniu
Copy link
Contributor Author

elkoniu commented Dec 29, 2024

@ardbiesheuvel @kraxel - I have addressed all the review comments and fixed all the Windows linter issues. Everything is green now.

@ardbiesheuvel ardbiesheuvel added the push Auto push patch series in PR if all checks pass label Dec 29, 2024
Copy link

mergify bot commented Dec 29, 2024

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

This commit adds:
- missing virtio subsystem ID for input device
- PrepareVirtioKeyboardDevicePath() handler to boot manager library

Signed-off-by: Paweł Poławski <ppolawsk@redhat.com>
This is virtio based keyboard driver designed to be used on ARM platform.
The driver implements basic and extended text input interface.

UEFI shell requires only basic text input interface, but Grub needs
extended text input to work on.

Signed-off-by: Paweł Poławski <ppolawsk@redhat.com>
Signed-off-by: Paweł Poławski <ppolawsk@redhat.com>
Signed-off-by: Paweł Poławski <ppolawsk@redhat.com>
Signed-off-by: Paweł Poławski <ppolawsk@redhat.com>
@elkoniu elkoniu force-pushed the arm-virtio-keyboard-driver branch from c2fac2d to 9a3efc9 Compare December 29, 2024 17:41
@elkoniu
Copy link
Contributor Author

elkoniu commented Dec 29, 2024

@ardbiesheuvel I have rebased PR manually to the latest master as GitHub was struggling with it for some reason.

@ardbiesheuvel ardbiesheuvel removed the push Auto push patch series in PR if all checks pass label Dec 29, 2024
@ardbiesheuvel ardbiesheuvel merged commit fc140c5 into tianocore:master Dec 29, 2024
124 of 125 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants