Re: [PATCH v12 05/13] virtio-iommu: Endpoint and domains structs and helpers

[Peter Xu]

On Tue, Jan 14, 2020 at 09:51:59AM +0100, Auger Eric wrote:
> Hi Peter,

Hi, Eric,

[...]

> > 
> >> +{
> >> +    VirtIOIOMMUEndpoint *ep;
> >> +
> >> +    ep = g_tree_lookup(s->endpoints, GUINT_TO_POINTER(ep_id));
> >> +    if (ep) {
> >> +        return ep;
> >> +    }
> >> +    if (!virtio_iommu_mr(s, ep_id)) {
> > 
> > Could I ask when this will trigger?
> 
> This can happen when a device is attached to a domain and its RID does
> not correspond to one of the devices protected by the iommu.

So will it happen only because of a kernel driver bug?

Also, I think the name of "virtio_iommu_mr" is confusing on that it
returned explicitly a MemoryRegion however it's never been used:

(since they're not in the same patch I'm pasting)

static IOMMUMemoryRegion *virtio_iommu_mr(VirtIOIOMMU *s, uint32_t sid)
{
    uint8_t bus_n, devfn;
    IOMMUPciBus *iommu_pci_bus;
    IOMMUDevice *dev;

    bus_n = PCI_BUS_NUM(sid);
    iommu_pci_bus = iommu_find_iommu_pcibus(s, bus_n);
    if (iommu_pci_bus) {
        devfn = sid & 0xFF;
        dev = iommu_pci_bus->pbdev[devfn];
        if (dev) {
            return &dev->iommu_mr;
        }
    }
    return NULL;
}

Maybe "return !!dev" would be enough, then make the return a boolean?
Then we can rename it to virtio_iommu_has_device().

PS. I think we can also drop IOMMU_PCI_DEVFN_MAX (after all you even
didn't use it here!) and use PCI_DEVFN_MAX, and replace 0xFF.

Thanks,

-- 
Peter Xu