On Fri, 29 Aug 2025, Mark Cave-Ayland wrote:
On 15/07/2025 14:38, BALATON Zoltan wrote:
On Tue, 15 Jul 2025, Mark Cave-Ayland wrote:
Use QOM casts to convert between VFIOPCIDevice and PCIDevice instead of
accessing pdev directly.
Signed-off-by: Mark Cave-Ayland <[email protected]>
---
hw/vfio/pci.c | 202 ++++++++++++++++++++++++++++++--------------------
1 file changed, 120 insertions(+), 82 deletions(-)
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 1093b28df7..fb9eb58da5 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -119,6 +119,7 @@ static void vfio_intx_mmap_enable(void *opaque)
static void vfio_intx_interrupt(void *opaque)
{
VFIOPCIDevice *vdev = opaque;
+ PCIDevice *pdev = PCI_DEVICE(vdev);
Don't do that. Opaque data is already type checked when it is registered
for the callback and cannot be changed so additional type checking here is
just a performance hit without any advantage. It's OK to do it in less
frequently called functions but don't add unnecessary casts to functions
that can be called a lot.
In general the QOM casts fall into the noise in a standard profile, but I can
see how it could be possible they might show up in the interrupt fast path.
I'll look at getting a vfio-pci perf test set up here to see if there is a
noticeable effect in this case, and if so think about what the best approach
is.
In general, casting opaque pointer of a callback is unnecessary because
these can be checked once when registered so it's not needed to check
again at every use. I think this should really be recommended practice to
not use QOM casts for these opaque pointers of callbacks. A lot of these
are also in hot path so not adding unnecessary checks is a good idea. In
this case when you're in the vfio implementation what's wrong with
accessing internal field of vfio anyway? An object's implemenattion should
be able to access its own fields so maybe the current way is OK and most
of this patch is not needed at all.
Regards,
BALATON Zoltan
