Skip to content

GSI Allocator: free GSIs when they are no longer used#74

Open
amphi wants to merge 2 commits intocyberus-technology:gardenlinuxfrom
amphi:gsi-allocator-bitmap
Open

GSI Allocator: free GSIs when they are no longer used#74
amphi wants to merge 2 commits intocyberus-technology:gardenlinuxfrom
amphi:gsi-allocator-bitmap

Conversation

@amphi
Copy link

@amphi amphi commented Feb 2, 2026

This PR changes the GsiAllocator to enable it to free GSIs when they are no longer used. To do that, it replaces the u32 that was used to track used GSIs with a simple bitmap. If you know a good bitmap crate that I should use, please tell me.

While I was here, I also made sure that errors from the allocator a propagated. I also used the bitmap for the IRQs of the GsiAllocator, just because it made it easier for me to propagate useful errors.

I am not completely sure whether I free the GSIs at all necessary places.

@amphi amphi self-assigned this Feb 2, 2026
@phip1611
Copy link
Member

phip1611 commented Feb 2, 2026

. If you know a good bitmap crate that I should use, please tell me.

I think a bitmap is trivial enough to keep the functionality in the module itself

@amphi amphi force-pushed the gsi-allocator-bitmap branch 2 times, most recently from 94c8bd8 to f05cdf5 Compare February 2, 2026 13:26
Copy link
Member

@phip1611 phip1611 left a comment

Choose a reason for hiding this comment

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

I think we can leave this as is, good design! Please address the remaining remarks

@amphi amphi force-pushed the gsi-allocator-bitmap branch 3 times, most recently from 63b9784 to 03f5bbd Compare February 6, 2026 14:23
struct InterruptAllocator {
// Although this is used as a bitmap, it is a collection of `u64`. Thus the
// name `words`.
words: Box<[usize]>,
Copy link
Member

Choose a reason for hiding this comment

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

Either adjust the comment to usize or use u64 here. I think even Box<[u128]> could be a good default or Box<[u8]> - please use something fixed if there is no good reason to do different

Copy link
Author

Choose a reason for hiding this comment

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

Yeah my bad, changed the comment to collection of usize

Copy link
Member

Choose a reason for hiding this comment

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

I don't understand why you use a usize at all. Why not u8?

@amphi amphi force-pushed the gsi-allocator-bitmap branch from 03f5bbd to 6e42550 Compare February 6, 2026 14:28
amphi added 2 commits February 6, 2026 15:31
The old implementation used a u32 to allocate new GSIs. The counter
increased every time a new GSI was allocated, and freeing GSIs was not
possible. Thus, Cloud Hypervisor could run out of GSIs and panic.

This new implementation can also free GSIs. Please note that this commit
only replaces the old mechanism, the next commit will introduce a
mechanism to free used GSIs.

While I was here I also propagated the errors that the allocator may
throw where necessary.

On-behalf-of: SAP sebastian.eydam@sap.com
Signed-off-by: Sebastian Eydam <sebastian.eydam@cyberus-technology.de>
These changes free GSIs when they are no longer used. That way we
won't exhaust the available GSIs anymore when attaching and
detaching devices.

On-behalf-of: SAP sebastian.eydam@sap.com
Signed-off-by: Sebastian Eydam <sebastian.eydam@cyberus-technology.de>
@amphi amphi force-pushed the gsi-allocator-bitmap branch from 6e42550 to 6637513 Compare February 6, 2026 14:31
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.

2 participants