Re: [PATCH v2 12/20] system/physmem: Add ats_do_translate helper

[Ruslan Ruslichenko <ruslichenko.r@gmail.com>]

On 10/15/26 11:40, Clément MATHIEU--DRIF wrote:
> 
> On Wed, 2026-10-14 at 10:21 +0200, Ruslan Ruslichenko wrote:
>> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>>
>>
>> On 10/13/26 16:27, Clément MATHIEU--DRIF wrote:
>>
>>> Do you need to operate at physmem level?
>>> We have an API for PCI device for ATS and PRI (see pci_ats_request_translation and pci_pri_unregister_notifier)
>>>
>>
>>
>> This helper is used by Remote-Port-ATS device, which is used for resolving ATS requests from remote masters via RemotePort link and it is TYPE_DEVICE itself.  
>> As it is not a PCI device itself, we cannot use PCI API unfortunately.
>>
> 
> Ok, I see,
> 
> The issue I see here is that we are basically re-implemeting what IOMMUs already do through the PCI API.  
> It looks like there is a risk here as some IOMMUs might want to update their state when being called on ATS/PRI, which won't be done here.
> 
> I'm not super familiar with RemotePort but I have a high level overview of what some companies do to bridge rtl simulators with a PCI bus in Qemu.  
> IIUC, the bus type of the simulator is known, and their bridge allocates an emulated device of the matching type in Qemu so they can call the official APIs as if there was a "real" emulated device.
> 
> In your use case, what kind of devices are you running in the external simulator? PCI I guess?
> 


The most common usage of ATS and remote-port-ats is indeed bound to PCI subsystem.
For reference, in Remote Port subsystem it is used by remote-port-pci-device and remote-port-pcie-root-port in Xilinx tree:
https://github.com/Xilinx/qemu/blob/master/hw/core/remote-port-pci-device.c
https://github.com/Xilinx/qemu/blob/master/hw/core/remote-port-pcie-root-port.c

And since there is already existing 'pci_ats_request_translation' API, I agree it is a cleaner and more correct approach.
I think, ATS device can use PCIDevice created by those models and call pci_ats_request_translation(), which removes the need for physmem-level helper.
Although, the 'ats_request_translation' is currently implemented only by the Intel IOMMU, ARM SMMU don't implement it, so this needs to be done too.

In our case we have a use case where a device on the platform bus, behind SMMU also wants to use ATS requests.
But those likely require separate approach and it belongs to SMMU for platform bus series, such as:
https://lists.nongnu.org/archive/html/qemu-devel/2025-12/msg02637.html

So will drop 'ats_do_translate' for the next revision. 

Thanks for the feedback!

--
Best Regards,
Ruslan

