Hi Shameer, On 12/4/25 7:57 PM, Shameer Kolothum wrote: > Hi Eric, > >> -----Original Message----- >> From: Eric Auger <[email protected]> >> Sent: 04 December 2025 16:39 >> To: Shameer Kolothum <[email protected]>; qemu- >> [email protected]; [email protected] >> Cc: [email protected]; Jason Gunthorpe <[email protected]>; Nicolin >> Chen <[email protected]>; [email protected]; [email protected]; >> Nathan Chen <[email protected]>; Matt Ochs <[email protected]>; >> [email protected]; [email protected]; >> [email protected]; [email protected]; >> [email protected]; [email protected]; [email protected]; >> Krishnakant Jaju <[email protected]>; Michael S . Tsirkin <[email protected]> >> Subject: Re: [PATCH v6 16/33] hw/pci/pci: Introduce a callback to retrieve >> the >> MSI doorbell GPA directly >> >> External email: Use caution opening links or attachments > ... > >>> --- a/target/arm/kvm.c >>> +++ b/target/arm/kvm.c >>> @@ -1620,26 +1620,42 @@ int kvm_arch_fixup_msi_route(struct >> kvm_irq_routing_entry *route, >>> return 0; >>> } >>> >>> + /* >>> + * We do have an IOMMU address space, but for some vIOMMU >> implementations >>> + * (e.g. accelerated SMMUv3) the translation tables are programmed into >>> + * the physical SMMUv3 in the host (nested S1=guest, S2=host). QEMU >> cannot >>> + * walk these tables in a safe way, so in that case we obtain the MSI >>> + * doorbell GPA directly from the vIOMMU backend and ignore the >> gIOVA >>> + * @address. >>> + */ >>> + if (pci_device_iommu_msi_direct_gpa(dev, &doorbell_gpa)) { >>> + goto set_doorbell; >>> + } >>> + >>> /* MSI doorbell address is translated by an IOMMU */ >>> >>> - RCU_READ_LOCK_GUARD(); >>> + rcu_read_lock(); >> what is the rationale behind the RCU changes? >> > Otherwise, it will crash if this fn returns via the goto set_doorbell path > with, > > include/qemu/rcu.h:101: rcu_read_unlock: Assertion `p_rcu_reader->depth != 0' > failed > > goto set_doorbell path skips over the scope where RCU_READ_LOCK_GUARD() > is active. When the function later reaches rcu_read_unlock() implicitly via > destructor, the guard was never entered, so we hit above. > > An alternative would be to wrap the translation logic in its own block, like > > if (pci_device_iommu_msi_direct_gpa(dev, &doorbell_gpa)) { > goto set_doorbell; > } > > { > RCU_READ_LOCK_GUARD(); > /* MSI translation logic */ > } > > set_doorbell: > route->u.msi.address_lo = doorbell_gpa; > route->u.msi.address_hi = doorbell_gpa >> 32; > return 0; > > Or could do: > > if (!pci_device_iommu_msi_direct_gpa(dev, &doorbell_gpa)) { > RCU_READ_LOCK_GUARD(); > /* translation logic */ > } > route->u.msi.address_lo = doorbell_gpa; > route->u.msi.address_hi = doorbell_gpa >> 32; > return 0; > > Both approaches work, but I find the explicit lock/unlock variant easier > to follow. Please let me know if there is a better way.
thank you for the explanation. It was not obvious to me. Maybe document that in the commit msg and keep the current version. Reviewed-by: Eric Auger <[email protected]> Eric > > Thanks, > Shameer >
