-
Notifications
You must be signed in to change notification settings - Fork 53
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
Support 64 bits and prefetchable BARs #792
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know enough about 64-bit BARs I'm afraid, I'll have to read up a little bit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh and some unit tests would be nice.
Thanks for the patch! As Thanos said, do you mind taking a look at adding a simple test in test/py/ ? Can you share your qemu client patches? Did you need further changes? Do you care about very large BAR sizes, or just locating them past 4g? |
Ok, I'll have a look at it
I simply used the branch
No, my use case was to expose a very large BAR of 2GiB (located above 4G), this patch is enough for this. Setting the PCI_BASE_ADDRESS_MEM_TYPE_64 bit made QEMU and the PCI host driver happy. |
1d5116b
to
edd4549
Compare
@@ -73,6 +73,17 @@ def test_vfu_realize_ctx_pci_bars(): | |||
ret = vfu_setup_region(ctx, index=VFU_PCI_DEV_BAR1_REGION_IDX, size=4096, | |||
flags=(VFU_REGION_FLAG_RW | VFU_REGION_FLAG_MEM)) | |||
assert ret == 0 | |||
ret = vfu_setup_region(ctx, index=VFU_PCI_DEV_BAR2_REGION_IDX, | |||
size=1048576, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a specific reason why you use these values?
@@ -1729,6 +1729,12 @@ vfu_realize_ctx(vfu_ctx_t *vfu_ctx) | |||
if (!(vfu_ctx->reg_info[i].flags & VFU_REGION_FLAG_MEM)) { | |||
vfu_ctx->pci.config_space->hdr.bars[i].io.region_type |= 0x1; | |||
} | |||
if ((vfu_ctx->reg_info[i].flags & VFU_REGION_FLAG_64_BITS)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jlevon in vfu_setup_region() we don't check whether the size is > 2^32 for 32-bit BARs, I think we should fail if VFU_REGION_FLAG_64_BITS
isn't set.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't support bars of that size anyway I think due to the region size uint32_t in private.h
@@ -1729,6 +1729,12 @@ vfu_realize_ctx(vfu_ctx_t *vfu_ctx) | |||
if (!(vfu_ctx->reg_info[i].flags & VFU_REGION_FLAG_MEM)) { | |||
vfu_ctx->pci.config_space->hdr.bars[i].io.region_type |= 0x1; | |||
} | |||
if ((vfu_ctx->reg_info[i].flags & VFU_REGION_FLAG_64_BITS)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can't support bars of that size anyway I think due to the region size uint32_t in private.h
Add two new flags for lib user to request 64bits and/or prefetchable BARs. Tested with a vfio-user client patched QEMU. Signed-off-by: Jérémy Fanguède <[email protected]>
Test that BARs created with VFU_REGION_FLAG_64_BITS and/or VFU_REGION_FLAG_PREFETCH flags do have the right flags in their header. Signed-off-by: Jérémy Fanguède <[email protected]>
edd4549
to
e89963e
Compare
Thanks for the contribution! |
Add two new flags for lib user to request 64bits and/or prefetchable BARs.
Tested with a vfio-user client patched QEMU.