>>
>>> On Tue, 2026-09-15 at 18:00 +0200, Ruslan Ruslichenko wrote:
>>>
>>>> From: Ruslan Ruslichenko <[[Ruslan_Ruslichenko@epam.com](mailto:Ruslan_Ruslichenko@epam.com)](mailto:[Ruslan_Ruslichenko@epam.com](mailto:Ruslan_Ruslichenko@epam.com))>
>>>>
>>>> Add 'ats_do_translate' method to enable manual IOMMU address
>>>> translation.
>>>>
>>>> This allows the Remote Port subsystem to resolve guest virtual
>>>> addresses to physical addresses by querying the QEMU IOMMU
>>>> translation pipeline. This is required for the Remote Port
>>>> subsystem to support translation  requests from external masters
>>>> in a co-simulation environment.
>>>>
>>>> Signed-off-by: Edgar E. Iglesias <[[edgar.iglesias@amd.com](mailto:edgar.iglesias@amd.com)](mailto:[edgar.iglesias@amd.com](mailto:edgar.iglesias@amd.com))>
>>>> Signed-off-by: Takahiro Nakata <[[takahiro.nakata.wr@renesas.com](mailto:takahiro.nakata.wr@renesas.com)](mailto:[takahiro.nakata.wr@renesas.com](mailto:takahiro.nakata.wr@renesas.com))>
>>>> Signed-off-by: Ruslan Ruslichenko <[[Ruslan_Ruslichenko@epam.com](mailto:Ruslan_Ruslichenko@epam.com)](mailto:[Ruslan_Ruslichenko@epam.com](mailto:Ruslan_Ruslichenko@epam.com))>
>>>> ---
>>>>  include/system/memory.h |  8 +++++++
>>>>  system/physmem.c        | 53 +++++++++++++++++++++++++++++++++++++++++
>>>>  2 files changed, 61 insertions(+)
>>>>
>>>> diff --git a/include/system/memory.h b/include/system/memory.h
>>>> index a7e03fc3e7..2032d3028d 100644
>>>> --- a/include/system/memory.h
>>>> +++ b/include/system/memory.h
>>>> @@ -2583,6 +2583,14 @@ static inline MemoryRegion *address_space_translate(AddressSpace *as,
>>>>                                addr, xlat, len, is_write, attrs);
>>>>  }
>>>>
>>>> +MemoryRegion *ats_do_translate(AddressSpace *as,
>>>> +    hwaddr addr,
>>>> +    hwaddr *xlat,
>>>> +    hwaddr *plen_out,
>>>> +    AddressSpace **target_as,
>>>> +    int *prot,
>>>> +    MemTxAttrs attrs);
>>>> +
>>>>  /* address_space_access_valid: check for validity of accessing an address
>>>>   * space range
>>>>   *
>>>> diff --git a/system/physmem.c b/system/physmem.c
>>>> index a4a6a4710c..1acda22ede 100644
>>>> --- a/system/physmem.c
>>>> +++ b/system/physmem.c
>>>> @@ -525,6 +525,59 @@ static MemoryRegionSection flatview_do_translate(FlatView *fv,
>>>>      return *section;
>>>>  }
>>>>
>>>> +/* This function is called from RCU critical section */
>>>> +MemoryRegion *ats_do_translate(AddressSpace *as,
>>>> +                               hwaddr addr,
>>>> +                               hwaddr *xlat,
>>>> +                               hwaddr *plen_out,
>>>> +                               AddressSpace **target_as,
>>>> +                               int *prot,
>>>> +                               MemTxAttrs attrs)
>>>> +{
>>>> +    MemoryRegionSection *section;
>>>> +    IOMMUMemoryRegion *iommu_mr;
>>>> +    hwaddr plen = (hwaddr)(-1);
>>>> +    FlatView *fv = address_space_to_flatview(as);
>>>> +    IOMMUMemoryRegionClass *imrc;
>>>> +    IOMMUTLBEntry iotlb;
>>>> +    int iommu_idx = 0;
>>>> +
>>>> +    if (!plen_out) {
>>>> +        plen_out = &plen;
>>>> +    }
>>>> +
>>>> +    /* This can be MMIO, so setup MMIO bit. */
>>>> +    section = address_space_translate_internal(
>>>> +            flatview_to_dispatch(fv), addr, xlat,
>>>> +            plen_out, true);
>>>> +
>>>> +    /* Illegal translation */
>>>> +    if (section->mr == &io_mem_unassigned) {
>>>> +        return NULL;
>>>> +    }
>>>> +
>>>> +    iommu_mr = memory_region_get_iommu(section->mr);
>>>> +    if (!iommu_mr) {
>>>> +        return NULL;
>>>> +    }
>>>> +
>>>> +    imrc = memory_region_get_iommu_class_nocheck(iommu_mr);
>>>> +
>>>> +    if (imrc->attrs_to_index) {
>>>> +        iommu_idx = imrc->attrs_to_index(iommu_mr, attrs);
>>>> +    }
>>>> +
>>>> +    iotlb = imrc->translate(iommu_mr, addr, IOMMU_RO, iommu_idx);
>>>> +
>>>> +    *xlat = ((iotlb.translated_addr & ~iotlb.addr_mask)
>>>> +            | (addr & iotlb.addr_mask));
>>>> +    *plen_out = MIN(*plen_out, (addr | iotlb.addr_mask) - addr + 1);
>>>> +    *prot = iotlb.perm;
>>>> +    *target_as = iotlb.target_as;
>>>> +
>>>> +    return section->mr;
>>>> +}
>>>> +
>>>>  /* Called from RCU critical section */
>>>>  IOMMUTLBEntry address_space_get_iotlb_entry(AddressSpace *as, hwaddr addr,
>>>>                                              bool is_write, MemTxAttrs attrs)
>>>
>>