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

openhcl_boot: Remove few kernel parameters for uio_hv_generic #763

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

namancse
Copy link

@namancse namancse commented Feb 3, 2025

Few of the module parameters that were earlier supported in uio_hv_generic driver no longer exist in kernel. Remove the logic which adds these parameters in kernel cmdline from openvmm.

Few of the module parameters that were earlier supported in uio_hv_generic
driver no longer exist in kernel. Remove the logic which adds these
parameters in kernel cmdline from openvmm.
@namancse namancse requested a review from a team as a code owner February 3, 2025 08:01
@jstarks
Copy link
Member

jstarks commented Feb 3, 2025

How does uio_hv_generic know the ring buffer size to use now?

@namancse
Copy link
Author

namancse commented Feb 6, 2025

Hi John, sorry for replying late, I missed the email due to some email filters.

This is the patch that adds the support to get ring size from a pre defined table.

https://lore.kernel.org/all/1711788723-8593-3-git-send-email-ssengar@linux.microsoft.com/

@mattkur
Copy link
Contributor

mattkur commented Feb 11, 2025

Hi John, sorry for replying late, I missed the email due to some email filters.

This is the patch that adds the support to get ring size from a pre defined table.

https://lore.kernel.org/all/1711788723-8593-3-git-send-email-ssengar@linux.microsoft.com/

Thanks for making this cleanup in openhcl.

Few questions:

  1. Do I understand properly that the command line parameters no longer exist, and therefore openhcl's setting them is redundant anyways?
  2. Can you confirm that those changes are in the kernel used by openhcl?
  3. Does the kernel now choose the same ring buffer size as what openhcl had chosen?

@jstarks : since you already commented ... any concerns?

@jstarks
Copy link
Member

jstarks commented Feb 11, 2025

I am unhappy that the kernel hard codes ring buffer sizes--we have asked for but did not receive an interface to make this configurable from user mode. The kernel has no way of knowing what an appropriate ring buffer size is.

Having said that, I have no objections to this specific change, since the kernel behavior is what it is, and the old command line args were inflexible (since they applied globally to all channels).

@mattkur
Copy link
Contributor

mattkur commented Feb 11, 2025

I am unhappy that the kernel hard codes ring buffer sizes--we have asked for but did not receive an interface to make this configurable from user mode. The kernel has no way of knowing what an appropriate ring buffer size is.

Having said that, I have no objections to this specific change, since the kernel behavior is what it is, and the old command line args were inflexible (since they applied globally to all channels).

Agreed.

Hi John, sorry for replying late, I missed the email due to some email filters.
This is the patch that adds the support to get ring size from a pre defined table.
https://lore.kernel.org/all/1711788723-8593-3-git-send-email-ssengar@linux.microsoft.com/

Thanks for making this cleanup in openhcl.

Few questions:

  1. Do I understand properly that the command line parameters no longer exist, and therefore openhcl's setting them is redundant anyways?
  2. Can you confirm that those changes are in the kernel used by openhcl?
  3. Does the kernel now choose the same ring buffer size as what openhcl had chosen?

@jstarks : since you already commented ... any concerns?

@namancse I'm happy to sign off if you can confirm the questions from above. Thanks again for the feedback!

@namancse
Copy link
Author

Hi Matt,
PFB my responses:

Do I understand properly that the command line parameters no longer exist, and therefore openhcl's setting them is redundant anyways?
Naman: Yes, this is true, the settings are redundant now.
Can you confirm that those changes are in the kernel used by openhcl?
Yes, the changes are there in OpenHCL kernel for quite some time now.
https://github.com/microsoft/OHCL-Linux-Kernel/blob/product/hcl-main/6.6/drivers/uio/uio_hv_generic.c#L42

Does the kernel now choose the same ring buffer size as what openhcl had chosen?
Naman: Yes, its same as earlier: 0x11000.
Earlier, we used to have settings which apply to all the devices, but now with the config table, individual devices can choose to have a preferred ring buffer sizes. There is a fallback mechanism to have default size if preferred size is not provided for that device. Below is the table:
https://github.com/microsoft/OHCL-Linux-Kernel/blob/product/hcl-main/6.6/drivers/hv/channel_mgmt.c#L38

@namancse namancse enabled auto-merge (squash) February 21, 2025 09:27
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.

3 participants