pci: vfio: Add the ability to expose the PASID capability on VFIO devices - #8790

Maybe we should do some proper capabilities handling - see #5251. We avoided it as it was a lot of code in the past but now with AI assistance maybe we can do it?

That would be a good idea given we have AI power. Do you think this current PR goes against it? I mean I tried to make this PR as generic as possible. The more advanced capability handling you're talking about seems to be one step above this and might take more time. Do you want me to keep this as a side task I can tackle later?

Maybe we should do some proper capabilities handling - see #5251. We avoided it as it was a lot of code in the past but now with AI assistance maybe we can do it?

That would be a good idea given we have AI power. Do you think this current PR goes against it? I mean I tried to make this PR as generic as possible. The more advanced capability handling you're talking about seems to be one step above this and might take more time. Do you want me to keep this as a side task I can tackle later?

My thought process is that if we build something to parse the capabilities and to construct the new capability set to expose this would be a good example to use (prove the abstraction works). The problem with doing this special case first is that we might end up with lots of special cases or its hard to reconcile after the fact.

Maybe we should do some proper capabilities handling - see #5251. We avoided it as it was a lot of code in the past but now with AI assistance maybe we can do it?

That would be a good idea given we have AI power. Do you think this current PR goes against it? I mean I tried to make this PR as generic as possible. The more advanced capability handling you're talking about seems to be one step above this and might take more time. Do you want me to keep this as a side task I can tackle later?

My thought process is that if we build something to parse the capabilities and to construct the new capability set to expose this would be a good example to use (prove the abstraction works). The problem with doing this special case first is that we might end up with lots of special cases or its hard to reconcile after the fact.

Makes sense, let me try and spend some time assessing the scope and the amount of work this would require.

@rbradford

The PCIe capability structure contains has variable length capabilities where they are encoded as approximately (capability id, size of capability, offset to next capability). From this you can find the next one. In theory you could delete the capabilities by adjusting the offset to skip to the next capability in a shadow version of the configuration space that CH could maintain. Unfortunately this part of the configuration includes RW fields - writes to which need to be passed to the real hardware.
So in order to maintain correct behaviour it would be necessary to maintain a table mapping offsets in the guest presented / shadow version of the PCI configuration space and those in the actual hardware version. Bearing in mind that the offsets to the next capabilities are in the same structure.

From what you wrote more than 3 years ago 😄 , it seems like we're missing a way to delete one or more capabilities from the list of extended capabilities exposed by the VFIO device, so the guest only sees a subset. You had said that the problem comes from the RW fields which need to be passed to the real hardware. I think the problem is actually not that hard to solve given we can simply emulate the offset to the next capability. Let's say you have 3 caps chained together and you want to hide the one in the middle (differently than by setting it to NULL), well we could use our existing ConfigPatch approach by patching the offset of the next cap in the first capability structure. That means when the guest reads the config space, it wouldn't see the cap in the middle, and RW access would not have to go through any emulation since we wouldn't be moving the existing caps. WDYT?

My current PR tackles a slightly different aspect, which is inserting a capability that doesn't exist on the VFIO device and that must be entirely emulated (both read/write accesses) from the VMM. This would be the logical extension to the capability deletion feature.

One more question, do you expect the "legacy"/non-extended caps to have the same deletion/addition ability?

@rbradford

The PCIe capability structure contains has variable length capabilities where they are encoded as approximately (capability id, size of capability, offset to next capability). From this you can find the next one. In theory you could delete the capabilities by adjusting the offset to skip to the next capability in a shadow version of the configuration space that CH could maintain. Unfortunately this part of the configuration includes RW fields - writes to which need to be passed to the real hardware.
So in order to maintain correct behaviour it would be necessary to maintain a table mapping offsets in the guest presented / shadow version of the PCI configuration space and those in the actual hardware version. Bearing in mind that the offsets to the next capabilities are in the same structure.

From what you wrote more than 3 years ago 😄 , it seems like we're missing a way to delete one or more capabilities from the list of extended capabilities exposed by the VFIO device, so the guest only sees a subset. You had said that the problem comes from the RW fields which need to be passed to the real hardware. I think the problem is actually not that hard to solve given we can simply emulate the offset to the next capability. Let's say you have 3 caps chained together and you want to hide the one in the middle (differently than by setting it to NULL), well we could use our existing ConfigPatch approach by patching the offset of the next cap in the first capability structure. That means when the guest reads the config space, it wouldn't see the cap in the middle, and RW access would not have to go through any emulation since we wouldn't be moving the existing caps. WDYT?

My current PR tackles a slightly different aspect, which is inserting a capability that doesn't exist on the VFIO device and that must be entirely emulated (both read/write accesses) from the VMM. This would be the logical extension to the capability deletion feature.

One more question, do you expect the "legacy"/non-extended caps to have the same deletion/addition ability?

I wonder if it would be better to parse them and then have a way to synthesize from scratch rather than try and "patch" in place?

@rbradford

