On Fri, Jul 05, 2019 at 07:01:36PM +0800, Liu Yi L wrote:
> +void pci_setup_pasid_ops(PCIDevice *dev, PCIPASIDOps *ops)
> +{
> + assert(ops && !dev->pasid_ops);
> + dev->pasid_ops = ops;
> +}
> +
> +bool pci_device_is_ops_set(PCIBus *bus, int32_t devfn)
Name should be "pci_device_is_pasid_ops_set". Or maybe you can simply
drop this function because as long as you check it in helper functions
like [1] below always then it seems even unecessary.
> +{
> + PCIDevice *dev;
> +
> + if (!bus) {
> + return false;
> + }
> +
> + dev = bus->devices[devfn];
> + return !!(dev && dev->pasid_ops);
> +}
> +
> +int pci_device_request_pasid_alloc(PCIBus *bus, int32_t devfn,
> + uint32_t min_pasid, uint32_t max_pasid)
>From VT-d spec I see that the virtual command "allocate pasid" does
not have bdf information so it's global, but here we've got bus/devfn.
I'm curious is that reserved for ARM or some other arch?
> +{
> + PCIDevice *dev;
> +
> + if (!bus) {
> + return -1;
> + }
> +
> + dev = bus->devices[devfn];
> + if (dev && dev->pasid_ops && dev->pasid_ops->alloc_pasid) {
[1]
> + return dev->pasid_ops->alloc_pasid(bus, devfn, min_pasid, max_pasid);
> + }
> + return -1;
> +}
> +
> +int pci_device_request_pasid_free(PCIBus *bus, int32_t devfn,
> + uint32_t pasid)
> +{
> + PCIDevice *dev;
> +
> + if (!bus) {
> + return -1;
> + }
> +
> + dev = bus->devices[devfn];
> + if (dev && dev->pasid_ops && dev->pasid_ops->free_pasid) {
> + return dev->pasid_ops->free_pasid(bus, devfn, pasid);
> + }
> + return -1;
> +}
> +
> static void pci_dev_get_w64(PCIBus *b, PCIDevice *dev, void *opaque)
> {
> Range *range = opaque;
> diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
> index d082707..16e5b8e 100644
> --- a/include/hw/pci/pci.h
> +++ b/include/hw/pci/pci.h
> @@ -262,6 +262,13 @@ struct PCIReqIDCache {
> };
> typedef struct PCIReqIDCache PCIReqIDCache;
>
> +typedef struct PCIPASIDOps PCIPASIDOps;
> +struct PCIPASIDOps {
> + int (*alloc_pasid)(PCIBus *bus, int32_t devfn,
> + uint32_t min_pasid, uint32_t max_pasid);
> + int (*free_pasid)(PCIBus *bus, int32_t devfn, uint32_t pasid);
> +};
> +
> struct PCIDevice {
> DeviceState qdev;
>
> @@ -351,6 +358,7 @@ struct PCIDevice {
> MSIVectorUseNotifier msix_vector_use_notifier;
> MSIVectorReleaseNotifier msix_vector_release_notifier;
> MSIVectorPollNotifier msix_vector_poll_notifier;
> + PCIPASIDOps *pasid_ops;
> };
>
> void pci_register_bar(PCIDevice *pci_dev, int region_num,
> @@ -484,6 +492,12 @@ typedef AddressSpace *(*PCIIOMMUFunc)(PCIBus *, void *, int);
> AddressSpace *pci_device_iommu_address_space(PCIDevice *dev);
> void pci_setup_iommu(PCIBus *bus, PCIIOMMUFunc fn, void *opaque);
>
> +void pci_setup_pasid_ops(PCIDevice *dev, PCIPASIDOps *ops);
> +bool pci_device_is_ops_set(PCIBus *bus, int32_t devfn);
> +int pci_device_request_pasid_alloc(PCIBus *bus, int32_t devfn,
> + uint32_t min_pasid, uint32_t max_pasid);
> +int pci_device_request_pasid_free(PCIBus *bus, int32_t devfn, uint32_t pasid);
> +
> static inline void
> pci_set_byte(uint8_t *config, uint8_t val)
> {
> --
> 2.7.4
>
Regards,
--
Peter Xu
