consume i8042 device from vm-superio

rust-vmm / vmm-reference

A VMM implementation based of rust-vmm components

Apache License 2.0

146 stars 61 forks source link

consume i8042 device from vm-superio #215

Closed ramyak-mehra closed 2 years ago

ramyak-mehra commented 2 years ago

Signed-off-by: Ramyak Mehra rmehra_be19@thapar.edu

Summary of the PR

Added I8042 wrapper inside devices/legacy.
add_i8042_deive func inside src/vmm/lib.rs on the vmm struct.
handled VcpuExit::IoOut at address i8042 port, inside vm-vcpu/src/vcpu/mod.rs

issue: #198

Requirements

Before submitting your PR, please make sure you addressed the following requirements:

[x] All commits in this PR are signed (with git commit -s), and the commit message has max 60 characters for the summary and max 75 characters for each description line.
[x] All added/changed functionality has a corresponding unit/integration test.

ramyak-mehra commented 2 years ago

I see the PIO bus is only available on x86_64. Should arch use MMIO or we are to leave the implementation of it for now?

andreeaflorescu commented 2 years ago

I see the PIO bus is only available on x86_64. Should arch use MMIO or we are to leave the implementation of it for now?

The i8042 is only used on x86_64. You should make sure to mark it as such using platform configuration macros.

ramyak-mehra commented 2 years ago

I see the PIO bus is only available on x86_64. Should arch use MMIO or we are to leave the implementation of it for now?

The i8042 is only used on x86_64. You should make sure to mark it as such using platform configuration macros.

I fixed the platform configuration issues. Is there a way we can test this implementation?

lauralt commented 2 years ago

I see the PIO bus is only available on x86_64. Should arch use MMIO or we are to leave the implementation of it for now?

The i8042 is only used on x86_64. You should make sure to mark it as such using platform configuration macros.

I fixed the platform configuration issues. Is there a way we can test this implementation?

Andreea explained here at the end how you can test it. Here is the command line definition. We will probably also need to remove i8042.nokbd.

andreeaflorescu commented 2 years ago

I see the PIO bus is only available on x86_64. Should arch use MMIO or we are to leave the implementation of it for now?

The i8042 is only used on x86_64. You should make sure to mark it as such using platform configuration macros.

I fixed the platform configuration issues. Is there a way we can test this implementation?

Andreea explained here at the end how you can test it. Here is the command line definition. We will probably also need to remove i8042.nokbd.

Can you try to see if this work? @ramyak-mehra

ramyak-mehra commented 2 years ago

I see the PIO bus is only available on x86_64. Should arch use MMIO or we are to leave the implementation of it for now?

The i8042 is only used on x86_64. You should make sure to mark it as such using platform configuration macros.

I fixed the platform configuration issues. Is there a way we can test this implementation?

Andreea explained here at the end how you can test it. Here is the command line definition. We will probably also need to remove i8042.nokbd.

Can you try to see if this work? @ramyak-mehra

On my local machine?

andreeaflorescu commented 2 years ago

Can you try to see if this work? @ramyak-mehra

On my local machine?

I mean to update the PR to modify the command line and check if the exit in tests still works. Now that's done, so it looks good to me. Can you also fix the failing tests? And maybe squash the commits.

ramyak-mehra commented 2 years ago

I did change the default kernel cmd line, but in the python tests we are passing the reboot=t and i8042.nokbd. should I remove it from there as well? I ran the tests by modifying that and the tests passed.

andreeaflorescu commented 2 years ago

I did change the default kernel cmd line, but in the python tests we are passing the reboot=t and i8042.nokbd. should I remove it from there as well? I ran the tests by modifying that and the tests passed.

We should add a test with reboot=t and i8042.nokbd and another one with the default cmdline.