The PCIe capability structure contains has variable length capabilities where they are encoded as approximately (capability id, size of capability, offset to next capability). From this you can find the next one. In theory you could delete the capabilities by adjusting the offset to skip to the next capability in a shadow version of the configuration space that CH could maintain. Unfortunately this part of the configuration includes RW fields - writes to which need to be passed to the real hardware.
So in order to maintain correct behaviour it would be necessary to maintain a table mapping offsets in the guest presented / shadow version of the PCI configuration space and those in the actual hardware version. Bearing in mind that the offsets to the next capabilities are in the same structure.

From what you wrote more than 3 years ago 😄 , it seems like we're missing a way to delete one or more capabilities from the list of extended capabilities exposed by the VFIO device, so the guest only sees a subset. You had said that the problem comes from the RW fields which need to be passed to the real hardware. I think the problem is actually not that hard to solve given we can simply emulate the offset to the next capability. Let's say you have 3 caps chained together and you want to hide the one in the middle (differently than by setting it to NULL), well we could use our existing ConfigPatch approach by patching the offset of the next cap in the first capability structure. That means when the guest reads the config space, it wouldn't see the cap in the middle, and RW access would not have to go through any emulation since we wouldn't be moving the existing caps. WDYT?
My current PR tackles a slightly different aspect, which is inserting a capability that doesn't exist on the VFIO device and that must be entirely emulated (both read/write accesses) from the VMM. This would be the logical extension to the capability deletion feature.
One more question, do you expect the "legacy"/non-extended caps to have the same deletion/addition ability?

I wonder if it would be better to parse them and then have a way to synthesize from scratch rather than try and "patch" in place?

Do you see any benefit in doing so? With that approach, that means you could chain the capabilities in different orders, but you would have to also "redirect" the accesses to the VFIO device. You'd be really shadowing everything, but I'm not sure that buys us anything. The current ConfigPatch approach is really not bad in the sense it lets us patch any byte from the PCI config space, and we only need to extend this with the writable support so that we can synthesize different behavior on RW fields.

@rbradford

The PCIe capability structure contains has variable length capabilities where they are encoded as approximately (capability id, size of capability, offset to next capability). From this you can find the next one. In theory you could delete the capabilities by adjusting the offset to skip to the next capability in a shadow version of the configuration space that CH could maintain. Unfortunately this part of the configuration includes RW fields - writes to which need to be passed to the real hardware.
So in order to maintain correct behaviour it would be necessary to maintain a table mapping offsets in the guest presented / shadow version of the PCI configuration space and those in the actual hardware version. Bearing in mind that the offsets to the next capabilities are in the same structure.

From what you wrote more than 3 years ago 😄 , it seems like we're missing a way to delete one or more capabilities from the list of extended capabilities exposed by the VFIO device, so the guest only sees a subset. You had said that the problem comes from the RW fields which need to be passed to the real hardware. I think the problem is actually not that hard to solve given we can simply emulate the offset to the next capability. Let's say you have 3 caps chained together and you want to hide the one in the middle (differently than by setting it to NULL), well we could use our existing ConfigPatch approach by patching the offset of the next cap in the first capability structure. That means when the guest reads the config space, it wouldn't see the cap in the middle, and RW access would not have to go through any emulation since we wouldn't be moving the existing caps. WDYT?
My current PR tackles a slightly different aspect, which is inserting a capability that doesn't exist on the VFIO device and that must be entirely emulated (both read/write accesses) from the VMM. This would be the logical extension to the capability deletion feature.
One more question, do you expect the "legacy"/non-extended caps to have the same deletion/addition ability?

I wonder if it would be better to parse them and then have a way to synthesize from scratch rather than try and "patch" in place?

Do you see any benefit in doing so? With that approach, that means you could chain the capabilities in different orders, but you would have to also "redirect" the accesses to the VFIO device. You'd be really shadowing everything, but I'm not sure that buys us anything. The current ConfigPatch approach is really not bad in the sense it lets us patch any byte from the PCI config space, and we only need to extend this with the writable support so that we can synthesize different behavior on RW fields.

Does the patching work if we need to remove a capability from in the middle (I think that's what was needed for #5251) ?

@rbradford

Does the patching work if we need to remove a capability from in the middle (I think that's what was needed for #5251) ?

Yes it will. We can patch only the next offset from the previous capability in the chain, and the guest will never see the one in the middle. For RW accesses, the guest keep accessing the same offsets that are defined at the VFIO level, therefore no need for indirection.

@rbradford

Does the patching work if we need to remove a capability from in the middle (I think that's what was needed for #5251) ?

Yes it will. We can patch only the next offset from the previous capability in the chain, and the guest will never see the one in the middle. For RW accesses, the guest keep accessing the same offsets that are defined at the VFIO level, therefore no need for indirection.

Okey dokey. Let's do it that way then!

@rbradford I've added one commit. This first new commit takes care of the unlinking we've talked about.

@rbradford how can I tell the CI typos that it's wrong?